Bug 1600211 - Forward AllPartsStopped to HttpChannelChild to ensure that we notify the listeners correctly. r=mayhemer

Differential Revision: https://phabricator.services.mozilla.com/D55223

--HG--
extra : moz-landing-system : lando

netwerk/ipc/DocumentLoadListener.cpp

@@ -384,6 +384,17 @@ void DocumentLoadListener::ResumeSuspendedChannel(
             aListener->OnStopRequest(aParams.request, rv);
           }
           rv = NS_OK;
+        },
+        [&](const OnAfterLastPartParams& aParams) {
+          nsCOMPtr<nsIMultiPartChannelListener> multiListener =
+              do_QueryInterface(aListener);
+          if (multiListener) {
+            if (NS_SUCCEEDED(rv)) {
+              multiListener->OnAfterLastPart(aParams.status);
+            } else {
+              multiListener->OnAfterLastPart(rv);
+            }
+          }
         });
   }
   // We don't expect to get new stream listener functions added
@@ -765,7 +776,9 @@ DocumentLoadListener::OnDataAvailable(nsIRequest* aRequest,
 //-----------------------------------------------------------------------------
 
 NS_IMETHODIMP
-DocumentLoadListener::OnAfterLastPart() {
+DocumentLoadListener::OnAfterLastPart(nsresult aStatus) {
+  mStreamListenerFunctions.AppendElement(StreamListenerFunction{
+      VariantIndex<3>{}, OnAfterLastPartParams{aStatus}});
   mIsFinished = true;
   return NS_OK;
 }

netwerk/ipc/DocumentLoadListener.h

@@ -274,8 +274,11 @@ class DocumentLoadListener : public nsIInterfaceRequestor,
     nsCOMPtr<nsIRequest> request;
     nsresult status;
   };
+  struct OnAfterLastPartParams {
+    nsresult status;
+  };
   typedef mozilla::Variant<OnStartRequestParams, OnDataAvailableParams,
-                           OnStopRequestParams>
+                           OnStopRequestParams, OnAfterLastPartParams>
       StreamListenerFunction;
   // TODO Backtrack this.
   // The set of nsIStreamListener functions that got called on this

netwerk/protocol/http/HttpChannelChild.cpp

@@ -582,6 +582,51 @@ mozilla::ipc::IPCResult HttpChannelChild::RecvOnStopRequest(
   return IPC_OK();
 }
 
+mozilla::ipc::IPCResult HttpChannelChild::RecvOnAfterLastPart(
+    const nsresult& aStatus) {
+  mEventQ->RunOrEnqueue(new NeckoTargetChannelFunctionEvent(
+      this, [self = UnsafePtr<HttpChannelChild>(this), aStatus]() {
+        self->OnAfterLastPart(aStatus);
+      }));
+  return IPC_OK();
+}
+
+void HttpChannelChild::OnAfterLastPart(const nsresult& aStatus) {
+  if (mOnStopRequestCalled) {
+    return;
+  }
+  mOnStopRequestCalled = true;
+
+  // notify "http-on-stop-connect" observers
+  gHttpHandler->OnStopRequest(this);
+
+  ReleaseListeners();
+
+  // If a preferred alt-data type was set, the parent would hold a reference to
+  // the cache entry in case the child calls openAlternativeOutputStream().
+  // (see nsHttpChannel::OnStopRequest)
+  if (!mPreferredCachedAltDataTypes.IsEmpty()) {
+    mAltDataCacheEntryAvailable = mCacheEntryAvailable;
+  }
+  mCacheEntryAvailable = false;
+
+  if (mLoadGroup) mLoadGroup->RemoveRequest(this, nullptr, mStatus);
+  CleanupBackgroundChannel();
+
+  if (mLoadFlags & LOAD_DOCUMENT_URI) {
+    // Keep IPDL channel open, but only for updating security info.
+    // If IPDL is already closed, then do nothing.
+    if (CanSend()) {
+      mKeptAlive = true;
+      SendDocumentChannelCleanup(true);
+    }
+  } else {
+    // The parent process will respond by sending a DeleteSelf message and
+    // making sure not to send any more messages after that.
+    TrySendDeletingChannel();
+  }
+}
+
 class SyntheticDiversionListener final : public nsIStreamListener {
   RefPtr<HttpChannelChild> mChannel;
 

netwerk/protocol/http/HttpChannelChild.h

@@ -216,6 +216,8 @@ class HttpChannelChild final : public PHttpChannelChild,
   mozilla::ipc::IPCResult RecvSetClassifierMatchedTrackingInfo(
       const ClassifierInfo& info) override;
 
+  mozilla::ipc::IPCResult RecvOnAfterLastPart(const nsresult& aStatus) override;
+
   virtual void ActorDestroy(ActorDestroyReason aWhy) override;
 
   virtual void DoNotifyListenerCleanup() override;
@@ -534,6 +536,7 @@ class HttpChannelChild final : public PHttpChannelChild,
   void DeleteSelf();
   void DoNotifyListener();
   void ContinueDoNotifyListener();
+  void OnAfterLastPart(const nsresult& aStatus);
 
   // Create a a new channel to be used in a redirection, based on the provided
   // response headers.

netwerk/protocol/http/HttpChannelParent.cpp

@@ -290,6 +290,7 @@ NS_INTERFACE_MAP_BEGIN(HttpChannelParent)
   NS_INTERFACE_MAP_ENTRY(nsIAsyncVerifyRedirectReadyCallback)
   NS_INTERFACE_MAP_ENTRY(nsIChannelEventSink)
   NS_INTERFACE_MAP_ENTRY(nsIRedirectResultListener)
+  NS_INTERFACE_MAP_ENTRY(nsIMultiPartChannelListener)
   NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsISupports, nsIParentRedirectingChannel)
   NS_INTERFACE_MAP_ENTRY_CONCRETE(HttpChannelParent)
 NS_INTERFACE_MAP_END
@@ -1590,6 +1591,18 @@ HttpChannelParent::OnStopRequest(nsIRequest* aRequest, nsresult aStatusCode) {
   return NS_OK;
 }
 
+//-----------------------------------------------------------------------------
+// HttpChannelParent::nsIMultiPartChannelListener
+//-----------------------------------------------------------------------------
+
+NS_IMETHODIMP
+HttpChannelParent::OnAfterLastPart(nsresult aStatus) {
+  if (!mIPCClosed) {
+    Unused << SendOnAfterLastPart(aStatus);
+  }
+  return NS_OK;
+}
+
 //-----------------------------------------------------------------------------
 // HttpChannelParent::nsIStreamListener
 //-----------------------------------------------------------------------------

netwerk/protocol/http/HttpChannelParent.h

@@ -23,6 +23,7 @@
 #include "nsIAuthPromptProvider.h"
 #include "mozilla/dom/ipc/IdType.h"
 #include "nsIDeprecationWarner.h"
+#include "nsIMultiPartChannel.h"
 
 class nsICacheEntry;
 
@@ -60,7 +61,8 @@ class HttpChannelParent final : public nsIInterfaceRequestor,
                                 public HttpChannelSecurityWarningReporter,
                                 public nsIAsyncVerifyRedirectReadyCallback,
                                 public nsIChannelEventSink,
-                                public nsIRedirectResultListener {
+                                public nsIRedirectResultListener,
+                                public nsIMultiPartChannelListener {
   virtual ~HttpChannelParent();
 
  public:
@@ -76,6 +78,7 @@ class HttpChannelParent final : public nsIInterfaceRequestor,
   NS_DECL_NSIASYNCVERIFYREDIRECTREADYCALLBACK
   NS_DECL_NSICHANNELEVENTSINK
   NS_DECL_NSIREDIRECTRESULTLISTENER
+  NS_DECL_NSIMULTIPARTCHANNELLISTENER
 
   NS_DECLARE_STATIC_IID_ACCESSOR(HTTP_CHANNEL_PARENT_IID)
 

netwerk/protocol/http/PHttpChannel.ipdl

@@ -238,6 +238,10 @@ child:
 
   // Tell the child information of matched URL againts SafeBrowsing tracking list
   async SetClassifierMatchedTrackingInfo(ClassifierInfo info);
+ 
+  // Forwards nsIMultiPartChannelListener::onAfterLastPart. Make sure we've
+  // disconnected our listeners, since we might not have on the last OnStopRequest.
+  async OnAfterLastPart(nsresult aStatus);
 
 both:
   // After receiving this message, the parent also calls

netwerk/protocol/http/ParentChannelListener.cpp

@@ -78,6 +78,14 @@ ParentChannelListener::OnStartRequest(nsIRequest* aRequest) {
 
   if (!mNextListener) return NS_ERROR_UNEXPECTED;
 
+  // If we're not a multi-part channel, then we can drop mListener and break the
+  // reference cycle. If we are, then this might be called again, so wait for
+  // OnAfterLastPart instead.
+  nsCOMPtr<nsIMultiPartChannel> multiPartChannel = do_QueryInterface(aRequest);
+  if (multiPartChannel) {
+    mIsMultiPart = true;
+  }
+
   LOG(("ParentChannelListener::OnStartRequest [this=%p]\n", this));
   return mNextListener->OnStartRequest(aRequest);
 }
@@ -94,11 +102,7 @@ ParentChannelListener::OnStopRequest(nsIRequest* aRequest,
        this, static_cast<uint32_t>(aStatusCode)));
   nsresult rv = mNextListener->OnStopRequest(aRequest, aStatusCode);
 
-  // If we're not a multi-part channel, then we can drop mListener and break the
-  // reference cycle. If we are, then this might be called again, so wait for
-  // OnAfterLastPart instead.
-  nsCOMPtr<nsIMultiPartChannel> multiPartChannel = do_QueryInterface(aRequest);
-  if (!multiPartChannel) {
+  if (!mIsMultiPart) {
     mNextListener = nullptr;
   }
   return rv;

netwerk/protocol/http/ParentChannelListener.h

@@ -90,6 +90,10 @@ class ParentChannelListener final : public nsIInterfaceRequestor,
   nsCOMPtr<nsINetworkInterceptController> mInterceptController;
 
   RefPtr<mozilla::dom::BrowserParent> mBrowserParent;
+
+  // True if we received OnStartRequest for a nsIMultiPartChannel, and are
+  // expected AllPartsStopped to be called when complete.
+  bool mIsMultiPart = false;
 };
 
 NS_DEFINE_STATIC_IID_ACCESSOR(ParentChannelListener, PARENT_CHANNEL_LISTENER)