Bug 551049 part 3: delay the delivery of NPP_URLNotify until the related stream is completely delivered and destroyed, and propagate errors from NPP_Write and NPN_DestroyStream back to NPP_URLNotify r=bent
authorBenjamin Smedberg <benjamin@smedbergs.us>
Sat, 06 Mar 2010 16:03:05 -0500
changeset 39283 beed4bc507c9c6a48cbaa7f828bf3304eb97931d
parent 39282 d5aa9bd8864d3ee31a4a977a41c61128e30d1390
child 39284 dc3af645f71324f778c3f59329c9bca6851b9e0d
push id12114
push userbsmedberg@mozilla.com
push dateThu, 11 Mar 2010 16:57:13 +0000
treeherdermozilla-central@dc3af645f713 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbent
bugs551049
milestone1.9.3a3pre
Bug 551049 part 3: delay the delivery of NPP_URLNotify until the related stream is completely delivered and destroyed, and propagate errors from NPP_Write and NPN_DestroyStream back to NPP_URLNotify r=bent
dom/plugins/BrowserStreamChild.cpp
dom/plugins/BrowserStreamChild.h
dom/plugins/PPluginInstance.ipdl
dom/plugins/PStreamNotify.ipdl
dom/plugins/PluginInstanceChild.cpp
dom/plugins/PluginInstanceChild.h
dom/plugins/StreamNotifyChild.h
--- a/dom/plugins/BrowserStreamChild.cpp
+++ b/dom/plugins/BrowserStreamChild.cpp
@@ -41,139 +41,141 @@
 
 namespace mozilla {
 namespace plugins {
 
 BrowserStreamChild::BrowserStreamChild(PluginInstanceChild* instance,
                                        const nsCString& url,
                                        const uint32_t& length,
                                        const uint32_t& lastmodified,
-                                       const PStreamNotifyChild* notifyData,
+                                       StreamNotifyChild* notifyData,
                                        const nsCString& headers,
                                        const nsCString& mimeType,
                                        const bool& seekable,
                                        NPError* rv,
                                        uint16_t* stype)
   : mInstance(instance)
-  , mClosed(false)
+  , mStreamStatus(kStreamOpen)
+  , mDestroyPending(NOT_DESTROYED)
+  , mNotifyPending(false)
+  , mInstanceDying(false)
   , mState(CONSTRUCTING)
   , mURL(url)
   , mHeaders(headers)
-  , mDestroyPending(kDestroyNotPending)
-  , mDeliverDataTracker(this)
+  , mStreamNotify(notifyData)
+  , mDeliveryTracker(this)
 {
   PLUGIN_LOG_DEBUG(("%s (%s, %i, %i, %p, %s, %s)", FULLFUNCTION,
                     url.get(), length, lastmodified, (void*) notifyData,
                     headers.get(), mimeType.get()));
 
   AssertPluginThread();
 
   memset(&mStream, 0, sizeof(mStream));
   mStream.ndata = static_cast<AStream*>(this);
   mStream.url = NullableStringGet(mURL);
   mStream.end = length;
   mStream.lastmodified = lastmodified;
+  mStream.headers = NullableStringGet(mHeaders);
   if (notifyData)
-    mStream.notifyData =
-      static_cast<const StreamNotifyChild*>(notifyData)->mClosure;
-  mStream.headers = NullableStringGet(mHeaders);
+    mStream.notifyData = notifyData->mClosure;
 }
 
 NPError
 BrowserStreamChild::StreamConstructed(
-            const nsCString& url,
-            const uint32_t& length,
-            const uint32_t& lastmodified,
-            PStreamNotifyChild* notifyData,
-            const nsCString& headers,
             const nsCString& mimeType,
             const bool& seekable,
             uint16_t* stype)
 {
   NPError rv = NPERR_NO_ERROR;
 
   *stype = NP_NORMAL;
   rv = mInstance->mPluginIface->newstream(
     &mInstance->mData, const_cast<char*>(NullableStringGet(mimeType)),
     &mStream, seekable, stype);
   if (rv != NPERR_NO_ERROR) {
-    mClosed = true;
     mState = DELETING;
+    mStreamNotify = NULL;
   }
   else {
     mState = ALIVE;
+
+    if (mStreamNotify)
+      mStreamNotify->SetAssociatedStream(this);
   }
 
   return rv;
 }
 
+BrowserStreamChild::~BrowserStreamChild()
+{
+  NS_ASSERTION(!mStreamNotify, "Should have nulled it by now!");
+}
+
 bool
 BrowserStreamChild::RecvWrite(const int32_t& offset,
                               const Buffer& data,
                               const uint32_t& newlength)
 {
   PLUGIN_LOG_DEBUG_FUNCTION;
 
   AssertPluginThread();
 
   if (ALIVE != mState)
     NS_RUNTIMEABORT("Unexpected state: received data after NPP_DestroyStream?");
 
-  if (mClosed)
+  if (kStreamOpen != mStreamStatus)
     return true;
 
   mStream.end = newlength;
 
   NS_ASSERTION(data.Length() > 0, "Empty data");
 
   PendingData* newdata = mPendingData.AppendElement();
   newdata->offset = offset;
   newdata->data = data;
   newdata->curpos = 0;
 
-  if (mDeliverDataTracker.empty())
-    MessageLoop::current()->PostTask(FROM_HERE,
-      mDeliverDataTracker.NewRunnableMethod(&BrowserStreamChild::DeliverData));
+  EnsureDeliveryPending();
 
   return true;
 }
 
 bool
 BrowserStreamChild::AnswerNPP_StreamAsFile(const nsCString& fname)
 {
   PLUGIN_LOG_DEBUG(("%s (fname=%s)", FULLFUNCTION, fname.get()));
 
   AssertPluginThread();
 
   if (ALIVE != mState)
     NS_RUNTIMEABORT("Unexpected state: received file after NPP_DestroyStream?");
 
-  if (mClosed)
+  if (kStreamOpen != mStreamStatus)
     return true;
 
   mInstance->mPluginIface->asfile(&mInstance->mData, &mStream,
                                   fname.get());
   return true;
 }
 
 bool
 BrowserStreamChild::RecvNPP_DestroyStream(const NPReason& reason)
 {
   PLUGIN_LOG_DEBUG_METHOD;
 
   if (ALIVE != mState)
     NS_RUNTIMEABORT("Unexpected state: recevied NPP_DestroyStream twice?");
 
   mState = DYING;
-  mDestroyPending = reason;
+  mDestroyPending = DESTROY_PENDING;
+  if (NPRES_DONE != reason)
+    mStreamStatus = reason;
 
-  if (mDeliverDataTracker.empty())
-    MessageLoop::current()->PostTask(FROM_HERE,
-      mDeliverDataTracker.NewRunnableMethod(&BrowserStreamChild::DeliverData));
-
+  EnsureDeliveryPending();
   return true;
 }
 
 bool
 BrowserStreamChild::Recv__delete__()
 {
   AssertPluginThread();
 
@@ -185,120 +187,131 @@ BrowserStreamChild::Recv__delete__()
 
 NPError
 BrowserStreamChild::NPN_RequestRead(NPByteRange* aRangeList)
 {
   PLUGIN_LOG_DEBUG_FUNCTION;
 
   AssertPluginThread();
 
-  if (ALIVE != mState || mClosed)
+  if (ALIVE != mState || kStreamOpen != mStreamStatus)
     return NPERR_GENERIC_ERROR;
 
   IPCByteRanges ranges;
   for (; aRangeList; aRangeList = aRangeList->next) {
     IPCByteRange br = {aRangeList->offset, aRangeList->length};
     ranges.push_back(br);
   }
 
   NPError result;
   CallNPN_RequestRead(ranges, &result);
   return result;
 }
 
 void
 BrowserStreamChild::NPN_DestroyStream(NPReason reason)
 {
-  mClosed = true;
+  mStreamStatus = reason;
   if (ALIVE == mState)
-    SendNPN_DestroyStream(NPRES_NETWORK_ERR);
+    SendNPN_DestroyStream(reason);
 
-  ClearSuspendedTimer();
-  mPendingData.Clear();
-  MaybeDeliverNPP_DestroyStream();
+  EnsureDeliveryPending();
+}
+
+void
+BrowserStreamChild::EnsureDeliveryPending()
+{
+  MessageLoop::current()->PostTask(FROM_HERE,
+    mDeliveryTracker.NewRunnableMethod(&BrowserStreamChild::Deliver));
 }
 
 void
-BrowserStreamChild::DeliverData()
+BrowserStreamChild::Deliver()
 {
-  if (mState != ALIVE && mState != DYING)
-    NS_RUNTIMEABORT("Unexpected state");
-
-  if (mClosed) {
-    ClearSuspendedTimer();
-    MaybeDeliverNPP_DestroyStream();
-    return;
+  while (kStreamOpen == mStreamStatus && mPendingData.Length()) {
+    if (DeliverPendingData() && kStreamOpen == mStreamStatus) {
+      SetSuspendedTimer();
+      return;
+    }
   }
+  ClearSuspendedTimer();
 
-  while (mPendingData.Length()) {
-    while (mPendingData[0].curpos < mPendingData[0].data.Length()) {
-      int32_t r = mInstance->mPluginIface->writeready(&mInstance->mData, &mStream);
-      if (r == 0) {
-        SetSuspendedTimer();
-        return;
-      }
-      if (!mPendingData.Length())
-        break;
-      r = mInstance->mPluginIface->write(
-        &mInstance->mData, &mStream,
-        mPendingData[0].offset + mPendingData[0].curpos, // offset
-        mPendingData[0].data.Length() - mPendingData[0].curpos, // length
-        const_cast<char*>(mPendingData[0].data.BeginReading() + mPendingData[0].curpos));
-      if (r == 0) {
-        SetSuspendedTimer();
-        return;
-      }
-      if (r < 0) { // error condition
-        NPN_DestroyStream(NPRES_NETWORK_ERR);
-        return;
-      }
-      if (!mPendingData.Length())
-        break;
+  NS_ASSERTION(kStreamOpen != mStreamStatus || 0 == mPendingData.Length(),
+               "Exit out of the data-delivery loop with pending data");
+  mPendingData.Clear();
 
-      mPendingData[0].curpos += r;
-    }
-    if (!mPendingData.Length())
-      break;
-    mPendingData.RemoveElementAt(0);
-  }
-
-  ClearSuspendedTimer();
-  MaybeDeliverNPP_DestroyStream();
-}
-
-void
-BrowserStreamChild::MaybeDeliverNPP_DestroyStream()
-{
-  if (kDestroyNotPending != mDestroyPending) {
-    NS_ASSERTION(mPendingData.Length() == 0, "Pending data?");
-
+  if (DESTROY_PENDING == mDestroyPending) {
+    mDestroyPending = DESTROYED;
     if (mState != DYING)
       NS_RUNTIMEABORT("mDestroyPending but state not DYING");
 
-    mClosed = true;
-    NPReason reason = mDestroyPending;
-    mDestroyPending = kDestroyNotPending;
+    NS_ASSERTION(NPRES_DONE != mStreamStatus, "Success status set too early!");
+    if (kStreamOpen == mStreamStatus)
+      mStreamStatus = NPRES_DONE;
 
     (void) mInstance->mPluginIface
-      ->destroystream(&mInstance->mData, &mStream, reason);
-
+      ->destroystream(&mInstance->mData, &mStream, mStreamStatus);
+  }
+  if (DESTROYED == mDestroyPending && mNotifyPending) {
+    NS_ASSERTION(mStreamNotify, "mDestroyPending but no mStreamNotify?");
+      
+    mNotifyPending = false;
+    mStreamNotify->NPP_URLNotify(mStreamStatus);
+    delete mStreamNotify;
+    mStreamNotify = NULL;
+  }
+  if (DYING == mState && DESTROYED == mDestroyPending
+      && !mStreamNotify && !mInstanceDying) {
     SendStreamDestroyed();
     mState = DELETING;
   }
 }
 
+bool
+BrowserStreamChild::DeliverPendingData()
+{
+  if (mState != ALIVE && mState != DYING)
+    NS_RUNTIMEABORT("Unexpected state");
+
+  NS_ASSERTION(mPendingData.Length(), "Called from Deliver with empty pending");
+
+  while (mPendingData[0].curpos < mPendingData[0].data.Length()) {
+    int32_t r = mInstance->mPluginIface->writeready(&mInstance->mData, &mStream);
+    if (kStreamOpen != mStreamStatus)
+      return false;
+    if (0 == r) // plugin wants to suspend delivery
+      return true;
+
+    r = mInstance->mPluginIface->write(
+      &mInstance->mData, &mStream,
+      mPendingData[0].offset + mPendingData[0].curpos, // offset
+      mPendingData[0].data.Length() - mPendingData[0].curpos, // length
+      const_cast<char*>(mPendingData[0].data.BeginReading() + mPendingData[0].curpos));
+    if (kStreamOpen != mStreamStatus)
+      return false;
+    if (0 == r)
+      return true;
+    if (r < 0) { // error condition
+      NPN_DestroyStream(NPRES_NETWORK_ERR);
+      return false;
+    }
+    mPendingData[0].curpos += r;
+  }
+  mPendingData.RemoveElementAt(0);
+  return false;
+}
 
 void
 BrowserStreamChild::SetSuspendedTimer()
 {
   if (mSuspendedTimer.IsRunning())
     return;
   mSuspendedTimer.Start(
-    base::TimeDelta::FromMilliseconds(100),
-    this, &BrowserStreamChild::DeliverData);
+    base::TimeDelta::FromMilliseconds(100), // 100ms copied from Mozilla plugin host
+    this, &BrowserStreamChild::Deliver);
 }
 
 void
 BrowserStreamChild::ClearSuspendedTimer()
 {
   mSuspendedTimer.Stop();
 }
 
--- a/dom/plugins/BrowserStreamChild.h
+++ b/dom/plugins/BrowserStreamChild.h
@@ -40,41 +40,36 @@
 
 #include "mozilla/plugins/PBrowserStreamChild.h"
 #include "mozilla/plugins/AStream.h"
 
 namespace mozilla {
 namespace plugins {
 
 class PluginInstanceChild;
-class PStreamNotifyChild;
+class StreamNotifyChild;
 
 class BrowserStreamChild : public PBrowserStreamChild, public AStream
 {
 public:
   BrowserStreamChild(PluginInstanceChild* instance,
                      const nsCString& url,
                      const uint32_t& length,
                      const uint32_t& lastmodified,
-                     const PStreamNotifyChild* notifyData,
+                     StreamNotifyChild* notifyData,
                      const nsCString& headers,
                      const nsCString& mimeType,
                      const bool& seekable,
                      NPError* rv,
                      uint16_t* stype);
-  virtual ~BrowserStreamChild() { }
+  virtual ~BrowserStreamChild();
 
   NS_OVERRIDE virtual bool IsBrowserStream() { return true; }
 
   NPError StreamConstructed(
-            const nsCString& url,
-            const uint32_t& length,
-            const uint32_t& lastmodified,
-            PStreamNotifyChild* notifyData,
-            const nsCString& headers,
             const nsCString& mimeType,
             const bool& seekable,
             uint16_t* stype);
 
   virtual bool RecvWrite(const int32_t& offset,
                          const Buffer& data,
                          const uint32_t& newsize);
   virtual bool AnswerNPP_StreamAsFile(const nsCString& fname);
@@ -90,64 +85,121 @@ public:
   {
     if (s != &mStream)
       NS_RUNTIMEABORT("Incorrect stream data");
   }
 
   NPError NPN_RequestRead(NPByteRange* aRangeList);
   void NPN_DestroyStream(NPReason reason);
 
+  void NotifyPending() {
+    NS_ASSERTION(!mNotifyPending, "Pending twice?");
+    mNotifyPending = true;
+    EnsureDeliveryPending();
+  }
+
+  /**
+   * During instance destruction, artificially cancel all outstanding streams.
+   *
+   * @return false if we are already in the DELETING state.
+   */
+  bool InstanceDying() {
+    if (DELETING == mState)
+      return false;
+
+    mInstanceDying = true;
+    return true;
+  }
+
+  void FinishDelivery() {
+    NS_ASSERTION(mInstanceDying, "Should only be called after InstanceDying");
+    NS_ASSERTION(DELETING != mState, "InstanceDying didn't work?");
+    mStreamStatus = NPRES_NETWORK_ERR;
+    Deliver();
+    NS_ASSERTION(!mStreamNotify, "Didn't deliver NPN_URLNotify?");
+  }
+
 private:
+  friend class StreamNotifyChild;
   using PBrowserStreamChild::SendNPN_DestroyStream;
 
   /**
-   * Deliver the data currently in mPending, scheduling
+   * Post an event to ensure delivery of pending data/destroy/urlnotify events
+   * outside of the current RPC stack.
+   */
+  void EnsureDeliveryPending();
+
+  /**
+   * Deliver data, destruction, notify scheduling
    * or cancelling the suspended timer as needed.
    */
-  void DeliverData();
-  void MaybeDeliverNPP_DestroyStream();
+  void Deliver();
+
+  /**
+   * Deliver one chunk of pending data.
+   * @return true if the plugin indicated a pause was necessary
+   */
+  bool DeliverPendingData();
+
   void SetSuspendedTimer();
   void ClearSuspendedTimer();
 
   PluginInstanceChild* mInstance;
   NPStream mStream;
-  bool mClosed;
+
+  static const NPReason kStreamOpen = -1;
+
+  /**
+   * The plugin's notion of whether a stream has been "closed" (no more
+   * data delivery) differs from the plugin host due to asynchronous delivery
+   * of data and NPN_DestroyStream. While the plugin-visible stream is open,
+   * mStreamStatus should be kStreamOpen (-1). mStreamStatus will be a
+   * failure code if either the parent or child indicates stream failure.
+   */
+  NPReason mStreamStatus;
+
+  /**
+   * Delivery of NPP_DestroyStream and NPP_URLNotify must be postponed until
+   * all data has been delivered.
+   */
+  enum {
+    NOT_DESTROYED, // NPP_DestroyStream not yet received
+    DESTROY_PENDING, // NPP_DestroyStream received, not yet delivered
+    DESTROYED // NPP_DestroyStream delivered, NPP_URLNotify may still be pending
+  } mDestroyPending;
+  bool mNotifyPending;
+
+  // When NPP_Destroy is called for our instance (manager), this flag is set
+  // cancels the stream and avoids sending StreamDestroyed.
+  bool mInstanceDying;
+
   enum {
     CONSTRUCTING,
     ALIVE,
     DYING,
     DELETING
   } mState;
   nsCString mURL;
   nsCString mHeaders;
-
-  static const NPReason kDestroyNotPending = -1;
-
-  /**
-   * When NPP_DestroyStream is delivered with pending data, we postpone
-   * delivering or acknowledging it until the pending data has been written.
-   *
-   * The default/initial value is kDestroyNotPending
-   */
-  NPReason mDestroyPending;
+  StreamNotifyChild* mStreamNotify;
 
   struct PendingData
   {
     int32_t offset;
     Buffer data;
     int32_t curpos;
   };
   nsTArray<PendingData> mPendingData;
 
   /**
    * Asynchronous RecvWrite messages are never delivered to the plugin
    * immediately, because that may be in the midst of an unexpected RPC
    * stack frame. It instead posts a runnable using this tracker to cancel
    * in case we are destroyed.
    */
-  ScopedRunnableMethodFactory<BrowserStreamChild> mDeliverDataTracker;
+  ScopedRunnableMethodFactory<BrowserStreamChild> mDeliveryTracker;
   base::RepeatingTimer<BrowserStreamChild> mSuspendedTimer;
 };
 
 } // namespace plugins
 } // namespace mozilla
 
 #endif /* mozilla_plugins_BrowserStreamChild_h */
--- a/dom/plugins/PPluginInstance.ipdl
+++ b/dom/plugins/PPluginInstance.ipdl
@@ -108,17 +108,17 @@ parent:
   rpc NPN_GetURL(nsCString url, nsCString target)
     returns (NPError result);
   rpc NPN_PostURL(nsCString url, nsCString target, nsCString buffer, bool file)
     returns (NPError result);
 
   /**
    * Covers both NPN_GetURLNotify and NPN_PostURLNotify.
    * @TODO This would be more readable as an overloaded method,
-   *       but IPDL doesn't allow that for constructors (or any method?).
+   *       but IPDL doesn't allow that for constructors.
    */
   rpc PStreamNotify(nsCString url, nsCString target, bool post,
                     nsCString buffer, bool file)
     returns (NPError result);
 
   async NPN_InvalidateRect(NPRect rect);
 
   rpc NPN_PushPopupsEnabledState(bool aState)
--- a/dom/plugins/PStreamNotify.ipdl
+++ b/dom/plugins/PStreamNotify.ipdl
@@ -15,13 +15,13 @@ namespace plugins {
 rpc protocol PStreamNotify
 {
   manager PPluginInstance;
 
 child:
   /**
    * Represents NPP_URLNotify
    */
-  rpc __delete__(NPReason reason);
+  async __delete__(NPReason reason);
 };
 
 } // namespace plugins
 } // namespace mozilla
--- a/dom/plugins/PluginInstanceChild.cpp
+++ b/dom/plugins/PluginInstanceChild.cpp
@@ -70,21 +70,21 @@ using mozilla::gfx::SharedDIB;
 // these, an ugly stall condition occurs. To ensure child stays in sync, we use
 // a timer callback to schedule work on in-calls.
 #define CHILD_MODALPUMPTIMEOUT 50
 #define CHILD_MODALLOOPTIMER   654321
 
 #endif // defined(OS_WIN)
 
 PluginInstanceChild::PluginInstanceChild(const NPPluginFuncs* aPluginIface,
-                                         const nsCString& aMimeType) :
-    mPluginIface(aPluginIface)
+                                         const nsCString& aMimeType)
+    : mPluginIface(aPluginIface)
+    , mQuirks(0)
     , mCachedWindowActor(nsnull)
     , mCachedElementActor(nsnull)
-    , mQuirks(0)
 #if defined(OS_WIN)
     , mPluginWindowHWND(0)
     , mPluginWndProc(0)
     , mPluginParentHWND(0)
     , mNestedEventHook(0)
     , mNestedPumpHook(0)
     , mNestedEventLevelDepth(0)
     , mNestedEventState(false)
@@ -1283,35 +1283,34 @@ PluginInstanceChild::AnswerPBrowserStrea
     const nsCString& headers,
     const nsCString& mimeType,
     const bool& seekable,
     NPError* rv,
     uint16_t* stype)
 {
     AssertPluginThread();
     *rv = static_cast<BrowserStreamChild*>(aActor)
-          ->StreamConstructed(url, length, lastmodified,
-                              notifyData, headers, mimeType, seekable,
-                              stype);
+          ->StreamConstructed(mimeType, seekable, stype);
     return true;
 }
 
 PBrowserStreamChild*
 PluginInstanceChild::AllocPBrowserStream(const nsCString& url,
                                          const uint32_t& length,
                                          const uint32_t& lastmodified,
                                          PStreamNotifyChild* notifyData,
                                          const nsCString& headers,
                                          const nsCString& mimeType,
                                          const bool& seekable,
                                          NPError* rv,
                                          uint16_t *stype)
 {
     AssertPluginThread();
-    return new BrowserStreamChild(this, url, length, lastmodified, notifyData,
+    return new BrowserStreamChild(this, url, length, lastmodified,
+                                  static_cast<StreamNotifyChild*>(notifyData),
                                   headers, mimeType, seekable, rv, stype);
 }
 
 bool
 PluginInstanceChild::DeallocPBrowserStream(PBrowserStreamChild* stream)
 {
     AssertPluginThread();
     delete stream;
@@ -1343,39 +1342,68 @@ PluginInstanceChild::AllocPStreamNotify(
                                         const bool& file,
                                         NPError* result)
 {
     AssertPluginThread();
     NS_RUNTIMEABORT("not reached");
     return NULL;
 }
 
-bool
-StreamNotifyChild::Answer__delete__(const NPReason& reason)
+void
+StreamNotifyChild::ActorDestroy(ActorDestroyReason why)
 {
-    AssertPluginThread();
-    return static_cast<PluginInstanceChild*>(Manager())
-        ->NotifyStream(this, reason);
+    if (AncestorDeletion == why && mBrowserStream) {
+        NS_ERROR("Pending NPP_URLNotify not called when closing an instance.");
+
+        // reclaim responsibility for deleting ourself
+        mBrowserStream = NULL;
+        mBrowserStream->mStreamNotify = NULL;
+    }
+}
+
+
+void
+StreamNotifyChild::SetAssociatedStream(BrowserStreamChild* bs)
+{
+    NS_ASSERTION(bs, "Shouldn't be null");
+    NS_ASSERTION(!mBrowserStream, "Two streams for one streamnotify?");
+
+    mBrowserStream = bs;
 }
 
 bool
-PluginInstanceChild::NotifyStream(StreamNotifyChild* notifyData,
-                                  NPReason reason)
+StreamNotifyChild::Recv__delete__(const NPReason& reason)
 {
-    if (notifyData->mClosure)
-        mPluginIface->urlnotify(&mData, notifyData->mURL.get(), reason,
-                                notifyData->mClosure);
+    AssertPluginThread();
+
+    if (mBrowserStream)
+        mBrowserStream->NotifyPending();
+    else
+        NPP_URLNotify(reason);
+
     return true;
 }
 
+void
+StreamNotifyChild::NPP_URLNotify(NPReason reason)
+{
+    PluginInstanceChild* instance = static_cast<PluginInstanceChild*>(Manager());
+
+    if (mClosure)
+        instance->mPluginIface->urlnotify(instance->GetNPP(), mURL.get(),
+                                          reason, mClosure);
+}
+
 bool
 PluginInstanceChild::DeallocPStreamNotify(PStreamNotifyChild* notifyData)
 {
     AssertPluginThread();
-    delete notifyData;
+
+    if (!static_cast<StreamNotifyChild*>(notifyData)->mBrowserStream)
+        delete notifyData;
     return true;
 }
 
 PluginScriptableObjectChild*
 PluginInstanceChild::GetActorForNPObject(NPObject* aObject)
 {
     AssertPluginThread();
     NS_ASSERTION(aObject, "Null pointer!");
@@ -1501,16 +1529,29 @@ DeleteObject(DeletingObjectEntry* e, voi
 }
 
 bool
 PluginInstanceChild::AnswerNPP_Destroy(NPError* aResult)
 {
     PLUGIN_LOG_DEBUG_METHOD;
     AssertPluginThread();
 
+    nsTArray<PBrowserStreamChild*> streams;
+    ManagedPBrowserStreamChild(streams);
+
+    // First make sure none of these streams become deleted
+    for (PRUint32 i = 0; i < streams.Length(); ) {
+        if (static_cast<BrowserStreamChild*>(streams[i])->InstanceDying())
+            ++i;
+        else
+            streams.RemoveElementAt(i);
+    }
+    for (PRUint32 i = 0; i < streams.Length(); ++i)
+        static_cast<BrowserStreamChild*>(streams[i])->FinishDelivery();
+
     for (PRUint32 i = 0; i < mPendingAsyncCalls.Length(); ++i)
         mPendingAsyncCalls[i]->Cancel();
     mPendingAsyncCalls.TruncateLength(0);
 
     mTimers.Clear();
 
     PluginModuleChild::current()->NPP_Destroy(this);
     mData.ndata = 0;
--- a/dom/plugins/PluginInstanceChild.h
+++ b/dom/plugins/PluginInstanceChild.h
@@ -54,21 +54,23 @@
 #include "nsRect.h"
 #include "nsTHashtable.h"
 
 namespace mozilla {
 namespace plugins {
 
 class PBrowserStreamChild;
 class BrowserStreamChild;
+class StreamNotifyChild;
 
 class PluginInstanceChild : public PPluginInstanceChild
 {
     friend class BrowserStreamChild;
     friend class PluginStreamChild;
+    friend class StreamNotifyChild; 
 
 #ifdef OS_WIN
     friend LRESULT CALLBACK PluginWindowProc(HWND hWnd,
                                              UINT message,
                                              WPARAM wParam,
                                              LPARAM lParam);
 #endif
 
@@ -178,18 +180,16 @@ public:
     GetActorForNPObject(NPObject* aObject);
 
     NPError
     NPN_NewStream(NPMIMEType aMIMEType, const char* aWindow,
                   NPStream** aStream);
 
     void InvalidateRect(NPRect* aInvalidRect);
 
-    bool NotifyStream(StreamNotifyChild* notifyData, NPReason reason);
-
     uint32_t ScheduleTimer(uint32_t interval, bool repeat, TimerFunc func);
     void UnscheduleTimer(uint32_t id);
 
 private:
     friend class PluginModuleChild;
 
     // Quirks mode support for various plugin mime types
     enum PluginQuirks {
--- a/dom/plugins/StreamNotifyChild.h
+++ b/dom/plugins/StreamNotifyChild.h
@@ -39,34 +39,56 @@
 #ifndef mozilla_plugins_StreamNotifyChild_h
 #define mozilla_plugins_StreamNotifyChild_h
 
 #include "mozilla/plugins/PStreamNotifyChild.h"
 
 namespace mozilla {
 namespace plugins {
 
+class BrowserStreamChild;
+
 class StreamNotifyChild : public PStreamNotifyChild
 {
   friend class PluginInstanceChild;
   friend class BrowserStreamChild;
 
 public:
   StreamNotifyChild(const nsCString& aURL)
     : mURL(aURL)
     , mClosure(NULL)
+    , mBrowserStream(NULL)
   { }
 
+  NS_OVERRIDE virtual void ActorDestroy(ActorDestroyReason why);
+
   void SetValid(void* aClosure) {
     mClosure = aClosure;
   }
 
-  bool Answer__delete__(const NPReason& reason);
+  void NPP_URLNotify(NPReason reason);
 
 private:
+  NS_OVERRIDE virtual bool Recv__delete__(const NPReason& reason);
+
+  /**
+   * If a stream is created for this this URLNotify, we associate the objects
+   * so that the NPP_URLNotify call is not fired before the stream data is
+   * completely delivered. The BrowserStreamChild takes responsibility for
+   * calling NPP_URLNotify and deleting this object.
+   */
+  void SetAssociatedStream(BrowserStreamChild* bs);
+
   nsCString mURL;
   void* mClosure;
+
+  /**
+   * If mBrowserStream is true, it is responsible for deleting this C++ object
+   * and DeallocPStreamNotify is not, so that the delayed delivery of
+   * NPP_URLNotify is possible.
+   */
+  BrowserStreamChild* mBrowserStream;
 };
 
 } // namespace plugins
 } // namespace mozilla
 
 #endif