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 443133 58e3cc92a9e49a92ce07affb1c427df354bd913a
parent 443132 831a02dbf05dc05facbae533ca474d2c4dbb7518
child 443134 d063578fc2699975f1bbc67a7190bf3fcc5494aa
push id1618
push userCallek@gmail.com
push dateThu, 11 Jan 2018 17:45:48 +0000
treeherdermozilla-release@882ca853e05a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbackout
bugs1412007, 676619
milestone58.0a1
backs out35fc92e75cf75947444747ca5b279cbaf34e051e
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
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;