Bug 1633935 - P1 allow OnStartRequest go thru pBg IPC, r=mayhemer

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

netwerk/protocol/http/HttpBackgroundChannelChild.cpp

@@ -151,6 +151,32 @@ IPCResult HttpBackgroundChannelChild::RecvOnStartRequestSent() {
   return IPC_OK();
 }
 
+IPCResult HttpBackgroundChannelChild::RecvOnStartRequest(
+    const nsHttpResponseHead& aResponseHead, const bool& aUseResponseHead,
+    const nsHttpHeaderArray& aRequestHeaders,
+    const HttpChannelOnStartRequestArgs& aArgs) {
+  LOG(("HttpBackgroundChannelChild::RecvOnStartRequest [this=%p]\n", this));
+  MOZ_ASSERT(OnSocketThread());
+
+  MOZ_ASSERT(mChannelChild, "no channel child in RecvOnStartRequest");
+  if (NS_WARN_IF(!mChannelChild)) {
+    return IPC_OK();
+  }
+
+  // TODO: OnStartRequest is off-main-thread so it's unnecessary to dispatch
+  // another IPC message for sync reason. Directly call here to behave the same
+  // as before. This is no longer needed and removed in the next patches.
+  RecvOnStartRequestSent();
+
+  mChannelChild->ProcessOnStartRequest(aResponseHead, aUseResponseHead,
+                                       aRequestHeaders, aArgs);
+  // Allow to queue other runnable since OnStartRequest Event already hits the
+  // child's mEventQ.
+  OnStartRequestReceived();
+
+  return IPC_OK();
+}
+
 IPCResult HttpBackgroundChannelChild::RecvOnTransportAndData(
     const nsresult& aChannelStatus, const nsresult& aTransportStatus,
     const uint64_t& aOffset, const uint32_t& aCount, const nsCString& aData,

netwerk/protocol/http/HttpBackgroundChannelChild.h

@@ -51,6 +51,11 @@ class HttpBackgroundChannelChild final : public PHttpBackgroundChannelChild {
 #endif
 
  protected:
+  IPCResult RecvOnStartRequest(const nsHttpResponseHead& aResponseHead,
+                               const bool& aUseResponseHead,
+                               const nsHttpHeaderArray& aRequestHeaders,
+                               const HttpChannelOnStartRequestArgs& aArgs);
+
   IPCResult RecvOnTransportAndData(const nsresult& aChannelStatus,
                                    const nsresult& aTransportStatus,
                                    const uint64_t& aOffset,

netwerk/protocol/http/HttpBackgroundChannelParent.cpp

@@ -166,6 +166,37 @@ bool HttpBackgroundChannelParent::OnStartRequestSent() {
   return SendOnStartRequestSent();
 }
 
+bool HttpBackgroundChannelParent::OnStartRequest(
+    const nsHttpResponseHead& aResponseHead, const bool& aUseResponseHead,
+    const nsHttpHeaderArray& aRequestHeaders,
+    const HttpChannelOnStartRequestArgs& aArgs) {
+  LOG(("HttpBackgroundChannelParent::OnStartRequest [this=%p]\n", this));
+  AssertIsInMainProcess();
+
+  if (NS_WARN_IF(!mIPCOpened)) {
+    return false;
+  }
+
+  if (!IsOnBackgroundThread()) {
+    MutexAutoLock lock(mBgThreadMutex);
+    nsresult rv = mBackgroundThread->Dispatch(
+        NewRunnableMethod<const nsHttpResponseHead, const bool,
+                          const nsHttpHeaderArray,
+                          const HttpChannelOnStartRequestArgs>(
+            "net::HttpBackgroundChannelParent::OnStartRequest", this,
+            &HttpBackgroundChannelParent::OnStartRequest, aResponseHead,
+            aUseResponseHead, aRequestHeaders, aArgs),
+        NS_DISPATCH_NORMAL);
+
+    MOZ_DIAGNOSTIC_ASSERT(NS_SUCCEEDED(rv));
+
+    return NS_SUCCEEDED(rv);
+  }
+
+  return SendOnStartRequest(aResponseHead, aUseResponseHead, aRequestHeaders,
+                            aArgs);
+}
+
 bool HttpBackgroundChannelParent::OnTransportAndData(
     const nsresult& aChannelStatus, const nsresult& aTransportStatus,
     const uint64_t& aOffset, const uint32_t& aCount, const nsCString& aData) {

netwerk/protocol/http/HttpBackgroundChannelParent.h

@@ -42,6 +42,12 @@ class HttpBackgroundChannelParent final : public PHttpBackgroundChannelParent {
   // To send OnStartRequestSend message over background channel.
   bool OnStartRequestSent();
 
+  // To send OnStartRequest message over background channel.
+  bool OnStartRequest(const nsHttpResponseHead& aResponseHead,
+                      const bool& aUseResponseHead,
+                      const nsHttpHeaderArray& aRequestHeaders,
+                      const HttpChannelOnStartRequestArgs& aArgs);
+
   // To send OnTransportAndData message over background channel.
   bool OnTransportAndData(const nsresult& aChannelStatus,
                           const nsresult& aTransportStatus,

netwerk/protocol/http/HttpChannelChild.cpp

@@ -389,6 +389,26 @@ void HttpChannelChild::AssociateApplicationCache(const nsCString& aGroupID,
   mApplicationCache->InitAsHandle(aGroupID, aClientID);
 }
 
+void HttpChannelChild::ProcessOnStartRequest(
+    const nsHttpResponseHead& aResponseHead, const bool& aUseResponseHead,
+    const nsHttpHeaderArray& aRequestHeaders,
+    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!");
+
+  mEventQ->RunOrEnqueue(new NeckoTargetChannelFunctionEvent(
+      this, [self = UnsafePtr<HttpChannelChild>(this), aResponseHead,
+             aUseResponseHead, aRequestHeaders, aArgs]() {
+        self->OnStartRequest(aResponseHead, aUseResponseHead, aRequestHeaders,
+                             aArgs);
+      }));
+}
+
 mozilla::ipc::IPCResult HttpChannelChild::RecvOnStartRequest(
     const nsHttpResponseHead& aResponseHead, const bool& aUseResponseHead,
     const nsHttpHeaderArray& aRequestHeaders,

netwerk/protocol/http/HttpChannelChild.h

@@ -267,6 +267,10 @@ class HttpChannelChild final : public PHttpChannelChild,
   already_AddRefed<nsIEventTarget> GetODATarget();
 
   [[nodiscard]] nsresult ContinueAsyncOpen();
+  void ProcessOnStartRequest(const nsHttpResponseHead& aResponseHead,
+                             const bool& aUseResponseHead,
+                             const nsHttpHeaderArray& aRequestHeaders,
+                             const HttpChannelOnStartRequestArgs& aArgs);
 
   // Callbacks while receiving OnTransportAndData/OnStopRequest/OnProgress/
   // OnStatus/FlushedForDiversion/DivertMessages on background IPC channel.

netwerk/protocol/http/HttpChannelParent.cpp

@@ -1531,15 +1531,30 @@ HttpChannelParent::OnStartRequest(nsIRequest* aRequest) {
   }
 
   rv = NS_OK;
-  if (mIPCClosed ||
-      !SendOnStartRequest(
+  bool ipcResult = false;
+  if (!mIPCClosed) {
+    // TODO: For multipart channel, we would deliever everything across
+    // pBackground as well.
+    // TODO: OnStartRequestSent is no longer needed since
+    // OnStartRequest/ODA/OnStopRequest are on the same thread.
+    if (!mIsMultiPart) {
+      ipcResult = mBgParent->OnStartRequest(
           *responseHead, useResponseHead,
           cleanedUpRequest ? cleanedUpRequestHeaders : requestHead->Headers(),
-          args)) {
-    rv = NS_ERROR_UNEXPECTED;
+          args);
+    } else {
+      ipcResult = SendOnStartRequest(
+          *responseHead, useResponseHead,
+          cleanedUpRequest ? cleanedUpRequestHeaders : requestHead->Headers(),
+          args);
+    }
   }
   requestHead->Exit();
 
+  if (mIPCClosed || !ipcResult) {
+    rv = NS_ERROR_UNEXPECTED;
+  }
+
   // OnStartRequest is sent to content process successfully.
   // Notify PHttpBackgroundChannelChild that all following IPC mesasges
   // should be run after OnStartRequest is handled.

netwerk/protocol/http/PHttpBackgroundChannel.ipdl

@@ -26,6 +26,11 @@ child:
   // is processed. For synchronizing the event sequence.
   async OnStartRequestSent();
 
+  async OnStartRequest(nsHttpResponseHead  responseHead,
+                       bool                useResponseHead,
+                       nsHttpHeaderArray   requestHeaders,
+                       HttpChannelOnStartRequestArgs args);
+
   // Combines a single OnDataAvailable and its associated OnProgress &
   // OnStatus calls into one IPDL message
   // This is duplicated on PHttpChannel, which gets used for multi-part