From 3fb74c33fbed74cb3ba1a5d90283cbfa23c19188 Mon Sep 17 00:00:00 2001 From: "smichaud@pobox.com" Date: Fri, 9 Nov 2007 08:16:58 -0800 Subject: [PATCH] Fix Cocoa widgets appshell workarounds for Camino crashing on exit. b=400321 r=joshmoz sr=roc a=belzner --- widget/src/cocoa/nsAppShell.h | 1 + widget/src/cocoa/nsAppShell.mm | 29 ++++++++++++++++++++++++----- widget/src/cocoa/nsWindowMap.mm | 7 ------- 3 files changed, 25 insertions(+), 12 deletions(-) diff --git a/widget/src/cocoa/nsAppShell.h b/widget/src/cocoa/nsAppShell.h index 7f73dfaad0a..3ba1f51fae9 100644 --- a/widget/src/cocoa/nsAppShell.h +++ b/widget/src/cocoa/nsAppShell.h @@ -87,6 +87,7 @@ protected: PRPackedBool mRunningEventLoop; PRPackedBool mStarted; PRPackedBool mTerminated; + PRPackedBool mNotifiedWillTerminate; PRPackedBool mSkippedNativeCallback; // mHadMoreEventsCount and kHadMoreEventsCountMax are used in diff --git a/widget/src/cocoa/nsAppShell.mm b/widget/src/cocoa/nsAppShell.mm index 5a91a4cc88e..878d96fddec 100644 --- a/widget/src/cocoa/nsAppShell.mm +++ b/widget/src/cocoa/nsAppShell.mm @@ -108,6 +108,7 @@ nsAppShell::nsAppShell() , mRunningEventLoop(PR_FALSE) , mStarted(PR_FALSE) , mTerminated(PR_FALSE) +, mNotifiedWillTerminate(PR_FALSE) , mSkippedNativeCallback(PR_FALSE) , mHadMoreEventsCount(0) , mRecursionDepth(0) @@ -140,7 +141,28 @@ nsAppShell::~nsAppShell() } [mDelegate release]; - [mMainPool release]; + // When you quit Camino with at least one window open, embedding is shut + // down (by a call to NS_TermEmbedding()) and we (the current appshell) + // are destroyed as the last browser window (class BrowserWindow) is closed. + // (The call to NS_TermEmbedding() is ultimately made from + // [BrowserWindowController windowWillClose:].) This means that some code + // runs _after_ the appshell is destroyed. This code assumes that various + // objects which have a retain count >= 1 will remain in existence, and that + // an autorelease pool will still be available. But because mMainPool sits + // so low on the autorelease stack, if we release it here there's a good + // chance that all the aforementioned objects (including the other + // autorelease pools) will be released, and havoc will result. + // + // So if we've been terminated using [NSApplication terminate:] (which + // Camino always uses), we don't release mMainPool here. This won't cause + // leaks, because after [NSApplication terminate:] sends an + // NSApplicationWillTerminate notification it calls [NSApplication + // _deallocHardCore:], which (after it uses [NSArray + // makeObjectsPerformSelector:] to close all remaining windows) calls + // [NSAutoreleasePool releaseAllPools] (to release all autorelease pools + // on the current thread, which is the main thread). + if (!mNotifiedWillTerminate) + [mMainPool release]; } // Init @@ -305,14 +327,11 @@ nsAppShell::ProcessGeckoEvents(void* aInfo) void nsAppShell::WillTerminate() { + mNotifiedWillTerminate = PR_TRUE; if (mTerminated) return; mTerminated = PR_TRUE; - // Ugly hack to stop _NSAutoreleaseNoPool errors on shutdown from Camino -- - // these seem to be triggered by our call here to NS_ProcessPendingEvents(). - [[NSAutoreleasePool alloc] init]; - // Calling [NSApp terminate:] causes (among other things) an // NSApplicationWillTerminate notification to be posted and the main run // loop to die before returning (in the call to [NSApp run]). So this is diff --git a/widget/src/cocoa/nsWindowMap.mm b/widget/src/cocoa/nsWindowMap.mm index 4337eb12d47..e5e55eb7cc9 100644 --- a/widget/src/cocoa/nsWindowMap.mm +++ b/widget/src/cocoa/nsWindowMap.mm @@ -217,18 +217,11 @@ - (void)windowWillClose:(NSNotification*)inNotification { - // We can get _NSAutoreleaseNoPool errors on shutdown here (from Camino) - // unless we use a local pool -- possibly triggered by a call we're now - // making to NS_ProcessPendingEvents() from nsAppShell::WillTerminate(). - NSAutoreleasePool* localPool = [[NSAutoreleasePool alloc] init]; - // postpone our destruction [[self retain] autorelease]; // remove ourselves from the window map (which owns us) [[WindowDataMap sharedWindowDataMap] removeDataForWindow:[inNotification object]]; - - [localPool release]; } @end