Closed Bug 952201 Opened 6 years ago Closed 6 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)

defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 33
Tracking Status
e10s + ---

People

(Reporter: cpeterson, Assigned: mrbkap)

References

()

Details

Attachments

(1 file, 4 obsolete files)

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
tracking-e10s: --- → +
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"
Taking and marking m1.
Assignee: nobody → mrbkap
Blocks: e10s-m1, 928230
Attached patch patch v1 (obsolete) — Splinter Review
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.
Attachment #8447513 - Flags: review?(felipc)
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 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-
Sorry for the lag in getting back to this. I'm going to have a new patch here today or tomorrow.
Attached patch patch v2 (obsolete) — Splinter Review
(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
Attachment #8455688 - Flags: review?(bmcbride)
Attached patch patch v2.1 (obsolete) — Splinter Review
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)
Attached patch patch v2.2Splinter Review
Try is green with this patch.
Attachment #8456588 - Attachment is obsolete: true
Attachment #8457456 - Flags: review?(bmcbride)
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+
https://hg.mozilla.org/mozilla-central/rev/53f37bbfd7ab
Status: NEW → RESOLVED
Closed: 6 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Firefox 33
Depends on: 1041475
You need to log in before you can comment on or make changes to this bug.