Backed out changeset 35fc92e75cf7 (bug 1412007) for failing broswer-chrome browser_bug676619.js r=backout on a CLOSED TREE
authorANDREEA PAVEL <apavel@mozilla.com>
Thu, 02 Nov 2017 18:09:15 +0200
changeset 692149 58e3cc92a9e49a92ce07affb1c427df354bd913a
parent 692148 831a02dbf05dc05facbae533ca474d2c4dbb7518
child 692150 01abd97b6450c6c76ef2365b86251caa4502e7bd
child 692180 afcda78f4f7366629bd92a0f2fc7227ff30c38c9
child 692183 d063578fc2699975f1bbc67a7190bf3fcc5494aa
child 692237 dd79f0be2d6ff5a61bedc5a09925aa4b78e30c1b
child 692240 4c99d37c43d18d1313641475139ecdba9cd4450d
child 692241 f67d3b4e10b5d5dc3d5b20d82826cb98799fa954
child 693912 40e539c197d888c07d7dde699dbd7e2ba402d66e
child 693913 9606b9da2deabe3eeaeb72c574c49d8a3c08a2db
child 693928 c0b1f09b3e545f90ffe8607649fa7433d070cdfe
child 694982 3a97ee540cfd64ab0b682f5fddeca805fd159ce0
child 695824 89f84422a3fbf8f5a948f9bfe8e292d0b641ee4d
child 698827 8eb4894f0f1a2d11b96de0957e11c26dbf3e489a
child 698845 7061f7c30b750206f969a0e9234eff0064ee39be
push id87414
push userbmo:mstriemer@mozilla.com
push dateThu, 02 Nov 2017 16:36:21 +0000
reviewersbackout
bugs1412007, 676619
milestone58.0a1
backs out35fc92e75cf75947444747ca5b279cbaf34e051e
Backed out changeset 35fc92e75cf7 (bug 1412007) for failing broswer-chrome browser_bug676619.js r=backout on a CLOSED TREE
netwerk/protocol/http/HttpChannelChild.cpp
netwerk/protocol/http/HttpChannelChild.h
--- a/netwerk/protocol/http/HttpChannelChild.cpp
+++ b/netwerk/protocol/http/HttpChannelChild.cpp
@@ -168,17 +168,17 @@ HttpChannelChild::HttpChannelChild()
   , mCacheFetchCount(0)
   , mCacheExpirationTime(nsICacheEntry::NO_EXPIRATION_TIME)
   , mSendResumeAt(false)
   , mDeletingChannelSent(false)
   , mIPCOpen(false)
   , mKeptAlive(false)
   , mUnknownDecoderInvolved(false)
   , mDivertingToParent(false)
-  , mFlushedForDiversion(eNotFlushed)
+  , mFlushedForDiversion(false)
   , mSuspendSent(false)
   , mSynthesizedResponse(false)
   , mShouldInterceptSubsequentRedirect(false)
   , mRedirectingForSubsequentSynthesizedResponse(false)
   , mPostRedirectChannelShouldIntercept(false)
   , mPostRedirectChannelShouldUpgrade(false)
   , mShouldParentIntercept(false)
   , mSuspendParentAfterSynthesizeResponse(false)
@@ -488,17 +488,17 @@ HttpChannelChild::RecvOnStartRequest(con
                                      const int16_t& redirectCount,
                                      const uint32_t& cacheKey,
                                      const nsCString& altDataType,
                                      const int64_t& altDataLen)
 {
   LOG(("HttpChannelChild::RecvOnStartRequest [this=%p]\n", this));
   // mFlushedForDiversion and mDivertingToParent should NEVER be set at this
   // stage, as they are set in the listener's OnStartRequest.
-  MOZ_RELEASE_ASSERT(mFlushedForDiversion == eNotFlushed,
+  MOZ_RELEASE_ASSERT(!mFlushedForDiversion,
     "mFlushedForDiversion should be unset before OnStartRequest!");
   MOZ_RELEASE_ASSERT(!mDivertingToParent,
     "mDivertingToParent should be unset before OnStartRequest!");
 
 
   mRedirectCount = redirectCount;
 
   mEventQ->RunOrEnqueue(new StartRequestEvent(this, channelStatus, responseHead,
@@ -549,17 +549,17 @@ HttpChannelChild::OnStartRequest(const n
                                  const uint32_t& cacheKey,
                                  const nsCString& altDataType,
                                  const int64_t& altDataLen)
 {
   LOG(("HttpChannelChild::OnStartRequest [this=%p]\n", this));
 
   // mFlushedForDiversion and mDivertingToParent should NEVER be set at this
   // stage, as they are set in the listener's OnStartRequest.
-  MOZ_RELEASE_ASSERT(mFlushedForDiversion == eNotFlushed,
+  MOZ_RELEASE_ASSERT(!mFlushedForDiversion,
     "mFlushedForDiversion should be unset before OnStartRequest!");
   MOZ_RELEASE_ASSERT(!mDivertingToParent,
     "mDivertingToParent should be unset before OnStartRequest!");
 
   if (!mCanceled && NS_SUCCEEDED(mStatus)) {
     mStatus = channelStatus;
   }
 
@@ -610,16 +610,18 @@ HttpChannelChild::OnStartRequest(const n
   //
   // gHttpHandler->OnExamineResponse(this);
 
   mTracingEnabled = false;
 
   DoOnStartRequest(this, mListenerContext);
 }
 
+namespace {
+
 class SyntheticDiversionListener final : public nsIStreamListener
 {
   RefPtr<HttpChannelChild> mChannel;
 
   ~SyntheticDiversionListener()
   {
   }
 
@@ -637,17 +639,16 @@ public:
     return NS_OK;
   }
 
   NS_IMETHOD
   OnStopRequest(nsIRequest* aRequest, nsISupports* aContext,
                 nsresult aStatus) override
   {
     mChannel->SendDivertOnStopRequest(aStatus);
-    mChannel->MaybeSendDivertComplete();
     return NS_OK;
   }
 
   NS_IMETHOD
   OnDataAvailable(nsIRequest* aRequest, nsISupports* aContext,
                   nsIInputStream* aInputStream, uint64_t aOffset,
                   uint32_t aCount) override
   {
@@ -662,16 +663,18 @@ public:
     return NS_OK;
   }
 
   NS_DECL_ISUPPORTS
 };
 
 NS_IMPL_ISUPPORTS(SyntheticDiversionListener, nsIStreamListener);
 
+} // anonymous namespace
+
 void
 HttpChannelChild::DoOnStartRequest(nsIRequest* aRequest, nsISupports* aContext)
 {
   LOG(("HttpChannelChild::DoOnStartRequest [this=%p]\n", this));
 
   // In theory mListener should not be null, but in practice sometimes it is.
   MOZ_ASSERT(mListener);
   if (!mListener) {
@@ -761,17 +764,17 @@ void
 HttpChannelChild::ProcessOnTransportAndData(const nsresult& aChannelStatus,
                                             const nsresult& aTransportStatus,
                                             const uint64_t& aOffset,
                                             const uint32_t& aCount,
                                             const nsCString& aData)
 {
   LOG(("HttpChannelChild::ProcessOnTransportAndData [this=%p]\n", this));
   MOZ_ASSERT(OnSocketThread());
-  MOZ_RELEASE_ASSERT(mFlushedForDiversion == eNotFlushed,
+  MOZ_RELEASE_ASSERT(!mFlushedForDiversion,
                      "Should not be receiving any more callbacks from parent!");
   mEventQ->RunOrEnqueue(new TransportAndDataEvent(this, aChannelStatus,
                                                   aTransportStatus, aData,
                                                   aOffset, aCount),
                         mDivertingToParent);
 }
 
 class MaybeDivertOnDataHttpEvent : public NeckoTargetChannelEvent<HttpChannelChild>
@@ -820,17 +823,17 @@ HttpChannelChild::OnTransportAndData(con
 
   if (!mCanceled && NS_SUCCEEDED(mStatus)) {
     mStatus = channelStatus;
   }
 
   // For diversion to parent, just SendDivertOnDataAvailable.
   if (mDivertingToParent) {
     MOZ_ASSERT(NS_IsMainThread());
-    MOZ_RELEASE_ASSERT(mFlushedForDiversion == eNotFlushed,
+    MOZ_RELEASE_ASSERT(!mFlushedForDiversion,
       "Should not be processing any more callbacks from parent!");
 
     SendDivertOnDataAvailable(data, offset, count);
     return;
   }
 
   if (mCanceled)
     return;
@@ -989,17 +992,17 @@ class StopRequestEvent : public NeckoTar
 };
 
 void
 HttpChannelChild::ProcessOnStopRequest(const nsresult& aChannelStatus,
                                        const ResourceTimingStruct& aTiming)
 {
   LOG(("HttpChannelChild::ProcessOnStopRequest [this=%p]\n", this));
   MOZ_ASSERT(OnSocketThread());
-  MOZ_RELEASE_ASSERT(mFlushedForDiversion == eNotFlushed,
+  MOZ_RELEASE_ASSERT(!mFlushedForDiversion,
     "Should not be receiving any more callbacks from parent!");
 
   mEventQ->RunOrEnqueue(new StopRequestEvent(this, aChannelStatus, aTiming),
                         mDivertingToParent);
 }
 
 class MaybeDivertOnStopHttpEvent : public NeckoTargetChannelEvent<HttpChannelChild>
 {
@@ -1023,51 +1026,32 @@ void
 HttpChannelChild::MaybeDivertOnStop(const nsresult& aChannelStatus)
 {
   LOG(("HttpChannelChild::MaybeDivertOnStop [this=%p, "
        "mDivertingToParent=%d status=%" PRIx32 "]", this,
        static_cast<bool>(mDivertingToParent),
        static_cast<uint32_t>(aChannelStatus)));
   if (mDivertingToParent) {
     SendDivertOnStopRequest(aChannelStatus);
-    MaybeSendDivertComplete();
   }
 }
 
 void
-HttpChannelChild::MaybeSendDivertComplete()
-{
-  if (mFlushedForDiversion == eNotFlushed) {
-    mFlushedForDiversion = eReadyToBeFlushed;
-    return;
-  }
-
-  if (mFlushedForDiversion == ePendingToBeFlushed) {
-    mFlushedForDiversion = eFlushed;
-    SendDivertComplete();
-    return;
-  }
-
-  MOZ_CRASH("We should not be already in this state!");
-}
-
-void
 HttpChannelChild::OnStopRequest(const nsresult& channelStatus,
                                 const ResourceTimingStruct& timing)
 {
   LOG(("HttpChannelChild::OnStopRequest [this=%p status=%" PRIx32 "]\n",
        this, static_cast<uint32_t>(channelStatus)));
   MOZ_ASSERT(NS_IsMainThread());
 
   if (mDivertingToParent) {
-    MOZ_RELEASE_ASSERT(mFlushedForDiversion == eNotFlushed,
+    MOZ_RELEASE_ASSERT(!mFlushedForDiversion,
       "Should not be processing any more callbacks from parent!");
 
     SendDivertOnStopRequest(channelStatus);
-    MaybeSendDivertComplete();
     return;
   }
 
   if (mUnknownDecoderInvolved) {
    LOG(("UnknownDecoder is involved queue OnStopRequest call. [this=%p]",
         this));
     MOZ_ASSERT(NS_IsMainThread());
     mUnknownDecoderEventQ.AppendElement(
@@ -1857,23 +1841,19 @@ void
 HttpChannelChild::FlushedForDiversion()
 {
   LOG(("HttpChannelChild::FlushedForDiversion [this=%p]\n", this));
   MOZ_RELEASE_ASSERT(mDivertingToParent);
 
   // Once this is set, it should not be unset before HttpChannelChild is taken
   // down. After it is set, no OnStart/OnData/OnStop callbacks should be
   // received from the parent channel, nor dequeued from the ChannelEventQueue.
-
-  if (mFlushedForDiversion == eReadyToBeFlushed) {
-    mFlushedForDiversion = eFlushed;
-    SendDivertComplete();
-  } else {
-    mFlushedForDiversion = ePendingToBeFlushed;
-  }
+  mFlushedForDiversion = true;
+
+  SendDivertComplete();
 }
 
 void
 HttpChannelChild::ProcessSetClassifierMatchedInfo(const nsCString& aList,
                                                   const nsCString& aProvider,
                                                   const nsCString& aFullHash)
 {
   LOG(("HttpChannelChild::ProcessSetClassifierMatchedInfo [this=%p]\n", this));
--- a/netwerk/protocol/http/HttpChannelChild.h
+++ b/netwerk/protocol/http/HttpChannelChild.h
@@ -314,41 +314,19 @@ private:
   // If nsUnknownDecoder is involved OnStartRequest call will be delayed and
   // this queue keeps OnDataAvailable data until OnStartRequest is finally
   // called.
   nsTArray<UniquePtr<ChannelEvent>> mUnknownDecoderEventQ;
   Atomic<bool, ReleaseAcquire> mUnknownDecoderInvolved;
 
   // Once set, OnData and possibly OnStop will be diverted to the parent.
   Atomic<bool, ReleaseAcquire> mDivertingToParent;
-
-  enum FlushedForDiversionEnum {
-    // This is the initial state.
-    eNotFlushed,
-
-    // This is set when SendOnStopRequest() is called and the previous state was
-    // eNotFlushed. FlushedForDiversion() has not been called yet, but when,
-    // eventually, it will be, SendDivertComplete() can immediately called.
-    eReadyToBeFlushed,
-
-    // This is set by FlushedForDiversion() when SetOnStopRequest() has not been
-    // called yet. When finally SetOnStopRequest() will be called,
-    // SendDivertComplete() will be executed as well.
-    ePendingToBeFlushed,
-
-    // This is the final step. No OnStart/OnData/OnStop callbacks should be
-    // received from the parent channel, nor dequeued from the
-    // ChannelEventQueue.
-    eFlushed,
-  };
-
-  // Atomic becuase it can be touched onSocketThread() for debugging reasons
-  // only.
-  Atomic<FlushedForDiversionEnum, ReleaseAcquire> mFlushedForDiversion;
-
+  // Once set, no OnStart/OnData/OnStop callbacks should be received from the
+  // parent channel, nor dequeued from the ChannelEventQueue.
+  Atomic<bool, ReleaseAcquire> mFlushedForDiversion;
   // Set if SendSuspend is called. Determines if SendResume is needed when
   // diverting callbacks to parent.
   bool mSuspendSent;
 
   // Set if a response was synthesized, indicating that any forthcoming redirects
   // should be intercepted.
   bool mSynthesizedResponse;
 
@@ -457,34 +435,28 @@ private:
                            const nsHttpResponseHead* responseHead);
 
   // Override the default security info pointer during a non-IPC redirection.
   void OverrideSecurityInfoForNonIPCRedirect(nsISupports* securityInfo);
 
   // Collect telemetry for the successful rate of OMT.
   void CollectOMTTelemetry();
 
-  // When SendDivertOnStopRequest() is called, this method is used to check
-  // mFlushedForDiversion and maybe call SendDivertComplete(). See
-  // mFlushedForDiversion state.
-  void MaybeSendDivertComplete();
-
   // The result of RetargetDeliveryTo for this channel.
   // |notRequested| represents OMT is not requested by the channel owner.
   LABELS_HTTP_CHILD_OMT_STATS mOMTResult = LABELS_HTTP_CHILD_OMT_STATS::notRequested;
 
   friend class AssociateApplicationCacheEvent;
   friend class StartRequestEvent;
   friend class StopRequestEvent;
   friend class TransportAndDataEvent;
   friend class MaybeDivertOnDataHttpEvent;
   friend class MaybeDivertOnStopHttpEvent;
   friend class ProgressEvent;
   friend class StatusEvent;
-  friend class SyntheticDiversionListener;
   friend class FailedAsyncOpenEvent;
   friend class Redirect1Event;
   friend class Redirect3Event;
   friend class DeleteSelfEvent;
   friend class HttpFlushedForDiversionEvent;
   friend class CancelEvent;
   friend class HttpAsyncAborter<HttpChannelChild>;
   friend class InterceptStreamListener;