Bug 1530691 - Ignore OnStartRequest/StopRequest/DataAvailable after HttpChannelChild::ActorDestroy(Deletion) is called r=mayhemer
authorValentin Gosu <valentin.gosu@gmail.com>
Thu, 07 Mar 2019 20:59:56 +0000
changeset 463018 a75985a8a0863ad2df578643c7507193115973f8
parent 463017 a5641ee4b22773ce34c7aaab1201dca44cca42a2
child 463019 9b8ef8d817750751af255a622bcf208a7db1c3b2
push id35665
push usershindli@mozilla.com
push dateFri, 08 Mar 2019 09:39:15 +0000
treeherdermozilla-central@54ed5eac2abc [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmayhemer
bugs1530691
milestone67.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1530691 - Ignore OnStartRequest/StopRequest/DataAvailable after HttpChannelChild::ActorDestroy(Deletion) is called r=mayhemer Differential Revision: https://phabricator.services.mozilla.com/D21908
netwerk/protocol/http/HttpChannelChild.cpp
netwerk/protocol/http/HttpChannelChild.h
--- a/netwerk/protocol/http/HttpChannelChild.cpp
+++ b/netwerk/protocol/http/HttpChannelChild.cpp
@@ -175,16 +175,17 @@ HttpChannelChild::HttpChannelChild()
       mFlushedForDiversion(false),
       mIsFromCache(false),
       mCacheNeedToReportBytesReadInitialized(false),
       mNeedToReportBytesRead(true),
       mCacheEntryAvailable(false),
       mAltDataCacheEntryAvailable(false),
       mSendResumeAt(false),
       mKeptAlive(false),
+      mIPCActorDeleted(false),
       mSuspendSent(false),
       mSynthesizedResponse(false),
       mShouldInterceptSubsequentRedirect(false),
       mRedirectingForSubsequentSynthesizedResponse(false),
       mPostRedirectChannelShouldIntercept(false),
       mPostRedirectChannelShouldUpgrade(false),
       mShouldParentIntercept(false),
       mSuspendParentAfterSynthesizeResponse(false) {
@@ -528,17 +529,21 @@ void HttpChannelChild::OnStartRequest(
   // stage, as they are set in the listener's OnStartRequest.
   MOZ_RELEASE_ASSERT(
       !mFlushedForDiversion,
       "mFlushedForDiversion should be unset before OnStartRequest!");
   MOZ_RELEASE_ASSERT(
       !mDivertingToParent,
       "mDivertingToParent should be unset before OnStartRequest!");
 
-  if (mOnStartRequestCalled && !mIPCOpen) {
+  // If this channel was aborted by ActorDestroy, then there may be other
+  // OnStartRequest/OnStopRequest/OnDataAvailable IPC messages that need to
+  // be handled. In that case we just ignore them to avoid calling the listener
+  // twice.
+  if (mOnStartRequestCalled && mIPCActorDeleted) {
     return;
   }
 
   if (!mCanceled && NS_SUCCEEDED(mStatus)) {
     mStatus = channelStatus;
   }
 
   // Cookies headers should not be visible to the child process
@@ -658,21 +663,21 @@ void HttpChannelChild::DoOnStartRequest(
   }
 
   if (mSynthesizedResponsePump && mLoadFlags & LOAD_CALL_CONTENT_SNIFFERS) {
     mSynthesizedResponsePump->PeekStream(CallTypeSniffers,
                                          static_cast<nsIChannel*>(this));
   }
 
   nsresult rv = mListener->OnStartRequest(aRequest);
+  mOnStartRequestCalled = true;
   if (NS_FAILED(rv)) {
     Cancel(rv);
     return;
   }
-  mOnStartRequestCalled = true;
 
   if (mDivertingToParent) {
     mListener = nullptr;
     mCompressListener = nullptr;
     if (mLoadGroup) {
       mLoadGroup->RemoveRequest(this, nullptr, mStatus);
     }
 
@@ -1028,17 +1033,21 @@ void HttpChannelChild::MaybeDivertOnStop
 
 void HttpChannelChild::OnStopRequest(
     const nsresult& channelStatus, const ResourceTimingStruct& timing,
     const nsHttpHeaderArray& aResponseTrailers) {
   LOG(("HttpChannelChild::OnStopRequest [this=%p status=%" PRIx32 "]\n", this,
        static_cast<uint32_t>(channelStatus)));
   MOZ_ASSERT(NS_IsMainThread());
 
-  if (mOnStopRequestCalled && !mIPCOpen) {
+  // If this channel was aborted by ActorDestroy, then there may be other
+  // OnStartRequest/OnStopRequest/OnDataAvailable IPC messages that need to
+  // be handled. In that case we just ignore them to avoid calling the listener
+  // twice.
+  if (mOnStopRequestCalled && mIPCActorDeleted) {
     return;
   }
 
   if (mDivertingToParent) {
     MOZ_RELEASE_ASSERT(
         !mFlushedForDiversion,
         "Should not be processing any more callbacks from parent!");
 
@@ -3825,16 +3834,19 @@ void HttpChannelChild::ActorDestroy(Acto
     AutoEventEnqueuer ensureSerialDispatch(mEventQ);
 
     mStatus = NS_ERROR_DOCSHELL_DYING;
     HandleAsyncAbort();
 
     // Cleanup the background channel before we resume the eventQ so we don't
     // get any other events.
     CleanupBackgroundChannel();
+
+    mIPCActorDeleted = true;
+    mCanceled = true;
   }
 }
 
 mozilla::ipc::IPCResult HttpChannelChild::RecvLogBlockedCORSRequest(
     const nsString& aMessage, const nsCString& aCategory) {
   Unused << LogBlockedCORSRequest(aMessage, aCategory);
   return IPC_OK();
 }
--- a/netwerk/protocol/http/HttpChannelChild.h
+++ b/netwerk/protocol/http/HttpChannelChild.h
@@ -408,16 +408,20 @@ class HttpChannelChild final : public PH
   uint8_t mCacheEntryAvailable : 1;
   uint8_t mAltDataCacheEntryAvailable : 1;
 
   // If ResumeAt is called before AsyncOpen, we need to send extra data upstream
   uint8_t mSendResumeAt : 1;
 
   uint8_t mKeptAlive : 1;  // IPC kept open, but only for security info
 
+  // Set when ActorDestroy(ActorDestroyReason::Deletion) is called
+  // The channel must ignore any following OnStart/Stop/DataAvailable messages
+  uint8_t mIPCActorDeleted : 1;
+
   // Set if SendSuspend is called. Determines if SendResume is needed when
   // diverting callbacks to parent.
   uint8_t mSuspendSent : 1;
 
   // Set if a response was synthesized, indicating that any forthcoming
   // redirects should be intercepted.
   uint8_t mSynthesizedResponse : 1;