Backed out 3 changesets (bug 1456466) for mochitest failures in /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/MozPromise.h on a CLOSED TREE

Backed out changeset fd0feb0058fc (bug 1456466)
Backed out changeset e3a50afee79e (bug 1456466)
Backed out changeset 89ce7df05344 (bug 1456466)

--HG--
extra : rebase_source : b3f92fad7452a251948b88e6bf561acda162072b

dom/serviceworkers/ServiceWorkerDescriptor.cpp

@@ -12,9 +12,6 @@
 namespace mozilla {
 namespace dom {
 
-using mozilla::ipc::PrincipalInfo;
-using mozilla::ipc::PrincipalInfoToPrincipal;
-
 ServiceWorkerDescriptor::ServiceWorkerDescriptor(uint64_t aId,
                                                  nsIPrincipal* aPrincipal,
                                                  const nsACString& aScope,
@@ -98,14 +95,6 @@ ServiceWorkerDescriptor::PrincipalInfo() const
   return mData->principalInfo();
 }
 
-nsCOMPtr<nsIPrincipal>
-ServiceWorkerDescriptor::GetPrincipal() const
-{
-  AssertIsOnMainThread();
-  nsCOMPtr<nsIPrincipal> ref =  PrincipalInfoToPrincipal(mData->principalInfo());
-  return Move(ref);
-}
-
 const nsCString&
 ServiceWorkerDescriptor::Scope() const
 {

dom/serviceworkers/ServiceWorkerDescriptor.h

@@ -7,7 +7,6 @@
 #define _mozilla_dom_ServiceWorkerDescriptor_h
 
 #include "mozilla/UniquePtr.h"
-#include "nsCOMPtr.h"
 #include "nsString.h"
 
 class nsIPrincipal;
@@ -70,9 +69,6 @@ public:
   const mozilla::ipc::PrincipalInfo&
   PrincipalInfo() const;
 
-  nsCOMPtr<nsIPrincipal>
-  GetPrincipal() const;
-
   const nsCString&
   Scope() const;
 

dom/serviceworkers/ServiceWorkerRegistration.cpp

@@ -198,40 +198,7 @@ ServiceWorkerRegistration::Update(ErrorResult& aRv)
     aRv.Throw(NS_ERROR_DOM_INVALID_STATE_ERR);
     return nullptr;
   }
-
-  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<ServiceWorkerRegistration> self = this;
-
-  mInner->Update(aRv)->Then(
-    global->EventTargetFor(TaskCategory::Other), __func__,
-    [outer, self](const ServiceWorkerRegistrationDescriptor& aDesc) {
-      nsIGlobalObject* global = self->GetParentObject();
-      if (!global) {
-        outer->MaybeReject(NS_ERROR_DOM_INVALID_STATE_ERR);
-        return;
-      }
-      RefPtr<ServiceWorkerRegistration> ref =
-        global->GetOrCreateServiceWorkerRegistration(aDesc);
-      if (!ref) {
-        outer->MaybeReject(NS_ERROR_DOM_INVALID_STATE_ERR);
-        return;
-      }
-      outer->MaybeResolve(ref);
-    }, [outer] (ErrorResult&& aRv) {
-      outer->MaybeReject(aRv);
-    });
-
-  return outer.forget();
+  return mInner->Update(aRv);
 }
 
 already_AddRefed<Promise>

dom/serviceworkers/ServiceWorkerRegistration.h

@@ -12,7 +12,6 @@
 #include "mozilla/dom/ServiceWorkerBinding.h"
 #include "mozilla/dom/ServiceWorkerRegistrationBinding.h"
 #include "mozilla/dom/ServiceWorkerRegistrationDescriptor.h"
-#include "mozilla/dom/ServiceWorkerUtils.h"
 
 // Support for Notification API extension.
 #include "mozilla/dom/NotificationBinding.h"
@@ -44,7 +43,7 @@ public:
     virtual void
     ClearServiceWorkerRegistration(ServiceWorkerRegistration* aReg) = 0;
 
-    virtual RefPtr<ServiceWorkerRegistrationPromise>
+    virtual already_AddRefed<Promise>
     Update(ErrorResult& aRv) = 0;
 
     virtual already_AddRefed<Promise>

dom/serviceworkers/ServiceWorkerRegistrationDescriptor.cpp

@@ -13,9 +13,6 @@
 namespace mozilla {
 namespace dom {
 
-using mozilla::ipc::PrincipalInfo;
-using mozilla::ipc::PrincipalInfoToPrincipal;
-
 Maybe<IPCServiceWorkerDescriptor>
 ServiceWorkerRegistrationDescriptor::NewestInternal() const
 {
@@ -133,14 +130,6 @@ ServiceWorkerRegistrationDescriptor::PrincipalInfo() const
   return mData->principalInfo();
 }
 
-nsCOMPtr<nsIPrincipal>
-ServiceWorkerRegistrationDescriptor::GetPrincipal() const
-{
-  AssertIsOnMainThread();
-  nsCOMPtr<nsIPrincipal> ref =  PrincipalInfoToPrincipal(mData->principalInfo());
-  return Move(ref);
-}
-
 const nsCString&
 ServiceWorkerRegistrationDescriptor::Scope() const
 {

dom/serviceworkers/ServiceWorkerRegistrationDescriptor.h

@@ -73,9 +73,6 @@ public:
   const mozilla::ipc::PrincipalInfo&
   PrincipalInfo() const;
 
-  nsCOMPtr<nsIPrincipal>
-  GetPrincipal() const;
-
   const nsCString&
   Scope() const;
 

dom/serviceworkers/ServiceWorkerRegistrationImpl.cpp

@@ -43,7 +43,6 @@ namespace dom {
 
 ServiceWorkerRegistrationMainThread::ServiceWorkerRegistrationMainThread(const ServiceWorkerRegistrationDescriptor& aDescriptor)
   : mOuter(nullptr)
-  , mDescriptor(aDescriptor)
   , mScope(NS_ConvertUTF8toUTF16(aDescriptor.Scope()))
   , mListeningForEvents(false)
 {
@@ -107,7 +106,6 @@ ServiceWorkerRegistrationMainThread::UpdateFound()
 void
 ServiceWorkerRegistrationMainThread::UpdateState(const ServiceWorkerRegistrationDescriptor& aDescriptor)
 {
-  mDescriptor = aDescriptor;
   mOuter->UpdateState(aDescriptor);
 }
 
@@ -170,47 +168,15 @@ UpdateInternal(nsIPrincipal* aPrincipal,
 
 class MainThreadUpdateCallback final : public ServiceWorkerUpdateFinishCallback
 {
-  RefPtr<ServiceWorkerRegistrationPromise::Private> mPromise;
+  PromiseWindowProxy mPromise;
 
   ~MainThreadUpdateCallback()
   { }
 
 public:
-  MainThreadUpdateCallback()
-    : mPromise(new ServiceWorkerRegistrationPromise::Private(__func__))
-  {
-  }
-
-  void
-  UpdateSucceeded(ServiceWorkerRegistrationInfo* aRegistration) override
-  {
-    mPromise->Resolve(aRegistration->Descriptor(), __func__);
-  }
-
-  void
-  UpdateFailed(ErrorResult& aStatus) override
-  {
-    mPromise->Reject(Move(aStatus), __func__);
-  }
-
-  RefPtr<ServiceWorkerRegistrationPromise>
-  Promise() const
-  {
-    return mPromise;
-  }
-};
-
-class WorkerThreadUpdateCallback final : public ServiceWorkerUpdateFinishCallback
-{
-  RefPtr<ServiceWorkerRegistrationPromise::Private> mPromise;
-
-  ~WorkerThreadUpdateCallback()
-  {
-  }
-
-public:
-  explicit WorkerThreadUpdateCallback(ServiceWorkerRegistrationPromise::Private* aPromise)
-    : mPromise(aPromise)
+  explicit MainThreadUpdateCallback(nsPIDOMWindowInner* aWindow,
+                                    Promise* aPromise)
+    : mPromise(aWindow, aPromise)
   {
     MOZ_ASSERT(NS_IsMainThread());
   }
@@ -218,13 +184,115 @@ public:
   void
   UpdateSucceeded(ServiceWorkerRegistrationInfo* aRegistration) override
   {
-    mPromise->Resolve(aRegistration->Descriptor(), __func__);
+    RefPtr<Promise> promise = mPromise.Get();
+    nsCOMPtr<nsPIDOMWindowInner> win = mPromise.GetWindow();
+    if (!promise || !win) {
+      return;
+    }
+
+    nsCOMPtr<nsIRunnable> r = NS_NewRunnableFunction(
+      "MainThreadUpdateCallback::UpdateSucceeded",
+      [promise = Move(promise)] () {
+        promise->MaybeResolveWithUndefined();
+      });
+    MOZ_ALWAYS_SUCCEEDS(
+      win->EventTargetFor(TaskCategory::Other)->Dispatch(r.forget()));
   }
 
   void
   UpdateFailed(ErrorResult& aStatus) override
   {
-    mPromise->Reject(Move(aStatus), __func__);
+    if (RefPtr<Promise> promise = mPromise.Get()) {
+      promise->MaybeReject(aStatus);
+    }
+  }
+};
+
+class UpdateResultRunnable final : public WorkerRunnable
+{
+  RefPtr<PromiseWorkerProxy> mPromiseProxy;
+  IPC::Message mSerializedErrorResult;
+
+  ~UpdateResultRunnable()
+  {}
+
+public:
+  UpdateResultRunnable(PromiseWorkerProxy* aPromiseProxy, ErrorResult& aStatus)
+    : WorkerRunnable(aPromiseProxy->GetWorkerPrivate())
+    , mPromiseProxy(aPromiseProxy)
+  {
+    // ErrorResult is not thread safe.  Serialize it for transfer across
+    // threads.
+    IPC::WriteParam(&mSerializedErrorResult, aStatus);
+    aStatus.SuppressException();
+  }
+
+  bool
+  WorkerRun(JSContext* aCx, WorkerPrivate* aWorkerPrivate) override
+  {
+    // Deserialize the ErrorResult now that we are back in the worker
+    // thread.
+    ErrorResult status;
+    PickleIterator iter = PickleIterator(mSerializedErrorResult);
+    Unused << IPC::ReadParam(&mSerializedErrorResult, &iter, &status);
+
+    Promise* promise = mPromiseProxy->WorkerPromise();
+    if (status.Failed()) {
+      promise->MaybeReject(status);
+    } else {
+      promise->MaybeResolveWithUndefined();
+    }
+    status.SuppressException();
+    mPromiseProxy->CleanUp();
+    return true;
+  }
+};
+
+class WorkerThreadUpdateCallback final : public ServiceWorkerUpdateFinishCallback
+{
+  RefPtr<PromiseWorkerProxy> mPromiseProxy;
+
+  ~WorkerThreadUpdateCallback()
+  {
+  }
+
+public:
+  explicit WorkerThreadUpdateCallback(PromiseWorkerProxy* aPromiseProxy)
+    : mPromiseProxy(aPromiseProxy)
+  {
+    MOZ_ASSERT(NS_IsMainThread());
+  }
+
+  void
+  UpdateSucceeded(ServiceWorkerRegistrationInfo* aRegistration) override
+  {
+    ErrorResult rv(NS_OK);
+    Finish(rv);
+  }
+
+  void
+  UpdateFailed(ErrorResult& aStatus) override
+  {
+    Finish(aStatus);
+  }
+
+  void
+  Finish(ErrorResult& aStatus)
+  {
+    if (!mPromiseProxy) {
+      return;
+    }
+
+    RefPtr<PromiseWorkerProxy> proxy = mPromiseProxy.forget();
+
+    MutexAutoLock lock(proxy->Lock());
+    if (proxy->CleanedUp()) {
+      return;
+    }
+
+    RefPtr<UpdateResultRunnable> r =
+      new UpdateResultRunnable(proxy, aStatus);
+    r->Dispatch();
   }
 };
 
@@ -262,12 +330,19 @@ class SWRUpdateRunnable final : public Runnable
   };
 
 public:
-  explicit SWRUpdateRunnable(const ServiceWorkerDescriptor& aDescriptor)
+  explicit SWRUpdateRunnable(PromiseWorkerProxy* aPromiseProxy)
     : Runnable("dom::SWRUpdateRunnable")
-    , mPromise(new ServiceWorkerRegistrationPromise::Private(__func__))
-    , mDescriptor(aDescriptor)
+    , mPromiseProxy(aPromiseProxy)
+    , mDescriptor(aPromiseProxy->GetWorkerPrivate()->GetServiceWorkerDescriptor())
     , mDelayed(false)
   {
+    MOZ_ASSERT(mPromiseProxy);
+
+    // This runnable is used for update calls originating from a worker thread,
+    // which may be delayed in some cases.
+    MOZ_ASSERT(mPromiseProxy->GetWorkerPrivate()->IsServiceWorker());
+    MOZ_ASSERT(mPromiseProxy->GetWorkerPrivate());
+    mPromiseProxy->GetWorkerPrivate()->AssertIsOnWorkerThread();
   }
 
   NS_IMETHOD
@@ -276,15 +351,21 @@ public:
     MOZ_ASSERT(NS_IsMainThread());
     ErrorResult result;
 
-    nsCOMPtr<nsIPrincipal> principal = mDescriptor.GetPrincipal();
-    if (NS_WARN_IF(!principal)) {
-      mPromise->Reject(NS_ERROR_DOM_INVALID_STATE_ERR, __func__);
-      return NS_OK;
+    nsCOMPtr<nsIPrincipal> principal;
+    // UpdateInternal may try to reject the promise synchronously leading
+    // to a deadlock.
+    {
+      MutexAutoLock lock(mPromiseProxy->Lock());
+      if (mPromiseProxy->CleanedUp()) {
+        return NS_OK;
+      }
+
+      principal = mPromiseProxy->GetWorkerPrivate()->GetPrincipal();
     }
+    MOZ_ASSERT(principal);
 
     RefPtr<ServiceWorkerManager> swm = ServiceWorkerManager::GetInstance();
     if (NS_WARN_IF(!swm)) {
-      mPromise->Reject(NS_ERROR_DOM_INVALID_STATE_ERR, __func__);
       return NS_OK;
     }
 
@@ -330,22 +411,18 @@ public:
     }
 
     RefPtr<WorkerThreadUpdateCallback> cb =
-      new WorkerThreadUpdateCallback(mPromise);
+      new WorkerThreadUpdateCallback(mPromiseProxy);
     UpdateInternal(principal, mDescriptor.Scope(), cb);
-
     return NS_OK;
   }
 
-  RefPtr<ServiceWorkerRegistrationPromise>
-  Promise() const
+private:
+  ~SWRUpdateRunnable()
   {
-    return mPromise;
+    MOZ_ASSERT(NS_IsMainThread());
   }
 
-private:
-  ~SWRUpdateRunnable() = default;
-
-  RefPtr<ServiceWorkerRegistrationPromise::Private> mPromise;
+  RefPtr<PromiseWorkerProxy> mPromiseProxy;
   const ServiceWorkerDescriptor mDescriptor;
   bool mDelayed;
 };
@@ -544,22 +621,31 @@ public:
 };
 } // namespace
 
-RefPtr<ServiceWorkerRegistrationPromise>
+already_AddRefed<Promise>
 ServiceWorkerRegistrationMainThread::Update(ErrorResult& aRv)
 {
   MOZ_ASSERT(NS_IsMainThread());
   MOZ_DIAGNOSTIC_ASSERT(mOuter);
 
-  nsCOMPtr<nsIPrincipal> principal = mDescriptor.GetPrincipal();
-  if (!principal) {
-    return ServiceWorkerRegistrationPromise::CreateAndReject(
-      NS_ERROR_DOM_INVALID_STATE_ERR, __func__);
+  nsCOMPtr<nsIGlobalObject> go = mOuter->GetParentObject();
+  if (!go) {
+    aRv.Throw(NS_ERROR_FAILURE);
+    return nullptr;
   }
 
-  RefPtr<MainThreadUpdateCallback> cb = new MainThreadUpdateCallback();
-  UpdateInternal(principal, NS_ConvertUTF16toUTF8(mScope), cb);
+  RefPtr<Promise> promise = Promise::Create(go, aRv);
+  if (NS_WARN_IF(aRv.Failed())) {
+    return nullptr;
+  }
 
-  return cb->Promise();
+  nsCOMPtr<nsIDocument> doc = mOuter->GetOwner()->GetExtantDoc();
+  MOZ_ASSERT(doc);
+
+  RefPtr<MainThreadUpdateCallback> cb =
+    new MainThreadUpdateCallback(mOuter->GetOwner(), promise);
+  UpdateInternal(doc->NodePrincipal(), NS_ConvertUTF16toUTF8(mScope), cb);
+
+  return promise.forget();
 }
 
 already_AddRefed<Promise>
@@ -804,7 +890,6 @@ private:
 
 ServiceWorkerRegistrationWorkerThread::ServiceWorkerRegistrationWorkerThread(const ServiceWorkerRegistrationDescriptor& aDescriptor)
   : mOuter(nullptr)
-  , mDescriptor(aDescriptor)
   , mScope(NS_ConvertUTF8toUTF16(aDescriptor.Scope()))
 {
 }
@@ -843,56 +928,36 @@ ServiceWorkerRegistrationWorkerThread::ClearServiceWorkerRegistration(ServiceWor
   mOuter = nullptr;
 }
 
-RefPtr<ServiceWorkerRegistrationPromise>
+already_AddRefed<Promise>
 ServiceWorkerRegistrationWorkerThread::Update(ErrorResult& aRv)
 {
-  if (NS_WARN_IF(!mWorkerRef->GetPrivate())) {
-    return ServiceWorkerRegistrationPromise::CreateAndReject(
-      NS_ERROR_DOM_INVALID_STATE_ERR, __func__);
-  }
+  WorkerPrivate* worker = GetCurrentThreadWorkerPrivate();
+  MOZ_ASSERT(worker);
+  worker->AssertIsOnWorkerThread();
 
-  RefPtr<StrongWorkerRef> workerRef =
-    StrongWorkerRef::Create(mWorkerRef->GetPrivate(),
-                            "ServiceWorkerRegistration::Update");
-  if (NS_WARN_IF(!workerRef)) {
-    return ServiceWorkerRegistrationPromise::CreateAndReject(
-      NS_ERROR_DOM_INVALID_STATE_ERR, __func__);
+  RefPtr<Promise> promise = Promise::Create(worker->GlobalScope(), aRv);
+  if (aRv.Failed()) {
+    return nullptr;
   }
 
   // Avoid infinite update loops by ignoring update() calls during top
   // level script evaluation.  See:
   // https://github.com/slightlyoff/ServiceWorker/issues/800
-  if (workerRef->Private()->LoadScriptAsPartOfLoadingServiceWorkerScript()) {
-    return ServiceWorkerRegistrationPromise::CreateAndResolve(mDescriptor,
-                                                              __func__);
+  if (worker->LoadScriptAsPartOfLoadingServiceWorkerScript()) {
+    promise->MaybeResolveWithUndefined();
+    return promise.forget();
   }
 
-  // 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 ServiceWorkerRegistrationPromise::CreateAndReject(
-      NS_ERROR_DOM_INVALID_STATE_ERR, __func__);
+  RefPtr<PromiseWorkerProxy> proxy = PromiseWorkerProxy::Create(worker, promise);
+  if (!proxy) {
+    aRv.Throw(NS_ERROR_DOM_ABORT_ERR);
+    return nullptr;
   }
 
-  RefPtr<SWRUpdateRunnable> r =
-    new SWRUpdateRunnable(workerRef->Private()->GetServiceWorkerDescriptor());
-  nsresult rv = workerRef->Private()->DispatchToMainThread(r);
-  if (NS_FAILED(rv)) {
-    return ServiceWorkerRegistrationPromise::CreateAndReject(
-      NS_ERROR_DOM_INVALID_STATE_ERR, __func__);
-  }
+  RefPtr<SWRUpdateRunnable> r = new SWRUpdateRunnable(proxy);
+  MOZ_ALWAYS_SUCCEEDS(worker->DispatchToMainThread(r.forget()));
 
-  RefPtr<ServiceWorkerRegistrationPromise::Private> outer =
-    new ServiceWorkerRegistrationPromise::Private(__func__);
-
-  r->Promise()->Then(workerRef->Private()->HybridEventTarget(), __func__,
-    [workerRef, outer] (const ServiceWorkerRegistrationDescriptor& aDesc) {
-      outer->Resolve(aDesc, __func__);
-    }, [workerRef, outer] (ErrorResult&& aRv) {
-      outer->Reject(Move(aRv), __func__);
-    });
-
-  return outer.forget();
+  return promise.forget();
 }
 
 already_AddRefed<Promise>

dom/serviceworkers/ServiceWorkerRegistrationImpl.h

@@ -42,7 +42,7 @@ public:
   void
   ClearServiceWorkerRegistration(ServiceWorkerRegistration* aReg) override;
 
-  RefPtr<ServiceWorkerRegistrationPromise>
+  already_AddRefed<Promise>
   Update(ErrorResult& aRv) override;
 
   already_AddRefed<Promise>
@@ -93,7 +93,6 @@ private:
   RegistrationRemovedInternal();
 
   ServiceWorkerRegistration* mOuter;
-  ServiceWorkerRegistrationDescriptor mDescriptor;
   const nsString mScope;
   bool mListeningForEvents;
 };
@@ -122,7 +121,7 @@ public:
   void
   ClearServiceWorkerRegistration(ServiceWorkerRegistration* aReg) override;
 
-  RefPtr<ServiceWorkerRegistrationPromise>
+  already_AddRefed<Promise>
   Update(ErrorResult& aRv) override;
 
   already_AddRefed<Promise>
@@ -158,7 +157,6 @@ private:
   GetWorkerPrivate(const MutexAutoLock& aProofOfLock);
 
   ServiceWorkerRegistration* mOuter;
-  const ServiceWorkerRegistrationDescriptor mDescriptor;
   const nsString mScope;
   RefPtr<WorkerListener> mListener;
   RefPtr<WeakWorkerRef> mWorkerRef;

dom/serviceworkers/test/worker_update.js

@@ -3,7 +3,7 @@
 // test actual update scenarios with a SJS test.
 onmessage = function(e) {
   self.registration.update().then(function(v) {
-    return v instanceof ServiceWorkerRegistration ? 'FINISH' : 'FAIL';
+    return v === undefined ? 'FINISH' : 'FAIL';
   }).catch(function(e) {
     return 'FAIL';
   }).then(function(result) {

testing/web-platform/meta/MANIFEST.json

@@ -364339,12 +364339,6 @@
      {}
     ]
    ],
-   "service-workers/service-worker/update-result.https.html": [
-    [
-     "/service-workers/service-worker/update-result.https.html",
-     {}
-    ]
-   ],
    "service-workers/service-worker/update.https.html": [
     [
      "/service-workers/service-worker/update.https.html",
@@ -599326,10 +599320,6 @@
    "aac5705d6844e4a33200418504adb57053a45be2",
    "testharness"
   ],
-  "service-workers/service-worker/update-result.https.html": [
-   "06741e887be9746d7354394f74c054dd920d1b60",
-   "testharness"
-  ],
   "service-workers/service-worker/update.https.html": [
    "d55da98b05b5885084474ebdbabdf6c0998f8bca",
    "testharness"

testing/web-platform/tests/service-workers/service-worker/update-result.https.html

@@ -1,23 +0,0 @@
-<!DOCTYPE html>
-<title>Service Worker: update() should resolve a ServiceWorkerRegistration</title>
-<script src="/resources/testharness.js"></script>
-<script src="/resources/testharnessreport.js"></script>
-<script src="resources/test-helpers.sub.js"></script>
-<body>
-<script>
-
-promise_test(async function(t) {
-  const script = './resources/empty.js';
-  const scope = './resources/empty.html?update-result';
-
-  let reg = await navigator.serviceWorker.register(script, { scope });
-  t.add_cleanup(async _ => await reg.unregister());
-  await wait_for_state(t, reg.installing, 'activated');
-
-  let result = await reg.update();
-  assert_true(result instanceof ServiceWorkerRegistration,
-              'update() should resolve a ServiceWorkerRegistration');
-}, 'ServiceWorkerRegistration.update() should resolve a registration object');
-
-</script>
-</body>