358607 - Crash [@ nsCocoaWindow::GetAttention] when I call goQuitApplication()

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, defect)

Description

[Jesse Ruderman]

Calling goQuitApplication() makes Firefox crash.

Since I use goQuitApplication for automated testing, this crash makes it very hard for me to do automated testing on Gecko to find security holes, regressions that cause crashes, etc.

I initially reported this as a comment in bug 343033, but hwaara said I should split it because the stacks are different.

Comment 1

[Jesse Ruderman]

Attachment: quit-browser.html

Comment 2

[Jesse Ruderman]

The testcase needs privs, so it should be saved and loaded with a file: URL.

Comment 3

[Jesse Ruderman]

Attachment: stack trace

Comment 4

[Mats Palmgren (inactive)]

Attachment: fix?

The problem is the static gHiddenWindowMenuBar is used after the hidden
window has been destroyed.
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/widget/src/cocoa/nsCocoaWindow.mm&rev=1.72&root=/cvsroot&mark=1103,1109-1113#1102
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/widget/src/cocoa/nsCocoaWindow.mm&rev=1.72&root=/cvsroot&mark=158,160-161#157
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/toolkit/components/startup/src/nsAppStartup.cpp&rev=1.13&root=/cvsroot&mark=304#289

GetHiddenWindowMenuBar() doesn't seem to be called that often so I think
we can look it up every time.

The change in nsAppShellService.cpp is just to remove a warning:
WARNING: requested removal of nonexistent window: file nsWindowWatcher.cpp, line 1114
which happens because nsAppShellService::CreateHiddenWindow() does not
actually register the window.

Comment 5

[Håkan Waara]

Comment on attachment 244024 [details] [diff] [review]
fix?

Looks fine to me, fwiw. I was debugging this yesterday and was more or less on the same track.

I think the reason for bug 343033 is more, and related strangeness to how the hidden window menubar is used. In that bug, someone holds a strong ref to it for too long (seems to never be released, actually).

Comment 6

[Josh Aas]

Comment on attachment 244024 [details] [diff] [review]
fix?

#if defined(XP_MAC) || defined(XP_MACOSX)

When you check in the change to nsAppShellService.cpp, can you please kill off tests for XP_MAC elsewhere in the file (just leave XP_MACOSX).

Comment 7

[Mats Palmgren (inactive)]

(In reply to comment #6)
> can you please kill off
> tests for XP_MAC elsewhere in the file (just leave XP_MACOSX).

Did so and checked in to trunk at 2006-11-15 19:25 PDT.

-> FIXED