Backed out 2 changesets (bug 1418074) for bustage in dom/fetch/Fetch.cpp. r=backout a=backout
authorSebastian Hengst <archaeopteryx@coole-files.de>
Mon, 08 Jan 2018 15:12:31 +0200
changeset 445574 1b3d0019f65e91d5ec62760f9ba0104cc98ea052
parent 445573 8c014d799aa40e09db34c577786f6460b642607d
child 445575 f47b24dad3907240e9ffcc6298b173820ae2f41f
push id1618
push userCallek@gmail.com
push dateThu, 11 Jan 2018 17:45:48 +0000
treeherdermozilla-release@882ca853e05a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbackout, backout
bugs1418074
milestone58.0
backs out9875bb519e72d8cf45ca75ca494c8bb51447749b
2327b280b3fceb883ec2120f3635b2f099bda300
Backed out 2 changesets (bug 1418074) for bustage in dom/fetch/Fetch.cpp. r=backout a=backout Backed out changeset 9875bb519e72 (bug 1418074) Backed out changeset 2327b280b3fc (bug 1418074)
dom/fetch/Fetch.cpp
--- a/dom/fetch/Fetch.cpp
+++ b/dom/fetch/Fetch.cpp
@@ -157,41 +157,27 @@ private:
   ~AbortSignalProxy()
   {
     NS_ProxyRelease(
       "AbortSignalProxy::mSignalMainThread",
       mMainThreadEventTarget, mSignalMainThread.forget());
   }
 };
 
-class WorkerFetchResolver;
-
-class WorkerNotifier final : public WorkerHolder
-{
-  RefPtr<WorkerFetchResolver> mResolver;
-
-public:
-  explicit WorkerNotifier(WorkerFetchResolver* aResolver)
-    : WorkerHolder("WorkerNotifier", AllowIdleShutdownStart)
-    , mResolver(aResolver)
-  {}
-
-  bool
-  Notify(Status aStatus) override;
-};
-
 class WorkerFetchResolver final : public FetchDriverObserver
 {
-  // Thread-safe:
+  friend class MainThreadFetchRunnable;
+  friend class WorkerDataAvailableRunnable;
+  friend class WorkerFetchResponseEndBase;
+  friend class WorkerFetchResponseEndRunnable;
+  friend class WorkerFetchResponseRunnable;
+
   RefPtr<PromiseWorkerProxy> mPromiseProxy;
   RefPtr<AbortSignalProxy> mSignalProxy;
-
-  // Touched only on the worker thread.
   RefPtr<FetchObserver> mFetchObserver;
-  UniquePtr<WorkerHolder> mWorkerHolder;
 
 public:
   // Returns null if worker is shutting down.
   static already_AddRefed<WorkerFetchResolver>
   Create(workers::WorkerPrivate* aWorkerPrivate, Promise* aPromise,
          AbortSignal* aSignal, FetchObserver* aObserver)
   {
     MOZ_ASSERT(aWorkerPrivate);
@@ -205,21 +191,16 @@ public:
     RefPtr<AbortSignalProxy> signalProxy;
     if (aSignal) {
       signalProxy =
         new AbortSignalProxy(aSignal, aWorkerPrivate->MainThreadEventTarget());
     }
 
     RefPtr<WorkerFetchResolver> r =
       new WorkerFetchResolver(proxy, signalProxy, aObserver);
-
-    if (NS_WARN_IF(!r->HoldWorker(aWorkerPrivate))) {
-      return nullptr;
-    }
-
     return r.forget();
   }
 
   AbortSignal*
   GetAbortSignalForMainThread()
   {
     MOZ_ASSERT(NS_IsMainThread());
 
@@ -237,71 +218,28 @@ public:
 
     if (!mSignalProxy) {
       return nullptr;
     }
 
     return mSignalProxy->GetSignalForTargetThread();
   }
 
-  PromiseWorkerProxy*
-  PromiseProxy() const
-  {
-    MOZ_ASSERT(NS_IsMainThread());
-    return mPromiseProxy;
-  }
-
-  Promise*
-  WorkerPromise(WorkerPrivate* aWorkerPrivate) const
-  {
-    MOZ_ASSERT(aWorkerPrivate);
-    aWorkerPrivate->AssertIsOnWorkerThread();
-
-    return mPromiseProxy->WorkerPromise();
-  }
-
-  FetchObserver*
-  GetFetchObserver(WorkerPrivate* aWorkerPrivate) const
-  {
-    MOZ_ASSERT(aWorkerPrivate);
-    aWorkerPrivate->AssertIsOnWorkerThread();
-
-    return mFetchObserver;
-  }
-
   void
   OnResponseAvailableInternal(InternalResponse* aResponse) override;
 
   void
   OnResponseEnd(FetchDriverObserver::EndReason eReason) override;
 
   bool
   NeedOnDataAvailable() override;
 
   void
   OnDataAvailable() override;
 
-  void
-  Shutdown(WorkerPrivate* aWorkerPrivate)
-  {
-    MOZ_ASSERT(aWorkerPrivate);
-    aWorkerPrivate->AssertIsOnWorkerThread();
-
-    mPromiseProxy->CleanUp();
-
-    mFetchObserver = nullptr;
-
-    if (mSignalProxy) {
-      mSignalProxy->Shutdown();
-      mSignalProxy = nullptr;
-    }
-
-    mWorkerHolder = nullptr;
-  }
-
 private:
    WorkerFetchResolver(PromiseWorkerProxy* aProxy,
                        AbortSignalProxy* aSignalProxy,
                        FetchObserver* aObserver)
     : mPromiseProxy(aProxy)
     , mSignalProxy(aSignalProxy)
     , mFetchObserver(aObserver)
   {
@@ -309,28 +247,16 @@ private:
     MOZ_ASSERT(mPromiseProxy);
   }
 
   ~WorkerFetchResolver()
   {}
 
   virtual void
   FlushConsoleReport() override;
-
-  bool
-  HoldWorker(WorkerPrivate* aWorkerPrivate)
-  {
-    UniquePtr<WorkerNotifier> wn(new WorkerNotifier(this));
-    if (NS_WARN_IF(!wn->HoldWorker(aWorkerPrivate, Canceling))) {
-      return false;
-    }
-
-    mWorkerHolder = Move(wn);
-    return true;
-  }
 };
 
 class MainThreadFetchResolver final : public FetchDriverObserver
 {
   RefPtr<Promise> mPromise;
   RefPtr<Response> mResponse;
   RefPtr<FetchObserver> mFetchObserver;
   RefPtr<AbortSignal> mSignal;
@@ -396,17 +322,17 @@ public:
     MOZ_ASSERT(mResolver);
   }
 
   NS_IMETHOD
   Run() override
   {
     AssertIsOnMainThread();
     RefPtr<FetchDriver> fetch;
-    RefPtr<PromiseWorkerProxy> proxy = mResolver->PromiseProxy();
+    RefPtr<PromiseWorkerProxy> proxy = mResolver->mPromiseProxy;
 
     {
       // Acquire the proxy mutex while getting data from the WorkerPrivate...
       MutexAutoLock lock(proxy->Lock());
       if (proxy->CleanedUp()) {
         NS_WARNING("Aborting Fetch because worker already shut down");
         return NS_OK;
       }
@@ -623,33 +549,31 @@ public:
   }
 
   bool
   WorkerRun(JSContext* aCx, WorkerPrivate* aWorkerPrivate) override
   {
     MOZ_ASSERT(aWorkerPrivate);
     aWorkerPrivate->AssertIsOnWorkerThread();
 
-    RefPtr<Promise> promise = mResolver->WorkerPromise(aWorkerPrivate);
-    RefPtr<FetchObserver> fetchObserver =
-      mResolver->GetFetchObserver(aWorkerPrivate);
+    RefPtr<Promise> promise = mResolver->mPromiseProxy->WorkerPromise();
 
     if (mInternalResponse->Type() != ResponseType::Error) {
-      if (fetchObserver) {
-        fetchObserver->SetState(FetchState::Complete);
+      if (mResolver->mFetchObserver) {
+        mResolver->mFetchObserver->SetState(FetchState::Complete);
       }
 
       RefPtr<nsIGlobalObject> global = aWorkerPrivate->GlobalScope();
       RefPtr<Response> response =
         new Response(global, mInternalResponse,
                      mResolver->GetAbortSignalForTargetThread());
       promise->MaybeResolve(response);
     } else {
-      if (fetchObserver) {
-        fetchObserver->SetState(FetchState::Errored);
+      if (mResolver->mFetchObserver) {
+        mResolver->mFetchObserver->SetState(FetchState::Errored);
       }
 
       ErrorResult result;
       result.ThrowTypeError<MSG_FETCH_FAILED>();
       promise->MaybeReject(result);
     }
     return true;
   }
@@ -667,22 +591,19 @@ public:
   }
 
   bool
   WorkerRun(JSContext* aCx, WorkerPrivate* aWorkerPrivate) override
   {
     MOZ_ASSERT(aWorkerPrivate);
     aWorkerPrivate->AssertIsOnWorkerThread();
 
-    RefPtr<FetchObserver> fetchObserver =
-      mResolver->GetFetchObserver(aWorkerPrivate);
-
-    if (fetchObserver &&
-        fetchObserver->State() == FetchState::Requesting) {
-      fetchObserver->SetState(FetchState::Responding);
+    if (mResolver->mFetchObserver &&
+        mResolver->mFetchObserver->State() == FetchState::Requesting) {
+      mResolver->mFetchObserver->SetState(FetchState::Responding);
     }
 
     return true;
   }
 };
 
 class WorkerFetchResponseEndBase
 {
@@ -694,17 +615,27 @@ public:
     : mResolver(aResolver)
   {
     MOZ_ASSERT(aResolver);
   }
 
   void
   WorkerRunInternal(WorkerPrivate* aWorkerPrivate)
   {
-    mResolver->Shutdown(aWorkerPrivate);
+    MOZ_ASSERT(aWorkerPrivate);
+    aWorkerPrivate->AssertIsOnWorkerThread();
+
+    mResolver->mPromiseProxy->CleanUp();
+
+    mResolver->mFetchObserver = nullptr;
+
+    if (mResolver->mSignalProxy) {
+      mResolver->mSignalProxy->Shutdown();
+      mResolver->mSignalProxy = nullptr;
+    }
   }
 };
 
 class WorkerFetchResponseEndRunnable final : public MainThreadWorkerRunnable
                                            , public WorkerFetchResponseEndBase
 {
   FetchDriverObserver::EndReason mReason;
 
@@ -717,17 +648,18 @@ public:
     , mReason(aReason)
   {
   }
 
   bool
   WorkerRun(JSContext* aCx, WorkerPrivate* aWorkerPrivate) override
   {
     if (mReason == FetchDriverObserver::eAborted) {
-      mResolver->WorkerPromise(aWorkerPrivate)->MaybeReject(NS_ERROR_DOM_ABORT_ERR);
+      RefPtr<Promise> promise = mResolver->mPromiseProxy->WorkerPromise();
+      promise->MaybeReject(NS_ERROR_DOM_ABORT_ERR);
     }
 
     WorkerRunInternal(aWorkerPrivate);
     return true;
   }
 
   nsresult
   Cancel() override
@@ -755,28 +687,16 @@ public:
   {
     WorkerRunInternal(aWorkerPrivate);
     return true;
   }
 
   // Control runnable cancel already calls Run().
 };
 
-bool
-WorkerNotifier::Notify(Status aStatus)
-{
-  if (mResolver) {
-    // This will nullify this object.
-    // No additional operation after this line!
-    mResolver->Shutdown(mWorkerPrivate);
-  }
-
-  return true;
-}
-
 void
 WorkerFetchResolver::OnResponseAvailableInternal(InternalResponse* aResponse)
 {
   AssertIsOnMainThread();
 
   MutexAutoLock lock(mPromiseProxy->Lock());
   if (mPromiseProxy->CleanedUp()) {
     return;