From c826ffa601c24294ab9d107bc18d7f861b206c99 Mon Sep 17 00:00:00 2001 From: "sharparrow1@yahoo.com" Date: Mon, 13 Aug 2007 13:47:04 -0700 Subject: [PATCH] Bug 3477743: plugin crash. patch by myself and Johnny Stenback, r+sr=roc --- layout/generic/nsObjectFrame.cpp | 216 +++++++++++++++++++++++-------- layout/generic/nsObjectFrame.h | 7 + view/public/nsIView.h | 9 ++ view/src/nsView.cpp | 5 +- widget/src/windows/nsWindow.cpp | 16 ++- 5 files changed, 197 insertions(+), 56 deletions(-) diff --git a/layout/generic/nsObjectFrame.cpp b/layout/generic/nsObjectFrame.cpp index 16d608bf5c4..c87e89ecb0f 100644 --- a/layout/generic/nsObjectFrame.cpp +++ b/layout/generic/nsObjectFrame.cpp @@ -122,6 +122,8 @@ #include "nsPIPluginHost.h" #include "nsIPluginDocument.h" +#include "nsThreadUtils.h" + #ifdef MOZ_CAIRO_GFX #include "gfxContext.h" #endif @@ -341,6 +343,8 @@ public: nsresult Destroy(); + void PrepareToStop(PRBool aDelayedStop); + //nsIEventListener interface nsEventStatus ProcessEvent(const nsGUIEvent & anEvent); @@ -379,6 +383,11 @@ public: void GUItoMacEvent(const nsGUIEvent& anEvent, EventRecord* origEvent, EventRecord& aMacEvent); #endif + void SetOwner(nsObjectFrame *aOwner) + { + mOwner = aOwner; + } + private: void FixUpURLS(const nsString &name, nsAString &value); @@ -393,6 +402,10 @@ private: nsCOMPtr mPluginHost; PRPackedBool mContentFocused; PRPackedBool mWidgetVisible; // used on Mac to store our widget's visible state + + // If true, destroy the widget on destruction. Used when plugin stop + // is being delayed to a safer point in time. + PRPackedBool mDestroyWidget; PRUint16 mNumCachedAttrs; PRUint16 mNumCachedParams; char **mCachedAttrParamNames; @@ -516,7 +529,7 @@ nsObjectFrame::Destroy() // we need to finish with the plugin before native window is destroyed // doing this in the destructor is too late. - StopPlugin(); + StopPluginInternal(PR_TRUE); nsObjectFrameSuper::Destroy(); } @@ -1368,7 +1381,7 @@ nsresult nsObjectFrame::PrepareInstanceOwner() { // First, have to stop any possibly running plugins. - StopPlugin(); + StopPluginInternal(PR_FALSE); NS_ASSERTION(!mInstanceOwner, "Must not have an instance owner here"); @@ -1440,47 +1453,49 @@ nsObjectFrame::TryNotifyContentObjectWrapper() } } -void -nsObjectFrame::StopPlugin() +class nsStopPluginRunnable : public nsRunnable { - if (mInstanceOwner != nsnull) { - nsCOMPtr inst; - mInstanceOwner->GetInstance(*getter_AddRefs(inst)); - if (inst) { - nsPluginWindow *win; - mInstanceOwner->GetWindow(win); - nsPluginNativeWindow *window = (nsPluginNativeWindow *)win; - nsCOMPtr nullinst; +public: + nsStopPluginRunnable(nsPluginInstanceOwner *aInstanceOwner) + : mInstanceOwner(aInstanceOwner) + { + } - PRBool doCache = PR_TRUE; - PRBool doCallSetWindowAfterDestroy = PR_FALSE; + NS_IMETHOD Run(); - // first, determine if the plugin wants to be cached - inst->GetValue(nsPluginInstanceVariable_DoCacheBool, - (void *) &doCache); - if (!doCache) { - // then determine if the plugin wants Destroy to be called after - // Set Window. This is for bug 50547. - inst->GetValue(nsPluginInstanceVariable_CallSetWindowAfterDestroyBool, - (void *) &doCallSetWindowAfterDestroy); - if (doCallSetWindowAfterDestroy) { - inst->Stop(); - inst->Destroy(); - - if (window) - window->CallSetWindow(nullinst); - else - inst->SetWindow(nsnull); - } - else { - if (window) - window->CallSetWindow(nullinst); - else - inst->SetWindow(nsnull); +private: + nsRefPtr mInstanceOwner; +}; - inst->Stop(); - inst->Destroy(); - } +static void +DoStopPlugin(nsPluginInstanceOwner *aInstanceOwner) +{ + nsCOMPtr inst; + aInstanceOwner->GetInstance(*getter_AddRefs(inst)); + if (inst) { + nsPluginWindow *win; + aInstanceOwner->GetWindow(win); + nsPluginNativeWindow *window = (nsPluginNativeWindow *)win; + nsCOMPtr nullinst; + + PRBool doCache = PR_TRUE; + PRBool doCallSetWindowAfterDestroy = PR_FALSE; + + // first, determine if the plugin wants to be cached + inst->GetValue(nsPluginInstanceVariable_DoCacheBool, (void *)&doCache); + if (!doCache) { + // then determine if the plugin wants Destroy to be called after + // Set Window. This is for bug 50547. + inst->GetValue(nsPluginInstanceVariable_CallSetWindowAfterDestroyBool, + (void *)&doCallSetWindowAfterDestroy); + if (doCallSetWindowAfterDestroy) { + inst->Stop(); + inst->Destroy(); + + if (window) + window->CallSetWindow(nullinst); + else + inst->SetWindow(nsnull); } else { if (window) @@ -1489,21 +1504,82 @@ nsObjectFrame::StopPlugin() inst->SetWindow(nsnull); inst->Stop(); + inst->Destroy(); } + } + else { + if (window) + window->CallSetWindow(nullinst); + else + inst->SetWindow(nsnull); - nsCOMPtr pluginHost = do_GetService(kCPluginManagerCID); - if (pluginHost) - pluginHost->StopPluginInstance(inst); - - // the frame is going away along with its widget - // so tell the window to forget its widget too - if (window) - window->SetPluginWidget(nsnull); + inst->Stop(); } - mInstanceOwner->Destroy(); - NS_RELEASE(mInstanceOwner); + nsCOMPtr pluginHost = do_GetService(kCPluginManagerCID); + if (pluginHost) + pluginHost->StopPluginInstance(inst); + + // the frame is going away along with its widget so tell the + // window to forget its widget too + if (window) + window->SetPluginWidget(nsnull); } + + aInstanceOwner->Destroy(); +} + +NS_IMETHODIMP +nsStopPluginRunnable::Run() +{ + DoStopPlugin(mInstanceOwner); + + return NS_OK; +} + +void +nsObjectFrame::StopPlugin() +{ + StopPluginInternal(PR_FALSE); +} + +void +nsObjectFrame::StopPluginInternal(PRBool aDelayedStop) +{ + if (mInstanceOwner == nsnull) { + return; + } + + mInstanceOwner->PrepareToStop(aDelayedStop); + +#ifdef XP_WIN + // We only deal with delayed stopping of plugins on Win32 for now, + // as that's the only platform where we need to (AFAIK) and it's + // unclear how safe widget parenting is on other platforms. + if (aDelayedStop) { + // nsStopPluginRunnable will hold a strong reference to + // mInstanceOwner, and thus keep it alive as long as it needs it. + nsCOMPtr evt = new nsStopPluginRunnable(mInstanceOwner); + NS_DispatchToCurrentThread(evt); + + // If we're asked to do a delayed stop it means we're stopping the + // plugin because we're destroying the frame. In that case, tell + // the view to disown the widget (i.e. leave it up to us to + // destroy it). + nsIView *view = GetView(); + if (view) { + view->DisownWidget(); + } + } else +#endif + { + DoStopPlugin(mInstanceOwner); + } + + // Break relationship between frame and plugin instance owner + mInstanceOwner->SetOwner(nsnull); + + NS_RELEASE(mInstanceOwner); } void @@ -1655,6 +1731,7 @@ nsPluginInstanceOwner::nsPluginInstanceOwner() mNumCachedParams = 0; mCachedAttrParamNames = nsnull; mCachedAttrParamValues = nsnull; + mDestroyWidget = PR_FALSE; } nsPluginInstanceOwner::~nsPluginInstanceOwner() @@ -3423,6 +3500,43 @@ nsPluginInstanceOwner::Destroy() target->RemoveEventListener(NS_LITERAL_STRING("draggesture"), listener, PR_TRUE); } + if (mDestroyWidget && mWidget) { + mWidget->Destroy(); + } + + return NS_OK; +} + +/* + * Prepare to stop + */ +void +nsPluginInstanceOwner::PrepareToStop(PRBool aDelayedStop) +{ + if (!mWidget) { + return; + } + +#ifdef XP_WIN + if (aDelayedStop) { + // To delay stopping a plugin we need to reparent the plugin + // so that we can safely tear down the + // plugin after its frame (and view) is gone. + + // Also hide and disable the widget to avoid it from appearing in + // odd places after reparenting it, but before it gets destroyed. + mWidget->Show(PR_FALSE); + mWidget->Enable(PR_FALSE); + + // Reparent the plugins native window. This relies on the widget + // and plugin et al not holding any other references to its + // parent. + mWidget->SetParent(nsnull); + + mDestroyWidget = PR_TRUE; + } +#endif + // Unregister scroll position listener nsIFrame* parentWithView = mOwner->GetAncestorWithView(); nsIView* curView = parentWithView ? parentWithView->GetView() : nsnull; @@ -3433,10 +3547,6 @@ nsPluginInstanceOwner::Destroy() curView = curView->GetParent(); } - - mOwner = nsnull; // break relationship between frame and plugin instance owner - - return NS_OK; } // Paints are handled differently, so we just simulate an update event. diff --git a/layout/generic/nsObjectFrame.h b/layout/generic/nsObjectFrame.h index b7973d1c243..d4f2fdeb215 100644 --- a/layout/generic/nsObjectFrame.h +++ b/layout/generic/nsObjectFrame.h @@ -113,6 +113,13 @@ public: virtual void TryNotifyContentObjectWrapper(); virtual void StopPlugin(); + /* + * Stop a plugin instance. If aDelayedStop is true, the plugin will + * be stopped at a later point when it's safe to do so (i.e. not + * while destroying the frame tree). Delayed stopping is only + * implemented on Win32 for now. + */ + void StopPluginInternal(PRBool aDelayedStop); /* fail on any requests to get a cursor from us because plugins set their own! see bug 118877 */ NS_IMETHOD GetCursor(const nsPoint& aPoint, nsIFrame::Cursor& aCursor) diff --git a/view/public/nsIView.h b/view/public/nsIView.h index 25fd94f7fee..1d90ef902cb 100644 --- a/view/public/nsIView.h +++ b/view/public/nsIView.h @@ -299,6 +299,14 @@ public: */ PRBool HasWidget() const { return mWindow != nsnull; } + /** + * If called, will make the view disown the widget and leave it up + * to other code to destroy it. + */ + void DisownWidget() { + mWidgetDisowned = PR_TRUE; + } + #ifdef DEBUG /** * Output debug info to FILE @@ -329,6 +337,7 @@ protected: nsRect mDimBounds; // relative to parent float mOpacity; PRUint32 mVFlags; + PRBool mWidgetDisowned; virtual ~nsIView() {} }; diff --git a/view/src/nsView.cpp b/view/src/nsView.cpp index 8783083b97a..7f25d44fec4 100644 --- a/view/src/nsView.cpp +++ b/view/src/nsView.cpp @@ -183,6 +183,7 @@ nsView::nsView(nsViewManager* aViewManager, nsViewVisibility aVisibility) mVFlags = 0; mViewManager = aViewManager; mDirtyRegion = nsnull; + mWidgetDisowned = PR_FALSE; } void nsView::DropMouseGrabbing() { @@ -250,7 +251,9 @@ nsView::~nsView() NS_IF_RELEASE(wrapper); mWindow->SetClientData(nsnull); - mWindow->Destroy(); + if (!mWidgetDisowned) { + mWindow->Destroy(); + } NS_RELEASE(mWindow); } delete mDirtyRegion; diff --git a/widget/src/windows/nsWindow.cpp b/widget/src/windows/nsWindow.cpp index 851c572e038..6579ad1f83e 100644 --- a/widget/src/windows/nsWindow.cpp +++ b/widget/src/windows/nsWindow.cpp @@ -1642,8 +1642,20 @@ NS_IMETHODIMP nsWindow::SetParent(nsIWidget *aNewParent) return NS_OK; } - NS_WARNING("Null aNewParent passed to SetParent"); - return NS_ERROR_FAILURE; + + nsCOMPtr kungFuDeathGrip(this); + + nsIWidget* parent = GetParent(); + + if (parent) { + parent->RemoveChild(this); + } + + if (mWnd) { + ::SetParent(mWnd, nsnull); + } + + return NS_OK; }