Bug 1320744 - Part 3, implement nsIThreadRetargetableRequest in HttpChannelChild. r=mayhemer

MozReview-Commit-ID: FyLXlkQde3h

netwerk/protocol/http/HttpBaseChannel.cpp

@@ -150,7 +150,8 @@ private:
 NS_IMPL_ISUPPORTS(AddHeadersToChannelVisitor, nsIHttpHeaderVisitor)
 
 HttpBaseChannel::HttpBaseChannel()
-  : mStartPos(UINT64_MAX)
+  : mCanceled(false)
+  , mStartPos(UINT64_MAX)
   , mStatus(NS_OK)
   , mLoadFlags(LOAD_NORMAL)
   , mCaps(0)
@@ -158,7 +159,6 @@ HttpBaseChannel::HttpBaseChannel()
   , mPriority(PRIORITY_NORMAL)
   , mRedirectionLimit(gHttpHandler->RedirectionLimit())
   , mApplyConversion(true)
-  , mCanceled(false)
   , mIsPending(false)
   , mWasOpened(false)
   , mRequestObserversCalled(false)

netwerk/protocol/http/HttpBaseChannel.h

@@ -8,6 +8,7 @@
 #ifndef mozilla_net_HttpBaseChannel_h
 #define mozilla_net_HttpBaseChannel_h
 
+#include "mozilla/Atomics.h"
 #include "nsHttp.h"
 #include "nsAutoPtr.h"
 #include "nsHashPropertyBag.h"
@@ -445,6 +446,10 @@ private:
   void ReleaseMainThreadOnlyReferences();
 
 protected:
+  // Use Release-Acquire ordering to ensure the OMT ODA is ignored while channel
+  // is canceled on main thread.
+  Atomic<bool, ReleaseAcquire> mCanceled;
+
   nsTArray<Pair<nsString, nsString>> mSecurityConsoleMessages;
 
   nsCOMPtr<nsIStreamListener>       mListener;
@@ -488,7 +493,6 @@ protected:
   uint8_t                           mRedirectionLimit;
 
   uint32_t                          mApplyConversion            : 1;
-  uint32_t                          mCanceled                   : 1;
   uint32_t                          mIsPending                  : 1;
   uint32_t                          mWasOpened                  : 1;
   // if 1 all "http-on-{opening|modify|etc}-request" observers have been called

netwerk/protocol/http/HttpChannelChild.cpp

@@ -20,6 +20,7 @@
 #include "mozilla/net/NeckoChild.h"
 #include "mozilla/net/HttpChannelChild.h"
 
+#include "nsCOMPtr.h"
 #include "nsISupportsPrimitives.h"
 #include "nsChannelClassifier.h"
 #include "nsGlobalWindow.h"
@@ -42,8 +43,11 @@
 #include "nsIDeprecationWarner.h"
 #include "nsICompressConvStats.h"
 #include "nsIDocument.h"
+#include "nsIDOMDocument.h"
 #include "nsIDOMWindowUtils.h"
+#include "nsIEventTarget.h"
 #include "nsStreamUtils.h"
+#include "nsThreadUtils.h"
 
 #ifdef OS_POSIX
 #include "chrome/common/file_descriptor_set_posix.h"
@@ -261,6 +265,7 @@ NS_INTERFACE_MAP_BEGIN(HttpChannelChild)
   NS_INTERFACE_MAP_ENTRY(nsIHttpChannelChild)
   NS_INTERFACE_MAP_ENTRY_CONDITIONAL(nsIAssociatedContentSecurity, GetAssociatedContentSecurity())
   NS_INTERFACE_MAP_ENTRY(nsIDivertableChannel)
+  NS_INTERFACE_MAP_ENTRY(nsIThreadRetargetableRequest)
 NS_INTERFACE_MAP_END_INHERITING(HttpBaseChannel)
 
 //-----------------------------------------------------------------------------
@@ -645,7 +650,7 @@ class TransportAndDataEvent : public ChannelEvent
   already_AddRefed<nsIEventTarget> GetEventTarget()
   {
     MOZ_ASSERT(mChild);
-    nsCOMPtr<nsIEventTarget> target = mChild->GetNeckoTarget();
+    nsCOMPtr<nsIEventTarget> target = mChild->GetODATarget();
     return target.forget();
   }
  private:
@@ -753,11 +758,27 @@ HttpChannelChild::OnTransportAndData(const nsresult& channelStatus,
   // necko msg in between them.
   AutoEventEnqueuer ensureSerialDispatch(mEventQ);
 
-  DoOnStatus(this, transportStatus);
-
   const int64_t progressMax = mResponseHead->ContentLength();
   const int64_t progress = offset + count;
-  DoOnProgress(this, progress, progressMax);
+
+  // OnTransportAndData will be run on retargeted thread if applicable, however
+  // OnStatus/OnProgress event can only be fired on main thread. We need to
+  // dispatch the status/progress event handling back to main thread with the
+  // appropriate event target for networking.
+  if (NS_IsMainThread()) {
+    DoOnStatus(this, transportStatus);
+    DoOnProgress(this, progress, progressMax);
+  } else {
+    RefPtr<HttpChannelChild> self = this;
+    nsCOMPtr<nsIEventTarget> neckoTarget = GetNeckoTarget();
+    DebugOnly<nsresult> rv =
+      neckoTarget->Dispatch(
+        NS_NewRunnableFunction([self, transportStatus, progress, progressMax]() {
+          self->DoOnStatus(self, transportStatus);
+          self->DoOnProgress(self, progress, progressMax);
+        }), NS_DISPATCH_NORMAL);
+    MOZ_ASSERT(NS_SUCCEEDED(rv));
+  }
 
   // OnDataAvailable
   //
@@ -782,6 +803,8 @@ void
 HttpChannelChild::DoOnStatus(nsIRequest* aRequest, nsresult status)
 {
   LOG(("HttpChannelChild::DoOnStatus [this=%p]\n", this));
+  MOZ_ASSERT(NS_IsMainThread());
+
   if (mCanceled)
     return;
 
@@ -815,6 +838,8 @@ void
 HttpChannelChild::DoOnProgress(nsIRequest* aRequest, int64_t progress, int64_t progressMax)
 {
   LOG(("HttpChannelChild::DoOnProgress [this=%p]\n", this));
+  MOZ_ASSERT(NS_IsMainThread());
+
   if (mCanceled)
     return;
 
@@ -846,7 +871,7 @@ HttpChannelChild::DoOnDataAvailable(nsIRequest* aRequest, nsISupports* aContext,
 
   nsresult rv = mListener->OnDataAvailable(aRequest, aContext, aStream, offset, count);
   if (NS_FAILED(rv)) {
-    Cancel(rv);
+    CancelOnMainThread(rv);
   }
 }
 
@@ -929,6 +954,7 @@ HttpChannelChild::OnStopRequest(const nsresult& channelStatus,
 {
   LOG(("HttpChannelChild::OnStopRequest [this=%p status=%" PRIx32 "]\n",
        this, static_cast<uint32_t>(channelStatus)));
+  MOZ_ASSERT(NS_IsMainThread());
 
   if (mDivertingToParent) {
     MOZ_RELEASE_ASSERT(!mFlushedForDiversion,
@@ -938,6 +964,30 @@ HttpChannelChild::OnStopRequest(const nsresult& channelStatus,
     return;
   }
 
+  // In thread retargeting is enabled, there might be Runnable for
+  // DoOnStatus/DoOnProgress sit in the main thread event target. We need to
+  // ensure OnStopRequest is fired after that by postponing the
+  // ChannelEventQueue processing to the end of main thread event target.
+  // This workaround can be removed after bug 1338493 is complete.
+  if (mODATarget) {
+    {
+      MutexAutoLock lock(mEventTargetMutex);
+      mODATarget = nullptr;
+    }
+    mEventQ->Suspend();
+    UniquePtr<ChannelEvent> stopEvent =
+      MakeUnique<StopRequestEvent>(this, channelStatus, timing);
+    mEventQ->PrependEvent(stopEvent);
+
+    nsCOMPtr<nsIEventTarget> neckoTarget = GetNeckoTarget();
+    MOZ_ASSERT(neckoTarget);
+
+    DebugOnly<nsresult> rv = neckoTarget->Dispatch(
+      NewRunnableMethod(mEventQ, &ChannelEventQueue::Resume), NS_DISPATCH_NORMAL);
+    MOZ_ASSERT(NS_SUCCEEDED(rv));
+    return;
+  }
+
   if (mUnknownDecoderInvolved) {
    LOG(("UnknownDecoder is involved queue OnStopRequest call. [this=%p]",
         this));
@@ -1025,6 +1075,7 @@ void
 HttpChannelChild::DoOnStopRequest(nsIRequest* aRequest, nsresult aChannelStatus, nsISupports* aContext)
 {
   LOG(("HttpChannelChild::DoOnStopRequest [this=%p]\n", this));
+  MOZ_ASSERT(NS_IsMainThread());
   MOZ_ASSERT(!mIsPending);
 
   // NB: We use aChannelStatus here instead of mStatus because if there was an
@@ -2221,6 +2272,21 @@ HttpChannelChild::GetNeckoTarget()
   return target.forget();
 }
 
+already_AddRefed<nsIEventTarget>
+HttpChannelChild::GetODATarget()
+{
+  nsCOMPtr<nsIEventTarget> target;
+  {
+    MutexAutoLock lock(mEventTargetMutex);
+    target = mODATarget ? mODATarget : mNeckoTarget;
+  }
+
+  if (!target) {
+    target = do_GetMainThread();
+  }
+  return target.forget();
+}
+
 nsresult
 HttpChannelChild::ContinueAsyncOpen()
 {
@@ -2960,6 +3026,48 @@ HttpChannelChild::GetDivertingToParent(bool* aDiverting)
   return NS_OK;
 }
 
+//-----------------------------------------------------------------------------
+// HttpChannelChild::nsIThreadRetargetableRequest
+//-----------------------------------------------------------------------------
+
+NS_IMETHODIMP
+HttpChannelChild::RetargetDeliveryTo(nsIEventTarget* aNewTarget)
+{
+  LOG(("HttpChannelChild::RetargetDeliveryTo [this=%p, aNewTarget=%p]",
+       this, aNewTarget));
+
+  MOZ_ASSERT(NS_IsMainThread(), "Should be called on main thread only");
+  MOZ_ASSERT(!mODATarget);
+  MOZ_ASSERT(aNewTarget);
+
+  NS_ENSURE_ARG(aNewTarget);
+  if (aNewTarget == NS_GetCurrentThread()) {
+    NS_WARNING("Retargeting delivery to same thread");
+    return NS_OK;
+  }
+
+  // Ensure that |mListener| and any subsequent listeners can be retargeted
+  // to another thread.
+  nsresult rv = NS_OK;
+  nsCOMPtr<nsIThreadRetargetableStreamListener> retargetableListener =
+      do_QueryInterface(mListener, &rv);
+  if (!retargetableListener || NS_FAILED(rv)) {
+    NS_WARNING("Listener is not retargetable");
+    return NS_ERROR_NO_INTERFACE;
+  }
+
+  rv = retargetableListener->CheckListenerChain();
+  if (NS_FAILED(rv)) {
+    NS_WARNING("Subsequent listeners are not retargetable");
+    return rv;
+  }
+
+  {
+    MutexAutoLock lock(mEventTargetMutex);
+    mODATarget = aNewTarget;
+  }
+  return NS_OK;
+}
 
 void
 HttpChannelChild::ResetInterception()
@@ -3004,6 +3112,53 @@ HttpChannelChild::TrySendDeletingChannel()
   MOZ_ASSERT(NS_SUCCEEDED(rv));
 }
 
+class CancelEvent final : public ChannelEvent
+{
+public:
+  CancelEvent(HttpChannelChild* aChild, nsresult aRv)
+    : mChild(aChild)
+    , mRv(aRv)
+  {
+    MOZ_ASSERT(!NS_IsMainThread());
+    MOZ_ASSERT(aChild);
+  }
+
+  void Run() {
+    MOZ_ASSERT(NS_IsMainThread());
+    mChild->Cancel(mRv);
+  }
+
+  already_AddRefed<nsIEventTarget> GetEventTarget()
+  {
+    MOZ_ASSERT(mChild);
+    nsCOMPtr<nsIEventTarget> target = mChild->GetNeckoTarget();
+    return target.forget();
+  }
+
+private:
+  HttpChannelChild* mChild;
+  const nsresult mRv;
+};
+
+void
+HttpChannelChild::CancelOnMainThread(nsresult aRv)
+{
+  LOG(("HttpChannelChild::CancelOnMainThread [this=%p]", this));
+
+  if (NS_IsMainThread()) {
+    Cancel(aRv);
+    return;
+  }
+
+  mEventQ->Suspend();
+  // Cancel is expected to preempt any other channel events, thus we put this
+  // event in the front of mEventQ to make sure nsIStreamListener not receiving
+  // any ODA/OnStopRequest callbacks.
+  UniquePtr<ChannelEvent> cancelEvent = MakeUnique<CancelEvent>(this, aRv);
+  mEventQ->PrependEvent(cancelEvent);
+  mEventQ->Resume();
+}
+
 void
 HttpChannelChild::OverrideWithSynthesizedResponse(nsAutoPtr<nsHttpResponseHead>& aResponseHead,
                                                   nsIInputStream* aSynthesizedInput,

netwerk/protocol/http/HttpChannelChild.h

@@ -30,8 +30,10 @@
 #include "nsIChildChannel.h"
 #include "nsIHttpChannelChild.h"
 #include "nsIDivertableChannel.h"
+#include "nsIThreadRetargetableRequest.h"
 #include "mozilla/net/DNS.h"
 
+class nsIEventTarget;
 class nsInputStreamPump;
 
 namespace mozilla {
@@ -51,6 +53,7 @@ class HttpChannelChild final : public PHttpChannelChild
                              , public nsIChildChannel
                              , public nsIHttpChannelChild
                              , public nsIDivertableChannel
+                             , public nsIThreadRetargetableRequest
 {
   virtual ~HttpChannelChild();
 public:
@@ -64,6 +67,7 @@ public:
   NS_DECL_NSICHILDCHANNEL
   NS_DECL_NSIHTTPCHANNELCHILD
   NS_DECL_NSIDIVERTABLECHANNEL
+  NS_DECL_NSITHREADRETARGETABLEREQUEST
 
   HttpChannelChild();
 
@@ -201,6 +205,9 @@ private:
   // Get event target for processing network events.
   already_AddRefed<nsIEventTarget> GetNeckoTarget();
 
+  // Get event target for ODA.
+  already_AddRefed<nsIEventTarget> GetODATarget();
+
   MOZ_MUST_USE nsresult ContinueAsyncOpen();
 
   void DoOnStartRequest(nsIRequest* aRequest, nsISupports* aContext);
@@ -228,6 +235,10 @@ private:
   // main thread if invoking on non-main thread.
   void TrySendDeletingChannel();
 
+  // Try invoke Cancel if on main thread, or prepend a CancelEvent in mEventQ to
+  // ensure Cacnel is processed before any other channel events.
+  void CancelOnMainThread(nsresult aRv);
+
   RequestHeaderTuples mClientSetRequestHeaders;
   nsCOMPtr<nsIChildChannel> mRedirectChannelChild;
   RefPtr<InterceptStreamListener> mInterceptListener;
@@ -301,7 +312,9 @@ private:
 
   // EventTarget for labeling networking events.
   nsCOMPtr<nsIEventTarget> mNeckoTarget;
-  // Used to ensure atomicity of mNeckoTarget;
+  // Target thread for delivering ODA.
+  nsCOMPtr<nsIEventTarget> mODATarget;
+  // Used to ensure atomicity of mNeckoTarget / mODATarget;
   Mutex mEventTargetMutex;
 
   void FinishInterceptedRedirect();
@@ -376,6 +389,7 @@ private:
   friend class Redirect3Event;
   friend class DeleteSelfEvent;
   friend class HttpFlushedForDiversionEvent;
+  friend class CancelEvent;
   friend class HttpAsyncAborter<HttpChannelChild>;
   friend class InterceptStreamListener;
   friend class InterceptedChannelContent;

netwerk/protocol/http/nsHttpChannel.cpp

@@ -6358,8 +6358,8 @@ nsHttpChannel::ContinueBeginConnectWithResult()
     }
 
     LOG(("nsHttpChannel::ContinueBeginConnectWithResult result [this=%p rv=%" PRIx32
-         " mCanceled=%i]\n",
+         " mCanceled=%u]\n",
-         this, static_cast<uint32_t>(rv), mCanceled));
+         this, static_cast<uint32_t>(rv), static_cast<bool>(mCanceled)));
     return rv;
 }
 

parser/html/nsHtml5StreamParser.cpp

@@ -982,11 +982,7 @@ nsHtml5StreamParser::OnStartRequest(nsIRequest* aRequest, nsISupports* aContext)
   }
 
   if (NS_FAILED(rv)) {
-    // for now skip warning if we're on child process, since we don't support
+    NS_WARNING("Failed to retarget HTML data delivery to the parser thread.");
-    // off-main thread delivery there yet.  This will change with bug 1015466
-    if (!XRE_IsContentProcess()) {
-      NS_WARNING("Failed to retarget HTML data delivery to the parser thread.");
-    }
   }
 
   if (mCharsetSource == kCharsetFromParentFrame) {