Bug 1633935 - P3 send OS*R and ODA via pHttpChannel for multipart channel, r=mayhemer,necko-reviewers

Depends on D76969

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

netwerk/protocol/http/HttpBackgroundChannelChild.cpp

@@ -83,11 +83,12 @@ bool HttpBackgroundChannelChild::ChannelClosed() {
   return !mChannelChild;
 }
 
-void HttpBackgroundChannelChild::OnStartRequestReceived() {
+void HttpBackgroundChannelChild::OnStartRequestReceived(
+    Maybe<uint32_t> aMultiPartID) {
   LOG(("HttpBackgroundChannelChild::OnStartRequestReceived [this=%p]\n", this));
   MOZ_ASSERT(OnSocketThread());
   MOZ_ASSERT(mChannelChild);
-  MOZ_ASSERT(!mStartReceived);  // Should only be called once.
+  MOZ_ASSERT(!mStartReceived || *aMultiPartID > 0);
 
   mStartReceived = true;
 
@@ -149,7 +150,7 @@ IPCResult HttpBackgroundChannelChild::RecvOnStartRequest(
                                        aRequestHeaders, aArgs);
   // Allow to queue other runnable since OnStartRequest Event already hits the
   // child's mEventQ.
-  OnStartRequestReceived();
+  OnStartRequestReceived(aArgs.multiPartID());
 
   return IPC_OK();
 }

netwerk/protocol/http/HttpBackgroundChannelChild.h

@@ -44,7 +44,7 @@ class HttpBackgroundChannelChild final : public PHttpBackgroundChannelChild {
 
   // Callback when OnStartRequest is received and handled by HttpChannelChild.
   // Enqueued messages in background channel will be flushed.
-  void OnStartRequestReceived();
+  void OnStartRequestReceived(Maybe<uint32_t> aMultiPartID);
 
 #ifdef MOZ_DIAGNOSTIC_ASSERT_ENABLED
   bool IsQueueEmpty() const { return mQueuedRunnables.IsEmpty(); }

netwerk/protocol/http/HttpChannelChild.cpp

@@ -395,9 +395,6 @@ void HttpChannelChild::ProcessOnStartRequest(
     const HttpChannelOnStartRequestArgs& aArgs) {
   LOG(("HttpChannelChild::ProcessOnStartRequest [this=%p]\n", this));
   MOZ_ASSERT(OnSocketThread());
-  MOZ_ASSERT(!mMultiPartID,
-             "Should only send OnStartRequest on the main-thread channel when "
-             "using multi-part!");
   MOZ_RELEASE_ASSERT(!mFlushedForDiversion,
                      "Should not be receiving any more callbacks from parent!");
 
@@ -442,14 +439,17 @@ mozilla::ipc::IPCResult HttpChannelChild::RecvOnStartRequest(
     // We don't need to notify the background channel if this is a multipart
     // stream, since all messages will be sent over the main-thread IPDL in
     // that case.
-    if (mBgChild && !aArgs.multiPartID()) {
-      MOZ_RELEASE_ASSERT(gSocketTransportService);
-      DebugOnly<nsresult> rv = gSocketTransportService->Dispatch(
-          NewRunnableMethod(
-              "HttpBackgroundChannelChild::OnStartRequestReceived", mBgChild,
-              &HttpBackgroundChannelChild::OnStartRequestReceived),
-          NS_DISPATCH_NORMAL);
-    }
+
+    // TODO: Never reach here. Remove later.
+
+    // if (mBgChild && !aArgs.multiPartID()) {
+    //   MOZ_RELEASE_ASSERT(gSocketTransportService);
+    //   DebugOnly<nsresult> rv = gSocketTransportService->Dispatch(
+    //       NewRunnableMethod(
+    //           "HttpBackgroundChannelChild::OnStartRequestReceived", mBgChild,
+    //           &HttpBackgroundChannelChild::OnStartRequestReceived),
+    //       NS_DISPATCH_NORMAL);
+    // }
   }
 
   return IPC_OK();
@@ -767,9 +767,6 @@ void HttpChannelChild::ProcessOnTransportAndData(
     const uint64_t& aOffset, const uint32_t& aCount, const nsCString& aData) {
   LOG(("HttpChannelChild::ProcessOnTransportAndData [this=%p]\n", this));
   MOZ_ASSERT(OnSocketThread());
-  MOZ_ASSERT(
-      !mMultiPartID,
-      "Should only send ODA on the main-thread channel when using multi-part!");
   MOZ_RELEASE_ASSERT(!mFlushedForDiversion,
                      "Should not be receiving any more callbacks from parent!");
   mEventQ->RunOrEnqueue(
@@ -998,9 +995,6 @@ void HttpChannelChild::ProcessOnStopRequest(
     const nsTArray<ConsoleReportCollected>& aConsoleReports) {
   LOG(("HttpChannelChild::ProcessOnStopRequest [this=%p]\n", this));
   MOZ_ASSERT(OnSocketThread());
-  MOZ_ASSERT(
-      !mMultiPartID,
-      "Should only send ODA on the main-thread channel when using multi-part!");
   MOZ_RELEASE_ASSERT(!mFlushedForDiversion,
                      "Should not be receiving any more callbacks from parent!");
 

netwerk/protocol/http/HttpChannelParent.cpp

@@ -83,8 +83,7 @@ HttpChannelParent::HttpChannelParent(dom::BrowserParent* iframeEmbedding,
       mCacheNeedFlowControlInitialized(false),
       mNeedFlowControl(true),
       mSuspendedForFlowControl(false),
-      mAfterOnStartRequestBegun(false),
-      mIsMultiPart(false) {
+      mAfterOnStartRequestBegun(false) {
   LOG(("Creating HttpChannelParent [this=%p]\n", this));
 
   // Ensure gHttpHandler is initialized: we need the atom table up and running.
@@ -1371,13 +1370,14 @@ HttpChannelParent::OnStartRequest(nsIRequest* aRequest) {
 
   Maybe<uint32_t> multiPartID;
   bool isLastPartOfMultiPart = false;
+  DebugOnly<bool> isMultiPart = false;
 
   RefPtr<HttpBaseChannel> chan = do_QueryObject(aRequest);
   if (!chan) {
     nsCOMPtr<nsIMultiPartChannel> multiPartChannel =
         do_QueryInterface(aRequest);
     if (multiPartChannel) {
-      mIsMultiPart = true;
+      isMultiPart = true;
       nsCOMPtr<nsIChannel> baseChannel;
       multiPartChannel->GetBaseChannel(getter_AddRefs(baseChannel));
       chan = do_QueryObject(baseChannel);
@@ -1388,7 +1388,7 @@ HttpChannelParent::OnStartRequest(nsIRequest* aRequest) {
       multiPartChannel->GetIsLastPart(&isLastPartOfMultiPart);
     }
   }
-  MOZ_ASSERT(multiPartID || !mIsMultiPart, "Changed multi-part state?");
+  MOZ_ASSERT(multiPartID || !isMultiPart, "Changed multi-part state?");
 
   if (!chan) {
     LOG(("  aRequest is not HttpBaseChannel"));
@@ -1531,27 +1531,15 @@ HttpChannelParent::OnStartRequest(nsIRequest* aRequest) {
   }
 
   rv = NS_OK;
-  bool ipcResult = false;
-  if (!mIPCClosed) {
-    // TODO: For multipart channel, we would deliever everything across
-    // pBackground as well.
-    if (!mIsMultiPart) {
-      ipcResult = mBgParent->OnStartRequest(
+  if (mIPCClosed ||
+      !mBgParent->OnStartRequest(
           *responseHead, useResponseHead,
           cleanedUpRequest ? cleanedUpRequestHeaders : requestHead->Headers(),
-          args);
-    } else {
-      ipcResult = SendOnStartRequest(
-          *responseHead, useResponseHead,
-          cleanedUpRequest ? cleanedUpRequestHeaders : requestHead->Headers(),
-          args);
-    }
+          args)) {
+    rv = NS_ERROR_UNEXPECTED;
   }
   requestHead->Exit();
 
-  if (mIPCClosed || !ipcResult) {
-    rv = NS_ERROR_UNEXPECTED;
-  }
   return rv;
 }
 
@@ -1585,20 +1573,11 @@ HttpChannelParent::OnStopRequest(nsIRequest* aRequest, nsresult aStatusCode) {
 
   // If we're handling a multi-part stream, then send this directly
   // over PHttpChannel to make synchronization easier.
-  if (!mIPCClosed && mIsMultiPart) {
-    // See the child code for why we do this.
-    TimeStamp lastActTabOpt = nsHttp::GetLastActiveTabLoadOptimizationHit();
-    if (!SendOnStopRequest(
-            aStatusCode, GetTimingAttributes(mChannel), lastActTabOpt,
-            responseTrailer ? *responseTrailer : nsHttpHeaderArray(),
-            consoleReports)) {
-      return NS_ERROR_UNEXPECTED;
-    }
-  } else if (mIPCClosed || !mBgParent ||
-             !mBgParent->OnStopRequest(
-                 aStatusCode, GetTimingAttributes(mChannel),
-                 responseTrailer ? *responseTrailer : nsHttpHeaderArray(),
-                 consoleReports)) {
+  if (mIPCClosed || !mBgParent ||
+      !mBgParent->OnStopRequest(
+          aStatusCode, GetTimingAttributes(mChannel),
+          responseTrailer ? *responseTrailer : nsHttpHeaderArray(),
+          consoleReports)) {
     return NS_ERROR_UNEXPECTED;
   }
 
@@ -1706,16 +1685,9 @@ HttpChannelParent::OnDataAvailable(nsIRequest* aRequest,
     // is ready to send OnTransportAndData.
     MOZ_ASSERT(mIPCClosed || mBgParent);
 
-    // If we're handling a multi-part stream, then send this directly
-    // over PHttpChannel to make synchronization easier.
-    if (!mIPCClosed && mIsMultiPart) {
-      if (!SendOnTransportAndData(channelStatus, transportStatus, aOffset,
-                                  toRead, data)) {
-        return NS_ERROR_UNEXPECTED;
-      }
-    } else if (mIPCClosed || !mBgParent ||
-               !mBgParent->OnTransportAndData(channelStatus, transportStatus,
-                                              aOffset, toRead, data)) {
+    if (mIPCClosed || !mBgParent ||
+        !mBgParent->OnTransportAndData(channelStatus, transportStatus, aOffset,
+                                       toRead, data)) {
       return NS_ERROR_UNEXPECTED;
     }
 

netwerk/protocol/http/HttpChannelParent.h

@@ -356,13 +356,6 @@ class HttpChannelParent final : public nsIInterfaceRequestor,
   // Used to ensure methods can't be called before OnStartRequest.
   uint8_t mAfterOnStartRequestBegun : 1;
 
-  // Set to true if we get OnStartRequest called with an nsIMultiPartChannel,
-  // and expect multiple OnStartRequest calls.
-  // When this happens we send OnTransportAndData and OnStopRequest over
-  // PHttpChannel instead of PHttpBackgroundChannel to make synchronizing all
-  // the parts easier.
-  uint8_t mIsMultiPart : 1;
-
   // Number of events to wait before actually invoking AsyncOpen on the main
   // channel. For each asynchronous step required before InvokeAsyncOpen, should
   // increase 1 to mAsyncOpenBarrier and invoke TryInvokeAsyncOpen after