Closed
Bug 1067340
Opened 8 years ago
Closed 8 years ago
content process crash in nsDOMWindowUtils::GetCurrentInnerWindowID
Categories
(Core :: DOM: Security, defect)
Tracking
()
VERIFIED
FIXED
mozilla35
Tracking | Status | |
---|---|---|
e10s | ? | --- |
People
(Reporter: kairo, Assigned: ehsan.akhgari)
References
Details
(Keywords: crash)
Crash Data
Attachments
(1 file, 2 obsolete files)
This bug was filed from the Socorro interface and is report bp-b292686d-5758-4462-acfa-e103b2140915. ============================================================= Top frames: 0 xul.dll nsDOMWindowUtils::GetCurrentInnerWindowID(unsigned __int64*) dom/base/nsDOMWindowUtils.cpp 1 xul.dll getInnerWindowID(nsIRequest*) content/base/src/nsCSPContext.cpp 2 xul.dll nsCSPContext::SetRequestContext(nsIURI*, nsIURI*, nsIChannel*) content/base/src/nsCSPContext.cpp 3 xul.dll nsDocument::InitCSP(nsIChannel*) content/base/src/nsDocument.cpp 4 xul.dll nsDocument::StartDocumentLoad(char const*, nsIChannel*, nsILoadGroup*, nsISupports*, nsIStreamListener**, bool, nsIContentSink*) content/base/src/nsDocument.cpp 5 xul.dll nsHTMLDocument::StartDocumentLoad(char const*, nsIChannel*, nsILoadGroup*, nsISupports*, nsIStreamListener**, bool, nsIContentSink*) content/html/document/src/nsHTMLDocument.cpp 6 xul.dll nsContentDLF::CreateDocument(char const*, nsIChannel*, nsILoadGroup*, nsIDocShell*, nsID const&, nsIStreamListener**, nsIContentViewer**) layout/build/nsContentDLF.cpp 7 xul.dll nsContentDLF::CreateInstance(char const*, nsIChannel*, nsILoadGroup*, char const*, nsIDocShell*, nsISupports*, nsIStreamListener**, nsIContentViewer**) layout/build/nsContentDLF.cpp 8 xul.dll nsDocShell::NewContentViewerObj(char const*, nsIRequest*, nsILoadGroup*, nsIStreamListener**, nsIContentViewer**) docshell/base/nsDocShell.cpp 9 xul.dll nsDocShell::CreateContentViewer(char const*, nsIRequest*, nsIStreamListener**) docshell/base/nsDocShell.cpp 10 xul.dll nsDSURIContentListener::DoContent(char const*, bool, nsIRequest*, nsIStreamListener**, bool*) docshell/base/nsDSURIContentListener.cpp This crash started rising on Nightly 35 with the 20140911064110 (so regression window is on 9/10). For stats and more reports see https://crash-stats.mozilla.com/report/list?signature=nsDOMWindowUtils%3A%3AGetCurrentInnerWindowID%28unsigned%20__int64%2A%29 Given that it's in CSP code, I'm trying DOM:Security for now as a component.
![]() |
Reporter | |
Comment 1•8 years ago
|
||
This is the first build after bug 1064885 so I guess any regression range I can get will point to this as the cause and this is an e10s regression. Also note that those crashes are all in content processes.
Summary: crash in nsDOMWindowUtils::GetCurrentInnerWindowID(unsigned __int64*) → content process crash in nsDOMWindowUtils::GetCurrentInnerWindowID
We're calling ExtractScheme on a null sIOService.
Blocks: 1061136
Flags: needinfo?(bobbyholley)
Sorry, wrong bug.
No longer blocks: 1061136
Flags: needinfo?(bobbyholley)
MOZ_RELEASE_ASSERT(nsContentUtils::IsCallerChrome()) at nsDOMWindowUtils::GetCurrentInnerWindowID
Flags: needinfo?(ehsan.akhgari)
Assignee | ||
Comment 5•8 years ago
|
||
Taking.
Assignee: nobody → ehsan.akhgari
tracking-e10s:
--- → ?
Component: DOM: Security → DOM
Flags: needinfo?(ehsan.akhgari)
Assignee | ||
Comment 6•8 years ago
|
||
Assignee | ||
Updated•8 years ago
|
Attachment #8490067 -
Flags: review?(bobbyholley)
Comment 7•8 years ago
|
||
Comment on attachment 8490067 [details] [diff] [review] Revert the caller chrome assertion in nsDOMWindowUtils::GetOuterWindowID because the function now has non-JS callers; r=bholley IMO C++ callers shouldn't be using DOMWindowUtils. This caller (nsCSPContext) should just QI the nsIDOMWindow to nsPIDOMWindow, and invoke GetOuterWindow()->WindowID(). If there's some reason we can't do that, this fix is fine FWIW.
Attachment #8490067 -
Flags: review?(bobbyholley) → review-
Assignee | ||
Comment 8•8 years ago
|
||
Assignee | ||
Updated•8 years ago
|
Attachment #8490067 -
Attachment is obsolete: true
Assignee | ||
Updated•8 years ago
|
Component: DOM → DOM: Security
Assignee | ||
Updated•8 years ago
|
Attachment #8490442 -
Flags: review?(sstamm)
Comment 9•8 years ago
|
||
Comment on attachment 8490442 [details] [diff] [review] Don't use nsIDOMWindowUtils in the CSP code, because it's not meant to be used from C++; r=geekboy Review of attachment 8490442 [details] [diff] [review]: ----------------------------------------------------------------- ::: content/base/src/nsCSPContext.cpp @@ -529,5 @@ > - if (NS_FAILED(rv)) { > - return 0; > - } > - > - return id; I think this changes the window ID that we're grabbing here for console messaging. Looking at the implementation of nsIDOMWindowUtils::GetCurrentInnerWindowID(), I notice that the inner window is fetched from the nsPIDOMWindow (not the outer window). While I think this is probably going to work (based on a scan of the implementation of nsIScriptError::InitWithWindowID), it's referred to as inner window everywhere and that seems wrong to me if we're passing around an outer window ID. Ehsan, you probably know more about how this works than me, but I'd like the names of the variables and what they contain to match, at least in the CSP code. Please either grab the inner window or replace all the references to "innerWindowID" in nsCSPContext.cpp/h and nsCSPUtils.cpp/h with either "outerWindowID" or the more generic "windowID".
Comment 10•8 years ago
|
||
Of course the context included by splinter is not helpful in comment 9. Here's the relevant context: > - nsCOMPtr<nsIDOMWindowUtils> du = do_GetInterface(window); > - rv = du->GetCurrentInnerWindowID(&id); > - if (NS_FAILED(rv)) { > - return 0; > - } > - return id; > + nsCOMPtr<nsPIDOMWindow> pwindow = do_QueryInterface(window); > + return pwindow->GetOuterWindow()->WindowID();
Assignee | ||
Comment 11•8 years ago
|
||
Comment on attachment 8490442 [details] [diff] [review] Don't use nsIDOMWindowUtils in the CSP code, because it's not meant to be used from C++; r=geekboy Sure, let's use the inner window.
Attachment #8490442 -
Attachment is obsolete: true
Attachment #8490442 -
Flags: review?(sstamm)
Assignee | ||
Comment 12•8 years ago
|
||
Assignee | ||
Updated•8 years ago
|
Attachment #8490900 -
Flags: review?(sstamm)
Comment 13•8 years ago
|
||
Comment on attachment 8490900 [details] [diff] [review] Don't use nsIDOMWindowUtils in the CSP code, because it's not meant to be used from C++; r=geekboy Review of attachment 8490900 [details] [diff] [review]: ----------------------------------------------------------------- Looks good.
Attachment #8490900 -
Flags: review?(sstamm) → review+
Comment 14•8 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/46859d329455 https://hg.mozilla.org/mozilla-central/rev/40efa924899c
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
Comment 15•8 years ago
|
||
Socorro [1] shows only 3 crashes over the past 4 weeks, and all of them are on Firefox 33 Beta. [1] - https://crash-stats.mozilla.com/report/list?product=Firefox&range_unit=days&range_value=28&signature=nsDOMWindowUtils%3A%3AGetCurrentInnerWindowID%28unsigned+__int64%2A%29
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•