Bug 719117 - Fix bug 724781 by preventing reentry to DoStopPlugin with a flag. part 2/2, r=bsmedberg
authorMats Palmgren <matspal@gmail.com>
Sat, 26 May 2012 00:34:11 +0200
changeset 94997 8915b49bc1bc761fd999025e85eb0d364a5815f8
parent 94996 2067107a71ca01c3d1fd1f7bf5b1df1410b3671b
child 94998 728450b22631101fa0be5891778bc3a4638bdbeb
push id22770
push userryanvm@gmail.com
push dateSat, 26 May 2012 12:07:39 +0000
treeherdermozilla-central@cd62c4b8f500 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbsmedberg
bugs719117, 724781
milestone15.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 719117 - Fix bug 724781 by preventing reentry to DoStopPlugin with a flag. part 2/2, r=bsmedberg
content/base/src/nsObjectLoadingContent.cpp
content/base/src/nsObjectLoadingContent.h
--- a/content/base/src/nsObjectLoadingContent.cpp
+++ b/content/base/src/nsObjectLoadingContent.cpp
@@ -312,31 +312,37 @@ nsPluginCrashedEvent::Run()
   return NS_OK;
 }
 
 class nsStopPluginRunnable : public nsRunnable, public nsITimerCallback
 {
 public:
   NS_DECL_ISUPPORTS_INHERITED
 
-  nsStopPluginRunnable(nsPluginInstanceOwner *aInstanceOwner)
-  : mInstanceOwner(aInstanceOwner)
+  nsStopPluginRunnable(nsPluginInstanceOwner* aInstanceOwner,
+                       nsObjectLoadingContent* aContent)
+    : mInstanceOwner(aInstanceOwner)
+    , mContentKungFuDeathGrip(aContent)
+    , mContent(aContent)
   {
     NS_ASSERTION(aInstanceOwner, "need an owner");
+    NS_ASSERTION(aContent, "need a nsObjectLoadingContent");
   }
 
   // nsRunnable
   NS_IMETHOD Run();
 
   // nsITimerCallback
   NS_IMETHOD Notify(nsITimer *timer);
 
 private:
   nsCOMPtr<nsITimer> mTimer;
   nsRefPtr<nsPluginInstanceOwner> mInstanceOwner;
+  nsCOMPtr<nsIObjectLoadingContent> mContentKungFuDeathGrip;
+  nsObjectLoadingContent* mContent;
 };
 
 NS_IMPL_ISUPPORTS_INHERITED1(nsStopPluginRunnable, nsRunnable, nsITimerCallback)
 
 NS_IMETHODIMP
 nsStopPluginRunnable::Notify(nsITimer *aTimer)
 {
   return Run();
@@ -365,17 +371,17 @@ nsStopPluginRunnable::Run()
       }
       NS_ERROR("Failed to setup a timer to stop the plugin later (at a safe "
                "time). Stopping the plugin now, this might crash.");
     }
   }
 
   mTimer = nsnull;
 
-  nsObjectLoadingContent::DoStopPlugin(mInstanceOwner, false);
+  mContent->DoStopPlugin(mInstanceOwner, false, true);
 
   return NS_OK;
 }
 
 class AutoNotifier {
   public:
     AutoNotifier(nsObjectLoadingContent* aContent, bool aNotify) :
       mContent(aContent), mNotify(aNotify) {
@@ -587,16 +593,17 @@ nsObjectLoadingContent::UnbindFromTree(b
 nsObjectLoadingContent::nsObjectLoadingContent()
   : mPendingInstantiateEvent(nsnull)
   , mChannel(nsnull)
   , mType(eType_Loading)
   , mInstantiating(false)
   , mUserDisabled(false)
   , mSuppressed(false)
   , mNetworkCreated(true)
+  , mIsStopping(false)
   , mSrcStreamLoading(false)
   , mFallbackReason(ePluginOtherState)
 {
   InitPrefCache();
   // If plugins.click_to_play is false, plugins should always play
   mShouldPlay = !gClickToPlayPlugins;
   // If plugins.click_to_play is true, track the activated state of plugins.
   mActivated = !gClickToPlayPlugins;
@@ -2130,17 +2137,19 @@ nsObjectLoadingContent::AsyncStartPlugin
 NS_IMETHODIMP
 nsObjectLoadingContent::GetSrcURI(nsIURI** aURI)
 {
   NS_IF_ADDREF(*aURI = mURI);
   return NS_OK;
 }
 
 static bool
-DoDelayedStop(nsPluginInstanceOwner *aInstanceOwner, bool aDelayedStop)
+DoDelayedStop(nsPluginInstanceOwner* aInstanceOwner,
+              nsObjectLoadingContent* aContent,
+              bool aDelayedStop)
 {
 #if (MOZ_PLATFORM_MAEMO==5)
   // Don't delay stop on Maemo/Hildon (bug 530739).
   if (aDelayedStop && aInstanceOwner->MatchPluginName("Shockwave Flash"))
     return false;
 #endif
   
   // Don't delay stopping QuickTime (bug 425157), Flip4Mac (bug 426524),
@@ -2148,43 +2157,56 @@ DoDelayedStop(nsPluginInstanceOwner *aIn
   if (aDelayedStop
 #if !(defined XP_WIN || defined MOZ_X11)
       && !aInstanceOwner->MatchPluginName("QuickTime")
       && !aInstanceOwner->MatchPluginName("Flip4Mac")
       && !aInstanceOwner->MatchPluginName("XStandard plugin")
       && !aInstanceOwner->MatchPluginName("CMISS Zinc Plugin")
 #endif
       ) {
-    nsCOMPtr<nsIRunnable> evt = new nsStopPluginRunnable(aInstanceOwner);
+    nsCOMPtr<nsIRunnable> evt =
+      new nsStopPluginRunnable(aInstanceOwner, aContent);
     NS_DispatchToCurrentThread(evt);
     return true;
   }
   return false;
 }
 
 void
-nsObjectLoadingContent::DoStopPlugin(nsPluginInstanceOwner *aInstanceOwner, bool aDelayedStop)
+nsObjectLoadingContent::DoStopPlugin(nsPluginInstanceOwner* aInstanceOwner,
+                                     bool aDelayedStop,
+                                     bool aForcedReentry)
 {
+  // DoStopPlugin can process events and there may be pending InDocCheckEvent
+  // events which can drop in underneath us and destroy the instance we are
+  // about to destroy unless we prevent that with the mIsStopping flag.
+  // (aForcedReentry is only true from the callback of an earlier delayed stop)
+  if (mIsStopping && !aForcedReentry) {
+    return;
+  }
+  mIsStopping = true;
+
   nsRefPtr<nsNPAPIPluginInstance> inst;
   aInstanceOwner->GetInstance(getter_AddRefs(inst));
   if (inst) {
-    if (DoDelayedStop(aInstanceOwner, aDelayedStop)) {
+    if (DoDelayedStop(aInstanceOwner, this, aDelayedStop)) {
       return;
     }
 
 #if defined(XP_MACOSX)
     aInstanceOwner->HidePluginWindow();
 #endif
 
     nsCOMPtr<nsIPluginHost> pluginHost = do_GetService(MOZ_PLUGIN_HOST_CONTRACTID);
     NS_ASSERTION(pluginHost, "Without a pluginHost, how can we have an instance to destroy?");
     static_cast<nsPluginHost*>(pluginHost.get())->StopPluginInstance(inst);
   }
   
   aInstanceOwner->Destroy();
+  mIsStopping = false;
 }
 
 NS_IMETHODIMP
 nsObjectLoadingContent::StopPluginInstance()
 {
   if (!mInstanceOwner) {
     return NS_OK;
   }
--- a/content/base/src/nsObjectLoadingContent.h
+++ b/content/base/src/nsObjectLoadingContent.h
@@ -205,17 +205,18 @@ class nsObjectLoadingContent : public ns
      */
     void RemovedFromDocument();
 
     static void Traverse(nsObjectLoadingContent *tmp,
                          nsCycleCollectionTraversalCallback &cb);
 
     void CreateStaticClone(nsObjectLoadingContent* aDest) const;
 
-    static void DoStopPlugin(nsPluginInstanceOwner *aInstanceOwner, bool aDelayedStop);
+    void DoStopPlugin(nsPluginInstanceOwner* aInstanceOwner, bool aDelayedStop,
+                      bool aForcedReentry = false);
 
     nsresult BindToTree(nsIDocument* aDocument, nsIContent* aParent,
                         nsIContent* aBindingParent,
                         bool aCompileEventHandler);
     void UnbindFromTree(bool aDeep = true,
                         bool aNullParent = true);
 
   private:
@@ -372,16 +373,19 @@ class nsObjectLoadingContent : public ns
     // Used to keep track of whether or not a plugin should be played.
     // This is used for click-to-play plugins.
     bool                        mShouldPlay : 1;
 
     // Used to keep track of whether or not a plugin has been played.
     // This is used for click-to-play plugins.
     bool                        mActivated : 1;
 
+    // Protects DoStopPlugin from reentry (bug 724781).
+    bool                        mIsStopping : 1;
+
     // Used to track when we might try to instantiate a plugin instance based on
     // a src data stream being delivered to this object. When this is true we don't
     // want plugin instance instantiation code to attempt to load src data again or
     // we'll deliver duplicate streams. Should be cleared when we are not loading
     // src data.
     bool mSrcStreamLoading;
 
     // A specific state that caused us to fallback