Closed
Bug 1275719
Opened 8 years ago
Closed 8 years ago
Error loading web extensions on recent builds
Categories
(Core :: XPCOM, defect, P1)
Core
XPCOM
Tracking
()
RESOLVED
FIXED
mozilla49
People
(Reporter: bsilverberg, Assigned: m_kato)
References
Details
Attachments
(1 file, 1 obsolete file)
When building Firefox from fx-team and/or mozilla-central, locally anyway, the resulting build is unable to load a Web Extension that uses a background script. This causes all of the web extension tests to fail, but it is also reproducible manually by simply trying to load an extension that has a background script.
This is happening for me on OS X 10.11.4, and it's also happening for :rpl on Linux and :aswan on OS X 10.11.5.
I've tried both artifact builds and full builds and the problem is the same. The error we are seeing is:
Extension error: console is not defined resource://devtools/shared/event-emitter.js:168 :: EventEmitter_emit@resource://devtools/shared/event-emitter.js:168:11
But that is actually masking the real error which is:
[Exception... "Cannot find interface information for parameter arg 0 [nsIWindowlessBrowser.document]" nsresult: "0x80570006 (NS_ERROR_XPC_CANT_GET_PARAM_IFACE_INFO)"
Some bisecting has determined that the regression range is:
changeset: 326073:daec005079bf
user: Makoto Kato <m_kato@ga2.so-net.ne.jp>
date: Mon May 23 16:47:48 2016 +0900
summary: Bug 1177943 - Part 6. Fix HiDPI support. r=smaug
changeset: 326072:a4093f040496
user: Makoto Kato <m_kato@ga2.so-net.ne.jp>
date: Tue Apr 26 17:18:04 2016 +0900
summary: Bug 1177943 - Part 5. Add IPC for e10s support. r=masayuki
changeset: 326071:f9d08027b095
user: Makoto Kato <m_kato@ga2.so-net.ne.jp>
date: Wed Apr 20 19:12:27 2016 +0900
summary: Bug 1177943 - Part 4. Implement remote nsICommandWithParams. r=smaug
changeset: 326070:024599d425ef
user: Makoto Kato <m_kato@ga2.so-net.ne.jp>
date: Tue Apr 26 18:04:41 2016 +0900
summary: Bug 1177943 - Part 3. Look up dictionary using new 10.8+ interface. r=masayuki
changeset: 326069:4b63f3496a31
user: Makoto Kato <m_kato@ga2.so-net.ne.jp>
date: Mon May 02 11:41:20 2016 +0900
summary: Bug 1177943 - Part 2. Add cmd_lookUpDictionary content command. r=masayuki
changeset: 326068:d355aa36fa26
user: Makoto Kato <m_kato@ga2.so-net.ne.jp>
date: Mon Apr 04 17:14:36 2016 +0900
summary: Bug 1177943 - Part 1. Add LookUpDictionary method to widget. r=masayuki
So it looks like something in Bug 1177943 introduced this.
Reporter | ||
Updated•8 years ago
|
Priority: -- → P1
Comment 1•8 years ago
|
||
'console is not defined' is completely unrelated but it would be great to fix it too :) bug 1275330
Reporter | ||
Comment 2•8 years ago
|
||
kmag seems to have found the culprit as discussed at https://bugzil.la/1177943#c67
Comment 3•8 years ago
|
||
[Tracking Requested - why for this release]:
This bug appears to impact all WebExtensions which use a background page, and has the potential to affect a large number of users and developers.
Blocks: 1177943
Severity: normal → critical
status-firefox49:
--- → affected
tracking-firefox49:
--- → ?
Comment 4•8 years ago
|
||
I've looked a bit more deeper into this and I can add more details:
- it interesting to notice that if in ext-backgroundPage.js we change:
let chromeDoc = chromeNav.document
into
let chromeDoc = chromeShell.contentViewer.DOMDocument
this seems to workaround the issue on the background page.
- the webextension test cases are not the only one that fails if I try to run them locally,
the following fails with the same exception (and it is not masked by the "undefined console" exception,
which is useful because I have been able to debug the issue in a simpler test):
gfx/tests/browser/browser_windowless_troubleshoot_crash.js
- in a debugging session using gdb on a locally compiled debug build, I've done a bit of tracing and
it looks that to execure "chromeNav.document":
- WindowlessBrowser::GetDocument calls nsWebBrowser::GetDocument
- nsWebBrowser::GetDocument calls nsDocShell::GetDocument
- nsDocShell::GetDocument calls nsContentViewer::GetDOMDocument
all it is fine until here, then the result value is processed by CallMethodHelper::GatherAndConvertResults
and in our scenario CallMethodHelper::GetInterfaceTypeFromParam[1] is called, and this last function is where
the NS_ERROR_XPC_CANT_GET_PARAM_IFACE_INFO exception is generated.
[1] https://dxr.mozilla.org/mozilla-central/source/js/xpconnect/src/XPCWrappedNative.cpp#1503
Comment 5•8 years ago
|
||
My workaround was to change the affected line to `let chromeDoc = interfaceRequestor.getInterface(Ci.nsIDOMWindow).document;`, for what that's worth…
Comment 6•8 years ago
|
||
Applying one of the workarounds for the ext-backgroundPage.js failures, I'm now seeing a local failure in toolkit/components/extensions/test/mochitest/test_chrome_ext_downloads_misc.html, where this line throws the same NS_ERROR_XPC_CANT_GET_PARAM_IFACE_INFO exception: https://dxr.mozilla.org/mozilla-central/source/toolkit/components/extensions/ext-downloads.js#613
It's the same pattern of getting the "document" property from an nsIWebNavigation throwing that exception...
Comment 7•8 years ago
|
||
I took my turn with gdb (lldb actually) and got a little further. Building on from Luca's comment 4, I think the problem happened with this stack:
frame #0: 0x000000010272f583 XUL`xptiInterfaceEntry::GetEntryForParam(this=0x0000000110513408, methodIndex=<unavailable>, param=<unavailable>, entry=0x00007fff5fbf6370) + 179 at xptiInterfaceInfo.cpp:377
frame #1: 0x0000000102731b1b XUL`xptiInterfaceInfo::GetIIDForParamNoAlloc(unsigned short, nsXPTParamInfo const*, nsID*) [inlined] xptiInterfaceEntry::GetIIDForParamNoAlloc(this=0x0000000110513408, methodIndex=<unavailable>, param=0x00007fff5fbf63b0, iid=0x00007fff5fbf6450) + 22 at xptiInterfaceInfo.cpp:453
frame #2: 0x0000000102731b05 XUL`xptiInterfaceInfo::GetIIDForParamNoAlloc(this=<unavailable>, methodIndex=<unavailable>, param=0x00007fff5fbf63b0, iid=0x00007fff5fbf6450) + 37 at xptiprivate.h:360
frame #3: 0x0000000103134e80 XUL`CallMethodHelper::GetInterfaceTypeFromParam(this=0x00007fff5fbf64e0, paramIndex=<unavailable>, datum_type=<unavailable>, result=0x00007fff5fbf6450) const + 144 at XPCWrappedNative.cpp:1514
frame #4: 0x0000000103133808 XUL`CallMethodHelper::GatherAndConvertResults(this=0x00007fff5fbf64e0) + 504 at XPCWrappedNative.cpp:1600
A selected fragment of code is:
370 uint16_t interfaceIndex = 0;
371 nsresult rv = GetInterfaceIndexForParam(methodIndex, param,
372 &interfaceIndex);
373 if (NS_FAILED(rv)) {
374 return rv;
375 }
376
377 xptiInterfaceEntry* theEntry = mTypelib->GetEntryAt(interfaceIndex - 1);
378
379 // This can happen if a declared interface is not available at runtime.
380 if(!theEntry)
381 {
382 *entry = nullptr;
383 return NS_ERROR_FAILURE;
382 }
lldb says that when we get to line 377 interfaceIndex is 0 but I don't really trust it since that with uint16_t's, xptiTypelibGuts::GetEntryAt() will get 65535 as its parameter which should fail an assertion (https://dxr.mozilla.org/mozilla-central/source/xpcom/reflect/xptinfo/xptiTypelibGuts.cpp#40). Regardless, down at line 380, theEntry is null, and the returned NS_ERROR_FAILURE bubbles all the way back to where GetInterfaceTypeFromParam() throws.
I think my monkey-work here is done, I don't know enough about what any of this actually means to interpret it. Kris, do you want to go deeper or suggest somebody to ask?
Flags: needinfo?(kmaglione+bmo)
Assignee | ||
Comment 8•8 years ago
|
||
xptiInterfaceEntry::GetEntryForParam accesses nsIDOMDocument that is parent. But xptiInterfaceEntry::GetShimForParam accesses nsIDocShell...
Why doesn't xptiInterfaceEntry::GetShimForParam traverse parent's object?
Assignee | ||
Comment 9•8 years ago
|
||
Assignee | ||
Updated•8 years ago
|
Component: WebExtensions → XPConnect
Product: Toolkit → Core
Comment 10•8 years ago
|
||
If we can't get this fixed promptly, can we back out whatever changeset introduced this? A lot of us are wasting a lot of time on this.
Assignee | ||
Comment 11•8 years ago
|
||
When using GetIIDForParamNoAlloc to get return paramter type, if param is nsIDOM*, it should get it by GetShimForParam.
When this situation, GetEntryFor Param tries to get nsIDOMDocument, so GetEntryForParam doesn't get entry. Then, GetShimForParam tries to get entry. But since it doesn't traverse parent objects, it will try to get nsIDocShell instead.
So it might not get correct entry.
Review commit: https://reviewboard.mozilla.org/r/55450/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/55450/
Assignee | ||
Updated•8 years ago
|
Attachment #8756748 -
Attachment is obsolete: true
Assignee | ||
Updated•8 years ago
|
Attachment #8756858 -
Flags: review?(bobbyholley)
Assignee | ||
Comment 12•8 years ago
|
||
Comment 14•8 years ago
|
||
Comment on attachment 8756858 [details]
MozReview Request: Bug 1275719 - GetShimForParam should traverse parent objects. r?bholley
Heading out on PTO soon. Ehsan wrote the shim stuff so he can probably review.
Attachment #8756858 -
Flags: review?(bobbyholley) → review?(ehsan)
Updated•8 years ago
|
Flags: needinfo?(kmaglione+bmo)
Comment 15•8 years ago
|
||
confirmed that this patch does appear to fix the extension loading bug.
Updated•8 years ago
|
Component: XPConnect → XPCOM
Comment 16•8 years ago
|
||
m_kato, could you perhaps explain how bug 1177943 caused this issue?
Flags: needinfo?(m_kato)
Comment 17•8 years ago
|
||
Comment on attachment 8756858 [details]
MozReview Request: Bug 1275719 - GetShimForParam should traverse parent objects. r?bholley
nfroyd offered to review this but ehsan or erahm, feel free to jump in if you get here first.
Attachment #8756858 -
Flags: review?(ehsan) → review?(nfroyd)
Comment 18•8 years ago
|
||
(In reply to Olli Pettay [:smaug] from comment #16)
> m_kato, could you perhaps explain how bug 1177943 caused this issue?
I too would like to understand this, as the linked explanation in comment 2 points at code that's nowhere near the code that's actually identified as failing.
Comment 19•8 years ago
|
||
I looked into this a bit myself, and the problem seems to be that the interface change from bug 1177943 adds an interface to the interface directory for nsIDocShell, and changes the (invalid) location in the directory where this lookup winds up pointing from nsIDOMNode (which does have a shim interface) to nsIDocShell (which doesn't). So, rather than getting an invalid but working nsIInterfaceInfo result, we get a nullptr, and the the call fails with NS_ERROR_XPC_CANT_GET_PARAM_IFACE_INFO.
It was only working before by sheer lucky accident.
Assignee | ||
Comment 20•8 years ago
|
||
(In reply to Olli Pettay [:smaug] from comment #16)
> m_kato, could you perhaps explain how bug 1177943 caused this issue?
Although Kris points out by comment #19, since it depends on updating nsIDocShell.idl, xpt is updated. Before landing, mBaseMethodIndex is 3. But after it, mBaseMethodIndex becomes 17 on my environment. Sinice document property's methodIndex is 12 on my environment, it cannot find correct type.
I don't know why this issue doesn't occurs until my landing. This issue depends on generated xpt.
Flags: needinfo?(m_kato)
Updated•8 years ago
|
Attachment #8756858 -
Flags: review?(nfroyd) → review+
Comment 21•8 years ago
|
||
Comment on attachment 8756858 [details]
MozReview Request: Bug 1275719 - GetShimForParam should traverse parent objects. r?bholley
https://reviewboard.mozilla.org/r/55450/#review52774
Sorry for losing track of this on Friday. r=me
Comment 22•8 years ago
|
||
Comment 23•8 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
Updated•8 years ago
|
Assignee: nobody → m_kato
You need to log in
before you can comment on or make changes to this bug.
Description
•