From 85e6a3ab1dfd65071458f20ec0d0285aaf6bb3dc Mon Sep 17 00:00:00 2001 From: Date: Thu, 22 Mar 2007 16:04:51 -0700 Subject: [PATCH] fix crash, suspend native events when instantiating plugins. patch by Steven Michaud. b=345627 r=josh sr=roc --- layout/generic/nsObjectFrame.cpp | 13 +++++++++++++ widget/public/nsIAppShell.idl | 25 ++++++++++++++++++++++++- widget/src/cocoa/nsAppShell.h | 2 ++ widget/src/cocoa/nsAppShell.mm | 14 ++++++++++++-- widget/src/mac/nsAppShell.cpp | 12 +++++++++++- widget/src/mac/nsAppShell.h | 2 ++ widget/src/xpwidgets/nsBaseAppShell.cpp | 18 +++++++++++++++++- widget/src/xpwidgets/nsBaseAppShell.h | 2 ++ 8 files changed, 83 insertions(+), 5 deletions(-) diff --git a/layout/generic/nsObjectFrame.cpp b/layout/generic/nsObjectFrame.cpp index 28a1ab7bb7a4..9b9e3f30b467 100644 --- a/layout/generic/nsObjectFrame.cpp +++ b/layout/generic/nsObjectFrame.cpp @@ -58,6 +58,7 @@ #include "nsReadableUtils.h" #include "prmem.h" #include "nsGkAtoms.h" +#include "nsIAppShell.h" #include "nsIDocument.h" #include "nsINodeInfo.h" #include "nsIURL.h" @@ -719,6 +720,14 @@ nsObjectFrame::InstantiatePlugin(nsIPluginHost* aPluginHost, const char* aMimeType, nsIURI* aURI) { + // If you add early return(s), be sure to balance this call to + // appShell->SuspendNative() with additional call(s) to + // appShell->ReturnNative(). + static NS_DEFINE_CID(kAppShellCID, NS_APPSHELL_CID); + nsCOMPtr appShell = do_GetService(kAppShellCID); + if (appShell) { + appShell->SuspendNative(); + } #ifdef DEBUG mInstantiating = PR_TRUE; @@ -745,6 +754,10 @@ nsObjectFrame::InstantiatePlugin(nsIPluginHost* aPluginHost, mInstantiating = PR_FALSE; #endif + if (appShell) { + appShell->ResumeNative(); + } + // XXX having to do this sucks. it'd be better to move the code from DidReflow // to FixupWindow or something. AddStateBits(NS_FRAME_IS_DIRTY); diff --git a/widget/public/nsIAppShell.idl b/widget/public/nsIAppShell.idl index b63e3a4862a1..2516ed730e9e 100644 --- a/widget/public/nsIAppShell.idl +++ b/widget/public/nsIAppShell.idl @@ -43,7 +43,7 @@ * Interface for the native event system layer. This interface is designed * to be used on the main application thread only. */ -[uuid(3785230a-da91-4eaa-8a25-56acc7ff35d0)] +[uuid(95f1f2b3-051b-4603-bb91-7058191dfc1e)] interface nsIAppShell : nsISupports { /** @@ -73,4 +73,27 @@ interface nsIAppShell : nsISupports */ void favorPerformanceHint(in boolean favorPerfOverStarvation, in unsigned long starvationDelay); + + /** + * Suspends the use of additional platform-specific methods (besides the + * nsIAppShell->run() event loop) to run Gecko events on the main + * application thread. Under some circumstances these "additional methods" + * can cause Gecko event handlers to be re-entered, sometimes leading to + * hangs and crashes. Calls to suspendNative() and resumeNative() may be + * nested. On some platforms (those that don't use any "additional + * methods") this will be a no-op. Does not (in itself) stop Gecko events + * from being processed on the main application thread. But if the + * nsIAppShell->run() event loop is blocked when this call is made, Gecko + * events will stop being processed until resumeNative() is called (even + * if a plugin or library is temporarily processing events on a nested + * event loop). + */ + void suspendNative(); + + /** + * Resumes the use of additional platform-specific methods to run Gecko + * events on the main application thread. Calls to suspendNative() and + * resumeNative() may be nested. On some platforms this will be a no-op. + */ + void resumeNative(); }; diff --git a/widget/src/cocoa/nsAppShell.h b/widget/src/cocoa/nsAppShell.h index e0f944b17782..056cf4839d56 100644 --- a/widget/src/cocoa/nsAppShell.h +++ b/widget/src/cocoa/nsAppShell.h @@ -50,6 +50,8 @@ class nsAppShell : public nsBaseAppShell { public: + NS_IMETHODIMP ResumeNative(void); + nsAppShell(); nsresult Init(); diff --git a/widget/src/cocoa/nsAppShell.mm b/widget/src/cocoa/nsAppShell.mm index e3437640f495..6c5f89e0cc1b 100644 --- a/widget/src/cocoa/nsAppShell.mm +++ b/widget/src/cocoa/nsAppShell.mm @@ -70,6 +70,15 @@ // nsAppShell implementation +NS_IMETHODIMP +nsAppShell::ResumeNative(void) +{ + nsresult retval = nsBaseAppShell::ResumeNative(); + if (NS_SUCCEEDED(retval) && (mSuspendNativeCount == 0)) + ScheduleNativeEventCallback(); + return retval; +} + nsAppShell::nsAppShell() : mAutoreleasePools(nsnull) , mPort(nil) @@ -195,8 +204,9 @@ nsAppShell::ProcessGeckoEvents() atStart:NO]; } - NativeEventCallback(); - + if (mSuspendNativeCount <= 0) + NativeEventCallback(); + [NSApp postEvent:[NSEvent otherEventWithType:NSApplicationDefined location:NSMakePoint(0,0) modifierFlags:0 diff --git a/widget/src/mac/nsAppShell.cpp b/widget/src/mac/nsAppShell.cpp index ec1b5eeeda70..8c01a1cd50c9 100644 --- a/widget/src/mac/nsAppShell.cpp +++ b/widget/src/mac/nsAppShell.cpp @@ -56,6 +56,15 @@ enum { // nsAppShell implementation +NS_IMETHODIMP +nsAppShell::ResumeNative(void) +{ + nsresult retval = nsBaseAppShell::ResumeNative(); + if (NS_SUCCEEDED(retval) && (mSuspendNativeCount == 0)) + ScheduleNativeEventCallback(); + return retval; +} + nsAppShell::nsAppShell() : mCFRunLoop(NULL) , mCFRunLoopSource(NULL) @@ -200,7 +209,8 @@ nsAppShell::ProcessGeckoEvents(void* aInfo) } } - self->NativeEventCallback(); + if (self->mSuspendNativeCount <= 0) + self->NativeEventCallback(); NS_RELEASE(self); } diff --git a/widget/src/mac/nsAppShell.h b/widget/src/mac/nsAppShell.h index 5042c1b0cb80..053a29c8a371 100644 --- a/widget/src/mac/nsAppShell.h +++ b/widget/src/mac/nsAppShell.h @@ -57,6 +57,8 @@ class nsMacMessagePump; class nsAppShell : public nsBaseAppShell { public: + NS_IMETHODIMP ResumeNative(void); + nsAppShell(); nsresult Init(); diff --git a/widget/src/xpwidgets/nsBaseAppShell.cpp b/widget/src/xpwidgets/nsBaseAppShell.cpp index 4cf7e74f5d7a..c56569057c7d 100644 --- a/widget/src/xpwidgets/nsBaseAppShell.cpp +++ b/widget/src/xpwidgets/nsBaseAppShell.cpp @@ -49,7 +49,8 @@ NS_IMPL_THREADSAFE_ISUPPORTS3(nsBaseAppShell, nsIAppShell, nsIThreadObserver, nsIObserver) nsBaseAppShell::nsBaseAppShell() - : mFavorPerf(0) + : mSuspendNativeCount(0) + , mFavorPerf(0) , mNativeEventPending(PR_FALSE) , mStarvationDelay(0) , mSwitchTime(0) @@ -177,6 +178,21 @@ nsBaseAppShell::FavorPerformanceHint(PRBool favorPerfOverStarvation, return NS_OK; } +NS_IMETHODIMP +nsBaseAppShell::SuspendNative(void) +{ + ++mSuspendNativeCount; + return NS_OK; +} + +NS_IMETHODIMP +nsBaseAppShell::ResumeNative(void) +{ + --mSuspendNativeCount; + NS_ASSERTION(mSuspendNativeCount >= 0, "Unbalanced call to nsBaseAppShell::ResumeNative!"); + return NS_OK; +} + //------------------------------------------------------------------------- // nsIThreadObserver methods: diff --git a/widget/src/xpwidgets/nsBaseAppShell.h b/widget/src/xpwidgets/nsBaseAppShell.h index 94d7d8aba1a0..0a30395d723e 100644 --- a/widget/src/xpwidgets/nsBaseAppShell.h +++ b/widget/src/xpwidgets/nsBaseAppShell.h @@ -94,6 +94,8 @@ protected: */ virtual PRBool ProcessNextNativeEvent(PRBool mayWait) = 0; + PRInt32 mSuspendNativeCount; + private: PRBool DoProcessNextNativeEvent(PRBool mayWait);