Bug 719117 - Fix bug 724781 by preventing reentry to DoStopPlugin with a flag. part 2/3, r=roc a=akeybl
authorMats Palmgren <matspal@gmail.com>
Mon, 28 May 2012 21:56:02 +0200
changeset 95880 3a2ab408f2e2507e565e8fcfaa1ee0ae63983c39
parent 95879 e8dd14a636a193138cb6b8bdc955ebe9f6f20a0a
child 95881 45a587498baee12f123c02ed744bc4217b7e73c9
push id886
push userlsblakk@mozilla.com
push dateMon, 04 Jun 2012 19:57:52 +0000
treeherdermozilla-beta@bbd8d5efd6d1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersroc, akeybl
bugs719117, 724781
milestone14.0a2
Bug 719117 - Fix bug 724781 by preventing reentry to DoStopPlugin with a flag. part 2/3, r=roc a=akeybl
content/base/src/nsObjectLoadingContent.cpp
content/base/src/nsObjectLoadingContent.h
--- a/content/base/src/nsObjectLoadingContent.cpp
+++ b/content/base/src/nsObjectLoadingContent.cpp
@@ -345,31 +345,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();
@@ -398,17 +404,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) {
@@ -620,16 +626,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;
@@ -2151,17 +2158,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),
@@ -2169,43 +2178,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
@@ -237,17 +237,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:
@@ -404,16 +405,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