1244723 - xpcshell should shutdown profile after all the registered cleanup functions, not before them

Status: RESOLVED FIXED

(Testing :: XPCShell Harness, defect)

Description

[Marco Bonardo [:mak]]

do_get_profile registers profile shutdown topics through do_register_cleanup, but by doing so the function will be invoked before the cleanup functions registered by the test...
Instead it should be the last one, after the test cleanup functions.

We should probably not use do_register_cleanup, instead store it separately and invoke it before we walk the clenaup functions array.

Comment 1

[Marco Bonardo [:mak]]

(In reply to Marco Bonardo [::mak] from comment #0)
> We should probably not use do_register_cleanup, instead store it separately
> and invoke it before we walk the clenaup functions array.

ehr, that should have been "after".

Comment 2

[Marco Bonardo [:mak]]

Attachment: MozReview Request: Bug 1244723 - xpcshell should shutdown profile after all the registered cleanup functions, not before them. r=ted

Review commit: https://reviewboard.mozilla.org/r/33035/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/33035/

Comment 3

[Marco Bonardo [:mak]]

looks like Try is showing some errors to be fixed.

Many (or all of them?) are "Unable to find a rejection expected by expectUncaughtRejection" that sounds like a positive thing!
Will check later and add another changeset on top.

Comment 4

[Marco Bonardo [:mak]]

Attachment: MozReview Request: Bug 1244723 - Remove some of the expected uncaught exceptions not happening anymore. r=ted

Review commit: https://reviewboard.mozilla.org/r/33247/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/33247/

Comment 5

[(not currently active) Ted Mielczarek]

Comment on attachment 8714343 [details]
MozReview Request: Bug 1244723 - xpcshell should shutdown profile after all the registered cleanup functions, not before them. r=ted

https://reviewboard.mozilla.org/r/33035/#review30025

Well that was silly. Good catch!

Comment 6

[(not currently active) Ted Mielczarek]

Comment on attachment 8714843 [details]
MozReview Request: Bug 1244723 - Remove some of the expected uncaught exceptions not happening anymore. r=ted

https://reviewboard.mozilla.org/r/33247/#review30027

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/b49ff0115100

Comment 8

[Marco Bonardo [:mak]]

The second part is not needed anymore since bug 989960 has temporarily been backed out.

Comment 9

[:Paolo Amadini]

Good to see less chances for error. There's no way to register an asynchronous cleanup function to be executed after profile shutdown notifications anymore (only synchronous through the observers), but I think this won't likely be needed in practice.

Remember to update the documentation of do_get_profile at:

https://developer.mozilla.org/en-US/docs/Mozilla/QA/Writing_xpcshell-based_unit_tests

Comment 10

[Marco Bonardo [:mak]]

(In reply to :Paolo Amadini from comment #9)
> Good to see less chances for error. There's no way to register an
> asynchronous cleanup function to be executed after profile shutdown
> notifications anymore (only synchronous through the observers), but I think
> this won't likely be needed in practice.

you can add an observer to do that, if needed, and you can use async shutdown tooling to wait for async functions.
the scope of cleanup functions is cleanup, not really do testing of shutdown or such.

> Remember to update the documentation of do_get_profile at:
> 
> https://developer.mozilla.org/en-US/docs/Mozilla/QA/Writing_xpcshell-
> based_unit_tests

what should I update there? it still looks correct to me.

Comment 11

[Marco Bonardo [:mak]]

ah I see, the note.

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/b49ff0115100