Bug 1416879 - Part 2: Allow for diversion cancellation and trigger for intercepted channels. r=bkelly, r=mayhemer
authorAndrew Sutherland <asutherland@asutherland.org>
Thu, 04 Jan 2018 18:38:07 -0500
changeset 450657 7052da34a62d03ad510ad273f8cad970157fd5ba
parent 450656 e3b4b0f2a73c7a6fdcf1389e08ed5f59487a0f3e
child 450658 f4ebb9ec34523bfd24d86b3769e5f4185db25d25
push id8531
push userryanvm@gmail.com
push dateFri, 12 Jan 2018 16:47:01 +0000
treeherdermozilla-beta@0bc627ade5a0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbkelly, mayhemer
bugs1416879
milestone59.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 1416879 - Part 2: Allow for diversion cancellation and trigger for intercepted channels. r=bkelly, r=mayhemer The diversion mechanism never expected to be dealing with data sourced from the content process, but that's exactly what happens with ServiceWorker-intercepted channels with the current child-intercept situation (which is being fixed). In order to allow timely cancellation of diverted intercepted channels, there needs to be a way to relay to the HttpChannelChild that it needs to be canceled so that the synthesized pump can be canceled and diversion can be marked as complete. This patch adds such a mechanism to ADivertableParentChannel and PHttpChannel for the exclusive use of InterceptedHttpChannel and then uses it.
netwerk/base/ADivertableParentChannel.h
netwerk/protocol/ftp/FTPChannelParent.cpp
netwerk/protocol/ftp/FTPChannelParent.h
netwerk/protocol/http/HttpChannelChild.cpp
netwerk/protocol/http/HttpChannelChild.h
netwerk/protocol/http/HttpChannelParent.cpp
netwerk/protocol/http/HttpChannelParent.h
netwerk/protocol/http/InterceptedHttpChannel.cpp
netwerk/protocol/http/InterceptedHttpChannel.h
netwerk/protocol/http/PHttpChannel.ipdl
--- a/netwerk/base/ADivertableParentChannel.h
+++ b/netwerk/base/ADivertableParentChannel.h
@@ -32,14 +32,29 @@ public:
   virtual nsresult SuspendForDiversion() = 0;
 
   // While messages are diverted back from the child to the parent calls to
   // suspend/resume the channel must also suspend/resume the message diversion.
   // These two functions will be called by nsHttpChannel and nsFtpChannel
   // Suspend()/Resume() functions.
   virtual nsresult SuspendMessageDiversion() = 0;
   virtual nsresult ResumeMessageDiversion() = 0;
+
+  // Cancel an ongoing diversion by using IPC to invoke Cancel() in the child.
+  // This is necessary because most of the channel's state machine is suspended
+  // during diversion, so an explicit action must be taken to interrupt the
+  // diversion process so cancellation can be fully processed.
+  //
+  // Historically, diversions were assumed to be shortlived, where it was merely
+  // a question of diverting some small amount of network traffic back to the
+  // parent.  However, Service Worker child interception made it possible for
+  // the data to entirely be sourced from the child, which makes diversion
+  // potentially long-lived.  Especially when large files are involved.
+  //
+  // This mechanism is expected to be removed when ServiceWorkers move from
+  // child intercept to parent intercept (in the short to medium term).
+  virtual nsresult CancelDiversion() = 0;
 };
 
 } // namespace net
 } // namespace mozilla
 
 #endif
--- a/netwerk/protocol/ftp/FTPChannelParent.cpp
+++ b/netwerk/protocol/ftp/FTPChannelParent.cpp
@@ -732,16 +732,25 @@ FTPChannelParent::SuspendMessageDiversio
 nsresult
 FTPChannelParent::ResumeMessageDiversion()
 {
   // This only need to resumes message queue.
   mEventQ->Resume();
   return NS_OK;
 }
 
+nsresult
+FTPChannelParent::CancelDiversion()
+{
+  // Only HTTP channels can have child-process-sourced-data that's long-lived
+  // so this isn't currently relevant for FTP channels and there is nothing to
+  // do.
+  return NS_OK;
+}
+
 void
 FTPChannelParent::DivertTo(nsIStreamListener *aListener)
 {
   MOZ_ASSERT(aListener);
   if (NS_WARN_IF(!mDivertingFromChild)) {
     MOZ_ASSERT(mDivertingFromChild,
                "Cannot DivertTo new listener if diverting is not set!");
     return;
--- a/netwerk/protocol/ftp/FTPChannelParent.h
+++ b/netwerk/protocol/ftp/FTPChannelParent.h
@@ -49,16 +49,18 @@ public:
 
   bool Init(const FTPChannelCreationArgs& aOpenArgs);
 
   // ADivertableParentChannel functions.
   void DivertTo(nsIStreamListener *aListener) override;
   nsresult SuspendForDiversion() override;
   nsresult SuspendMessageDiversion() override;
   nsresult ResumeMessageDiversion() override;
+  nsresult CancelDiversion() override;
+
 
   // Calls OnStartRequest for "DivertTo" listener, then notifies child channel
   // that it should divert OnDataAvailable and OnStopRequest calls to this
   // parent channel.
   void StartDiversion();
 
   // Handles calling OnStart/Stop if there are errors during diversion.
   // Called asynchronously from FailDiversion.
--- a/netwerk/protocol/http/HttpChannelChild.cpp
+++ b/netwerk/protocol/http/HttpChannelChild.cpp
@@ -3860,16 +3860,31 @@ HttpChannelChild::RecvSetPriority(const 
 
 mozilla::ipc::IPCResult
 HttpChannelChild::RecvAttachStreamFilter(Endpoint<extensions::PStreamFilterParent>&& aEndpoint)
 {
   extensions::StreamFilterParent::Attach(this, Move(aEndpoint));
   return IPC_OK();
 }
 
+mozilla::ipc::IPCResult
+HttpChannelChild::RecvCancelDiversion()
+{
+  MOZ_ASSERT(NS_IsMainThread());
+
+  // This method is a very special case for cancellation of a diverted
+  // intercepted channel.  Normally we would go through the mEventQ in order to
+  // serialize event execution in the face of sync XHR and now background
+  // channels.  However, similar to how CancelOnMainThread describes, Cancel()
+  // pre-empts everything.  (And frankly, we want this stack frame on the stack
+  // if a crash happens.)
+  Cancel(NS_ERROR_ABORT);
+  return IPC_OK();
+}
+
 void
 HttpChannelChild::ActorDestroy(ActorDestroyReason aWhy)
 {
   MOZ_ASSERT(NS_IsMainThread());
 
   // OnStartRequest might be dropped if IPDL is destroyed abnormally
   // and BackgroundChild might have pending IPC messages.
   // Clean up BackgroundChild at this time to prevent memleak.
--- a/netwerk/protocol/http/HttpChannelChild.h
+++ b/netwerk/protocol/http/HttpChannelChild.h
@@ -163,16 +163,18 @@ protected:
 
   mozilla::ipc::IPCResult RecvIssueDeprecationWarning(const uint32_t& warning,
                                                       const bool& asError) override;
 
   mozilla::ipc::IPCResult RecvSetPriority(const int16_t& aPriority) override;
 
   mozilla::ipc::IPCResult RecvAttachStreamFilter(Endpoint<extensions::PStreamFilterParent>&& aEndpoint) override;
 
+  mozilla::ipc::IPCResult RecvCancelDiversion() override;
+
   virtual void ActorDestroy(ActorDestroyReason aWhy) override;
 
   MOZ_MUST_USE bool
   GetAssociatedContentSecurity(nsIAssociatedContentSecurity** res = nullptr);
   virtual void DoNotifyListenerCleanup() override;
 
   NS_IMETHOD GetResponseSynthesized(bool* aSynthesized) override;
 
--- a/netwerk/protocol/http/HttpChannelParent.cpp
+++ b/netwerk/protocol/http/HttpChannelParent.cpp
@@ -1996,16 +1996,26 @@ nsresult
 HttpChannelParent::ResumeMessageDiversion()
 {
   LOG(("HttpChannelParent::SuspendMessageDiversion [this=%p]", this));
   // This only needs to resumes message queue.
   mEventQ->Resume();
   return NS_OK;
 }
 
+nsresult
+HttpChannelParent::CancelDiversion()
+{
+  LOG(("HttpChannelParent::CancelDiversion [this=%p]", this));
+  if (!mIPCClosed) {
+    Unused << SendCancelDiversion();
+  }
+  return NS_OK;
+}
+
 /* private, supporting function for ADivertableParentChannel */
 nsresult
 HttpChannelParent::ResumeForDiversion()
 {
   LOG(("HttpChannelParent::ResumeForDiversion [this=%p]\n", this));
   MOZ_ASSERT(mChannel);
   if (NS_WARN_IF(!mDivertingFromChild)) {
     MOZ_ASSERT(mDivertingFromChild,
--- a/netwerk/protocol/http/HttpChannelParent.h
+++ b/netwerk/protocol/http/HttpChannelParent.h
@@ -78,16 +78,17 @@ public:
 
   MOZ_MUST_USE bool Init(const HttpChannelCreationArgs& aOpenArgs);
 
   // ADivertableParentChannel functions.
   void DivertTo(nsIStreamListener *aListener) override;
   MOZ_MUST_USE nsresult SuspendForDiversion() override;
   MOZ_MUST_USE nsresult SuspendMessageDiversion() override;
   MOZ_MUST_USE nsresult ResumeMessageDiversion() override;
+  MOZ_MUST_USE nsresult CancelDiversion() override;
 
   // Calls OnStartRequest for "DivertTo" listener, then notifies child channel
   // that it should divert OnDataAvailable and OnStopRequest calls to this
   // parent channel.
   void StartDiversion();
 
   // Handles calling OnStart/Stop if there are errors during diversion.
   // Called asynchronously from FailDiversion.
--- a/netwerk/protocol/http/InterceptedHttpChannel.cpp
+++ b/netwerk/protocol/http/InterceptedHttpChannel.cpp
@@ -28,16 +28,17 @@ InterceptedHttpChannel::InterceptedHttpC
                                                const TimeStamp& aAsyncOpenTimestamp)
   : HttpAsyncAborter<InterceptedHttpChannel>(this)
   , mProgress(0)
   , mProgressReported(0)
   , mSynthesizedStreamLength(-1)
   , mResumeStartPos(0)
   , mSynthesizedOrReset(Invalid)
   , mCallingStatusAndProgress(false)
+  , mDiverting(false)
 {
   // Pre-set the creation and AsyncOpen times based on the original channel
   // we are intercepting.  We don't want our extra internal redirect to mask
   // any time spent processing the channel.
   mChannelCreationTime = aCreationTime;
   mChannelCreationTimestamp = aCreationTimestamp;
   mAsyncOpenTime = aAsyncOpenTimestamp;
 }
@@ -496,16 +497,25 @@ InterceptedHttpChannel::Cancel(nsresult 
   }
   mCanceled = true;
 
   MOZ_DIAGNOSTIC_ASSERT(NS_FAILED(aStatus));
   if (NS_SUCCEEDED(mStatus)) {
     mStatus = aStatus;
   }
 
+  // Everything is suspended during diversion until it completes.  Since the
+  // intercepted channel could be a long-running stream, we need to request that
+  // cancellation be triggered in the child, completing the diversion and
+  // allowing cancellation to run to completion.
+  if (mDiverting) {
+    Unused << mParentChannel->CancelDiversion();
+    // (We want the pump to be canceled as well, so don't directly return.)
+  }
+
   if (mPump) {
     return mPump->Cancel(mStatus);
   }
 
   return AsyncAbort(mStatus);
 }
 
 NS_IMETHODIMP
@@ -1109,28 +1119,30 @@ InterceptedHttpChannel::OnDataAvailable(
                                     aOffset, aCount);
 }
 
 NS_IMETHODIMP
 InterceptedHttpChannel::MessageDiversionStarted(ADivertableParentChannel* aParentChannel)
 {
   MOZ_ASSERT(!mParentChannel);
   mParentChannel = aParentChannel;
+  mDiverting = true;
   uint32_t suspendCount = mSuspendCount;
   while(suspendCount--) {
     mParentChannel->SuspendMessageDiversion();
   }
   return NS_OK;
 }
 
 NS_IMETHODIMP
 InterceptedHttpChannel::MessageDiversionStop()
 {
   MOZ_ASSERT(mParentChannel);
   mParentChannel = nullptr;
+  mDiverting = false;
   return NS_OK;
 }
 
 NS_IMETHODIMP
 InterceptedHttpChannel::SuspendInternal()
 {
   ++mSuspendCount;
   if (mPump) {
--- a/netwerk/protocol/http/InterceptedHttpChannel.h
+++ b/netwerk/protocol/http/InterceptedHttpChannel.h
@@ -89,16 +89,17 @@ private:
   nsCString mResumeEntityId;
   nsString mStatusHost;
   enum {
     Invalid = 0,
     Synthesized,
     Reset
   } mSynthesizedOrReset;
   Atomic<bool> mCallingStatusAndProgress;
+  bool mDiverting;
 
   InterceptedHttpChannel(PRTime aCreationTime,
                          const TimeStamp& aCreationTimestamp,
                          const TimeStamp& aAsyncOpenTimestamp);
   ~InterceptedHttpChannel() = default;
 
   virtual void
   ReleaseListeners() override;
--- a/netwerk/protocol/http/PHttpChannel.ipdl
+++ b/netwerk/protocol/http/PHttpChannel.ipdl
@@ -144,16 +144,19 @@ child:
 
   // When CORS blocks the request in the parent process, it doesn't have the
   // correct window ID, so send the message to the child for logging to the web
   // console.
   async LogBlockedCORSRequest(nsString message);
 
   async AttachStreamFilter(Endpoint<PStreamFilterParent> aEndpoint);
 
+  // See ADivertableParentChannel::CancelDiversion
+  async CancelDiversion();
+
 both:
   // After receiving this message, the parent also calls
   // SendFinishInterceptedRedirect, and makes sure not to send any more messages
   // after that. When receiving this message, the child will call
   // Send__delete__() and complete the steps required to finish the redirect.
   async FinishInterceptedRedirect();
 
   async SetPriority(int16_t priority);