Bug 1633935 - P10 wait PHttpChannel::OnStartRequestSent for permission/cookie update from parent, r=mayhemer,necko-reviewers

Depends on D77751

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

netwerk/protocol/http/HttpChannelChild.cpp

@@ -189,7 +189,9 @@ HttpChannelChild::HttpChannelChild()
       mShouldParentIntercept(false),
       mSuspendParentAfterSynthesizeResponse(false),
       mIsLastPartOfMultiPart(false),
-      mSuspendForWaitCompleteRedirectSetup(false) {
+      mSuspendForWaitCompleteRedirectSetup(false),
+      mRecvOnStartRequestSentCalled(false),
+      mSuspendedByWaitingForPermissionAndCookie(false) {
   LOG(("Creating HttpChannelChild @%p\n", this));
 
   mChannelCreationTime = PR_Now();
@@ -391,6 +393,20 @@ void HttpChannelChild::AssociateApplicationCache(const nsCString& aGroupID,
   mApplicationCache->InitAsHandle(aGroupID, aClientID);
 }
 
+mozilla::ipc::IPCResult HttpChannelChild::RecvOnStartRequestSent() {
+  LOG(("HttpChannelChild::RecvOnStartRequestSent [this=%p]\n", this));
+  MOZ_ASSERT(NS_IsMainThread());
+  MOZ_ASSERT(!mRecvOnStartRequestSentCalled);
+
+  mRecvOnStartRequestSentCalled = true;
+
+  if (mSuspendedByWaitingForPermissionAndCookie) {
+    mSuspendedByWaitingForPermissionAndCookie = false;
+    mEventQ->Resume();
+  }
+  return IPC_OK();
+}
+
 void HttpChannelChild::ProcessOnStartRequest(
     const nsHttpResponseHead& aResponseHead, const bool& aUseResponseHead,
     const nsHttpHeaderArray& aRequestHeaders,
@@ -407,6 +423,7 @@ void HttpChannelChild::ProcessOnStartRequest(
                              aArgs);
       }));
 }
+
 static void ResourceTimingStructArgsToTimingsStruct(
     const ResourceTimingStructArgs& aArgs, TimingStruct& aTimings) {
   aTimings.domainLookupStart = aArgs.domainLookupStart();
@@ -522,6 +539,20 @@ void HttpChannelChild::OnStartRequest(
                                       false);
   }
 
+  if (aArgs.shouldWaitForOnStartRequestSent() &&
+      !mRecvOnStartRequestSentCalled) {
+    LOG(("  > pending DoOnStartRequest until RecvOnStartRequestSent\n"));
+    MOZ_ASSERT(NS_IsMainThread());
+
+    mEventQ->Suspend();
+    mSuspendedByWaitingForPermissionAndCookie = true;
+    mEventQ->PrependEvent(MakeUnique<NeckoTargetChannelFunctionEvent>(
+        this, [self = UnsafePtr<HttpChannelChild>(this)]() {
+          self->DoOnStartRequest(self, nullptr);
+        }));
+    return;
+  }
+
   DoOnStartRequest(this, nullptr);
 }
 

netwerk/protocol/http/HttpChannelChild.h

@@ -132,6 +132,7 @@ class HttpChannelChild final : public PHttpChannelChild,
   nsresult CrossProcessRedirectFinished(nsresult aStatus);
 
  protected:
+  mozilla::ipc::IPCResult RecvOnStartRequestSent() override;
   mozilla::ipc::IPCResult RecvFailedAsyncOpen(const nsresult& status) override;
   mozilla::ipc::IPCResult RecvRedirect1Begin(
       const uint32_t& registrarId, const URIParams& newURI,
@@ -462,6 +463,13 @@ class HttpChannelChild final : public PHttpChannelChild,
   // CompleteRedirectSetup/RecvDeleteSelf.
   uint8_t mSuspendForWaitCompleteRedirectSetup : 1;
 
+  // True if RecvOnStartRequestSent was received.
+  uint8_t mRecvOnStartRequestSentCalled : 1;
+
+  // True if this channel is suspened by waiting for permission and cookie. That
+  // is, RecvOnStartRequestSent is received.
+  uint8_t mSuspendedByWaitingForPermissionAndCookie : 1;
+
   void FinishInterceptedRedirect();
   void CleanupRedirectingChannel(nsresult rv);
 

netwerk/protocol/http/HttpChannelParams.ipdlh

@@ -48,6 +48,7 @@ struct HttpChannelOnStartRequestArgs
   nsCString appCacheGroupId;
   nsCString appCacheClientId;
   nsIReferrerInfo overrideReferrerInfo;
+  bool shouldWaitForOnStartRequestSent;
 };
 
 } // namespace ipc

netwerk/protocol/http/HttpChannelParent.cpp

@@ -1374,9 +1374,8 @@ HttpChannelParent::OnStartRequest(nsIRequest* aRequest) {
 
   RefPtr<HttpBaseChannel> chan = do_QueryObject(aRequest);
   if (!chan) {
-    nsCOMPtr<nsIMultiPartChannel> multiPartChannel =
-        do_QueryInterface(aRequest);
-    if (multiPartChannel) {
+    if (nsCOMPtr<nsIMultiPartChannel> multiPartChannel =
+            do_QueryInterface(aRequest)) {
       isMultiPart = true;
       nsCOMPtr<nsIChannel> baseChannel;
       multiPartChannel->GetBaseChannel(getter_AddRefs(baseChannel));
@@ -1487,8 +1486,11 @@ HttpChannelParent::OnStartRequest(nsIRequest* aRequest) {
   nsHttpResponseHead* responseHead = chan->GetResponseHead();
   bool useResponseHead = !!responseHead;
   nsHttpResponseHead cleanedUpResponseHead;
-  if (responseHead &&
-      (responseHead->HasHeader(nsHttp::Set_Cookie) || multiPartID)) {
+
+  bool hasSetCookie =
+      responseHead && responseHead->HasHeader(nsHttp::Set_Cookie);
+
+  if (hasSetCookie || multiPartID) {
     cleanedUpResponseHead = *responseHead;
     cleanedUpResponseHead.ClearHeader(nsHttp::Set_Cookie);
     if (multiPartID) {
@@ -1530,7 +1532,19 @@ HttpChannelParent::OnStartRequest(nsIRequest* aRequest) {
     cleanedUpRequest = true;
   }
 
+  bool isDocument = chan->IsDocument();
+  if (!isDocument) {
+    rv = chan->GetIsMainDocumentChannel(&isDocument);
+    NS_ENSURE_SUCCESS(rv, rv);
+  }
+
+  // Bug 1645901: Currently Set-Cookie is passed to child process on main
+  // thread, which is racy with PBackground. We should have a way to set cookie
+  // in child for Set-Cookie response header.
+  args.shouldWaitForOnStartRequestSent() = isDocument || hasSetCookie;
+
   rv = NS_OK;
+
   if (mIPCClosed ||
       !mBgParent->OnStartRequest(
           *responseHead, useResponseHead,
@@ -1540,6 +1554,15 @@ HttpChannelParent::OnStartRequest(nsIRequest* aRequest) {
   }
   requestHead->Exit();
 
+  // Need to wait for the permission to content process, which is sent via
+  // PContent in AboutToLoadHttpFtpDocumentForChild. For multipart channel,
+  // send only one time since the permissions are the same.
+  if (NS_SUCCEEDED(rv) && args.shouldWaitForOnStartRequestSent() &&
+      multiPartID.valueOr(0) == 0) {
+    LOG(("HttpChannelParent::SendOnStartRequestSent\n"));
+    Unused << SendOnStartRequestSent();
+  }
+
   return rv;
 }
 

netwerk/protocol/http/PHttpChannel.ipdl

@@ -107,6 +107,13 @@ child:
   // AsyncOpen of nsHttpChannel on the parent.
   async FailedAsyncOpen(nsresult status);
 
+  // OnStartRequest is sent over PHttpBackgroundChannel. However, sometime we
+  // need to wait for some PContent IPCs, e.g., permission, cookies. Those IPC
+  // are sent just before the background thread OnStartRequest, which is racy.
+  // Therefore, need one main thread IPC event for synchronizing the event
+  // sequence.
+  async OnStartRequestSent();
+
   // Called to initiate content channel redirect, starts talking to sinks
   // on the content process and reports result via Redirect2Verify above
   async Redirect1Begin(uint32_t           registrarId,