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

RESOLVED INVALID

Status

()

Core
General
RESOLVED INVALID
7 years ago
4 years ago

People

(Reporter: int3, Unassigned)

Tracking

(Blocks: 1 bug)

unspecified
x86
All
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(e10s+)

Details

(Reporter)

Description

7 years ago
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]
(Reporter)

Updated

7 years ago
Blocks: 516755

Comment 1

7 years ago
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.

Updated

7 years ago
Component: Embedding: APIs → General
QA Contact: apis → general

Comment 2

7 years ago
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.
(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()).
tracking-e10s: --- → +
Sounds like this is invalid.
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.