Last Comment Bug 355273 - Crash [@nsMacWindow::WindowEventHandler] when selecting Quit from Dock menu while modal javascript dialog displayed
: Crash [@nsMacWindow::WindowEventHandler] when selecting Quit from Dock menu w...
Status: VERIFIED FIXED
: crash, regression, verified1.8.1.8
Product: Core Graveyard
Classification: Graveyard
Component: Widget: Mac (show other bugs)
: 1.8 Branch
: x86 Mac OS X
: -- critical (vote)
: ---
Assigned To: Mats Palmgren (:mats)
:
Mentors:
: 377350 (view as bug list)
Depends on: 394405
Blocks: 355097
  Show dependency treegraph
 
Reported: 2006-10-03 12:11 PDT by Ben Turner (not reading bugmail, use the needinfo flag!)
Modified: 2011-06-09 14:58 PDT (History)
11 users (show)
mconnor: blocking1.8.1-
mozillamarcia.knous: in‑litmus+
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Stack (6.43 KB, text/plain)
2006-10-03 12:17 PDT, Ben Turner (not reading bugmail, use the needinfo flag!)
no flags Details
Apple report (36.89 KB, text/plain)
2006-10-03 17:43 PDT, Marcia Knous [:marcia - use ni]
no flags Details
Patch rev. 1 (6.08 KB, patch)
2007-07-16 10:17 PDT, Mats Palmgren (:mats)
jaas: review+
roc: superreview+
dveditz: approval1.8.1.8+
Details | Diff | Review

Description Ben Turner (not reading bugmail, use the needinfo flag!) 2006-10-03 12:11:23 PDT
Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.1) Gecko/20061003 BonEcho/2.0

STR:

1. Type 'javascript: alert(1)' in the URL bar (without the single quotes).
2. Click and hold the mouse on the BonEcho icon in the dock so that the context menu appears.
3. Select 'Quit'.
4. BonEcho will crash.

This does not happen by selecting the 'Quit' menu item from the menu bar, nor when hitting cmd-Q on the keyboard.
Comment 1 Ben Turner (not reading bugmail, use the needinfo flag!) 2006-10-03 12:17:25 PDT
Created attachment 241097 [details]
Stack

The JS here is most likely nsCloseAllWindows.js:76, as that's what I've run into on my XULRunner debug build.
Comment 2 Ben Turner (not reading bugmail, use the needinfo flag!) 2006-10-03 12:33:49 PDT
Requesting blocking to get drivers' assessment.
Comment 3 Ben Turner (not reading bugmail, use the needinfo flag!) 2006-10-03 12:44:22 PDT
This doesn't happen on Firefox 1.5. Regression.
Comment 4 Ben Turner (not reading bugmail, use the needinfo flag!) 2006-10-03 13:52:21 PDT
In XULRunner I'm crashing here:

http://lxr.mozilla.org/mozilla1.8/source/widget/src/mac/nsMacWindow.cpp#926

dereferencing a null pointer. It's weird, though, because 'self' and 'mMacEventHandler' are both valid.
Comment 5 Adam Guthrie 2006-10-03 14:07:19 PDT
I'll bet self->mMacEventHandler isn't, though. ;)
Comment 6 Ben Turner (not reading bugmail, use the needinfo flag!) 2006-10-03 14:15:55 PDT
(In reply to comment #5)
> I'll bet self->mMacEventHandler isn't, though. ;)

It is. That's why it's weird. ;)

I think this stack may either be wrong or we're looking at some sort of race condition. If I break here I can't reproduce the crash. And I've seen some instances where the app shuts down normally even when I'm not in the debugger.
Comment 7 Marcia Knous [:marcia - use ni] 2006-10-03 17:43:35 PDT
Created attachment 241135 [details]
Apple report

Here is the apple crash data from my PPC crash using RC2 candidate, in case it helps.
Comment 8 Jesse Ruderman 2006-10-03 19:55:19 PDT
I think this is the same as bug 355097.
Comment 9 Josh Aas 2006-10-04 09:55:41 PDT
I did some debugging and when you quit with a sheet open like this we are hiding and destroying the sheet after we hide and destroy its parent window.

One way to stop this crash is to hide any sheet children of the parent before hiding the parent. However, while we may want to do that I don't think that code would ever get used if this was handled correctly - we shouldn't be quitting when we have sheets up.
Comment 10 Mike Connor [:mconnor] 2006-10-04 10:12:36 PDT
Not a topcrash, very corner case STR, not going to block on this, 1.8.1.1 possibly
Comment 11 Daniel Veditz [:dveditz] 2006-12-15 11:05:26 PST
If we get a fix please ask for branch approval, but not looking hopeful.
Comment 12 Jesse Ruderman 2007-03-22 15:47:12 PDT
WFM on trunk.  Selecting "Quit" from the dock icon while there's a modal alert() dialog open just makes Firefox beep.
Comment 13 Ben Turner (not reading bugmail, use the needinfo flag!) 2007-03-22 17:31:07 PDT
(In reply to comment #12)

Yeah, I don't think I ever saw this on trunk.
Comment 14 Henrik Skupin (:whimboo) 2007-04-13 14:26:55 PDT
*** Bug 377350 has been marked as a duplicate of this bug. ***
Comment 15 Henrik Skupin (:whimboo) 2007-04-13 14:29:52 PDT
Same happens for Thunderbird when closing the account wizard after opening a compose window while no account was created before. See bug 377350.
Comment 16 Henrik Skupin (:whimboo) 2007-04-13 16:08:25 PDT
Mark, the line where Thunderbird maybe crashes comes from your patch on bug 345564. There you reimported the code which was removed on bug 340592. Does it have something to do with the crash?
Comment 17 Mats Palmgren (:mats) 2007-07-16 10:17:53 PDT
Created attachment 272505 [details] [diff] [review]
Patch rev. 1

We get calls to nsMacWindow::WindowEventHandler() on a destroyed window.
Don't ask me why because that shouldn't happen since we call
::DisposeWindow() in the destructor - there shouldn't be any callbacks
after that, but there is.  Explicitly deregistering the event handlers
fixes it (it also seems to fix bug 355097).
The "mMacEventHandler.reset(nsnull)" isn't needed to fix this bug,
it's just a safe-guard in case we have more use-after-free issues...
Comment 18 Jesse Ruderman 2007-07-16 11:24:37 PDT
Where is DisposeWindow defined and implemented?  lxr can't find it.
Comment 19 Mats Palmgren (:mats) 2007-07-16 12:44:27 PDT
I think it's this one:
http://developer.apple.com/documentation/mac/Toolbox/Toolbox-258.html
Comment 20 Josh Aas 2007-07-17 14:07:03 PDT
Comment on attachment 272505 [details] [diff] [review]
Patch rev. 1

+  , mScrollEventHandler(0)
+  , mWindowEventHandler(0)

For consistency, please set these to NULL (not 0 or nsnull, which we use for gecko object pointers, this distinction being used for readability).
Comment 21 Mats Palmgren (:mats) 2007-07-22 06:29:31 PDT
Comment on attachment 272505 [details] [diff] [review]
Patch rev. 1

sr for branches please.
Comment 22 Daniel Veditz [:dveditz] 2007-07-23 19:34:41 PDT
Comment on attachment 272505 [details] [diff] [review]
Patch rev. 1

Only approving blocking bugs for 1.8.1.6
Comment 23 Daniel Veditz [:dveditz] 2007-08-29 15:45:59 PDT
Comment on attachment 272505 [details] [diff] [review]
Patch rev. 1

approved for 1.8.1.7, a=dveditz for release-drivers
Comment 24 Mats Palmgren (:mats) 2007-08-30 18:31:35 PDT
MOZILLA_1_8_BRANCH
mozilla/widget/src/mac/nsMacWindow.cpp 	1.158.2.29
mozilla/widget/src/mac/nsMacWindow.h 	1.58.2.9 

-> FIXED
Comment 25 Carsten Book [:Tomcat] 2007-09-03 15:57:42 PDT
verified fixed 1.8.1.7 using Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.1.7pre) Gecko/2007090303 BonEcho/2.0.0.7pre

no crash on steps to reproduce from this bug - adding verified keyword
Comment 26 Henrik Skupin (:whimboo) 2007-12-29 06:52:15 PST
Works fine with latest 1.8 branch builds. For Firefox 3 it's not possible anymore to close the application over the dock. I filed bug 410170 to cover this issue.
Comment 27 Marcia Knous [:marcia - use ni] 2008-03-14 12:05:27 PDT
https://litmus.mozilla.org/show_test.cgi?id=5202 has been added to the 2.0 test suite, 3.0 pending depending on behavior change.

Note You need to log in before you can comment on or make changes to this bug.