Closed Bug 811394 Opened 10 years ago Closed 10 years ago

Style Editor window close exits Firefox


(Core :: DOM: Core & HTML, defect)

19 Branch
Not set



Tracking Status
firefox19 - ---


(Reporter: cheesypoof2020, Assigned: bzbarsky)



(Keywords: regression)


(1 file)

User Agent: Mozilla/5.0 (Windows NT 6.1; rv:19.0) Gecko/19.0 Firefox/19.0
Build ID: 20121113030658

Steps to reproduce:

Click the close button on a 'Style Editor' window.

Actual results:

Firefox stops running.

Expected results:

Only the 'Style Editor' window should have closed.
Component: Untriaged → Developer Tools: Style Editor
Ever confirmed: true
Regression range:


My guess goes to bug 806996.
Keywords: regression
Regression window(m-i)
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:19.0) Gecko/19.0 Firefox/19.0 ID:20121109074532
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:19.0) Gecko/19.0 Firefox/19.0 ID:20121109080131

Regressed by Bug 807226
Blocks: 807226
Component: Developer Tools: Style Editor → DOM
Product: Firefox → Core
OS: Windows 7 → All
WARNING: requested removal of nonexistent window: file ../../../../../mozilla/embedding/components/windowwatcher/src/nsWindowWatcher.cpp, line 1250

That's my first guess so far...
OK, so StyleEditor.jsm has an unload handler that calls window.close().  This is getting invoked when we click the close button on the window titlebar, and calls close() on that same window, which ends up reentering nsXULWindow::Destroy, like so:

#0  nsXULWindow::Destroy (this=0x10ed60570) at nsXULWindow.cpp:409
#1  0x000000010410ab67 in nsWebShellWindow::Destroy (this=0x10ed60570) at nsWebShellWindow.cpp:757
#2  0x00000001040ec259 in nsChromeTreeOwner::Destroy (this=0x10e6a3dd0) at nsChromeTreeOwner.cpp:348
#3  0x00000001040ec28c in non-virtual thunk to nsChromeTreeOwner::Destroy() () at nsChromeTreeOwner.cpp:349
#4  0x0000000103614261 in nsGlobalWindow::ReallyCloseWindow (this=0x121645000) at nsGlobalWindow.cpp:6663
#5  0x0000000103613de2 in nsGlobalWindow::FinalClose (this=0x121645000) at nsGlobalWindow.cpp:6610
#6  0x0000000103613be5 in nsGlobalWindow::Close (this=0x121645000) at nsGlobalWindow.cpp:6553
#7  0x000000010361395c in nsGlobalWindow::Close (this=0x121676c00) at nsGlobalWindow.cpp:6488

and then we presumably lose track of whatever magic stuff keeps track of number of open windows and quits the app when the last window closes...

The only reason this used to work is that the unload handler never got called before, because by the time we got there we no longer had an nsIScriptContet.

Paul, I'm not sure what this unload handler is doing, and whether it _really_ expects to be called, but figured you might want to know.
Actually, I'm wrong.  The handler _did_ get called before.  But we used to run the onunload on the JSContext of the window, which means that in nsGlobalWindow::FinalClose we discovered that we're running on the window's context and set a termination function instead of closing for real.  And by the time the termination function ran we'd finished the nsXULWindow::Destroy and hence things were ok.

Oh, and of course the caller is chrome, so nsGlobalWindow::FinalClose won't do async close in this case....

I guess I should just fix nsWebShellWindow::Destroy to handle recursion.
Actually, if the unload handler had dispatched an event to some other window, which had then turned around and called close() on the first window, possibly with a sync XHR in there to make sure there's a null on the JSContext stack, I bet this could have happened even in the old setup.  So yeah, just have to fix nsWebShellWindow., thank you for finding and reporting this!  And thank you Loic  and Alice0775 for hunting down what caused the problem.
Assignee: nobody → bzbarsky
Whiteboard: [need review]
Attachment #681855 - Flags: review?(bugs) → review+
Thanks for the quick review!
Flags: in-testsuite?
Whiteboard: [need review]
Target Milestone: --- → mozilla19
Closed: 10 years ago
Resolution: --- → FIXED
Already in 19, no need to track.
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.