Bug 532208: Defer deletion of browser streams to later tasks. r=bsmedberg

dom/plugins/BrowserStreamChild.cpp

@@ -152,16 +152,6 @@ BrowserStreamChild::AnswerNPP_StreamAsFile(const nsCString& fname)
   return true;
 }
 
-bool
-BrowserStreamChild::Answer__delete__(const NPError& reason,
-                                     const bool& artificial)
-{
-  AssertPluginThread();
-  if (!artificial)
-    NPP_DestroyStream(reason);
-  return true;
-}
-
 NPError
 BrowserStreamChild::NPN_RequestRead(NPByteRange* aRangeList)
 {
@@ -180,18 +170,31 @@ BrowserStreamChild::NPN_RequestRead(NPByteRange* aRangeList)
   return result;
 }
 
-void
-BrowserStreamChild::NPP_DestroyStream(NPError reason)
+bool
+BrowserStreamChild::AnswerNPP_DestroyStream(const NPReason& reason,
+                                            NPError* err)
 {
   PLUGIN_LOG_DEBUG(("%s (reason=%i)", FULLFUNCTION, reason));
 
   AssertPluginThread();
 
-  if (mClosed)
-    return;
+  if (mClosed) {
+    NS_WARNING("NPP_DestroyStream on a closed stream");
+    return true;
+  }
 
-  mInstance->mPluginIface->destroystream(&mInstance->mData, &mStream, reason);
+  *err = mInstance->mPluginIface->destroystream(&mInstance->mData,
+                                                &mStream, reason);
   mClosed = true;
+  return true;
+}
+
+bool
+BrowserStreamChild::Recv__delete__()
+{
+  if (!mClosed)
+    NS_WARNING("__delete__ on an open stream");
+  return true;
 }
 
 } /* namespace plugins */

dom/plugins/BrowserStreamChild.h

@@ -81,8 +81,12 @@ public:
                                    int32_t* consumed);
 
   virtual bool AnswerNPP_StreamAsFile(const nsCString& fname);
-  virtual bool Answer__delete__(const NPError& reason,
-                                const bool& artificial);
+
+  NS_OVERRIDE
+  virtual bool AnswerNPP_DestroyStream(const NPReason& reason, NPError* err);
+
+  NS_OVERRIDE
+  virtual bool Recv__delete__();
 
 
   void EnsureCorrectInstance(PluginInstanceChild* i)

dom/plugins/BrowserStreamParent.cpp

@@ -2,6 +2,15 @@
 
 #include "BrowserStreamParent.h"
 #include "PluginInstanceParent.h"
+#include "PluginModuleParent.h"
+
+template<>
+struct RunnableMethodTraits<mozilla::plugins::BrowserStreamParent>
+{
+  typedef mozilla::plugins::BrowserStreamParent Cls;
+    static void RetainCallee(Cls* obj) { }
+    static void ReleaseCallee(Cls* obj) { }
+};
 
 namespace mozilla {
 namespace plugins {
@@ -10,12 +19,33 @@ BrowserStreamParent::BrowserStreamParent(PluginInstanceParent* npp,
                                          NPStream* stream)
   : mNPP(npp)
   , mStream(stream)
+  , mDeleteTask(nsnull)
 {
   mStream->pdata = static_cast<AStream*>(this);
 }
 
 BrowserStreamParent::~BrowserStreamParent()
 {
+  if (mDeleteTask) {
+    mDeleteTask->Cancel();
+    // MessageLoop::current() owns this
+    mDeleteTask = nsnull;
+  }
+}
+
+NPError
+BrowserStreamParent::NPP_DestroyStream(NPReason reason)
+{
+  PLUGIN_LOG_DEBUG(("%s (reason=%i)", FULLFUNCTION, reason));
+
+  if (!mDeleteTask) {
+    mDeleteTask = NewRunnableMethod(this, &BrowserStreamParent::Delete);
+    MessageLoop::current()->PostTask(FROM_HERE, mDeleteTask);
+  }
+
+  NPError err = NPERR_NO_ERROR;
+  CallNPP_DestroyStream(reason, &err);
+  return err;
 }
 
 bool
@@ -42,15 +72,6 @@ BrowserStreamParent::AnswerNPN_RequestRead(const IPCByteRanges& ranges,
   return true;
 }
 
-bool
-BrowserStreamParent::Answer__delete__(const NPError& reason,
-                                      const bool& artificial)
-{
-  if (!artificial)
-    NPN_DestroyStream(reason);
-  return true;
-}
-
 int32_t
 BrowserStreamParent::WriteReady()
 {
@@ -87,12 +108,21 @@ BrowserStreamParent::StreamAsFile(const char* fname)
   CallNPP_StreamAsFile(nsCString(fname));
 }
 
-NPError
-BrowserStreamParent::NPN_DestroyStream(NPReason reason)
+bool
+BrowserStreamParent::AnswerNPN_DestroyStream(const NPReason& reason,
+                                             NPError* result)
 {
   PLUGIN_LOG_DEBUG_FUNCTION;
 
-  return mNPP->mNPNIface->destroystream(mNPP->mNPP, mStream, reason);
+  *result = mNPP->mNPNIface->destroystream(mNPP->mNPP, mStream, reason);
+  return true;
+}
+
+void
+BrowserStreamParent::Delete()
+{
+  PBrowserStreamParent::Send__delete__(this);
+  // |this| just got deleted
 }
 
 } // namespace plugins

dom/plugins/BrowserStreamParent.h

@@ -44,6 +44,7 @@
 namespace mozilla {
 namespace plugins {
 
+class PluginModuleParent;
 class PluginInstanceParent;
 
 class BrowserStreamParent : public PBrowserStreamParent, public AStream
@@ -56,23 +57,25 @@ public:
                       NPStream* stream);
   virtual ~BrowserStreamParent();
 
+  NPError NPP_DestroyStream(NPReason reason);
+
   NS_OVERRIDE virtual bool IsBrowserStream() { return true; }
 
   virtual bool AnswerNPN_RequestRead(const IPCByteRanges& ranges,
                                      NPError* result);
 
-  virtual bool
-  Answer__delete__(const NPError& reason, const bool& artificial);
+  virtual bool AnswerNPN_DestroyStream(const NPReason& reason, NPError* result);
 
   int32_t WriteReady();
   int32_t Write(int32_t offset, int32_t len, void* buffer);
   void StreamAsFile(const char* fname);
 
 private:
-  NPError NPN_DestroyStream(NPError reason);
+  void Delete();
 
   PluginInstanceParent* mNPP;
   NPStream* mStream;
+  CancelableTask* mDeleteTask;
 };
 
 } // namespace plugins

dom/plugins/PBrowserStream.ipdl

@@ -66,17 +66,17 @@ child:
 
   rpc NPP_StreamAsFile(nsCString fname);
 
+  rpc NPP_DestroyStream(NPReason reason)
+    returns (NPError error);
+
+  async __delete__();
+
 parent:
   rpc NPN_RequestRead(IPCByteRanges ranges)
     returns (NPError result);
 
-both:
-  /**
-   * ~PBrowserStream is for both NPN_DestroyStream and NPP_DestroyStream.
-   * @param artificial True when the stream is closed as a by-product of
-   *                        some other call (such as a failure in NPP_Write).
-   */
-  rpc __delete__(NPReason reason, bool artificial);
+  rpc NPN_DestroyStream(NPReason reason)
+    returns (NPError error);
 };
 
 } // namespace plugins

dom/plugins/PluginInstanceParent.cpp

@@ -640,7 +640,7 @@ PluginInstanceParent::NPP_NewStream(NPMIMEType type, NPStream* stream,
         return NPERR_GENERIC_ERROR;
 
     if (NPERR_NO_ERROR != err)
-        PBrowserStreamParent::Call__delete__(bs, NPERR_GENERIC_ERROR, true);
+        bs->NPP_DestroyStream(NPERR_GENERIC_ERROR);
 
     return err;
 }
@@ -658,8 +658,7 @@ PluginInstanceParent::NPP_DestroyStream(NPStream* stream, NPReason reason)
         if (sp->mNPP != this)
             NS_RUNTIMEABORT("Mismatched plugin data");
 
-        PBrowserStreamParent::Call__delete__(sp, reason, false);
-        return NPERR_NO_ERROR;
+        return sp->NPP_DestroyStream(reason);
     }
     else {
         PluginStreamParent* sp =

dom/plugins/PluginModuleChild.cpp

@@ -898,7 +898,9 @@ _destroystream(NPP aNPP,
     if (s->IsBrowserStream()) {
         BrowserStreamChild* bs = static_cast<BrowserStreamChild*>(s);
         bs->EnsureCorrectInstance(p);
-        PBrowserStreamChild::Call__delete__(bs, aReason, false);
+        NPError err = NPERR_NO_ERROR;
+        bs->CallNPN_DestroyStream(aReason, &err);
+        return err;
     }
     else {
         PluginStreamChild* ps = static_cast<PluginStreamChild*>(s);