Bug 1514065 - resume the bp-suspension if the divsersion starts r=dragana
authorJunior Hsu <juhsu@mozilla.com>
Wed, 16 Jan 2019 00:57:11 +0000
changeset 514044 c93c9bc7d63a0d096818cbd52274460dfaee58c5
parent 514043 e56cc5e7b57a5d18ab72207f7f246a9b8c610c1c
child 514045 2fc4ce319e749d9d8b672bfdbc9d3fe1fb7276a6
push id1953
push userffxbld-merge
push dateMon, 11 Mar 2019 12:10:20 +0000
treeherdermozilla-release@9c35dcbaa899 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdragana
bugs1514065
milestone66.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 1514065 - resume the bp-suspension if the divsersion starts r=dragana Differential Revision: https://phabricator.services.mozilla.com/D14361
modules/libpref/init/all.js
netwerk/protocol/http/HttpChannelChild.cpp
netwerk/protocol/http/HttpChannelParent.cpp
--- a/modules/libpref/init/all.js
+++ b/modules/libpref/init/all.js
@@ -1900,21 +1900,20 @@ pref("network.http.rcwn.small_resource_s
 
 pref("network.http.rcwn.min_wait_before_racing_ms", 0);
 pref("network.http.rcwn.max_wait_before_racing_ms", 500);
 
 // The ratio of the transaction count for the focused window and the count of
 // all available active connections.
 pref("network.http.focused_window_transaction_ratio", "0.9");
 
-// XXX Disable for intranet downloading issue.
 // This is the size of the flow control window (KB) (i.e., the amount of data
 // that the parent can send to the child before getting an ack). 0 for disable
 // the flow control.
-pref("network.http.send_window_size", 0);
+pref("network.http.send_window_size", 1024);
 
 // Whether or not we give more priority to active tab.
 // Note that this requires restart for changes to take effect.
 #ifdef ANDROID
 // disabled because of bug 1382274
 pref("network.http.active_tab_priority", false);
 #else
 pref("network.http.active_tab_priority", true);
--- a/netwerk/protocol/http/HttpChannelChild.cpp
+++ b/netwerk/protocol/http/HttpChannelChild.cpp
@@ -875,17 +875,21 @@ void HttpChannelChild::OnTransportAndDat
       }
       mUnreportBytesRead = 0;
     }
   }
 }
 
 bool HttpChannelChild::NeedToReportBytesRead() {
   if (mCacheNeedToReportBytesReadInitialized) {
-    return mNeedToReportBytesRead;
+    // No need to send SendRecvBytes when diversion starts since the parent
+    // process will suspend for diversion triggered in during OnStrartRequest at
+    // child side, which is earlier. Parent will take over the flow control
+    // after the diverting starts. Sending |SendBytesRead| is redundant.
+    return mNeedToReportBytesRead && !mDivertingToParent;
   }
 
   // Might notify parent for partial cache, and the IPC message is ignored by
   // parent.
   int64_t contentLength = -1;
   if (gHttpHandler->SendWindowSize() == 0 || mIsFromCache ||
       NS_FAILED(GetContentLength(&contentLength)) ||
       contentLength < gHttpHandler->SendWindowSize()) {
--- a/netwerk/protocol/http/HttpChannelParent.cpp
+++ b/netwerk/protocol/http/HttpChannelParent.cpp
@@ -1604,16 +1604,17 @@ HttpChannelParent::OnDataAvailable(nsIRe
     aCount -= toRead;
     toRead = std::min<uint32_t>(aCount, kCopyChunkSize);
   }
 
   if (NeedFlowControl()) {
     // We're going to run out of sending window size
     if (mSendWindowSize > 0 && mSendWindowSize <= count) {
       MOZ_ASSERT(!mSuspendedForFlowControl);
+      LOG(("  suspend the channel due to e10s backpressure"));
       Unused << mChannel->Suspend();
       mSuspendedForFlowControl = true;
       mHasSuspendedByBackPressure = true;
     } else if (!mResumedTimestamp.IsNull()) {
       // Calculate the delay when the first packet arrived after resume
       Telemetry::AccumulateTimeDelta(
           Telemetry::NETWORK_BACK_PRESSURE_SUSPENSION_DELAY_TIME_MS,
           mResumedTimestamp);
@@ -1647,25 +1648,27 @@ bool HttpChannelParent::NeedFlowControl(
     mNeedFlowControl = false;
   }
   mCacheNeedFlowControlInitialized = true;
   return mNeedFlowControl;
 }
 
 mozilla::ipc::IPCResult HttpChannelParent::RecvBytesRead(
     const int32_t& aCount) {
-  if (!NeedFlowControl()) {
+  // no more flow control after diviersion starts
+  if (!NeedFlowControl() || mDivertingFromChild) {
     return IPC_OK();
   }
 
   LOG(("HttpChannelParent::RecvBytesRead [this=%p count=%" PRId32 "]\n", this,
        aCount));
 
   if (mSendWindowSize <= 0 && mSendWindowSize + aCount > 0) {
     MOZ_ASSERT(mSuspendedForFlowControl);
+    LOG(("  resume the channel due to e10s backpressure relief"));
     Unused << mChannel->Resume();
     mSuspendedForFlowControl = false;
 
     mResumedTimestamp = TimeStamp::Now();
   }
   mSendWindowSize += aCount;
   return IPC_OK();
 }
@@ -2071,16 +2074,24 @@ nsresult HttpChannelParent::SuspendForDi
     // of this suspend, therefore mEventQ should not be suspened so we need to
     // resume it once.
     mEventQ->Resume();
   }
 
   rv = mParentListener->SuspendForDiversion();
   MOZ_ASSERT(NS_SUCCEEDED(rv));
 
+  // After we suspend for diversion, we don't need the flow control since the
+  // channel is suspended until all the data is consumed and no more e10s later.
+  // No point to have another redundant suspension.
+  if (mSuspendedForFlowControl) {
+    Unused << mChannel->Resume();
+    mSuspendedForFlowControl = false;
+  }
+
   // Once this is set, no more OnStart/OnData/OnStop callbacks should be sent
   // to the child.
   mDivertingFromChild = true;
 
   return NS_OK;
 }
 
 nsresult HttpChannelParent::SuspendMessageDiversion() {