674313 - [e10s] nsIWindowWatcher::openWindow crosses chrome->content boundary

Status: RESOLVED INVALID

(Core :: General, defect)

Description

[Jez Ng [:int3]]

The dynamic analysis patch in bug 666713 seems to suggest that openWindow violates the chrome/content separation.

STRs:
1. Open 2 windows, then quit Firefox
2. Reopen Firefox and restore the last session

Actual Results:
content-log.txt has the following entry:

wrapper creation (XPCWrappedNative)
object:   [xpconnect wrapped nsIWebProgress @ 0x127618250 (native @ 0x1242f8888)]
0 sss_openWindowWithState() ["file:///Users/jez/src/mozilla-central/obj-ff-debug/dist/NightlyDebug.app/Contents/MacOS/components/nsSessionStore.js":3470]
1 sss_restoreLastSession() ["file:///Users/jez/src/mozilla-central/obj-ff-debug/dist/NightlyDebug.app/Contents/MacOS/components/nsSessionStore.js":1529]
2 BrowserOnClick() ["chrome://browser/content/browser.js":8083]

The line number points to the openWindow() call.

This can be verified in other ways; e.g. by launching the error console. In this case content-log.txt gives me

wrapper creation (XPCWrappedNative)
object:   [xpconnect wrapped nsIWebProgress @ 0x11ccba520 (native @ 0x11cca1898)]
0 toOpenWindowByType() ["chrome://browser/content/browser.js":8834]
1 toJavaScriptConsole() ["chrome://browser/content/browser.js":8816]
2 oncommand() ["chrome://browser/content/browser.xul":1]
3 <TOP LEVEL> ["<unknown>":0]

Comment 1

[Benjamin Smedberg]

This looks like a false positive from the analysis, although I don't quite understand it: it's reporting an nsIWebProgress, although the object in question should be nsIDOMWindow. But it's a chrome window, not a content window, so it shouldn't be triggering the warning.

Comment 2

[Drew Willcoxon :adw]

Thanks for pointing this out, Jezreel.

It looks like the logging done in xpcwrappednative.cpp's FinishCreate captures too much, i.e., non-content-related wrappers.  I'm not sure, but I think it might be logging "xpconnect wrapped nsIWebProgress" for a window, or some object related to the window that gets reflected into JS, because of the way XPCWrappedNative::ToString works.  It iterates through all the interfaces in the wrapper's XPCNativeSet.  I don't know how that set is populated, but judging from the ToString output it seems that an interface nsIFoo is added after the wrapper is QI'ed to nsIFoo.  So my guess is that this window, or some object related to the window (a docshell?), has only been used as an nsIWebProgress at the time that the wrapper creation is logged.

Comment 3

[Blake Kaplan (:mrbkap) (inactive)]

(In reply to Drew Willcoxon :adw from comment #2)
> It looks like the logging done in xpcwrappednative.cpp's FinishCreate
> captures too much, i.e., non-content-related wrappers.  I'm not sure, but I
> think it might be logging "xpconnect wrapped nsIWebProgress" for a window,
> or some object related to the window that gets reflected into JS, because of
> the way XPCWrappedNative::ToString works.  It iterates through all the
> interfaces in the wrapper's XPCNativeSet.  I don't know how that set is
> populated, but judging from the ToString output it seems that an interface
> nsIFoo is added after the wrapper is QI'ed to nsIFoo.  So my guess is that

This is exactly right. The set is populated with the list of interfaces declared in the nsIClassInfo implementation as well as additional interfaces that have been added via QI on the wrapper (in the latter case the wrapper will return true when asked HasMutatedSet()).

Comment 4

[Blake Kaplan (:mrbkap) (inactive)]

Sounds like this is invalid.