Fixing bug 347743. Delay plugin destruction on windows to get it out of layout frame destruction. r+sr=roc@ocallahan.org

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
@@ -332,6 +334,8 @@ public:
 
   nsresult Destroy();  
 
+  void PrepareToStop(PRBool aDelayedStop);
+
   //nsIEventListener interface
   nsEventStatus ProcessEvent(const nsGUIEvent & anEvent);
   
@@ -363,6 +367,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);
 
@@ -377,6 +386,10 @@ private:
   nsCOMPtr<nsIPluginHost>     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;
@@ -499,7 +512,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();
 }
@@ -1288,7 +1301,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");
 
@@ -1358,15 +1371,28 @@ nsObjectFrame::Instantiate(const char* aMimeType, nsIURI* aURI)
   return rv;
 }
 
-void
-nsObjectFrame::StopPlugin()
+class nsStopPluginRunnable : public nsRunnable
+{
+public:
+  nsStopPluginRunnable(nsPluginInstanceOwner *aInstanceOwner)
+    : mInstanceOwner(aInstanceOwner)
+  {
+  }
+
+  NS_IMETHOD Run();
+
+private:  
+  nsRefPtr<nsPluginInstanceOwner> mInstanceOwner;
+};
+
+static void
+DoStopPlugin(nsPluginInstanceOwner *aInstanceOwner)
 {
-  if (mInstanceOwner != nsnull) {
   nsCOMPtr<nsIPluginInstance> inst;
-    mInstanceOwner->GetInstance(*getter_AddRefs(inst));
+  aInstanceOwner->GetInstance(*getter_AddRefs(inst));
   if (inst) {
     nsPluginWindow *win;
-      mInstanceOwner->GetWindow(win);
+    aInstanceOwner->GetWindow(win);
     nsPluginNativeWindow *window = (nsPluginNativeWindow *)win;
     nsCOMPtr<nsIPluginInstance> nullinst;
 
@@ -1374,13 +1400,12 @@ nsObjectFrame::StopPlugin()
     PRBool doCallSetWindowAfterDestroy = PR_FALSE;
 
     // first, determine if the plugin wants to be cached
-      inst->GetValue(nsPluginInstanceVariable_DoCacheBool, 
-                     (void *) &doCache);
+    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);
+                     (void *)&doCallSetWindowAfterDestroy);
       if (doCallSetWindowAfterDestroy) {
         inst->Stop();
         inst->Destroy();
@@ -1413,15 +1438,66 @@ nsObjectFrame::StopPlugin()
     if (pluginHost)
       pluginHost->StopPluginInstance(inst);
 
-      // the frame is going away along with its widget
-      // so tell the window to forget its widget too
+    // the frame is going away along with its widget so tell the
+    // window to forget its widget too
     if (window)
       window->SetPluginWidget(nsnull);
   }
 
-    mInstanceOwner->Destroy();
-    NS_RELEASE(mInstanceOwner);
+  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<nsIRunnable> 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
@@ -1573,6 +1649,7 @@ nsPluginInstanceOwner::nsPluginInstanceOwner()
   mNumCachedParams = 0;
   mCachedAttrParamNames = nsnull;
   mCachedAttrParamValues = nsnull;
+  mDestroyWidget = PR_FALSE;
 }
 
 nsPluginInstanceOwner::~nsPluginInstanceOwner()
@@ -3131,6 +3208,45 @@ 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 to
+    // the top-level Gecko widget so that we can safely tear down the
+    // plugin after its frame (and view) is gone.
+
+    nsIWidget *appTopWidget = mWidget->GetTopLevelWindow();
+
+    // 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(appTopWidget);
+
+    mDestroyWidget = PR_TRUE;
+  }
+#endif
+
   // Unregister scroll position listener
   nsIFrame* parentWithView = mOwner->GetAncestorWithView();
   nsIView* curView = parentWithView ? parentWithView->GetView() : nsnull;
@@ -3141,10 +3257,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.

layout/generic/nsObjectFrame.h

@@ -112,6 +112,14 @@ public:
   virtual nsresult Instantiate(const char* aMimeType, nsIURI* aURI);
   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) 

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() {}
 };

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);
+    if (!mWidgetDisowned) {
       mWindow->Destroy();
+    }
     NS_RELEASE(mWindow);
   }
   delete mDirtyRegion;

widget/public/nsIWidget.h

@@ -1053,6 +1053,12 @@ class nsIWidget : public nsISupports {
      */
     NS_IMETHOD GetAnimatedResize(PRUint16* aAnimation) = 0;
 
+    /*
+     * Return the the nearest ancestor widget which does not have a
+     * Gecko parent.
+     */
+    virtual nsIWidget* GetTopLevelWindow(void) = 0;
+
 protected:
     // keep the list of children.  We also keep track of our siblings.
     // The ownership model is as follows: parent holds a strong ref to

widget/src/windows/nsWindow.cpp

@@ -7374,7 +7374,7 @@ nsWindow::ResolveIMECaretPos(nsWindow* aClient,
                              nsRect&   aResult)
 {
   // RootView coordinates -> Screen coordinates
-  GetTopLevelWindow()->WidgetToScreen(aEventResult, aResult);
+  GetTopLevelWindowInternal()->WidgetToScreen(aEventResult, aResult);
   // if aClient is nsnull, returns screen coordinates
   if (!aClient)
     return;
@@ -7766,6 +7766,26 @@ nsWindow::GetLastInputEventTime(PRUint32& aTime)
   return NS_OK;
 }
 
+nsIWidget* nsWindow::GetTopLevelWindow(void)
+{
+  HWND hWnd = (HWND)GetNativeData(NS_NATIVE_WINDOW);
+  HWND rootWnd = hWnd;
+  nsIWidget *topWidget = nsnull;
+
+  while (1) {
+    HWND parent = ::GetParent(rootWnd);
+
+    if (!parent || GetNSWindowPtr(parent) == nsnull) {
+      break;
+    }
+
+    rootWnd = parent;
+  }
+
+  return GetNSWindowPtr(rootWnd);
+}
+
+
 //-------------------------------------------------------------------------
 //-------------------------------------------------------------------------
 //-- NOTE!!! These hook functions can be removed when we migrate to
@@ -8184,7 +8204,7 @@ STDMETHODIMP_(LRESULT) nsWindow::LresultFromObject(REFIID riid, WPARAM wParam, L
 
 #ifdef MOZ_XUL
 
-nsWindow* nsWindow::GetTopLevelWindow()
+nsWindow* nsWindow::GetTopLevelWindowInternal()
 {
   nsWindow* curWindow = this;
 
@@ -8303,21 +8323,21 @@ void nsWindow::ResizeTranslucentWindow(PRInt32 aNewWidth, PRInt32 aNewHeight, PR
 
 NS_IMETHODIMP nsWindow::GetWindowTranslucency(PRBool& aTranslucent)
 {
-  aTranslucent = GetTopLevelWindow()->GetWindowTranslucencyInner();
+  aTranslucent = GetTopLevelWindowInternal()->GetWindowTranslucencyInner();
 
   return NS_OK;
 }
 
 NS_IMETHODIMP nsWindow::SetWindowTranslucency(PRBool aTranslucent)
 {
-  nsresult rv = GetTopLevelWindow()->SetWindowTranslucencyInner(aTranslucent);
+  nsresult rv = GetTopLevelWindowInternal()->SetWindowTranslucencyInner(aTranslucent);
 
   return rv;
 }
 
 NS_IMETHODIMP nsWindow::UpdateTranslucentWindowAlpha(const nsRect& aRect, PRUint8* aAlphas)
 {
-  GetTopLevelWindow()->UpdateTranslucentWindowAlphaInner(aRect, aAlphas);
+  GetTopLevelWindowInternal()->UpdateTranslucentWindowAlphaInner(aRect, aAlphas);
 
   return NS_OK;
 }

widget/src/windows/nsWindow.h

@@ -204,7 +204,9 @@ public:
 
   NS_IMETHOD              GetAttention(PRInt32 aCycleCount);
   NS_IMETHOD              GetLastInputEventTime(PRUint32& aTime);
-  nsWindow*               GetTopLevelWindow();
+  virtual nsIWidget*      GetTopLevelWindow(void);
+
+  nsWindow*               GetTopLevelWindowInternal();
 
 #ifdef MOZ_CAIRO_GFX
   gfxASurface             *GetThebesSurface();

widget/src/xpwidgets/nsBaseWidget.cpp

@@ -856,6 +856,13 @@ nsBaseWidget::GetAnimatedResize(PRUint16* aAnimation)
   return NS_ERROR_NOT_IMPLEMENTED;
 }
 
+nsIWidget* nsBaseWidget::GetTopLevelWindow(void)
+{
+  NS_WARNING("Not implemented");
+
+  return nsnull;
+}
+
 /**
  * Modifies aFile to point at an icon file with the given name and suffix.  The
  * suffix may correspond to a file extension with leading '.' if appropriate.

widget/src/xpwidgets/nsBaseWidget.h

@@ -133,6 +133,7 @@ public:
   NS_IMETHOD              SetIcon(const nsAString &anIconSpec);
   NS_IMETHOD              SetAnimatedResize(PRUint16 aAnimation);
   NS_IMETHOD              GetAnimatedResize(PRUint16* aAnimation);
+  virtual nsIWidget*      GetTopLevelWindow();
   virtual void            ConvertToDeviceCoordinates(nscoord  &aX,nscoord &aY) {}
   virtual void            FreeNativeData(void * data, PRUint32 aDataType) {}//~~~