Closed
Bug 749964
Opened 12 years ago
Closed 12 years ago
crash in nsWebBrowserPersist::SaveDocumentInternal
Categories
(Core Graveyard :: Embedding: APIs, defect)
Tracking
(firefox15 wontfix, firefox16- wontfix, firefox17- verified, firefox18 verified)
VERIFIED
FIXED
mozilla18
People
(Reporter: scoobidiver, Assigned: bzbarsky)
References
Details
(Keywords: crash, regression, testcase)
Crash Data
Attachments
(3 files)
2nd part, do no open this one directly, it needs to be opened from a script or it can't close itself
334 bytes,
text/html
|
Details | |
409 bytes,
text/html
|
Details | |
1.25 KB,
patch
|
khuey
:
review+
akeybl
:
approval-mozilla-aurora+
akeybl
:
approval-mozilla-beta-
|
Details | Diff | Splinter Review |
It first appeared in 15.0a1/20120427. The regression range might be: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=cc5254f9825f&tochange=450d8cd16316 Signature nsWebBrowserPersist::SaveDocumentInternal(nsIDOMDocument*, nsIURI*, nsIURI*) More Reports Search UUID 71515b0e-7192-4f92-8dd8-c1d6a2120428 Date Processed 2012-04-28 03:54:40 Uptime 51 Last Crash 4.5 weeks before submission Install Age 9.5 hours since version was first installed. Install Time 2012-04-27 18:26:05 Product Firefox Version 15.0a1 Build ID 20120427030500 Release Channel nightly OS Windows NT OS Version 6.1.7601 Service Pack 1 Build Architecture x86 Build Architecture Info GenuineIntel family 6 model 42 stepping 7 Crash Reason EXCEPTION_ACCESS_VIOLATION_READ Crash Address 0x38 App Notes AdapterVendorID: 0x8086, AdapterDeviceID: 0x0116, AdapterSubsysID: 1670103c, AdapterDriverVersion: 8.830.6.3000 D3D10 Layers? D3D10 Layers- D3D9 Layers? D3D9 Layers+ EMCheckCompatibility True Total Virtual Memory 4294836224 Available Virtual Memory 3880538112 System Memory Use Percentage 41 Available Page File 5484793856 Available Physical Memory 2487209984 Frame Module Signature Source 0 xul.dll nsWebBrowserPersist::SaveDocumentInternal embedding/components/webbrowserpersist/src/nsWebBrowserPersist.cpp:1538 1 xul.dll nsWebBrowserPersist::SaveDocument embedding/components/webbrowserpersist/src/nsWebBrowserPersist.cpp:475 2 xul.dll NS_InvokeByIndex_P xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp:102 3 xul.dll XPCWrappedNative::CallMethod js/xpconnect/src/XPCWrappedNative.cpp:2408 4 xul.dll XPC_WN_CallMethod js/xpconnect/src/XPCWrappedNativeJSOps.cpp:1549 5 mozjs.dll js::GetPropertyOperation js/src/jsinterpinlines.h:266 6 mozjs.dll js::Interpret js/src/jsinterp.cpp:2757 7 mozjs.dll JSScript::makeAnalysis js/src/jsinfer.cpp:5372 8 mozjs.dll js::RunScript js/src/jsinterp.cpp:475 9 mozjs.dll js::InvokeKernel js/src/jsinterp.cpp:535 10 mozjs.dll js::Invoke js/src/jsinterp.cpp:567 11 mozjs.dll JS_CallFunctionValue js/src/jsapi.cpp:5416 12 xul.dll nsJSContext::CallEventHandler dom/base/nsJSEnvironment.cpp:1889 13 xul.dll nsJSEventListener::HandleEvent dom/src/events/nsJSEventListener.cpp:225 14 xul.dll nsScriptSecurityManager::GetPrincipalAndFrame caps/src/nsScriptSecurityManager.cpp:2394 15 xul.dll nsTArray<XPCJSContextInfo,nsTArrayInfallibleAllocator>::AppendElements<JSContext obj-firefox/dist/include/nsTArray.h:899 16 xul.dll XPCJSContextStack::Push js/xpconnect/src/XPCThreadContext.cpp:128 17 xul.dll nsCxPusher::Push content/base/src/nsContentUtils.cpp:2935 18 xul.dll nsCxPusher::RePush content/base/src/nsContentUtils.cpp:2963 19 xul.dll nsEventTargetChainItem::HandleEventTargetChain content/events/src/nsEventDispatcher.cpp:349 20 xul.dll nsWindowRoot::PreHandleEvent dom/base/nsWindowRoot.cpp:194 21 mozjs.dll JS_FrameIterator js/src/jsdbgapi.cpp:507 22 xul.dll nsEventDispatcher::DispatchDOMEvent content/events/src/nsEventDispatcher.cpp:747 23 xul.dll nsINode::DispatchEvent content/base/src/nsGenericElement.cpp:1165 24 xul.dll nsContentUtils::DispatchXULCommand content/base/src/nsContentUtils.cpp:5844 More reports at: https://crash-stats.mozilla.com/report/list?signature=nsWebBrowserPersist%3A%3ASaveDocumentInternal%28nsIDOMDocument*%2C+nsIURI*%2C+nsIURI*%29
I believe this can be triggered by trying to save a pop-up that is about to be closed just in the right moment, i.e. a race condition, sadly I had no luck reproducing it another time yet (tried about 10 times). My original crash was bp-4d7f1e6c-04eb-4ef8-ae40-6ba3e2120820.
Oh, now I got it, the pop-up needs to open in a tab, e.g. because you have browser.link.open_newwindow.restriction set to 0. Now I can reliable reproduce it on a clean profile: bp-1780b256-2332-4829-91e5-f5b1e2120820 The save dialog does not close when the tab gets automatically closed, unlike in the pop-up case, so making it close in the tab case too would be a good fix? Good candidate for a first patch?
Attachment #653335 -
Attachment mime type: text/plain → text/html
Open this, let the "pop-up" open (but as a tab by having the appropriate about:config entries set, like browser.link.open_newwindow.restriction set to 0), quickly switch to it and do a "Save Page As…" before it closes, and click OK in the "Save As" file-picker dialog only after the tab has closed itself.
Comment on attachment 653338 [details]
1st part, open this
Huh? Is this some sort of security feature that it will ignore me when I explicitly set the MIME type to text/html the first time?
Attachment #653338 -
Attachment mime type: text/plain → text/html
Comment 6•12 years ago
|
||
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:17.0) Gecko/17.0 Firefox/17.0 ID:20121005042010 I tried to download attachment 668421 [details] and hit the same crash: bp-37f765eb-d2e3-44ca-969a-0346e2121005
Keywords: regressionwindow-wanted
OS: Windows 7 → All
Comment 7•12 years ago
|
||
It's reproducible all the time with the above mentioned attachment. Marcia, would you mind to check how often this crash occurs at the moment?
tracking-firefox17:
--- → ?
Assignee | ||
Comment 8•12 years ago
|
||
What happens here is that the "aDocument" argument passed to nsWebBrowserPersist::SaveDocument is some random non-document JS object. XPConnect happily makes up an nsIDOMDocument, but then we QI it to nsIDocument, get null, and crash.
Assignee | ||
Comment 9•12 years ago
|
||
Ah, looks like the JSObject passed in is a proxy. The proxy handler is js::DeadObjectProxy. So this is actually sort of fallout from bug 695480, which is why it appears in 15. I'll add the null-check here, but I wonder whether we should refuse to create XPCWrappedJS around js::DeadObjectProxy instances!
Blocks: hueyfix
Assignee | ||
Comment 10•12 years ago
|
||
Note that this will cause the saveDocument calls to throw, by the way...
(In reply to Boris Zbarsky (:bz) from comment #9) > I'll add the null-check here, but I wonder whether we should refuse to > create XPCWrappedJS around js::DeadObjectProxy instances! Probably. We should probably also make nsIDOMDocument builtinclass.
Assignee | ||
Comment 12•12 years ago
|
||
Yes, I was assuming there was some deep reason why it wasn't yet....
There might be! Node isn't builtinclass either.
Assignee | ||
Comment 14•12 years ago
|
||
Attachment #668714 -
Flags: review?(khuey)
Assignee | ||
Comment 15•12 years ago
|
||
Marcia, I hope you don't mind me stealing this. ;)
Assignee: mozillamarcia.knous → bzbarsky
Whiteboard: [need review]
Attachment #668714 -
Flags: review?(khuey) → review+
Assignee | ||
Comment 16•12 years ago
|
||
Comment on attachment 668714 [details] [diff] [review] Don't try to save if what we have is not a real document object. [Approval Request Comment] Bug caused by (feature/regressing bug #): hueyfix User impact if declined: Random crashes when trying to save self-closing moles, er, windows. Testing completed (on m-c, etc.): Patched build doesn't crash on the testcases in this bug. Instead it shows an alert that it could not save the document. Risk to taking this patch (and alternatives if risky): Low risk. Adds a null-check and exception instead of dereferencing null and crashing. String or UUID changes made by this patch: None.
Attachment #668714 -
Flags: approval-mozilla-beta?
Attachment #668714 -
Flags: approval-mozilla-aurora?
Assignee | ||
Comment 17•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/1c64ccca6860
Status: NEW → RESOLVED
Closed: 12 years ago
Keywords: regressionwindow-wanted
Resolution: --- → FIXED
Whiteboard: [need review]
Target Milestone: --- → mozilla18
Assignee | ||
Updated•12 years ago
|
status-firefox15:
--- → wontfix
status-firefox16:
--- → affected
status-firefox17:
--- → affected
tracking-firefox16:
--- → ?
Comment 18•12 years ago
|
||
Rank is #207 in the topcrasher list for Firefox 15 and #158 for Firefox 16.0b6. Is it something we could check with a crash test or is it not worth the effort?
Updated•12 years ago
|
Comment 19•12 years ago
|
||
Comment on attachment 668714 [details] [diff] [review] Don't try to save if what we have is not a real document object. [Triage Comment] Approving for Aurora 17 given this is just a null check fix for a low volume crasher. Please land early Monday PT to make the merge.
Attachment #668714 -
Flags: approval-mozilla-beta?
Attachment #668714 -
Flags: approval-mozilla-beta-
Attachment #668714 -
Flags: approval-mozilla-aurora?
Attachment #668714 -
Flags: approval-mozilla-aurora+
Assignee | ||
Comment 20•12 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/1ae019103173 Alex, is this wontfix for 16 then?
Comment 21•12 years ago
|
||
(In reply to Boris Zbarsky (:bz) from comment #20) > https://hg.mozilla.org/releases/mozilla-aurora/rev/1ae019103173 > > Alex, is this wontfix for 16 then? Yep - updated the status flag.
Comment 22•12 years ago
|
||
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/17.0 Firefox/17.0 http://bit.ly/WzbcSp No crashes in 17beta1 and a nice error message is displayed when saving the testcases which previously crashed the browser in F16.
Comment 23•12 years ago
|
||
Virgil, please keep the verifyme keyword on this bug until it's been verified across all fixed branches.
Keywords: verifyme
Comment 24•12 years ago
|
||
(In reply to Virgil Dicu [:virgil] [QA] from comment #22) > Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/17.0 Firefox/17.0 > > http://bit.ly/WzbcSp > > No crashes in 17beta1 and a nice error message is displayed when saving the > testcases which previously crashed the browser in F16. Verified fixed on Mozilla/5.0 (Windows NT 6.1; WOW64; rv:18.0) Gecko/18.0 Firefox/18.0b1
Updated•5 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•