Bug 1436812 P9 Implement the ServiceWorkerContainer::Register() method using the inner class. r=baku
authorBen Kelly <ben@wanderview.com>
Mon, 23 Apr 2018 09:46:55 -0700
changeset 468662 f0f62b226f25211bb05b7bf65c0d20394ec40da7
parent 468661 bfadc112f9f74a4fa2bfb8c53efab086f834124e
child 468663 9509a7028644878202c1cd2fa61a83e1a397a70a
push id9165
push userasasaki@mozilla.com
push dateThu, 26 Apr 2018 21:04:54 +0000
treeherdermozilla-beta@064c3804de2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbaku
bugs1436812
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 1436812 P9 Implement the ServiceWorkerContainer::Register() method using the inner class. r=baku
dom/interfaces/base/nsIServiceWorkerManager.idl
dom/serviceworkers/ServiceWorkerContainer.cpp
dom/serviceworkers/ServiceWorkerContainer.h
dom/serviceworkers/ServiceWorkerContainerImpl.cpp
dom/serviceworkers/ServiceWorkerContainerImpl.h
dom/serviceworkers/ServiceWorkerManager.cpp
dom/serviceworkers/ServiceWorkerManager.h
--- a/dom/interfaces/base/nsIServiceWorkerManager.idl
+++ b/dom/interfaces/base/nsIServiceWorkerManager.idl
@@ -113,29 +113,16 @@ interface nsIServiceWorkerManagerListene
 
   void onUnregister(in nsIServiceWorkerRegistrationInfo aInfo);
 };
 
 [scriptable, builtinclass, uuid(7404c8e8-4d47-4449-8ed1-47d1261d4e33)]
 interface nsIServiceWorkerManager : nsISupports
 {
   /**
-   * Registers a ServiceWorker with script loaded from `aScriptURI` to act as
-   * the ServiceWorker for aScope.  Requires a valid entry settings object on
-   * the stack. This means you must call this from content code 'within'
-   * a window.
-   *
-   * Returns a Promise.
-   */
-  nsISupports register(in mozIDOMWindow aWindow,
-                       in nsIURI aScope,
-                       in nsIURI aScriptURI,
-                       in uint16_t aUpdateViaCache);
-
-  /**
    * Unregister an existing ServiceWorker registration for `aScope`.
    * It keeps aCallback alive until the operation is concluded.
    */
   void unregister(in nsIPrincipal aPrincipal,
                   in nsIServiceWorkerUnregisterCallback aCallback,
                   in DOMString aScope);
 
   nsIServiceWorkerRegistrationInfo getRegistrationByPrincipal(in nsIPrincipal aPrincipal,
--- a/dom/serviceworkers/ServiceWorkerContainer.cpp
+++ b/dom/serviceworkers/ServiceWorkerContainer.cpp
@@ -176,24 +176,19 @@ IsFromAuthenticatedOrigin(nsIDocument* a
 
 } // anonymous namespace
 
 already_AddRefed<Promise>
 ServiceWorkerContainer::Register(const nsAString& aScriptURL,
                                  const RegistrationOptions& aOptions,
                                  ErrorResult& aRv)
 {
-  nsCOMPtr<nsISupports> promise;
-
-  nsCOMPtr<nsIServiceWorkerManager> swm = mozilla::services::GetServiceWorkerManager();
-  if (!swm) {
-    aRv.Throw(NS_ERROR_FAILURE);
-    return nullptr;
-  }
-
+  // Note, we can't use GetGlobalIfValid() from the start here.  If we
+  // hit a storage failure we want to log a message with the final
+  // scope string we put together below.
   nsIGlobalObject* global = GetParentObject();
   if (!global) {
     aRv.Throw(NS_ERROR_DOM_INVALID_STATE_ERR);
     return nullptr;
   }
 
   Maybe<ClientInfo> clientInfo = global->GetClientInfo();
   if (clientInfo.isNothing()) {
@@ -351,28 +346,41 @@ ServiceWorkerContainer::Register(const n
                                     NS_LITERAL_CSTRING("Service Workers"),
                                     aDoc, nsContentUtils::eDOM_PROPERTIES,
                                     "ServiceWorkerRegisterStorageError",
                                     param, 1);
   });
 
   window->NoteCalledRegisterForServiceWorkerScope(cleanedScopeURL);
 
-  // The spec says that the "client" passed to Register() must be the global
-  // where the ServiceWorkerContainer was retrieved from.
-  aRv = swm->Register(GetOwner(), scopeURI, scriptURI,
-                      static_cast<uint16_t>(aOptions.mUpdateViaCache),
-                      getter_AddRefs(promise));
-  if (NS_WARN_IF(aRv.Failed())) {
+  RefPtr<Promise> outer = Promise::Create(global, aRv);
+  if (aRv.Failed()) {
     return nullptr;
   }
 
-  RefPtr<Promise> ret = static_cast<Promise*>(promise.get());
-  MOZ_ASSERT(ret);
-  return ret.forget();
+  RefPtr<ServiceWorkerContainer> self = this;
+
+  mInner->Register(clientInfo.ref(), cleanedScopeURL, cleanedScriptURL,
+                   aOptions.mUpdateViaCache)->Then(
+    global->EventTargetFor(TaskCategory::Other), __func__,
+    [self, outer] (const ServiceWorkerRegistrationDescriptor& aDesc) {
+      ErrorResult rv;
+      nsIGlobalObject* global = self->GetGlobalIfValid(rv);
+      if (rv.Failed()) {
+        outer->MaybeReject(rv);
+        return;
+      }
+      RefPtr<ServiceWorkerRegistration> reg =
+        global->GetOrCreateServiceWorkerRegistration(aDesc);
+      outer->MaybeResolve(reg);
+    }, [self, outer] (ErrorResult&& aRv) {
+      outer->MaybeReject(aRv);
+    });
+
+  return outer.forget();
 }
 
 already_AddRefed<ServiceWorker>
 ServiceWorkerContainer::GetController()
 {
   RefPtr<ServiceWorker> ref = mControllerWorker;
   return ref.forget();
 }
--- a/dom/serviceworkers/ServiceWorkerContainer.h
+++ b/dom/serviceworkers/ServiceWorkerContainer.h
@@ -22,18 +22,20 @@ class ServiceWorker;
 // Lightweight serviceWorker APIs collection.
 class ServiceWorkerContainer final : public DOMEventTargetHelper
 {
 public:
   class Inner
   {
   public:
     virtual RefPtr<ServiceWorkerRegistrationPromise>
-    Register(const nsAString& aScriptURL,
-             const RegistrationOptions& aOptions) = 0;
+    Register(const ClientInfo& aClientInfo,
+             const nsACString& aScopeURL,
+             const nsACString& aScriptURL,
+             ServiceWorkerUpdateViaCache aUpdateViaCache) const = 0;
 
     virtual RefPtr<ServiceWorkerRegistrationPromise>
     GetRegistration(const ClientInfo& aClientInfo,
                     const nsACString& aURL) const = 0;
 
     virtual RefPtr<ServiceWorkerRegistrationListPromise>
     GetRegistrations(const ClientInfo& aClientInfo) const = 0;
 
--- a/dom/serviceworkers/ServiceWorkerContainerImpl.cpp
+++ b/dom/serviceworkers/ServiceWorkerContainerImpl.cpp
@@ -5,21 +5,28 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "ServiceWorkerContainerImpl.h"
 
 namespace mozilla {
 namespace dom {
 
 RefPtr<ServiceWorkerRegistrationPromise>
-ServiceWorkerContainerImpl::Register(const nsAString& aScriptURL,
-                                     const RegistrationOptions& aOptions)
+ServiceWorkerContainerImpl::Register(const ClientInfo& aClientInfo,
+                                     const nsACString& aScopeURL,
+                                     const nsACString& aScriptURL,
+                                     ServiceWorkerUpdateViaCache aUpdateViaCache) const
 {
-  // TODO
-  return nullptr;
+  RefPtr<ServiceWorkerManager> swm = ServiceWorkerManager::GetInstance();
+  if (NS_WARN_IF(!swm)) {
+    return ServiceWorkerRegistrationPromise::CreateAndReject(NS_ERROR_DOM_INVALID_STATE_ERR,
+                                                             __func__);
+  }
+
+  return swm->Register(aClientInfo, aScopeURL, aScriptURL, aUpdateViaCache);
 }
 
 RefPtr<ServiceWorkerRegistrationPromise>
 ServiceWorkerContainerImpl::GetRegistration(const ClientInfo& aClientInfo,
                                             const nsACString& aURL) const
 {
   RefPtr<ServiceWorkerManager> swm = ServiceWorkerManager::GetInstance();
   if (NS_WARN_IF(!swm)) {
--- a/dom/serviceworkers/ServiceWorkerContainerImpl.h
+++ b/dom/serviceworkers/ServiceWorkerContainerImpl.h
@@ -16,18 +16,20 @@ namespace dom {
 class ServiceWorkerContainerImpl final : public ServiceWorkerContainer::Inner
 {
   ~ServiceWorkerContainerImpl() = default;
 
 public:
   ServiceWorkerContainerImpl() = default;
 
   RefPtr<ServiceWorkerRegistrationPromise>
-  Register(const nsAString& aScriptURL,
-           const RegistrationOptions& aOptions) override;
+  Register(const ClientInfo& aClientInfo,
+           const nsACString& aScopeURL,
+           const nsACString& aScriptURL,
+           ServiceWorkerUpdateViaCache aUpdateViaCache) const override;
 
   RefPtr<ServiceWorkerRegistrationPromise>
   GetRegistration(const ClientInfo& aClientInfo,
                   const nsACString& aURL) const override;
 
   RefPtr<ServiceWorkerRegistrationListPromise>
   GetRegistrations(const ClientInfo& aClientInfo) const override;
 
--- a/dom/serviceworkers/ServiceWorkerManager.cpp
+++ b/dom/serviceworkers/ServiceWorkerManager.cpp
@@ -420,66 +420,51 @@ ServiceWorkerManager::MaybeStartShutdown
   RefPtr<TeardownRunnable> runnable = new TeardownRunnable(mActor);
   nsresult rv = NS_DispatchToMainThread(runnable);
   Unused << NS_WARN_IF(NS_FAILED(rv));
   mActor = nullptr;
 }
 
 class ServiceWorkerResolveWindowPromiseOnRegisterCallback final : public ServiceWorkerJob::Callback
 {
-  // The promise "returned" by the call to Update up to
-  // navigator.serviceWorker.register().
-  PromiseWindowProxy mPromise;
+  RefPtr<ServiceWorkerRegistrationPromise::Private> mPromise;
 
   ~ServiceWorkerResolveWindowPromiseOnRegisterCallback()
   {}
 
   virtual void
   JobFinished(ServiceWorkerJob* aJob, ErrorResult& aStatus) override
   {
     MOZ_ASSERT(NS_IsMainThread());
     MOZ_ASSERT(aJob);
-    RefPtr<Promise> promise = mPromise.Get();
-    if (!promise) {
-      return;
-    }
 
     if (aStatus.Failed()) {
-      promise->MaybeReject(aStatus);
-      return;
-    }
-
-    nsCOMPtr<nsPIDOMWindowInner> window = mPromise.GetWindow();
-    if (!window) {
+      mPromise->Reject(Move(aStatus), __func__);
       return;
     }
 
     MOZ_ASSERT(aJob->GetType() == ServiceWorkerJob::Type::Register);
     RefPtr<ServiceWorkerRegisterJob> registerJob =
       static_cast<ServiceWorkerRegisterJob*>(aJob);
     RefPtr<ServiceWorkerRegistrationInfo> reg = registerJob->GetRegistration();
 
-    RefPtr<ServiceWorkerRegistration> swr =
-      window->AsGlobal()->GetOrCreateServiceWorkerRegistration(reg->Descriptor());
-
-    nsCOMPtr<nsIRunnable> r = NS_NewRunnableFunction(
-      "ServiceWorkerResolveWindowPromiseOnRegisterCallback::JobFinished",
-      [promise = Move(promise), swr = Move(swr)] () {
-        promise->MaybeResolve(swr);
-      });
-    MOZ_ALWAYS_SUCCEEDS(
-      window->EventTargetFor(TaskCategory::Other)->Dispatch(r.forget()));
+    mPromise->Resolve(reg->Descriptor(), __func__);
   }
 
 public:
-  ServiceWorkerResolveWindowPromiseOnRegisterCallback(nsPIDOMWindowInner* aWindow,
-                                                      Promise* aPromise)
-    : mPromise(aWindow, aPromise)
+  ServiceWorkerResolveWindowPromiseOnRegisterCallback()
+    : mPromise(new ServiceWorkerRegistrationPromise::Private(__func__))
   {}
 
+  RefPtr<ServiceWorkerRegistrationPromise>
+  Promise() const
+  {
+    return mPromise;
+  }
+
   NS_INLINE_DECL_REFCOUNTING(ServiceWorkerResolveWindowPromiseOnRegisterCallback, override)
 };
 
 namespace {
 
 class PropagateSoftUpdateRunnable final : public Runnable
 {
 public:
@@ -743,83 +728,67 @@ private:
     }
   }
 
   RefPtr<GenericPromise::Private> mPromise;
 };
 
 } // namespace
 
-// If we return an error code here, the ServiceWorkerContainer will
-// automatically reject the Promise.
-NS_IMETHODIMP
-ServiceWorkerManager::Register(mozIDOMWindow* aWindow,
-                               nsIURI* aScopeURI,
-                               nsIURI* aScriptURI,
-                               uint16_t aUpdateViaCache,
-                               nsISupports** aPromise)
+RefPtr<ServiceWorkerRegistrationPromise>
+ServiceWorkerManager::Register(const ClientInfo& aClientInfo,
+                               const nsACString& aScopeURL,
+                               const nsACString& aScriptURL,
+                               ServiceWorkerUpdateViaCache aUpdateViaCache)
 {
-  MOZ_ASSERT(NS_IsMainThread());
-
-  if (NS_WARN_IF(!aWindow)) {
-    return NS_ERROR_DOM_INVALID_STATE_ERR;
+  nsCOMPtr<nsIURI> scopeURI;
+  nsresult rv = NS_NewURI(getter_AddRefs(scopeURI), aScopeURL, nullptr, nullptr);
+  if (NS_FAILED(rv)) {
+    return ServiceWorkerRegistrationPromise::CreateAndReject(rv, __func__);
   }
 
-  auto* window = nsPIDOMWindowInner::From(aWindow);
-
-  nsCOMPtr<nsIDocument> doc = window->GetExtantDoc();
-  MOZ_ASSERT(doc);
-
-  // Data URLs are not allowed.
-  nsCOMPtr<nsIPrincipal> documentPrincipal = doc->NodePrincipal();
-
-  nsCString cleanedScope;
-  nsresult rv = aScopeURI->GetSpecIgnoringRef(cleanedScope);
-  if (NS_WARN_IF(NS_FAILED(rv))) {
-    return NS_ERROR_FAILURE;
+  nsCOMPtr<nsIURI> scriptURI;
+  rv = NS_NewURI(getter_AddRefs(scriptURI), aScriptURL, nullptr, nullptr);
+  if (NS_FAILED(rv)) {
+    return ServiceWorkerRegistrationPromise::CreateAndReject(rv, __func__);
   }
 
-  nsAutoCString spec;
-  rv = aScriptURI->GetSpecIgnoringRef(spec);
-  if (NS_WARN_IF(NS_FAILED(rv))) {
-    return rv;
+  rv = ServiceWorkerScopeAndScriptAreValid(aClientInfo, scopeURI, scriptURI);
+  if (NS_FAILED(rv)) {
+    return ServiceWorkerRegistrationPromise::CreateAndReject(rv, __func__);
   }
 
-  ErrorResult result;
-  RefPtr<Promise> promise = Promise::Create(window->AsGlobal(), result);
-  if (result.Failed()) {
-    return result.StealNSResult();
-  }
+  // If the previous validation step passed then we must have a principal.
+  nsCOMPtr<nsIPrincipal> principal = aClientInfo.GetPrincipal();
 
   nsAutoCString scopeKey;
-  rv = PrincipalToScopeKey(documentPrincipal, scopeKey);
+  rv = PrincipalToScopeKey(principal, scopeKey);
   if (NS_WARN_IF(NS_FAILED(rv))) {
-    return rv;
+    return ServiceWorkerRegistrationPromise::CreateAndReject(rv, __func__);
   }
 
   RefPtr<ServiceWorkerJobQueue> queue = GetOrCreateJobQueue(scopeKey,
-                                                            cleanedScope);
+                                                            aScopeURL);
 
   RefPtr<ServiceWorkerResolveWindowPromiseOnRegisterCallback> cb =
-    new ServiceWorkerResolveWindowPromiseOnRegisterCallback(window, promise);
+    new ServiceWorkerResolveWindowPromiseOnRegisterCallback();
 
   nsCOMPtr<nsILoadGroup> loadGroup = do_CreateInstance(NS_LOADGROUP_CONTRACTID);
   RefPtr<ServiceWorkerRegisterJob> job = new ServiceWorkerRegisterJob(
-    documentPrincipal, cleanedScope, spec, loadGroup,
+    principal, aScopeURL, aScriptURL, loadGroup,
     static_cast<ServiceWorkerUpdateViaCache>(aUpdateViaCache)
   );
 
   job->AppendResultCallback(cb);
   queue->ScheduleJob(job);
 
   MOZ_ASSERT(NS_IsMainThread());
   Telemetry::Accumulate(Telemetry::SERVICE_WORKER_REGISTRATIONS, 1);
 
-  promise.forget(aPromise);
-  return NS_OK;
+  return cb->Promise();
 }
 
 /*
  * Implements the async aspects of the getRegistrations algorithm.
  */
 class GetRegistrationsRunnable final : public Runnable
 {
   const ClientInfo mClientInfo;
--- a/dom/serviceworkers/ServiceWorkerManager.h
+++ b/dom/serviceworkers/ServiceWorkerManager.h
@@ -189,16 +189,21 @@ public:
 
   void
   PropagateRemoveAll();
 
   void
   RemoveAll();
 
   RefPtr<ServiceWorkerRegistrationPromise>
+  Register(const ClientInfo& aClientInfo, const nsACString& aScopeURL,
+           const nsACString& aScriptURL,
+           ServiceWorkerUpdateViaCache aUpdateViaCache);
+
+  RefPtr<ServiceWorkerRegistrationPromise>
   GetRegistration(const ClientInfo& aClientInfo, const nsACString& aURL) const;
 
   RefPtr<ServiceWorkerRegistrationListPromise>
   GetRegistrations(const ClientInfo& aClientInfo) const;
 
   already_AddRefed<ServiceWorkerRegistrationInfo>
   GetRegistration(nsIPrincipal* aPrincipal, const nsACString& aScope) const;