Closed
Bug 953156
Opened 12 years ago
Closed 11 years ago
Chrome manifests for bootstrapped add-ons unregistered at shutdown, causing multiple calls to CheckForNewChrome
Categories
(Toolkit :: Add-ons Manager, defect)
Toolkit
Add-ons Manager
Tracking
()
RESOLVED
FIXED
mozilla29
People
(Reporter: darktrojan, Assigned: darktrojan)
Details
Attachments
(1 file, 3 obsolete files)
|
6.52 KB,
patch
|
Unfocused
:
review+
|
Details | Diff | Splinter Review |
I've noticed that as each add-on is shutdown at app shutdown, we call Components.manager.removeBootstrappedManifestLocation, and by extension, CheckForNewChrome. This doesn't need to happen if we're shutting down.
|
Assignee | |
Comment 1•12 years ago
|
||
I really don't know how I'd go about testing this.
Attachment #8351484 -
Flags: review?(bmcbride)
|
|
Assignee | |
Comment 2•12 years ago
|
||
Turns out I do know how I can test this.
Attachment #8351484 -
Attachment is obsolete: true
Attachment #8351484 -
Flags: review?(bmcbride)
Attachment #8351522 -
Flags: review?(bmcbride)
|
||
Comment 3•12 years ago
|
||
Comment on attachment 8351522 [details] [diff] [review]
953156-2.diff
Review of attachment 8351522 [details] [diff] [review]:
-----------------------------------------------------------------
Me like.
Attachment #8351522 -
Flags: review?(bmcbride) → review+
|
|
Assignee | |
Comment 4•12 years ago
|
||
Flags: in-testsuite+
|
||
Comment 5•12 years ago
|
||
|
|
Assignee | |
Comment 6•11 years ago
|
||
Hmm, I thought I'd run all those tests locally. Guess not.
Blair, looking at test_langpack.js (line 207 et al), I think I should manually unregister the manifest in the test. Does that seem sensible?
|
|
||
Comment 7•11 years ago
|
||
Yep.
(You're lucky I saw this, remember to needinfo me)
|
|
Assignee | |
Comment 8•11 years ago
|
||
Now with fixed test!
Attachment #8351522 -
Attachment is obsolete: true
Attachment #8356375 -
Flags: review?(bmcbride)
|
|
||
Comment 9•11 years ago
|
||
Comment on attachment 8356375 [details] [diff] [review]
953156-3.diff
Review of attachment 8356375 [details] [diff] [review]:
-----------------------------------------------------------------
Ship it... again.
Attachment #8356375 -
Flags: review?(bmcbride) → review+
|
|
Assignee | |
Comment 10•11 years ago
|
||
|
|
||
Comment 11•11 years ago
|
||
backed out for failures like https://tbpl.mozilla.org/php/getParsedLog.php?id=32630579&tree=Mozilla-Inbound
|
|
Assignee | |
Comment 12•11 years ago
|
||
Now with fixed, fixed test.
Attachment #8356375 -
Attachment is obsolete: true
Attachment #8356970 -
Flags: review?(bmcbride)
|
|
||
Comment 13•11 years ago
|
||
Comment on attachment 8356970 [details] [diff] [review]
953156-4.diff
Review of attachment 8356970 [details] [diff] [review]:
-----------------------------------------------------------------
Geez, at this rate, you'd almost think this was one of my patches ;-)
Attachment #8356970 -
Flags: review?(bmcbride) → review+
|
|
Assignee | |
Comment 14•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/fbc1d2e697f1
(In reply to Blair McBride [:Unfocused] from comment #13)
> Geez, at this rate, you'd almost think this was one of my patches ;-)
If this doesn't succeed, I'm WONTFIXing it.
(Especially since it passes Try.)
|
||
Comment 15•11 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
You need to log in
before you can comment on or make changes to this bug.
Description
•