Bug 1381748 - Cleanup FetchConsumer workflow - part 3 - shutdown workflow, r=catalinb
☠☠ backed out by 19f1250e37cd ☠ ☠
authorAndrea Marchesini <amarchesini@mozilla.com>
Wed, 26 Jul 2017 08:55:31 +0200
changeset 422381 da550c5c845ff908fccfcaa7732c4ceb54b35da8
parent 422380 067897f212ac50c81efb30aadd98cd01b103c901
child 422382 0989b2e5e0b9f091c8c68a321061508dbf49d094
push id1517
push userjlorenzo@mozilla.com
push dateThu, 14 Sep 2017 16:50:54 +0000
treeherdermozilla-release@3b41fd564418 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerscatalinb
bugs1381748
milestone56.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 1381748 - Cleanup FetchConsumer workflow - part 3 - shutdown workflow, r=catalinb
dom/fetch/FetchConsumer.cpp
dom/fetch/FetchConsumer.h
--- a/dom/fetch/FetchConsumer.cpp
+++ b/dom/fetch/FetchConsumer.cpp
@@ -37,19 +37,17 @@ public:
 
   ~FetchBodyWorkerHolder() = default;
 
   bool Notify(workers::Status aStatus) override
   {
     MOZ_ASSERT(aStatus > workers::Running);
     if (!mWasNotified) {
       mWasNotified = true;
-      // This will probably cause the releasing of the consumer.
-      // The WorkerHolder will be released as well.
-      mConsumer->ContinueConsumeBody(NS_BINDING_ABORTED, 0, nullptr);
+      mConsumer->ShutDownMainThreadConsuming();
     }
 
     return true;
   }
 };
 
 template <class Derived>
 class BeginConsumeBodyRunnable final : public Runnable
@@ -210,22 +208,16 @@ public:
                    const uint8_t* aResult) override
   {
     MOZ_ASSERT(NS_IsMainThread());
 
     // The loading is completed. Let's nullify the pump before continuing the
     // consuming of the body.
     mFetchBodyConsumer->NullifyConsumeBodyPump();
 
-    // If the binding requested cancel, we don't need to call
-    // ContinueConsumeBody, since that is the originator.
-    if (aStatus == NS_BINDING_ABORTED) {
-      return NS_OK;
-    }
-
     uint8_t* nonconstResult = const_cast<uint8_t*>(aResult);
     if (mFetchBodyConsumer->GetWorkerPrivate()) {
       RefPtr<ContinueConsumeBodyRunnable<Derived>> r =
         new ContinueConsumeBodyRunnable<Derived>(mFetchBodyConsumer,
                                                  aStatus,
                                                  aResultLength,
                                                  nonconstResult);
       if (!r->Dispatch()) {
@@ -247,16 +239,20 @@ public:
                                   nsresult aRv) override
   {
     // On error.
     if (NS_FAILED(aRv)) {
       OnStreamComplete(nullptr, nullptr, aRv, 0, nullptr);
       return;
     }
 
+    // The loading is completed. Let's nullify the pump before continuing the
+    // consuming of the body.
+    mFetchBodyConsumer->NullifyConsumeBodyPump();
+
     MOZ_ASSERT(aBlob);
 
     if (mFetchBodyConsumer->GetWorkerPrivate()) {
       RefPtr<ContinueConsumeBlobBodyRunnable<Derived>> r =
         new ContinueConsumeBlobBodyRunnable<Derived>(mFetchBodyConsumer,
                                                      aBlob->Impl());
 
       if (!r->Dispatch()) {
@@ -278,36 +274,16 @@ NS_IMPL_ADDREF(ConsumeBodyDoneObserver<D
 template <class Derived>
 NS_IMPL_RELEASE(ConsumeBodyDoneObserver<Derived>)
 template <class Derived>
 NS_INTERFACE_MAP_BEGIN(ConsumeBodyDoneObserver<Derived>)
   NS_INTERFACE_MAP_ENTRY(nsIStreamLoaderObserver)
   NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsISupports, nsIStreamLoaderObserver)
 NS_INTERFACE_MAP_END
 
-template <class Derived>
-class ShutDownMainThreadConsumingRunnable final : public WorkerMainThreadRunnable
-{
-  RefPtr<FetchBodyConsumer<Derived>> mBodyConsumer;
-
-public:
-  explicit ShutDownMainThreadConsumingRunnable(FetchBodyConsumer<Derived>* aBodyConsumer)
-    : WorkerMainThreadRunnable(aBodyConsumer->GetWorkerPrivate(),
-                               NS_LITERAL_CSTRING("Fetch :: Cancel Pump"))
-    , mBodyConsumer(aBodyConsumer)
-  {}
-
-  bool
-  MainThreadRun() override
-  {
-    mBodyConsumer->ShutDownMainThreadConsuming();
-    return true;
-  }
-};
-
 } // anonymous
 
 template <class Derived>
 /* static */ already_AddRefed<Promise>
 FetchBodyConsumer<Derived>::Create(nsIGlobalObject* aGlobal,
                                    nsIEventTarget* aMainThreadEventTarget,
                                    FetchBody<Derived>* aBody,
                                    FetchConsumeType aType,
@@ -474,23 +450,24 @@ FetchBodyConsumer<Derived>::RegisterWork
  * reflected in a lack of error return code.
  */
 template <class Derived>
 void
 FetchBodyConsumer<Derived>::BeginConsumeBodyMainThread()
 {
   AssertIsOnMainThread();
 
-  // Nothing to do.
+  AutoFailConsumeBody<Derived> autoReject(this);
+
   if (mShuttingDown) {
+    // We haven't started yet, but we have been terminated. AutoFailConsumeBody
+    // will dispatch a runnable to release resources.
     return;
   }
 
-  AutoFailConsumeBody<Derived> autoReject(this);
-
   nsCOMPtr<nsIInputStreamPump> pump;
   nsresult rv = NS_NewInputStreamPump(getter_AddRefs(pump),
                                       mBodyStream, -1, -1, 0, 0, false,
                                       mMainThreadEventTarget);
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return;
   }
 
@@ -563,21 +540,16 @@ FetchBodyConsumer<Derived>::ContinueCons
   auto autoReleaseObject = mozilla::MakeScopeExit([&] {
     self->ReleaseObject();
   });
 
   if (NS_WARN_IF(NS_FAILED(aStatus))) {
     localPromise->MaybeReject(NS_ERROR_DOM_ABORT_ERR);
   }
 
-  // We need to nullify mConsumeBodyPump on the main-thread and, in case it has
-  // not been created yet, we need to block the creation setting mShuttingDown
-  // to true.
-  ShutDownMainThreadConsuming();
-
   // Don't warn here since we warned above.
   if (NS_FAILED(aStatus)) {
     return;
   }
 
   // Finish successfully consuming body according to type.
   MOZ_ASSERT(aResult);
 
@@ -663,47 +635,36 @@ FetchBodyConsumer<Derived>::ContinueCons
 
   // Just a precaution to ensure ContinueConsumeBody is not called out of
   // sync with a body read.
   MOZ_ASSERT(mBody->BodyUsed());
 
   MOZ_ASSERT(mConsumePromise);
   RefPtr<Promise> localPromise = mConsumePromise.forget();
 
-  // Release the pump.
-  ShutDownMainThreadConsuming();
-
   RefPtr<dom::Blob> blob = dom::Blob::Create(mGlobal, aBlobImpl);
   MOZ_ASSERT(blob);
 
   localPromise->MaybeResolve(blob);
 
   ReleaseObject();
 }
 
 template <class Derived>
 void
 FetchBodyConsumer<Derived>::ShutDownMainThreadConsuming()
 {
   if (!NS_IsMainThread()) {
-    MOZ_ASSERT(mWorkerPrivate);
-    // In case of worker thread, we block the worker while the request is
-    // canceled on the main thread. This ensures that OnStreamComplete has a
-    // valid FetchConsumer around to call ShutDownMainThreadConsuming and we
-    // don't release the FetchConsumer on the main thread.
-    RefPtr<ShutDownMainThreadConsumingRunnable<Derived>> r =
-      new ShutDownMainThreadConsumingRunnable<Derived>(this);
+    RefPtr<FetchBodyConsumer<Derived>> self = this;
 
-    IgnoredErrorResult rv;
-    r->Dispatch(Killing, rv);
-    if (NS_WARN_IF(rv.Failed())) {
-      return;
-    }
+    nsCOMPtr<nsIRunnable> r = NS_NewRunnableFunction(
+      "FetchBodyConsumer::ShutDownMainThreadConsuming",
+      [self] () { self->ShutDownMainThreadConsuming(); });
 
-    MOZ_DIAGNOSTIC_ASSERT(mShuttingDown);
+    mMainThreadEventTarget->Dispatch(r.forget(), NS_DISPATCH_NORMAL);
     return;
   }
 
   // We need this because maybe, mConsumeBodyPump has not been created yet. We
   // must be sure that we don't try to do it.
   mShuttingDown = true;
 
   if (mConsumeBodyPump) {
--- a/dom/fetch/FetchConsumer.h
+++ b/dom/fetch/FetchConsumer.h
@@ -63,16 +63,17 @@ public:
   GetWorkerPrivate() const
   {
     return mWorkerPrivate;
   }
 
   void
   NullifyConsumeBodyPump()
   {
+    mShuttingDown = true;
     mConsumeBodyPump = nullptr;
   }
 
 private:
   FetchBodyConsumer(nsIEventTarget* aMainThreadEventTarget,
                     nsIGlobalObject* aGlobalObject,
                     workers::WorkerPrivate* aWorkerPrivate,
                     FetchBody<Derived>* aBody,