Bug 787778 - Add additional re-entrance checks in plugin object code. r=josh
authorJohn Schoenick <jschoenick@mozilla.com>
Thu, 04 Oct 2012 13:50:49 -0700
changeset 110347 576c66cb9f2f9e7f3c3418ebb677a709a1e555a3
parent 110346 6ea044d85c1e3ef0dddcc636890536ff7801cd43
child 110348 d9e070c062f0dba3d724ec9f7cfc82f151dfd919
push id93
push usernmatsakis@mozilla.com
push dateWed, 31 Oct 2012 21:26:57 +0000
reviewersjosh
bugs787778
milestone19.0a1
Bug 787778 - Add additional re-entrance checks in plugin object code. r=josh
content/base/src/nsObjectLoadingContent.cpp
--- a/content/base/src/nsObjectLoadingContent.cpp
+++ b/content/base/src/nsObjectLoadingContent.cpp
@@ -849,18 +849,20 @@ nsObjectLoadingContent::OnStopRequest(ns
 
   if (aRequest != mChannel) {
     return NS_BINDING_ABORTED;
   }
 
   mChannel = nullptr;
 
   if (mFinalListener) {
-    mFinalListener->OnStopRequest(aRequest, aContext, aStatusCode);
+    // This may re-enter in the case of plugin listeners
+    nsCOMPtr<nsIStreamListener> listenerGrip(mFinalListener);
     mFinalListener = nullptr;
+    listenerGrip->OnStopRequest(aRequest, aContext, aStatusCode);
   }
 
   // Return value doesn't matter
   return NS_OK;
 }
 
 
 // nsIStreamListener
@@ -872,18 +874,20 @@ nsObjectLoadingContent::OnDataAvailable(
 {
   NS_ENSURE_TRUE(nsContentUtils::IsCallerChrome(), NS_ERROR_NOT_AVAILABLE);
 
   if (aRequest != mChannel) {
     return NS_BINDING_ABORTED;
   }
 
   if (mFinalListener) {
-    return mFinalListener->OnDataAvailable(aRequest, aContext, aInputStream,
-                                           aOffset, aCount);
+    // This may re-enter in the case of plugin listeners
+    nsCOMPtr<nsIStreamListener> listenerGrip(mFinalListener);
+    return listenerGrip->OnDataAvailable(aRequest, aContext, aInputStream,
+                                         aOffset, aCount);
   }
 
   // We shouldn't have a connected channel with no final listener
   NS_NOTREACHED("Got data for channel with no connected final listener");
   mChannel = nullptr;
 
   return NS_ERROR_UNEXPECTED;
 }
@@ -1673,37 +1677,31 @@ nsObjectLoadingContent::LoadObject(bool 
     NS_NOTREACHED("Trying to load with bad channel state");
     rv = NS_ERROR_UNEXPECTED;
     return NS_OK;
   }
 
   ///
   /// Attempt to load new type
   ///
-  
-  // Remove blocker on entering into instantiate
-  mIsLoading = false;
-  
+
+  // We don't set mFinalListener until OnStartRequest has been called, to
+  // prevent re-entry ugliness with CloseChannel()
+  nsCOMPtr<nsIStreamListener> finalListener;
   switch (mType) {
     case eType_Image:
       if (!mChannel) {
         // We have a LoadImage() call, but UpdateObjectParameters requires a
         // channel for images, so this is not a valid state.
         NS_NOTREACHED("Attempting to load image without a channel?");
         rv = NS_ERROR_UNEXPECTED;
         break;
       }
-      rv = LoadImageWithChannel(mChannel, getter_AddRefs(mFinalListener));
-      if (mFinalListener) {
-        // Note that LoadObject is called from mChannel's OnStartRequest
-        // when loading with a channel
-        mSrcStreamLoading = true;
-        rv = mFinalListener->OnStartRequest(mChannel, nullptr);
-        mSrcStreamLoading = false;
-      }
+      rv = LoadImageWithChannel(mChannel, getter_AddRefs(finalListener));
+      // finalListener will receive OnStartRequest below
     break;
     case eType_Plugin:
     {
       if (mChannel) {
         nsRefPtr<nsPluginHost> pluginHost =
           already_AddRefed<nsPluginHost>(nsPluginHost::GetInst());
         if (!pluginHost) {
           NS_NOTREACHED("No pluginHost");
@@ -1720,28 +1718,18 @@ nsObjectLoadingContent::LoadObject(bool 
           // We're un-rendered, and can't instantiate a plugin. HasNewFrame will
           // re-start us when we can proceed.
           LOG(("OBJLC [%p]: Aborting load - plugin-type, but no frame", this));
           CloseChannel();
           break;
         }
         
         rv = pluginHost->NewEmbeddedPluginStreamListener(mURI, this, nullptr,
-                                                         getter_AddRefs(mFinalListener));
-        if (NS_SUCCEEDED(rv)) {
-          // Note that LoadObject is called from mChannel's OnStartRequest
-          // when loading with a channel
-
-          mSrcStreamLoading = true;
-          rv = mFinalListener->OnStartRequest(mChannel, nullptr);
-          mSrcStreamLoading = false;
-          if (NS_SUCCEEDED(rv)) {
-            NotifyContentObjectWrapper();
-          }
-        }
+                                                         getter_AddRefs(finalListener));
+        // finalListener will receive OnStartRequest below
       } else {
         rv = AsyncStartPluginInstance();
       }
     }
     break;
     case eType_Document:
     {
       if (!mChannel) {
@@ -1787,92 +1775,110 @@ nsObjectLoadingContent::LoadObject(bool 
       nsCOMPtr<nsIURILoader>
         uriLoader(do_GetService(NS_URI_LOADER_CONTRACTID, &rv));
       if (NS_FAILED(rv)) {
         NS_NOTREACHED("Failed to get uriLoader service");
         mType = eType_Null;
         break;
       }
       rv = uriLoader->OpenChannel(mChannel, nsIURILoader::DONT_RETARGET, req,
-                                  getter_AddRefs(mFinalListener));
-      if (NS_SUCCEEDED(rv)) {
-        // Note that LoadObject is called from mChannel's OnStartRequest
-        // when loading with a channel
-        mSrcStreamLoading = true;
-        rv = mFinalListener->OnStartRequest(mChannel, nullptr);
-        mSrcStreamLoading = false;
-      }
+                                  getter_AddRefs(finalListener));
+      // finalListener will receive OnStartRequest below
     }
     break;
     case eType_Loading:
       // If our type remains Loading, we need a channel to proceed
       rv = OpenChannel();
       if (NS_FAILED(rv)) {
         LOG(("OBJLC [%p]: OpenChannel returned failure (%u)", this, rv));
       }
     break;
     case eType_Null:
       // Handled below, silence compiler warnings
     break;
   };
 
+  //
+  // Loaded, handle notifications and fallback
+  //
   if (NS_FAILED(rv)) {
     // If we failed in the loading hunk above, switch to fallback
     LOG(("OBJLC [%p]: Loading failed, switching to fallback", this));
     mType = eType_Null;
   }
 
-  // Switching to fallback state
+  // If we didn't load anything, handle switching to fallback state
   if (mType == eType_Null) {
     LOG(("OBJLC [%p]: Loading fallback, type %u", this, fallbackType));
     NS_ASSERTION(!mFrameLoader && !mInstanceOwner,
                  "switched to type null but also loaded something");
 
     if (mChannel) {
       // If we were loading with a channel but then failed over, throw it away
-      // (this also closes mFinalListener)
       CloseChannel();
     }
 
     // Don't notify or send events - we'll handle those ourselves
     // (so really this is just setting mFallbackType)
     LoadFallback(fallbackType, false);
   }
 
   // Notify of our final state if we haven't already
   NotifyStateChanged(oldType, oldState, false, aNotify);
-
+  
   if (mType == eType_Null && !mContentType.IsEmpty() &&
       mFallbackType != eFallbackAlternate) {
     // if we have a content type and are not showing alternate
     // content, fire a pluginerror to trigger (we stopped LoadFallback
     // from doing so above, it doesn't know of our old state)
     FirePluginError(mFallbackType);
   }
 
+  //
+  // Pass load on to finalListener if loading with a channel
+  //
+
+  // If we re-entered and loaded something else, that load will have cleaned up
+  // our our listener.
+  if (!mIsLoading) {
+    LOG(("OBJLC [%p]: Re-entered before dispatching to final listener", this));
+  } else if (finalListener) {
+    NS_ASSERTION(mType != eType_Null && mType != eType_Loading,
+                 "We should not have a final listener with a non-loaded type");
+    // Note that we always enter into LoadObject() from ::OnStartRequest when
+    // loading with a channel.
+    mSrcStreamLoading = true;
+    // Remove blocker on entering into instantiate
+    // (this is otherwise unset by the stack class)
+    mIsLoading = false;
+    mFinalListener = finalListener;
+    finalListener->OnStartRequest(mChannel, nullptr);
+    mSrcStreamLoading = false;
+  }
+
   return NS_OK;
 }
 
+// This call can re-enter when dealing with plugin listeners
 nsresult
 nsObjectLoadingContent::CloseChannel()
 {
   if (mChannel) {
     LOG(("OBJLC [%p]: Closing channel\n", this));
-    // These three statements are carefully ordered:
-    // - onStopRequest should get a channel whose status is the same as the
-    //   status argument
-    // - onStopRequest must get a non-null channel
-    mChannel->Cancel(NS_BINDING_ABORTED);
-    if (mFinalListener) {
-      // NOTE mFinalListener is only created when we load with a channel, which
-      //      LoadObject() requires come from a OnStartRequest call
-      mFinalListener->OnStopRequest(mChannel, nullptr, NS_BINDING_ABORTED);
-      mFinalListener = nullptr;
+    // Null the values before potentially-reentering, and ensure they survive
+    // the call
+    nsCOMPtr<nsIChannel> channelGrip(mChannel);
+    nsCOMPtr<nsIStreamListener> listenerGrip(mFinalListener);
+    mChannel = nullptr;
+    mFinalListener = nullptr;
+    channelGrip->Cancel(NS_BINDING_ABORTED);
+    if (listenerGrip) {
+      // mFinalListener is only set by LoadObject after OnStartRequest
+      listenerGrip->OnStopRequest(channelGrip, nullptr, NS_BINDING_ABORTED);
     }
-    mChannel = nullptr;
   }
   return NS_OK;
 }
 
 nsresult
 nsObjectLoadingContent::OpenChannel()
 {
   nsCOMPtr<nsIContent> thisContent = 
@@ -1968,17 +1974,21 @@ nsObjectLoadingContent::UnloadObject(boo
   // state
   CancelImageRequests(false);
   if (mFrameLoader) {
     mFrameLoader->Destroy();
     mFrameLoader = nullptr;
   }
 
   if (aResetState) {
-    CloseChannel();
+    if (mType != eType_Plugin) {
+      // This can re-enter when dealing with plugins, and StopPluginInstance
+      // will handle it
+      CloseChannel();
+    }
     mChannelLoaded = false;
     mType = eType_Loading;
     mURI = mOriginalURI = mBaseURI = nullptr;
     mContentType.Truncate();
     mOriginalContentType.Truncate();
   }
 
   // This call should be last as it may re-enter