Closed Bug 811394 Opened 9 years ago Closed 9 years ago

Style Editor window close exits Firefox

Categories

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

19 Branch
x86
All
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla19
Tracking Status
firefox19 - ---

People

(Reporter: cheesypoof2020, Assigned: bzbarsky)

References

Details

(Keywords: regression)

Attachments

(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
Status: UNCONFIRMED → NEW
Ever confirmed: true
Regression range:

m-c
good=2012-11-09
bad=2012-11-10
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=90cea19e27e2&tochange=a47525b93528

My guess goes to bug 806996.
Keywords: regression
Regression window(m-i)
Good:
http://hg.mozilla.org/integration/mozilla-inbound/rev/983d136130c5
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:19.0) Gecko/19.0 Firefox/19.0 ID:20121109074532
Bad:
http://hg.mozilla.org/integration/mozilla-inbound/rev/bd9729ca4de2
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:19.0) Gecko/19.0 Firefox/19.0 ID:20121109080131
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=983d136130c5&tochange=bd9729ca4de2

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.
cheesypoof2020@yahoo.com, 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!

http://hg.mozilla.org/integration/mozilla-inbound/rev/6e9e10baf0dd
Flags: in-testsuite?
Whiteboard: [need review]
Target Milestone: --- → mozilla19
https://hg.mozilla.org/mozilla-central/rev/6e9e10baf0dd
Status: NEW → RESOLVED
Closed: 9 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.