Closed Bug 811394 Opened 9 years ago Closed 9 years ago
Style Editor window close exits Firefox
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.
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.
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
Component: Developer Tools: Style Editor → DOM
Product: Firefox → Core
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.
Try run pending at https://tbpl.mozilla.org/?tree=Try&rev=858c850cbaa2
Attachment #681855 - Flags: review?(bugs)
email@example.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]
Thanks for the quick review! http://hg.mozilla.org/integration/mozilla-inbound/rev/6e9e10baf0dd
Whiteboard: [need review]
Target Milestone: --- → mozilla19
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.