Backed out changeset da550c5c845f (bug 1381748)
authorCarsten "Tomcat" Book <cbook@mozilla.com>
Wed, 26 Jul 2017 14:44:29 +0200
changeset 419999 19f1250e37cdaafb4b608f2f4c188da3d20e5bef
parent 419998 000f28217a30b8cf649573ad2f52a03a8b426ef7
child 420000 299ae6eb5ec1b1be01a5a321c50aacfe8a62fbcf
push id7566
push usermtabara@mozilla.com
push dateWed, 02 Aug 2017 08:25:16 +0000
treeherdermozilla-beta@86913f512c3c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1381748
milestone56.0a1
backs outda550c5c845ff908fccfcaa7732c4ceb54b35da8
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
Backed out changeset da550c5c845f (bug 1381748)
dom/fetch/FetchConsumer.cpp
dom/fetch/FetchConsumer.h
--- a/dom/fetch/FetchConsumer.cpp
+++ b/dom/fetch/FetchConsumer.cpp
@@ -37,17 +37,19 @@ public:
 
   ~FetchBodyWorkerHolder() = default;
 
   bool Notify(workers::Status aStatus) override
   {
     MOZ_ASSERT(aStatus > workers::Running);
     if (!mWasNotified) {
       mWasNotified = true;
-      mConsumer->ShutDownMainThreadConsuming();
+      // This will probably cause the releasing of the consumer.
+      // The WorkerHolder will be released as well.
+      mConsumer->ContinueConsumeBody(NS_BINDING_ABORTED, 0, nullptr);
     }
 
     return true;
   }
 };
 
 template <class Derived>
 class BeginConsumeBodyRunnable final : public Runnable
@@ -208,16 +210,22 @@ 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()) {
@@ -239,20 +247,16 @@ 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()) {
@@ -274,16 +278,36 @@ 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,
@@ -450,24 +474,23 @@ FetchBodyConsumer<Derived>::RegisterWork
  * reflected in a lack of error return code.
  */
 template <class Derived>
 void
 FetchBodyConsumer<Derived>::BeginConsumeBodyMainThread()
 {
   AssertIsOnMainThread();
 
-  AutoFailConsumeBody<Derived> autoReject(this);
-
+  // Nothing to do.
   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;
   }
 
@@ -540,16 +563,21 @@ 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);
 
@@ -635,36 +663,47 @@ 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()) {
-    RefPtr<FetchBodyConsumer<Derived>> self = this;
+    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);
 
-    nsCOMPtr<nsIRunnable> r = NS_NewRunnableFunction(
-      "FetchBodyConsumer::ShutDownMainThreadConsuming",
-      [self] () { self->ShutDownMainThreadConsuming(); });
+    IgnoredErrorResult rv;
+    r->Dispatch(Killing, rv);
+    if (NS_WARN_IF(rv.Failed())) {
+      return;
+    }
 
-    mMainThreadEventTarget->Dispatch(r.forget(), NS_DISPATCH_NORMAL);
+    MOZ_DIAGNOSTIC_ASSERT(mShuttingDown);
     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,17 +63,16 @@ public:
   GetWorkerPrivate() const
   {
     return mWorkerPrivate;
   }
 
   void
   NullifyConsumeBodyPump()
   {
-    mShuttingDown = true;
     mConsumeBodyPump = nullptr;
   }
 
 private:
   FetchBodyConsumer(nsIEventTarget* aMainThreadEventTarget,
                     nsIGlobalObject* aGlobalObject,
                     workers::WorkerPrivate* aWorkerPrivate,
                     FetchBody<Derived>* aBody,