Closed Bug 715554 Opened 8 years ago Closed 8 years ago

Send quit-application in browser/components/places/tests/unit/test_clearHistory_shutdown.js

Categories

(Toolkit :: Places, defect)

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla12

People

(Reporter: espindola, Assigned: espindola)

References

Details

Attachments

(1 file)

No description provided.
ping. Do you have an ETA on when you can review this?
Comment on attachment 586127 [details] [diff] [review]
Send quit-application in browser/components/places/tests/unit/test_clearHistory_shutdown.js

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

::: browser/components/places/tests/unit/test_clearHistory_shutdown.js
@@ +142,5 @@
>  
>    shutdownPlaces();
> +
> +  // Shutdown the download manager.
> +  Services.obs.notifyObservers(null, "quit-application", null)

This causes incorrect notifications ordering, since profile notifications would be fired after this one (shutdownPlaces only notifies to history). The right way may be to add a shutdownDownloadManager() helper sending all three notifications to the dm...

But Bug 713172 is moving dm database shutdown to profile-before-change, and is almost ready to land, so it's probably better to just wait for it to land and this bug will be solved by that.
Attachment #586127 - Flags: review?(mak77) → review-
hm, well I was wrong about ordering. it may be fine, though I still think it's worth to just land Bug 713172
Comment on attachment 586127 [details] [diff] [review]
Send quit-application in browser/components/places/tests/unit/test_clearHistory_shutdown.js

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

So, code-wise correct, but unneeded
Attachment #586127 - Flags: review- → review+
oh, and in case it's missing a final semicolon (sorry for noise bugzilla is lagging like crazy today...)
And the semicolon is at

https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=e94c3b308519

sorry about missing the last comment on the first push.
https://hg.mozilla.org/mozilla-central/rev/2ac52594bdb7
https://hg.mozilla.org/mozilla-central/rev/e94c3b308519
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla12
Blocks: 711937
Flags: in-testsuite+
Version: unspecified → Trunk
You need to log in before you can comment on or make changes to this bug.