Closed
Bug 952201
Opened 10 years ago
Closed 10 years ago
[e10s] Can't install non-AMO addons; clicking on .xpi link causes "Error: You cannot use the AddonManager in child processes"
Categories
(Firefox :: Extension Compatibility, defect)
Firefox
Extension Compatibility
Tracking
()
RESOLVED
FIXED
Firefox 33
Tracking | Status | |
---|---|---|
e10s | + | --- |
People
(Reporter: cpeterson, Assigned: mrbkap)
References
()
Details
Attachments
(1 file, 4 obsolete files)
33.65 KB,
patch
|
Unfocused
:
review+
|
Details | Diff | Splinter Review |
STR: 1. With e10s enabled, load https://www.eff.org/https-everywhere 2. Click on "Install in Firefox" link RESULT: Nothing happens, but a debug build logs the following error messages: System JS : ERROR resource://gre/modules/AddonManager.jsm:20 - Error: You cannot use the AddonManager in child processes! [Parent 51764] WARNING: No docshells for remote frames!: file /mozilla/inbound/content/base/src/nsFrameLoader.cpp, line 565 System JS : ERROR chrome://browser/content/browser.js:163 - TypeError: aDocShell is null
Reporter | ||
Updated•10 years ago
|
tracking-e10s:
--- → +
Reporter | ||
Updated•10 years ago
|
Summary: [e10s] Clicking on .xpi link causes "Error: You cannot use the AddonManager in child processes" → [e10s] Can't install non-AMO addons; clicking on .xpi link causes "Error: You cannot use the AddonManager in child processes"
Assignee | ||
Comment 3•10 years ago
|
||
Taking and marking m1.
Assignee | ||
Comment 4•10 years ago
|
||
Assignee | ||
Comment 5•10 years ago
|
||
This is mostly a question of getting information from point A to point B, through all of the XPCOM stuff. Nothing here is all that exciting. Felipe, I'm not entirely sure who should be reviewing here. Let me know if there's a better choice.
Assignee | ||
Comment 6•10 years ago
|
||
Assignee | ||
Updated•10 years ago
|
Attachment #8447513 -
Flags: review?(felipc)
Assignee | ||
Comment 7•10 years ago
|
||
Comment on attachment 8447513 [details] [diff] [review] patch v1 Unfocused, I understand that you're the owner of this code?
Attachment #8447513 -
Flags: review?(felipc) → review?(bmcbride)
Comment 8•10 years ago
|
||
Comment on attachment 8447513 [details] [diff] [review] patch v1 Review of attachment 8447513 [details] [diff] [review]: ----------------------------------------------------------------- Most of this is adding a 'browser' param to functions, and passing it around along with everything else. Seems cleaner if we were to change the 'window' param to a generic 'target' param, and QI that when we finally need it in browser-addons.js Also, can you figure out what test this makes pass? We have a bunch of Add-ons Manager tests that are disabled in e10s - this should fix at least one of them.
Attachment #8447513 -
Flags: review?(bmcbride) → review-
Assignee | ||
Comment 9•10 years ago
|
||
Sorry for the lag in getting back to this. I'm going to have a new patch here today or tomorrow.
Assignee | ||
Comment 10•10 years ago
|
||
Assignee | ||
Comment 11•10 years ago
|
||
(Oops, hit the send button too soon.) Is this what you had in mind? It seems to work (and is probably safer in a way since it forced me to look at *all* of the users of window). I went with 'originator' instead of 'target', but I can change that easily if you want (it didn't shrink the size of the patch at all, though). As for the tests, it turns out that the entirety of the xpinstall browser tests is disabled. This patch didn't magically make them start passing. I'll start looking into why, but I would prefer not to gate this patch on them passing. I've pushed to try at <https://tbpl.mozilla.org/?tree=Try&rev=baeb5d69bc5e>.
Attachment #8447513 -
Attachment is obsolete: true
Attachment #8455684 -
Attachment is obsolete: true
Assignee | ||
Updated•10 years ago
|
Attachment #8455688 -
Flags: review?(bmcbride)
Assignee | ||
Comment 12•10 years ago
|
||
The last patch was orange on try. This one can be tracked at <https://tbpl.mozilla.org/?tree=Try&rev=2f0981996a17>.
Attachment #8455688 -
Attachment is obsolete: true
Attachment #8455688 -
Flags: review?(bmcbride)
Assignee | ||
Comment 13•10 years ago
|
||
Try is green with this patch.
Attachment #8456588 -
Attachment is obsolete: true
Assignee | ||
Updated•10 years ago
|
Attachment #8457456 -
Flags: review?(bmcbride)
Comment 14•10 years ago
|
||
Comment on attachment 8457456 [details] [diff] [review] patch v2.2 Review of attachment 8457456 [details] [diff] [review]: ----------------------------------------------------------------- Ship it!
Attachment #8457456 -
Flags: review?(bmcbride) → review+
Assignee | ||
Comment 15•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/53f37bbfd7ab Thanks a bunch for the quick reviews!
https://hg.mozilla.org/mozilla-central/rev/53f37bbfd7ab
Status: NEW → RESOLVED
Closed: 10 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Firefox 33
Comment 17•10 years ago
|
||
This busted Android. Filing.
Comment 18•10 years ago
|
||
Bustage filed at https://bugzilla.mozilla.org/show_bug.cgi?id=1041770
You need to log in
before you can comment on or make changes to this bug.
Description
•