Bug 1280629 - Part 1: Suspend the http channel if the child process is not able to consume on time. r=dragana
☠☠ backed out by 77273a4b047a ☠ ☠
authorJunior Hsu <juhsu@mozilla.com>
Wed, 15 Aug 2018 10:04:50 +0300
changeset 431597 e654f2b128f6483738646728911b29fcf426b76f
parent 431596 c7d7fe60813267d51474afc6c401ce51df2796b3
child 431598 64648fd6ef5ee1dece183850a31c05471fb3dabb
push id106496
push userarchaeopteryx@coole-files.de
push dateWed, 15 Aug 2018 07:08:15 +0000
treeherdermozilla-inbound@64648fd6ef5e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdragana
bugs1280629
milestone63.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 1280629 - Part 1: Suspend the http channel if the child process is not able to consume on time. r=dragana Reviewers: dragana Reviewed By: dragana Subscribers: reviewbot Bug #: 1280629 Differential Revision: https://phabricator.services.mozilla.com/D2745
modules/libpref/init/all.js
netwerk/protocol/http/HttpChannelChild.cpp
netwerk/protocol/http/HttpChannelChild.h
netwerk/protocol/http/HttpChannelParent.cpp
netwerk/protocol/http/HttpChannelParent.h
netwerk/protocol/http/PHttpChannel.ipdl
netwerk/protocol/http/nsHttpHandler.cpp
netwerk/protocol/http/nsHttpHandler.h
testing/web-platform/meta/2dcontext/imagebitmap/createImageBitmap-origin.sub.html.ini
testing/web-platform/meta/html/semantics/embedded-content/media-elements/playing-the-media-resource/pause-move-to-other-document.html.ini
testing/web-platform/meta/html/semantics/embedded-content/the-video-element/video_initially_paused.html.ini
--- a/modules/libpref/init/all.js
+++ b/modules/libpref/init/all.js
@@ -1811,16 +1811,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");
 
+// 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)
+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);
 #endif
--- a/netwerk/protocol/http/HttpChannelChild.cpp
+++ b/netwerk/protocol/http/HttpChannelChild.cpp
@@ -915,16 +915,60 @@ HttpChannelChild::OnTransportAndData(con
                                       count, NS_ASSIGNMENT_DEPEND);
   if (NS_FAILED(rv)) {
     Cancel(rv);
     return;
   }
 
   DoOnDataAvailable(this, mListenerContext, stringStream, offset, count);
   stringStream->Close();
+
+  if (NeedToReportBytesRead()) {
+    mUnreportBytesRead += count;
+    if (mUnreportBytesRead >= gHttpHandler->SendWindowSize() >> 2) {
+      if (NS_IsMainThread()) {
+        Unused << SendBytesRead(mUnreportBytesRead);
+      } else {
+        // PHttpChannel connects to the main thread
+        RefPtr<HttpChannelChild> self = this;
+        int32_t bytesRead = mUnreportBytesRead;
+        nsCOMPtr<nsIEventTarget> neckoTarget = GetNeckoTarget();
+        MOZ_ASSERT(neckoTarget);
+
+        DebugOnly<nsresult> rv = neckoTarget->Dispatch(
+          NS_NewRunnableFunction("net::HttpChannelChild::SendBytesRead",
+                                 [self, bytesRead]() {
+                                   Unused << self->SendBytesRead(bytesRead);
+                                 }),
+          NS_DISPATCH_NORMAL);
+        MOZ_ASSERT(NS_SUCCEEDED(rv));
+      }
+      mUnreportBytesRead = 0;
+    }
+  }
+}
+
+bool
+HttpChannelChild::NeedToReportBytesRead() {
+ if (mCacheNeedToReportBytesReadInitialized) {
+    return mNeedToReportBytesRead;
+  }
+
+  // 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()) {
+    mNeedToReportBytesRead = false;
+  }
+
+  mCacheNeedToReportBytesReadInitialized = true;
+  return mNeedToReportBytesRead;
 }
 
 void
 HttpChannelChild::DoOnStatus(nsIRequest* aRequest, nsresult status)
 {
   LOG(("HttpChannelChild::DoOnStatus [this=%p]\n", this));
   MOZ_ASSERT(NS_IsMainThread());
 
--- a/netwerk/protocol/http/HttpChannelChild.h
+++ b/netwerk/protocol/http/HttpChannelChild.h
@@ -253,16 +253,22 @@ private:
   void ProcessFlushedForDiversion();
   void ProcessDivertMessages();
   void ProcessNotifyTrackingProtectionDisabled();
   void ProcessNotifyTrackingResource();
   void ProcessSetClassifierMatchedInfo(const nsCString& aList,
                                        const nsCString& aProvider,
                                        const nsCString& aFullHash);
 
+  // Return true if we need to tell the parent the size of unreported received
+  // data
+  bool NeedToReportBytesRead();
+  bool mCacheNeedToReportBytesReadInitialized = false;
+  bool mNeedToReportBytesRead = true;
+  int32_t mUnreportBytesRead = 0;
 
   void DoOnStartRequest(nsIRequest* aRequest, nsISupports* aContext);
   void DoOnStatus(nsIRequest* aRequest, nsresult status);
   void DoOnProgress(nsIRequest* aRequest, int64_t progress, int64_t progressMax);
   void DoOnDataAvailable(nsIRequest* aRequest, nsISupports* aContext, nsIInputStream* aStream,
                          uint64_t offset, uint32_t count);
   void DoPreOnStopRequest(nsresult aStatus);
   void DoOnStopRequest(nsIRequest* aRequest, nsresult aChannelStatus, nsISupports* aContext);
--- a/netwerk/protocol/http/HttpChannelParent.cpp
+++ b/netwerk/protocol/http/HttpChannelParent.cpp
@@ -90,16 +90,18 @@ HttpChannelParent::HttpChannelParent(con
   mHttpHandler = gHttpHandler;
 
   if (iframeEmbedding.type() == PBrowserOrId::TPBrowserParent) {
     mTabParent = static_cast<dom::TabParent*>(iframeEmbedding.get_PBrowserParent());
   } else {
     mNestedFrameId = iframeEmbedding.get_TabId();
   }
 
+  mSendWindowSize = gHttpHandler->SendWindowSize();
+
   mEventQ = new ChannelEventQueue(static_cast<nsIParentRedirectingChannel*>(this));
 }
 
 HttpChannelParent::~HttpChannelParent()
 {
   LOG(("Destroying HttpChannelParent [this=%p]\n", this));
   CleanupBackgroundChannel();
 }
@@ -1617,16 +1619,18 @@ HttpChannelParent::OnDataAvailable(nsIRe
   uint32_t toRead = std::min<uint32_t>(aCount, kCopyChunkSize);
 
   nsCString data;
   if (!data.SetCapacity(toRead, fallible)) {
     LOG(("  out of memory!"));
     return NS_ERROR_OUT_OF_MEMORY;
   }
 
+  int32_t count = static_cast<int32_t>(aCount);
+
   while (aCount) {
     nsresult rv = NS_ReadInputStreamToString(aInputStream, data, toRead);
     if (NS_FAILED(rv)) {
       return rv;
     }
 
     // Either IPC channel is closed or background channel
     // is ready to send OnTransportAndData.
@@ -1638,19 +1642,75 @@ HttpChannelParent::OnDataAvailable(nsIRe
       return NS_ERROR_UNEXPECTED;
     }
 
     aOffset += toRead;
     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);
+      Unused << mChannel->Suspend();
+      mSuspendedForFlowControl = true;
+    }
+    mSendWindowSize -= count;
+  }
+
   return NS_OK;
 }
 
+bool
+HttpChannelParent::NeedFlowControl()
+{
+  if (mCacheNeedFlowControlInitialized) {
+    return mNeedFlowControl;
+  }
+
+  int64_t contentLength = -1;
+
+  RefPtr<nsHttpChannel> httpChannelImpl = do_QueryObject(mChannel);
+
+  // By design, we won't trigger the flow control if
+  // a. pref-out
+  // b. the resource is from cache or partial cache
+  // c. the resource is small
+  // Note that we served the cached resource first for partical cache, which is
+  // ignored here since we only take the first ODA into consideration.
+  if (gHttpHandler->SendWindowSize() == 0 ||
+      !httpChannelImpl ||
+      httpChannelImpl->IsReadingFromCache() ||
+      NS_FAILED(httpChannelImpl->GetContentLength(&contentLength)) ||
+      contentLength < gHttpHandler->SendWindowSize()) {
+    mNeedFlowControl = false;
+  }
+  mCacheNeedFlowControlInitialized = true;
+  return mNeedFlowControl;
+}
+
+mozilla::ipc::IPCResult
+HttpChannelParent::RecvBytesRead(const int32_t& aCount)
+{
+  if (!NeedFlowControl()) {
+    return IPC_OK();
+  }
+
+  LOG(("HttpBackgroundChannelParent::RecvBytesRead [this=%p count=%" PRId32 "]\n", this, aCount));
+
+  if (mSendWindowSize <= 0 && mSendWindowSize + aCount > 0) {
+    MOZ_ASSERT(mSuspendedForFlowControl);
+    Unused << mChannel->Resume();
+    mSuspendedForFlowControl = false;
+  }
+  mSendWindowSize += aCount;
+  return IPC_OK();
+}
+
 //-----------------------------------------------------------------------------
 // HttpChannelParent::nsIProgressEventSink
 //-----------------------------------------------------------------------------
 
 NS_IMETHODIMP
 HttpChannelParent::OnProgress(nsIRequest *aRequest,
                               nsISupports *aContext,
                               int64_t aProgress,
--- a/netwerk/protocol/http/HttpChannelParent.h
+++ b/netwerk/protocol/http/HttpChannelParent.h
@@ -204,16 +204,17 @@ protected:
   virtual mozilla::ipc::IPCResult RecvMarkOfflineCacheEntryAsForeign() override;
   virtual mozilla::ipc::IPCResult RecvDivertOnDataAvailable(const nsCString& data,
                                          const uint64_t& offset,
                                          const uint32_t& count) override;
   virtual mozilla::ipc::IPCResult RecvDivertOnStopRequest(const nsresult& statusCode) override;
   virtual mozilla::ipc::IPCResult RecvDivertComplete() override;
   virtual mozilla::ipc::IPCResult RecvRemoveCorsPreflightCacheEntry(const URIParams& uri,
                                                                     const mozilla::ipc::PrincipalInfo& requestingPrincipal) override;
+  virtual mozilla::ipc::IPCResult RecvBytesRead(const int32_t& aCount) override;
   virtual void ActorDestroy(ActorDestroyReason why) override;
 
   // Supporting function for ADivertableParentChannel.
   MOZ_MUST_USE nsresult ResumeForDiversion();
 
   // Asynchronously calls NotifyDiversionFailed.
   void FailDiversion(nsresult aErrorCode);
 
@@ -257,16 +258,25 @@ private:
   // will be returned and the callbacks will be invoked in order.
   already_AddRefed<GenericPromise> WaitForBgParent();
 
   // Remove the association with background channel after main-thread IPC
   // is about to be destroyed or no further event is going to be sent, i.e.,
   // DocumentChannelCleanup.
   void CleanupBackgroundChannel();
 
+  // Check if the channel needs to enable the flow control on the IPC channel.
+  // That is, we may suspend the channel if the ODA-s to child process are not
+  // consumed quickly enough. Otherwise, memory explosion could happen.
+  bool NeedFlowControl();
+  bool mCacheNeedFlowControlInitialized = false;
+  bool mNeedFlowControl = true;
+  bool mSuspendedForFlowControl = false;
+  int32_t mSendWindowSize;
+
   friend class HttpBackgroundChannelParent;
   friend class DivertDataAvailableEvent;
   friend class DivertStopRequestEvent;
   friend class DivertCompleteEvent;
 
   RefPtr<HttpBaseChannel>       mChannel;
   nsCOMPtr<nsICacheEntry>       mCacheEntry;
   nsCOMPtr<nsIAssociatedContentSecurity>  mAssociatedContentSecurity;
--- a/netwerk/protocol/http/PHttpChannel.ipdl
+++ b/netwerk/protocol/http/PHttpChannel.ipdl
@@ -86,16 +86,20 @@ parent:
   // to remove any matching entry from the CORS preflight cache.
   async RemoveCorsPreflightCacheEntry(URIParams uri,
                                       PrincipalInfo requestingPrincipal);
 
   // After receiving this message, the parent calls SendDeleteSelf, and makes
   // sure not to send any more messages after that.
   async DeletingChannel();
 
+  // Tell the parent the amount bytes read by child for the e10s back pressure
+  // flow control
+  async BytesRead(int32_t count);
+
   async __delete__();
 
 child:
   async OnStartRequest(nsresult            channelStatus,
                        nsHttpResponseHead  responseHead,
                        bool                useResponseHead,
                        nsHttpHeaderArray   requestHeaders,
                        ParentLoadInfoForwarderArgs loadInfoForwarder,
--- a/netwerk/protocol/http/nsHttpHandler.cpp
+++ b/netwerk/protocol/http/nsHttpHandler.cpp
@@ -216,16 +216,17 @@ nsHttpHandler::nsHttpHandler()
     , mThrottleEnabled(true)
     , mThrottleVersion(2)
     , mThrottleSuspendFor(3000)
     , mThrottleResumeFor(200)
     , mThrottleReadLimit(8000)
     , mThrottleReadInterval(500)
     , mThrottleHoldTime(600)
     , mThrottleMaxTime(3000)
+    , mSendWindowSize(1024)
     , mUrgentStartEnabled(true)
     , mTailBlockingEnabled(true)
     , mTailDelayQuantum(600)
     , mTailDelayQuantumAfterDCL(100)
     , mTailDelayMax(6000)
     , mTailTotalMax(0)
     , mRedirectionLimit(10)
     , mPhishyUserPassLength(1)
@@ -1740,16 +1741,21 @@ nsHttpHandler::PrefsChanged(const char *
       rv = Preferences::GetInt(HTTP_PREF("throttle.max-time-ms"), &val);
       mThrottleMaxTime = (uint32_t)clamped(val, 0, 120000);
       if (NS_SUCCEEDED(rv) && mConnMgr) {
         Unused << mConnMgr->UpdateParam(nsHttpConnectionMgr::THROTTLING_MAX_TIME,
                                         mThrottleMaxTime);
       }
     }
 
+    if (PREF_CHANGED(HTTP_PREF("send_window_size"))) {
+      Unused << Preferences::GetInt(HTTP_PREF("send_window_size"), &val);
+      mSendWindowSize = val >= 0 ? val : 0;
+    }
+
     if (PREF_CHANGED(HTTP_PREF("on_click_priority"))) {
         Unused << Preferences::GetBool(HTTP_PREF("on_click_priority"), &mUrgentStartEnabled);
     }
 
     if (PREF_CHANGED(HTTP_PREF("tailing.enabled"))) {
         Unused << Preferences::GetBool(HTTP_PREF("tailing.enabled"), &mTailBlockingEnabled);
     }
     if (PREF_CHANGED(HTTP_PREF("tailing.delay-quantum"))) {
--- a/netwerk/protocol/http/nsHttpHandler.h
+++ b/netwerk/protocol/http/nsHttpHandler.h
@@ -143,16 +143,17 @@ public:
     bool           IsTailBlockingEnabled() { return mTailBlockingEnabled; }
     uint32_t       TailBlockingDelayQuantum(bool aAfterDOMContentLoaded) {
       return aAfterDOMContentLoaded ? mTailDelayQuantumAfterDCL : mTailDelayQuantum;
     }
     uint32_t       TailBlockingDelayMax() { return mTailDelayMax; }
     uint32_t       TailBlockingTotalMax() { return mTailTotalMax; }
 
     uint32_t       ThrottlingReadLimit() { return mThrottleVersion == 1 ? 0 : mThrottleReadLimit; }
+    int32_t        SendWindowSize() { return mSendWindowSize * 1024; }
 
     // TCP Keepalive configuration values.
 
     // Returns true if TCP keepalive should be enabled for short-lived conns.
     bool TCPKeepaliveEnabledForShortLivedConns() {
       return mTCPKeepaliveShortLivedEnabled;
     }
     // Return time (secs) that a connection is consider short lived (for TCP
@@ -497,16 +498,18 @@ private:
     uint32_t mThrottleVersion;
     uint32_t mThrottleSuspendFor;
     uint32_t mThrottleResumeFor;
     uint32_t mThrottleReadLimit;
     uint32_t mThrottleReadInterval;
     uint32_t mThrottleHoldTime;
     uint32_t mThrottleMaxTime;
 
+    int32_t mSendWindowSize;
+
     bool mUrgentStartEnabled;
     bool mTailBlockingEnabled;
     uint32_t mTailDelayQuantum;
     uint32_t mTailDelayQuantumAfterDCL;
     uint32_t mTailDelayMax;
     uint32_t mTailTotalMax;
 
     uint8_t  mRedirectionLimit;
--- a/testing/web-platform/meta/2dcontext/imagebitmap/createImageBitmap-origin.sub.html.ini
+++ b/testing/web-platform/meta/2dcontext/imagebitmap/createImageBitmap-origin.sub.html.ini
@@ -1,9 +1,10 @@
 [createImageBitmap-origin.sub.html]
+  prefs: [network.http.send_window_size:0]
   [cross-origin HTMLImageElement]
     expected: FAIL
 
   [cross-origin SVGImageElement]
     expected: FAIL
 
   [cross-origin HTMLVideoElement]
     expected: FAIL
--- a/testing/web-platform/meta/html/semantics/embedded-content/media-elements/playing-the-media-resource/pause-move-to-other-document.html.ini
+++ b/testing/web-platform/meta/html/semantics/embedded-content/media-elements/playing-the-media-resource/pause-move-to-other-document.html.ini
@@ -1,2 +1,3 @@
 [pause-move-to-other-document.html]
   max-asserts: 103
+  prefs: [network.http.send_window_size:0]
--- a/testing/web-platform/meta/html/semantics/embedded-content/the-video-element/video_initially_paused.html.ini
+++ b/testing/web-platform/meta/html/semantics/embedded-content/the-video-element/video_initially_paused.html.ini
@@ -1,2 +1,3 @@
 [video_initially_paused.html]
   expected: FAIL
+  prefs: [network.http.send_window_size:0]