Fixing bug 430802 (and bug 405357). Prevent plugin instantiation code re-entrancy. r=jwatt@jwatt.org, sr=bzbarsky@mit.edu, a=beltzner

content/base/src/nsObjectLoadingContent.cpp

@@ -1666,6 +1666,15 @@ nsObjectLoadingContent::Instantiate(nsIObjectFrame* aFrame,
 {
   NS_ASSERTION(aFrame, "Must have a frame here");
 
+  // We're instantiating now, invalidate any pending async instantiate
+  // calls.
+  mPendingInstantiateEvent = nsnull;
+
+  // Mark that we're instantiating now so that we don't end up
+  // re-entering instantiation code.
+  PRBool oldInstantiatingValue = mInstantiating;
+  mInstantiating = PR_TRUE;
+
   nsCString typeToUse(aMIMEType);
   if (typeToUse.IsEmpty() && aURI) {
     IsPluginEnabledByExtension(aURI, typeToUse);
@@ -1687,7 +1696,11 @@ nsObjectLoadingContent::Instantiate(nsIObjectFrame* aFrame,
   NS_ASSERTION(aURI || !typeToUse.IsEmpty(), "Need a URI or a type");
   LOG(("OBJLC [%p]: Calling [%p]->Instantiate(<%s>, %p)\n", this, aFrame,
        typeToUse.get(), aURI));
-  return aFrame->Instantiate(typeToUse.get(), aURI);
+  nsresult rv = aFrame->Instantiate(typeToUse.get(), aURI);
+
+  mInstantiating = oldInstantiatingValue;
+
+  return rv;
 }
 
 nsresult

layout/generic/nsObjectFrame.cpp

@@ -581,7 +581,7 @@ nsObjectFrame::Init(nsIContent*      aContent,
                     nsIFrame*        aParent,
                     nsIFrame*        aPrevInFlow)
 {
-  mInstantiating = PR_FALSE;
+  mPreventInstantiation = PR_FALSE;
 
   PR_LOG(nsObjectFrameLM, PR_LOG_DEBUG,
          ("Initializing nsObjectFrame %p for content %p\n", this, aContent));
@@ -592,7 +592,7 @@ nsObjectFrame::Init(nsIContent*      aContent,
 void
 nsObjectFrame::Destroy()
 {
-  NS_ASSERTION(!mInstantiating, "about to crash due to bug 136927");
+  NS_ASSERTION(!mPreventInstantiation, "about to crash due to bug 136927");
 
   // we need to finish with the plugin before native window is destroyed
   // doing this in the destructor is too late.
@@ -831,6 +831,9 @@ nsObjectFrame::InstantiatePlugin(nsIPluginHost* aPluginHost,
                                  const char* aMimeType,
                                  nsIURI* aURI)
 {
+  NS_ASSERTION(mPreventInstantiation,
+               "Instantiation should be prevented here!");
+
   // If you add early return(s), be sure to balance this call to
   // appShell->SuspendNative() with additional call(s) to
   // appShell->ReturnNative().
@@ -839,9 +842,6 @@ nsObjectFrame::InstantiatePlugin(nsIPluginHost* aPluginHost,
     appShell->SuspendNative();
   }
 
-  NS_PRECONDITION(!mInstantiating, "How did that happen?");
-  mInstantiating = PR_TRUE;
-
   NS_ASSERTION(mContent, "We should have a content node.");
 
   nsIDocument* doc = mContent->GetOwnerDoc();
@@ -859,7 +859,6 @@ nsObjectFrame::InstantiatePlugin(nsIPluginHost* aPluginHost,
     rv = aPluginHost->InstantiateEmbeddedPlugin(aMimeType, aURI,
                                                 mInstanceOwner);
   }
-  mInstantiating = PR_FALSE;
 
   if (appShell) {
     appShell->ResumeNative();
@@ -1639,7 +1638,7 @@ nsObjectFrame::PrepareInstanceOwner()
 nsresult
 nsObjectFrame::Instantiate(nsIChannel* aChannel, nsIStreamListener** aStreamListener)
 {
-  if (mInstantiating) {
+  if (mPreventInstantiation) {
     return NS_OK;
   }
   
@@ -1656,10 +1655,12 @@ nsObjectFrame::Instantiate(nsIChannel* aChannel, nsIStreamListener** aStreamList
   // This must be done before instantiating the plugin
   FixupWindow(mRect.Size());
 
-  NS_ASSERTION(!mInstantiating, "Say what?");
-  mInstantiating = PR_TRUE;
+  NS_ASSERTION(!mPreventInstantiation, "Say what?");
+  mPreventInstantiation = PR_TRUE;
   rv = pluginHost->InstantiatePluginForChannel(aChannel, mInstanceOwner, aStreamListener);
-  mInstantiating = PR_FALSE;
+  NS_ASSERTION(mPreventInstantiation,
+               "Instantiation should still be prevented!");
+  mPreventInstantiation = PR_FALSE;
 
   return rv;
 }
@@ -1671,7 +1672,7 @@ nsObjectFrame::Instantiate(const char* aMimeType, nsIURI* aURI)
          ("nsObjectFrame::Instantiate(%s) called on frame %p\n", aMimeType,
           this));
 
-  if (mInstantiating) {
+  if (mPreventInstantiation) {
     return NS_OK;
   }
 
@@ -1691,6 +1692,8 @@ nsObjectFrame::Instantiate(const char* aMimeType, nsIURI* aURI)
     return rv;
   mInstanceOwner->SetPluginHost(pluginHost);
 
+  mPreventInstantiation = PR_TRUE;
+
   rv = InstantiatePlugin(pluginHost, aMimeType, aURI);
 
   // finish up
@@ -1699,6 +1702,11 @@ nsObjectFrame::Instantiate(const char* aMimeType, nsIURI* aURI)
     CallSetWindow();
   }
 
+  NS_ASSERTION(mPreventInstantiation,
+               "Instantiation should still be prevented!");
+
+  mPreventInstantiation = PR_FALSE;
+
   return rv;
 }
 
@@ -1921,6 +1929,11 @@ nsObjectFrame::StopPluginInternal(PRBool aDelayedStop)
   // get reinstantiated we'll send the right messages to the plug-in.
   mWindowlessRect.Empty();
 
+  PRBool oldVal = mPreventInstantiation;
+  mPreventInstantiation = PR_TRUE;
+
+  nsWeakFrame weakFrame(this);
+
 #ifdef XP_WIN
   if (aDelayedStop) {
     // If we're asked to do a delayed stop it means we're stopping the
@@ -1942,6 +1955,14 @@ nsObjectFrame::StopPluginInternal(PRBool aDelayedStop)
 
   DoStopPlugin(owner, aDelayedStop);
 
+  // If |this| is still alive, reset mPreventInstantiation.
+  if (weakFrame.IsAlive()) {
+    NS_ASSERTION(mPreventInstantiation,
+                 "Instantiation should still be prevented!");
+
+    mPreventInstantiation = oldVal;
+  }
+
   // Break relationship between frame and plugin instance owner
   owner->SetOwner(nsnull);
 }

layout/generic/nsObjectFrame.h

@@ -196,7 +196,7 @@ private:
   // For assertions that make it easier to determine if a crash is due
   // to the underlying problem described in bug 136927, and to prevent
   // reentry into instantiation.
-  PRBool mInstantiating;
+  PRBool mPreventInstantiation;
 };