885389 - xpcshell head.js doesn't notify profile-after-change

Status: RESOLVED FIXED

(Testing :: XPCShell Harness, defect)

Description

[Richard Newman [:rnewman]]

Attachment: Incorrectly absolute patch.

It only sends profile-do-change.

I found this while trying to figure out why FormHistory wasn't uniniting itself (Bug 878677).

Is there a reason why it doesn't?

Adding a call to do so doesn't seem to break anything:

15:31.89 INFO | Result summary:
15:31.89 INFO | Passed: 1643
15:31.89 INFO | Failed: 2
15:31.89 INFO | Todo: 3

Only the usual kinds of failures:

15:05.50 TEST-UNEXPECTED-FAIL | /Users/rnewman/moz/hg/services-central/obj-ff-dbg/_tests/xpcshell/toolkit/mozapps/update/test/unit/test_0201_app_launch_apply_update.js | test failed (with xpcshell return code: 3), see following log:
15:05.50 >>>>>>>
15:05.50 ### XPCOM_MEM_LEAK_LOG defined -- logging leaks to /var/folders/2b/fnxwv4mj1bqc8qbkhdzc13s80000gn/T/tmp3Jabbx/runxpcshelltests_leaks.log
15:05.50
15:05.50 TEST-INFO | (xpcshell/head.js) | test MAIN run_test pending (1)
15:05.50
15:05.50 TEST-INFO | (xpcshell/head.js) | test pending (2)
15:05.50
15:05.50 TEST-UNEXPECTED-FAIL | /Users/rnewman/moz/hg/services-central/obj-ff-dbg/_tests/xpcshell/toolkit/mozapps/update/test/unit/head_update.js:3059 | ReferenceError: BUNDLE_NAME is not defined - See following stack:
15:05.50 JS frame :: getAppDir@/Users/rnewman/moz/hg/services-central/obj-ff-dbg/_tests/xpcshell/toolkit/mozapps/update/test/unit/head_update.js:3059
15:05.50 JS frame :: symlinkUpdateFilesIntoBundleDirectory@/Users/rnewman/moz/hg/services-central/obj-ff-dbg/_tests/xpcshell/toolkit/mozapps/update/test/unit/test_0201_app_launch_apply_update.js:59
15:05.50 JS frame :: run_test@/Users/rnewman/moz/hg/services-central/obj-ff-dbg/_tests/xpcshell/toolkit/mozapps/update/test/unit/test_0201_app_launch_apply_update.js:118
15:05.50 JS frame :: _execute_test@/Users/rnewman/moz/hg/services-central/testing/xpcshell/head.js:325
15:05.50 JS frame :: @-e:1
15:05.50 /Users/rnewman/moz/hg/services-central/obj-ff-dbg/_tests/xpcshell/toolkit/mozapps/update/test/unit/test_0201_app_launch_apply_update.js:212: TypeError: gProcess is undefined


 4:01.72 TEST-UNEXPECTED-FAIL | /Users/rnewman/moz/hg/services-central/obj-ff-dbg/_tests/xpcshell/netwerk/test/unit/test_protocolproxyservice.js | "127.0.0.1" != "127.0.0.1" - See following stack:
 4:01.72 JS frame :: /Users/rnewman/moz/hg/services-central/obj-ff-dbg/_tests/xpcshell/netwerk/test/unit/test_protocolproxyservice.js :: myipaddress_callback :: line 598
 4:01.72 JS frame :: /Users/rnewman/moz/hg/services-central/obj-ff-dbg/_tests/xpcshell/netwerk/test/unit/test_protocolproxyservice.js :: resolveCallback.prototype.onProxyAvailable :: line 152

Comment 1

[Richard Newman [:rnewman]]

Per mak:

This is for the simple reason there are various components that are inited by profile-after-change and it would be crazy to always invoke it and load a bunch of useless components. profile-after-change must indeed be notified manually.

Comment 2

[(not currently active) Ted Mielczarek]

Comment on attachment 765424 [details] [diff] [review]
Incorrectly absolute patch.

That being said, I think it would be fine to add a parameter to do_get_profile to send this notification instead of making tests send it manually.

Comment 3

[Richard Newman [:rnewman]]

Then let's keep this one open.

Comment 4

[Marco Bonardo [:mak]]

(In reply to Ted Mielczarek [:ted.mielczarek] from comment #2)
> Comment on attachment 765424 [details] [diff] [review]
> Incorrectly absolute patch.
> 
> That being said, I think it would be fine to add a parameter to
> do_get_profile to send this notification instead of making tests send it
> manually.

Possibly, though it will still initialize a lot of components that the test doesn't need, and that may be an issue for 2 reasons:
1. unexpected failures due to not setting up the env for those components properly (after all it's not the component you are testing)
2. slower test runs, with increased use of resources

My idea of "sending it manually" is not through a global notifyObservers() call, but by directly invoking .observe() of the component manually, so that only that specific component is initialized, that is what we did so far.

Comment 5

[Georg Fritzsche [:gfritzsche]]

Attachment: Make do_get_profile() optionally notify "profile-after-change"

Comment 6

[(not currently active) Ted Mielczarek]

This could also probably stand a test in testing/xpcshell/example.

Comment 7

[Georg Fritzsche [:gfritzsche]]

Attachment: Add test coverage for do_get_profile() observer notifications

Comment 8

[(not currently active) Ted Mielczarek]

Comment on attachment 8595312 [details] [diff] [review]
Add test coverage for do_get_profile() observer notifications

Review of attachment 8595312 [details] [diff] [review]:
-----------------------------------------------------------------

Very thorough, nice!

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/025e4c11ed7e
https://hg.mozilla.org/integration/mozilla-inbound/rev/d0e1fb64877c

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/025e4c11ed7e
https://hg.mozilla.org/mozilla-central/rev/d0e1fb64877c