Closed Bug 1275719 Opened 8 years ago Closed 8 years ago

Error loading web extensions on recent builds

Categories

(Core :: XPCOM, defect, P1)

defect

Tracking

()

RESOLVED FIXED
mozilla49
Tracking Status
firefox49 + fixed

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.
Priority: -- → P1
'console is not defined' is completely unrelated but it would be great to fix it too :) bug 1275330
kmag seems to have found the culprit as discussed at https://bugzil.la/1177943#c67
[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
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
My workaround was to change the affected line to `let chromeDoc = interfaceRequestor.getInterface(Ci.nsIDOMWindow).document;`, for what that's worth…
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...
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)
xptiInterfaceEntry::GetEntryForParam accesses nsIDOMDocument that is parent. But xptiInterfaceEntry::GetShimForParam accesses nsIDocShell... Why doesn't xptiInterfaceEntry::GetShimForParam traverse parent's object?
Attached patch Trace parent object on GetShimForParam (obsolete) (deleted) — Splinter Review
Component: WebExtensions → XPConnect
Product: Toolkit → Core
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.
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/
Attachment #8756748 - Attachment is obsolete: true
Attachment #8756858 - Flags: review?(bobbyholley)
Tracking for 49 based on the potential to affect a large # of users.
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)
Flags: needinfo?(kmaglione+bmo)
confirmed that this patch does appear to fix the extension loading bug.
Component: XPConnect → XPCOM
m_kato, could you perhaps explain how bug 1177943 caused this issue?
Flags: needinfo?(m_kato)
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)
(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.
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.
(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)
Attachment #8756858 - Flags: review?(nfroyd) → review+
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
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
Assignee: nobody → m_kato
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: