Bug 1149358: Ensure that plugin streams are not manipulated by PluginAsyncSurrogate if plugin destruction is imminent; r=jimm a=lhenry
authorAaron Klotz <aklotz@mozilla.com>
Wed, 01 Apr 2015 16:53:47 -0600
changeset 265405 e8ed559c3ebe31e5de48ee7ef42dbfe3f6d9ccc6
parent 265404 651367982d90cf595d343d33f987b355b2c67503
child 265406 9e708c930a8fadf9dd959f38ffe3b69afbf62fdf
push id4718
push userraliiev@mozilla.com
push dateMon, 11 May 2015 18:39:53 +0000
treeherdermozilla-beta@c20c4ef55f08 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjimm, lhenry
bugs1149358
milestone39.0a2
Bug 1149358: Ensure that plugin streams are not manipulated by PluginAsyncSurrogate if plugin destruction is imminent; r=jimm a=lhenry
dom/plugins/ipc/PluginAsyncSurrogate.cpp
dom/plugins/ipc/PluginInstanceParent.cpp
--- a/dom/plugins/ipc/PluginAsyncSurrogate.cpp
+++ b/dom/plugins/ipc/PluginAsyncSurrogate.cpp
@@ -431,26 +431,30 @@ PluginAsyncSurrogate::SetStreamType(NPSt
     return false;
   }
   return streamListener->SetStreamType(aStreamType);
 }
 
 void
 PluginAsyncSurrogate::OnInstanceCreated(PluginInstanceParent* aInstance)
 {
-  for (uint32_t i = 0, len = mPendingNewStreamCalls.Length(); i < len; ++i) {
-    PendingNewStreamCall& curPendingCall = mPendingNewStreamCalls[i];
-    uint16_t streamType = NP_NORMAL;
-    NPError curError = aInstance->NPP_NewStream(
-                    const_cast<char*>(NullableStringGet(curPendingCall.mType)),
-                    curPendingCall.mStream, curPendingCall.mSeekable,
-                    &streamType);
-    if (curError != NPERR_NO_ERROR) {
-      // If we failed here then the send failed and we need to clean up
-      DestroyAsyncStream(curPendingCall.mStream);
+  if (!mDestroyPending) {
+    // If NPP_Destroy has already been called then these streams have already
+    // been cleaned up on the browser side and are no longer valid.
+    for (uint32_t i = 0, len = mPendingNewStreamCalls.Length(); i < len; ++i) {
+      PendingNewStreamCall& curPendingCall = mPendingNewStreamCalls[i];
+      uint16_t streamType = NP_NORMAL;
+      NPError curError = aInstance->NPP_NewStream(
+                      const_cast<char*>(NullableStringGet(curPendingCall.mType)),
+                      curPendingCall.mStream, curPendingCall.mSeekable,
+                      &streamType);
+      if (curError != NPERR_NO_ERROR) {
+        // If we failed here then the send failed and we need to clean up
+        DestroyAsyncStream(curPendingCall.mStream);
+      }
     }
   }
   mPendingNewStreamCalls.Clear();
   mInstantiated = true;
 }
 
 /**
  * During asynchronous initialization it might be necessary to wait for the
@@ -538,32 +542,32 @@ PluginAsyncSurrogate::AsyncCallArriving(
   if (--mAsyncCallsInFlight == 0) {
     mPluginDestructionGuard.reset(nullptr);
   }
 }
 
 void
 PluginAsyncSurrogate::NotifyAsyncInitFailed()
 {
-  // Clean up any pending NewStream requests
-  for (uint32_t i = 0, len = mPendingNewStreamCalls.Length(); i < len; ++i) {
-    PendingNewStreamCall& curPendingCall = mPendingNewStreamCalls[i];
-    DestroyAsyncStream(curPendingCall.mStream);
+  if (!mDestroyPending) {
+    // Clean up any pending NewStream requests
+    for (uint32_t i = 0, len = mPendingNewStreamCalls.Length(); i < len; ++i) {
+      PendingNewStreamCall& curPendingCall = mPendingNewStreamCalls[i];
+      DestroyAsyncStream(curPendingCall.mStream);
+    }
+    mPendingNewStreamCalls.Clear();
   }
-  mPendingNewStreamCalls.Clear();
 
   nsNPAPIPluginInstance* inst =
     static_cast<nsNPAPIPluginInstance*>(mInstance->ndata);
   if (!inst) {
       return;
   }
   nsPluginInstanceOwner* owner = inst->GetOwner();
-  if (!owner) {
-      return;
-  }
+  MOZ_ASSERT(owner);
   owner->NotifyHostAsyncInitFailed();
 }
 
 // static
 NPObject*
 PluginAsyncSurrogate::ScriptableAllocate(NPP aInstance, NPClass* aClass)
 {
   PLUGIN_LOG_DEBUG_FUNCTION;
--- a/dom/plugins/ipc/PluginInstanceParent.cpp
+++ b/dom/plugins/ipc/PluginInstanceParent.cpp
@@ -1734,20 +1734,27 @@ PluginInstanceParent::RecvAsyncNPP_NewRe
     //     calling NPP_SetWindow!
     mUseSurrogate = false;
 
     mSurrogate->AsyncCallArriving();
     if (aResult == NPERR_NO_ERROR) {
         mSurrogate->SetAcceptingCalls(true);
     }
 
+    // It is possible for a plugin instance to outlive its owner (eg. When a
+    // PluginDestructionGuard was on the stack at the time the owner was being
+    // destroyed). We need to handle that case.
     nsPluginInstanceOwner* owner = GetOwner();
-    // It is possible for a plugin instance to outlive its owner when async
-    // plugin init is turned on, so we need to handle that case.
-    if (aResult != NPERR_NO_ERROR || !owner) {
+    if (!owner) {
+        // We can't do anything at this point, just return. Any pending browser
+        // streams will be cleaned up when the plugin instance is destroyed.
+        return true;
+    }
+
+    if (aResult != NPERR_NO_ERROR) {
         mSurrogate->NotifyAsyncInitFailed();
         return true;
     }
 
     // Now we need to do a bunch of exciting post-NPP_New housekeeping.
     owner->NotifyHostCreateWidget();
 
     MOZ_ASSERT(mSurrogate);