Bug 1455695 P1 Make ServiceWorkerRegistration::Inner::Unregister() use MozPromise. r=baku
authorBen Kelly <ben@wanderview.com>
Mon, 30 Apr 2018 07:55:01 -0700
changeset 416379 cd48fe9eb3dd3cc05b5662e9a5d861846ae47f89
parent 416378 c9a94881c7b4ec7328553c82995bb74c3dbbb641
child 416380 8f25c38c4fd68b8b424e020eaeba1b7c780c790b
push id33926
push userapavel@mozilla.com
push dateTue, 01 May 2018 10:13:46 +0000
treeherdermozilla-central@d2a4720d1c33 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbaku
bugs1455695
milestone61.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 1455695 P1 Make ServiceWorkerRegistration::Inner::Unregister() use MozPromise. r=baku
dom/serviceworkers/ServiceWorkerRegistration.cpp
dom/serviceworkers/ServiceWorkerRegistration.h
dom/serviceworkers/ServiceWorkerRegistrationImpl.cpp
dom/serviceworkers/ServiceWorkerRegistrationImpl.h
--- a/dom/serviceworkers/ServiceWorkerRegistration.cpp
+++ b/dom/serviceworkers/ServiceWorkerRegistration.cpp
@@ -238,17 +238,42 @@ ServiceWorkerRegistration::Update(ErrorR
 
 already_AddRefed<Promise>
 ServiceWorkerRegistration::Unregister(ErrorResult& aRv)
 {
   if (!mInner) {
     aRv.Throw(NS_ERROR_DOM_INVALID_STATE_ERR);
     return nullptr;
   }
-  return mInner->Unregister(aRv);
+
+  nsIGlobalObject* global = GetParentObject();
+  if (!global) {
+    aRv.Throw(NS_ERROR_DOM_INVALID_STATE_ERR);
+    return nullptr;
+  }
+
+  RefPtr<Promise> outer = Promise::Create(global, aRv);
+  if (NS_WARN_IF(aRv.Failed())) {
+    return nullptr;
+  }
+
+  RefPtr<DOMMozPromiseRequestHolder<GenericPromise>> holder =
+    new DOMMozPromiseRequestHolder<GenericPromise>(global);
+
+  mInner->Unregister()->Then(
+    global->EventTargetFor(TaskCategory::Other), __func__,
+    [outer, holder] (bool aSuccess) {
+      holder->Complete();
+      outer->MaybeResolve(aSuccess);
+    }, [outer, holder] (nsresult aRv) {
+      holder->Complete();
+      outer->MaybeReject(aRv);
+    })->Track(*holder);
+
+  return outer.forget();
 }
 
 already_AddRefed<PushManager>
 ServiceWorkerRegistration::GetPushManager(JSContext* aCx, ErrorResult& aRv)
 {
   if (!mInner) {
     aRv.Throw(NS_ERROR_DOM_INVALID_STATE_ERR);
     return nullptr;
--- a/dom/serviceworkers/ServiceWorkerRegistration.h
+++ b/dom/serviceworkers/ServiceWorkerRegistration.h
@@ -42,18 +42,18 @@ public:
     SetServiceWorkerRegistration(ServiceWorkerRegistration* aReg) = 0;
 
     virtual void
     ClearServiceWorkerRegistration(ServiceWorkerRegistration* aReg) = 0;
 
     virtual RefPtr<ServiceWorkerRegistrationPromise>
     Update(ErrorResult& aRv) = 0;
 
-    virtual already_AddRefed<Promise>
-    Unregister(ErrorResult& aRv) = 0;
+    virtual RefPtr<GenericPromise>
+    Unregister() = 0;
 
     virtual already_AddRefed<Promise>
     ShowNotification(JSContext* aCx,
                      const nsAString& aTitle,
                      const NotificationOptions& aOptions,
                      ErrorResult& aRv) = 0;
 
     virtual already_AddRefed<Promise>
--- a/dom/serviceworkers/ServiceWorkerRegistrationImpl.cpp
+++ b/dom/serviceworkers/ServiceWorkerRegistrationImpl.cpp
@@ -5,17 +5,16 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "ServiceWorkerRegistrationImpl.h"
 
 #include "ipc/ErrorIPCUtils.h"
 #include "mozilla/dom/DOMPrefs.h"
 #include "mozilla/dom/Notification.h"
 #include "mozilla/dom/Promise.h"
-#include "mozilla/dom/PromiseWindowProxy.h"
 #include "mozilla/dom/PromiseWorkerProxy.h"
 #include "mozilla/dom/PushManagerBinding.h"
 #include "mozilla/dom/PushManager.h"
 #include "mozilla/dom/ServiceWorkerRegistrationBinding.h"
 #include "mozilla/dom/WorkerCommon.h"
 #include "mozilla/dom/WorkerPrivate.h"
 #include "mozilla/dom/WorkerRef.h"
 #include "mozilla/dom/WorkerScope.h"
@@ -370,204 +369,164 @@ private:
   const ServiceWorkerDescriptor mDescriptor;
   bool mDelayed;
 };
 
 NS_IMPL_ISUPPORTS(SWRUpdateRunnable::TimerCallback, nsITimerCallback)
 
 class UnregisterCallback final : public nsIServiceWorkerUnregisterCallback
 {
-  PromiseWindowProxy mPromise;
+  RefPtr<GenericPromise::Private> mPromise;
 
 public:
   NS_DECL_ISUPPORTS
 
-  explicit UnregisterCallback(nsPIDOMWindowInner* aWindow,
-                              Promise* aPromise)
-    : mPromise(aWindow, aPromise)
+  UnregisterCallback()
+    : mPromise(new GenericPromise::Private(__func__))
   {
-    MOZ_ASSERT(aPromise);
   }
 
   NS_IMETHOD
   UnregisterSucceeded(bool aState) override
   {
-    MOZ_ASSERT(NS_IsMainThread());
-    RefPtr<Promise> promise = mPromise.Get();
-    nsCOMPtr<nsPIDOMWindowInner> win = mPromise.GetWindow();
-    if (!promise || !win) {
-      return NS_OK;
-    }
-
-    nsCOMPtr<nsIRunnable> r = NS_NewRunnableFunction(
-      "UnregisterCallback::UnregisterSucceeded",
-      [promise = Move(promise), aState] () {
-        promise->MaybeResolve(aState);
-      });
-    MOZ_ALWAYS_SUCCEEDS(
-      win->EventTargetFor(TaskCategory::Other)->Dispatch(r.forget()));
+    mPromise->Resolve(aState, __func__);
     return NS_OK;
   }
 
   NS_IMETHOD
   UnregisterFailed() override
   {
-    MOZ_ASSERT(NS_IsMainThread());
-
-    if (RefPtr<Promise> promise = mPromise.Get()) {
-      promise->MaybeReject(NS_ERROR_DOM_SECURITY_ERR);
-    }
+    mPromise->Reject(NS_ERROR_DOM_SECURITY_ERR, __func__);
     return NS_OK;
   }
 
+  RefPtr<GenericPromise>
+  Promise() const
+  {
+    return mPromise;
+  }
+
 private:
-  ~UnregisterCallback()
-  { }
+  ~UnregisterCallback() = default;
 };
 
 NS_IMPL_ISUPPORTS(UnregisterCallback, nsIServiceWorkerUnregisterCallback)
 
-class FulfillUnregisterPromiseRunnable final : public WorkerRunnable
-{
-  RefPtr<PromiseWorkerProxy> mPromiseWorkerProxy;
-  Maybe<bool> mState;
-public:
-  FulfillUnregisterPromiseRunnable(PromiseWorkerProxy* aProxy,
-                                   const Maybe<bool>& aState)
-    : WorkerRunnable(aProxy->GetWorkerPrivate())
-    , mPromiseWorkerProxy(aProxy)
-    , mState(aState)
-  {
-    MOZ_ASSERT(NS_IsMainThread());
-    MOZ_ASSERT(mPromiseWorkerProxy);
-  }
-
-  bool
-  WorkerRun(JSContext* aCx, WorkerPrivate* aWorkerPrivate) override
-  {
-    RefPtr<Promise> promise = mPromiseWorkerProxy->WorkerPromise();
-    if (mState.isSome()) {
-      promise->MaybeResolve(mState.value());
-    } else {
-      promise->MaybeReject(NS_ERROR_DOM_SECURITY_ERR);
-    }
-
-    mPromiseWorkerProxy->CleanUp();
-    return true;
-  }
-};
-
 class WorkerUnregisterCallback final : public nsIServiceWorkerUnregisterCallback
 {
-  RefPtr<PromiseWorkerProxy> mPromiseWorkerProxy;
+  RefPtr<ThreadSafeWorkerRef> mWorkerRef;
+  RefPtr<GenericPromise::Private> mPromise;
 public:
   NS_DECL_ISUPPORTS
 
-  explicit WorkerUnregisterCallback(PromiseWorkerProxy* aProxy)
-    : mPromiseWorkerProxy(aProxy)
+  WorkerUnregisterCallback(RefPtr<ThreadSafeWorkerRef>&& aWorkerRef,
+                           RefPtr<GenericPromise::Private>&& aPromise)
+    : mWorkerRef(Move(aWorkerRef))
+    , mPromise(Move(aPromise))
   {
-    MOZ_ASSERT(aProxy);
+    MOZ_DIAGNOSTIC_ASSERT(mWorkerRef);
+    MOZ_DIAGNOSTIC_ASSERT(mPromise);
   }
 
   NS_IMETHOD
   UnregisterSucceeded(bool aState) override
   {
-    MOZ_ASSERT(NS_IsMainThread());
-    Finish(Some(aState));
+    mPromise->Resolve(aState, __func__);
+    mWorkerRef = nullptr;
     return NS_OK;
   }
 
   NS_IMETHOD
   UnregisterFailed() override
   {
-    MOZ_ASSERT(NS_IsMainThread());
-    Finish(Nothing());
+    mPromise->Reject(NS_ERROR_DOM_SECURITY_ERR, __func__);
+    mWorkerRef = nullptr;
     return NS_OK;
   }
 
 private:
-  ~WorkerUnregisterCallback()
-  {}
-
-  void
-  Finish(const Maybe<bool>& aState)
-  {
-    MOZ_ASSERT(NS_IsMainThread());
-    if (!mPromiseWorkerProxy) {
-      return;
-    }
-
-    RefPtr<PromiseWorkerProxy> proxy = mPromiseWorkerProxy.forget();
-    MutexAutoLock lock(proxy->Lock());
-    if (proxy->CleanedUp()) {
-      return;
-    }
-
-    RefPtr<WorkerRunnable> r =
-      new FulfillUnregisterPromiseRunnable(proxy, aState);
-
-    r->Dispatch();
-  }
+  ~WorkerUnregisterCallback() = default;
 };
 
 NS_IMPL_ISUPPORTS(WorkerUnregisterCallback, nsIServiceWorkerUnregisterCallback);
 
 /*
  * If the worker goes away, we still continue to unregister, but we don't try to
  * resolve the worker Promise (which doesn't exist by that point).
  */
 class StartUnregisterRunnable final : public Runnable
 {
-  RefPtr<PromiseWorkerProxy> mPromiseWorkerProxy;
-  const nsString mScope;
+  // The promise is protected by the mutex.
+  Mutex mMutex;
+
+  RefPtr<ThreadSafeWorkerRef> mWorkerRef;
+  RefPtr<GenericPromise::Private> mPromise;
+  const ServiceWorkerRegistrationDescriptor mDescriptor;
+
+  ~StartUnregisterRunnable()
+  {
+    MutexAutoLock lock(mMutex);
+    if (mPromise) {
+      mPromise->Reject(NS_ERROR_DOM_INVALID_STATE_ERR, __func__);
+    }
+  }
 
 public:
-  StartUnregisterRunnable(PromiseWorkerProxy* aProxy, const nsAString& aScope)
+  StartUnregisterRunnable(StrongWorkerRef* aWorkerRef,
+                          GenericPromise::Private* aPromise,
+                          const ServiceWorkerRegistrationDescriptor& aDescriptor)
     : Runnable("dom::StartUnregisterRunnable")
-    , mPromiseWorkerProxy(aProxy)
-    , mScope(aScope)
+    , mMutex("StartUnregisterRunnable")
+    , mWorkerRef(new ThreadSafeWorkerRef(aWorkerRef))
+    , mPromise(aPromise)
+    , mDescriptor(aDescriptor)
   {
-    MOZ_ASSERT(aProxy);
+    MOZ_DIAGNOSTIC_ASSERT(mWorkerRef);
+    MOZ_DIAGNOSTIC_ASSERT(mPromise);
   }
 
   NS_IMETHOD
   Run() override
   {
     MOZ_ASSERT(NS_IsMainThread());
 
-    // XXXnsm: There is a rare chance of this failing if the worker gets
-    // destroyed. In that case, unregister() called from a SW is no longer
-    // guaranteed to run. We should fix this by having a main thread proxy
-    // maintain a strongref to ServiceWorkerRegistrationInfo and use its
-    // principal. Can that be trusted?
-    nsCOMPtr<nsIPrincipal> principal;
+    nsCOMPtr<nsIPrincipal> principal = mDescriptor.GetPrincipal();
+    if (!principal) {
+      mPromise->Reject(NS_ERROR_DOM_INVALID_STATE_ERR, __func__);
+      return NS_OK;
+    }
+
+    nsCOMPtr<nsIServiceWorkerManager> swm =
+      mozilla::services::GetServiceWorkerManager();
+    if (!swm) {
+      mPromise->Reject(NS_ERROR_DOM_INVALID_STATE_ERR, __func__);
+      return NS_OK;
+    }
+
+    RefPtr<GenericPromise::Private> promise;
     {
-      MutexAutoLock lock(mPromiseWorkerProxy->Lock());
-      if (mPromiseWorkerProxy->CleanedUp()) {
-        return NS_OK;
-      }
-
-      WorkerPrivate* worker = mPromiseWorkerProxy->GetWorkerPrivate();
-      MOZ_ASSERT(worker);
-      principal = worker->GetPrincipal();
+      MutexAutoLock lock(mMutex);
+      promise = mPromise.forget();
     }
-    MOZ_ASSERT(principal);
 
     RefPtr<WorkerUnregisterCallback> cb =
-      new WorkerUnregisterCallback(mPromiseWorkerProxy);
-    nsCOMPtr<nsIServiceWorkerManager> swm =
-      mozilla::services::GetServiceWorkerManager();
-    nsresult rv = swm->Unregister(principal, cb, mScope);
+      new WorkerUnregisterCallback(Move(mWorkerRef), Move(promise));
+
+    nsresult rv = swm->Unregister(principal,
+                                  cb,
+                                  NS_ConvertUTF8toUTF16(mDescriptor.Scope()));
     if (NS_WARN_IF(NS_FAILED(rv))) {
-      cb->UnregisterFailed();
+      mPromise->Reject(rv, __func__);
+      return NS_OK;
     }
 
     return NS_OK;
   }
 };
+
 } // namespace
 
 RefPtr<ServiceWorkerRegistrationPromise>
 ServiceWorkerRegistrationMainThread::Update(ErrorResult& aRv)
 {
   MOZ_ASSERT(NS_IsMainThread());
   MOZ_DIAGNOSTIC_ASSERT(mOuter);
 
@@ -578,79 +537,44 @@ ServiceWorkerRegistrationMainThread::Upd
   }
 
   RefPtr<MainThreadUpdateCallback> cb = new MainThreadUpdateCallback();
   UpdateInternal(principal, NS_ConvertUTF16toUTF8(mScope), cb);
 
   return cb->Promise();
 }
 
-already_AddRefed<Promise>
-ServiceWorkerRegistrationMainThread::Unregister(ErrorResult& aRv)
+RefPtr<GenericPromise>
+ServiceWorkerRegistrationMainThread::Unregister()
 {
   MOZ_ASSERT(NS_IsMainThread());
   MOZ_DIAGNOSTIC_ASSERT(mOuter);
 
-  nsCOMPtr<nsIGlobalObject> go = mOuter->GetParentObject();
-  if (!go) {
-    aRv.Throw(NS_ERROR_FAILURE);
-    return nullptr;
-  }
-
-  // Although the spec says that the same-origin checks should also be done
-  // asynchronously, we do them in sync because the Promise created by the
-  // WebIDL infrastructure due to a returned error will be resolved
-  // asynchronously. We aren't making any internal state changes in these
-  // checks, so ordering of multiple calls is not affected.
-  nsCOMPtr<nsIDocument> document = mOuter->GetOwner()->GetExtantDoc();
-  if (!document) {
-    aRv.Throw(NS_ERROR_FAILURE);
-    return nullptr;
-  }
-
-  nsCOMPtr<nsIURI> scopeURI;
-  nsCOMPtr<nsIURI> baseURI = document->GetBaseURI();
-  // "If the origin of scope is not client's origin..."
-  nsresult rv = NS_NewURI(getter_AddRefs(scopeURI), mScope, nullptr, baseURI);
-  if (NS_WARN_IF(NS_FAILED(rv))) {
-    aRv.Throw(NS_ERROR_DOM_SECURITY_ERR);
-    return nullptr;
+  nsCOMPtr<nsIServiceWorkerManager> swm =
+    mozilla::services::GetServiceWorkerManager();
+  if (!swm) {
+    return GenericPromise::CreateAndReject(NS_ERROR_DOM_INVALID_STATE_ERR,
+                                           __func__);
   }
 
-  nsCOMPtr<nsIPrincipal> documentPrincipal = document->NodePrincipal();
-  rv = documentPrincipal->CheckMayLoad(scopeURI, true /* report */,
-                                       false /* allowIfInheritsPrinciple */);
-  if (NS_FAILED(rv)) {
-    aRv.Throw(NS_ERROR_DOM_SECURITY_ERR);
-    return nullptr;
-  }
-
-  nsAutoCString uriSpec;
-  aRv = scopeURI->GetSpecIgnoringRef(uriSpec);
-  if (NS_WARN_IF(aRv.Failed())) {
-    return nullptr;
+  nsCOMPtr<nsIPrincipal> principal = mDescriptor.GetPrincipal();
+  if (!principal) {
+    return GenericPromise::CreateAndReject(NS_ERROR_DOM_INVALID_STATE_ERR,
+                                           __func__);
   }
 
-  nsCOMPtr<nsIServiceWorkerManager> swm =
-    mozilla::services::GetServiceWorkerManager();
+  RefPtr<UnregisterCallback> cb = new UnregisterCallback();
 
-  RefPtr<Promise> promise = Promise::Create(go, aRv);
-  if (NS_WARN_IF(aRv.Failed())) {
-    return nullptr;
+  nsresult rv = swm->Unregister(principal, cb,
+                                NS_ConvertUTF8toUTF16(mDescriptor.Scope()));
+  if (NS_FAILED(rv)) {
+    return GenericPromise::CreateAndReject(rv, __func__);
   }
 
-  RefPtr<UnregisterCallback> cb = new UnregisterCallback(mOuter->GetOwner(), promise);
-
-  NS_ConvertUTF8toUTF16 scope(uriSpec);
-  aRv = swm->Unregister(documentPrincipal, cb, scope);
-  if (aRv.Failed()) {
-    return nullptr;
-  }
-
-  return promise.forget();
+  return cb->Promise();
 }
 
 // Notification API extension.
 already_AddRefed<Promise>
 ServiceWorkerRegistrationMainThread::ShowNotification(JSContext* aCx,
                                                       const nsAString& aTitle,
                                                       const NotificationOptions& aOptions,
                                                       ErrorResult& aRv)
@@ -907,46 +831,50 @@ ServiceWorkerRegistrationWorkerThread::U
   if (NS_FAILED(rv)) {
     outer->Reject(NS_ERROR_DOM_INVALID_STATE_ERR, __func__);
     return outer.forget();
   }
 
   return outer.forget();
 }
 
-already_AddRefed<Promise>
-ServiceWorkerRegistrationWorkerThread::Unregister(ErrorResult& aRv)
+RefPtr<GenericPromise>
+ServiceWorkerRegistrationWorkerThread::Unregister()
 {
-  WorkerPrivate* worker = GetCurrentThreadWorkerPrivate();
-  MOZ_ASSERT(worker);
-  worker->AssertIsOnWorkerThread();
+  if (NS_WARN_IF(!mWorkerRef->GetPrivate())) {
+    return GenericPromise::CreateAndReject(
+      NS_ERROR_DOM_INVALID_STATE_ERR, __func__);
+  }
 
-  if (!worker->IsServiceWorker()) {
-    // For other workers, the registration probably originated from
-    // getRegistration(), so we may have to validate origin etc. Let's do this
-    // this later.
-    aRv.Throw(NS_ERROR_DOM_SECURITY_ERR);
-    return nullptr;
+  RefPtr<StrongWorkerRef> workerRef =
+    StrongWorkerRef::Create(mWorkerRef->GetPrivate(), __func__);
+  if (NS_WARN_IF(!workerRef)) {
+    return GenericPromise::CreateAndReject(
+      NS_ERROR_DOM_INVALID_STATE_ERR, __func__);
   }
 
-  RefPtr<Promise> promise = Promise::Create(worker->GlobalScope(), aRv);
-  if (aRv.Failed()) {
-    return nullptr;
+  // Eventually we need to support all workers, but for right now this
+  // code assumes we're on a service worker global as self.registration.
+  if (NS_WARN_IF(!workerRef->Private()->IsServiceWorker())) {
+    return GenericPromise::CreateAndReject(
+      NS_ERROR_DOM_SECURITY_ERR, __func__);
   }
 
-  RefPtr<PromiseWorkerProxy> proxy = PromiseWorkerProxy::Create(worker, promise);
-  if (!proxy) {
-    aRv.Throw(NS_ERROR_DOM_ABORT_ERR);
-    return nullptr;
+  RefPtr<GenericPromise::Private> outer = new GenericPromise::Private(__func__);
+
+  RefPtr<StartUnregisterRunnable> r =
+    new StartUnregisterRunnable(workerRef, outer, mDescriptor);
+
+  nsresult rv = workerRef->Private()->DispatchToMainThread(r);
+  if (NS_FAILED(rv)) {
+    outer->Reject(NS_ERROR_DOM_INVALID_STATE_ERR, __func__);
+    return outer.forget();
   }
 
-  RefPtr<StartUnregisterRunnable> r = new StartUnregisterRunnable(proxy, mScope);
-  MOZ_ALWAYS_SUCCEEDS(worker->DispatchToMainThread(r.forget()));
-
-  return promise.forget();
+  return outer.forget();
 }
 
 void
 ServiceWorkerRegistrationWorkerThread::InitListener()
 {
   MOZ_ASSERT(!mListener);
   WorkerPrivate* worker = GetCurrentThreadWorkerPrivate();
   MOZ_ASSERT(worker);
--- a/dom/serviceworkers/ServiceWorkerRegistrationImpl.h
+++ b/dom/serviceworkers/ServiceWorkerRegistrationImpl.h
@@ -40,18 +40,18 @@ public:
   SetServiceWorkerRegistration(ServiceWorkerRegistration* aReg) override;
 
   void
   ClearServiceWorkerRegistration(ServiceWorkerRegistration* aReg) override;
 
   RefPtr<ServiceWorkerRegistrationPromise>
   Update(ErrorResult& aRv) override;
 
-  already_AddRefed<Promise>
-  Unregister(ErrorResult& aRv) override;
+  RefPtr<GenericPromise>
+  Unregister() override;
 
   already_AddRefed<Promise>
   ShowNotification(JSContext* aCx,
                    const nsAString& aTitle,
                    const NotificationOptions& aOptions,
                    ErrorResult& aRv) override;
 
   already_AddRefed<Promise>
@@ -120,18 +120,18 @@ public:
   SetServiceWorkerRegistration(ServiceWorkerRegistration* aReg) override;
 
   void
   ClearServiceWorkerRegistration(ServiceWorkerRegistration* aReg) override;
 
   RefPtr<ServiceWorkerRegistrationPromise>
   Update(ErrorResult& aRv) override;
 
-  already_AddRefed<Promise>
-  Unregister(ErrorResult& aRv) override;
+  RefPtr<GenericPromise>
+  Unregister() override;
 
   already_AddRefed<Promise>
   ShowNotification(JSContext* aCx,
                    const nsAString& aTitle,
                    const NotificationOptions& aOptions,
                    ErrorResult& aRv) override;
 
   already_AddRefed<Promise>