Bug 1113957 - ServiceWorker unregistration uses job queue. r=baku
authorNikhil Marathe <nsm.nikhil@gmail.com>
Thu, 22 Jan 2015 14:10:38 -0800
changeset 239414 237edb7e4ae58499f0851fc66ac12dbd9c6f7bd0
parent 239413 4056ebeecf61515b6fce5ae8a3f828870855e538
child 239415 186127d710767bf094464a151d5f88b3157992f5
push id497
push usermleibovic@mozilla.com
push dateWed, 28 Jan 2015 16:43:37 +0000
reviewersbaku
bugs1113957
milestone38.0a1
Bug 1113957 - ServiceWorker unregistration uses job queue. r=baku
dom/workers/ServiceWorkerManager.cpp
dom/workers/ServiceWorkerManager.h
dom/workers/ServiceWorkerRegistration.cpp
dom/workers/test/serviceworkers/mochitest.ini
dom/workers/test/serviceworkers/test_unregister.html
--- a/dom/workers/ServiceWorkerManager.cpp
+++ b/dom/workers/ServiceWorkerManager.cpp
@@ -1416,95 +1416,127 @@ ServiceWorkerManager::CheckReadyPromise(
       new ServiceWorkerRegistration(aWindow, scope);
     aPromise->MaybeResolve(swr);
     return true;
   }
 
   return false;
 }
 
+class ServiceWorkerUnregisterJob MOZ_FINAL : public ServiceWorkerJob
+{
+  nsRefPtr<ServiceWorkerRegistrationInfo> mRegistration;
+  const nsCString mScope;
+  nsCOMPtr<nsIServiceWorkerUnregisterCallback> mCallback;
+
+  ~ServiceWorkerUnregisterJob()
+  { }
+
+public:
+  ServiceWorkerUnregisterJob(ServiceWorkerJobQueue* aQueue,
+                             const nsACString& aScope,
+                             nsIServiceWorkerUnregisterCallback* aCallback)
+    : ServiceWorkerJob(aQueue)
+    , mScope(aScope)
+    , mCallback(aCallback)
+  {
+    AssertIsOnMainThread();
+  }
+
+  void
+  Start() MOZ_OVERRIDE
+  {
+    AssertIsOnMainThread();
+    nsCOMPtr<nsIRunnable> r =
+      NS_NewRunnableMethod(this, &ServiceWorkerUnregisterJob::UnregisterAndDone);
+    MOZ_ALWAYS_TRUE(NS_SUCCEEDED(NS_DispatchToMainThread(r)));
+  }
+
+private:
+  // You probably want UnregisterAndDone().
+  nsresult
+  Unregister()
+  {
+    AssertIsOnMainThread();
+
+    nsRefPtr<ServiceWorkerManager> swm = ServiceWorkerManager::GetInstance();
+
+    nsRefPtr<ServiceWorkerManager::ServiceWorkerDomainInfo> domainInfo =
+      swm->GetDomainInfo(mScope);
+    MOZ_ASSERT(domainInfo);
+
+    // "Let registration be the result of running [[Get Registration]]
+    // algorithm passing scope as the argument."
+    nsRefPtr<ServiceWorkerRegistrationInfo> registration;
+    if (!domainInfo->mServiceWorkerRegistrationInfos.Get(mScope,
+                                                         getter_AddRefs(registration))) {
+      // "If registration is null, then, resolve promise with false."
+      return mCallback->UnregisterSucceeded(false);
+    }
+
+    MOZ_ASSERT(registration);
+
+    // "Set registration's uninstalling flag."
+    registration->mPendingUninstall = true;
+    // "Resolve promise with true"
+    nsresult rv = mCallback->UnregisterSucceeded(true);
+    if (NS_WARN_IF(NS_FAILED(rv))) {
+      return rv;
+    }
+
+    // "If no service worker client is using registration..."
+    if (!registration->IsControllingDocuments()) {
+      // "If registration's uninstalling flag is set.."
+      if (!registration->mPendingUninstall) {
+        return NS_OK;
+      }
+
+      // "Invoke [[Clear Registration]]..."
+      registration->Clear();
+      domainInfo->RemoveRegistration(registration);
+    }
+
+    return NS_OK;
+  }
+
+  // The unregister job is done irrespective of success or failure of any sort.
+  void
+  UnregisterAndDone()
+  {
+    Done(Unregister());
+  }
+};
+
 NS_IMETHODIMP
 ServiceWorkerManager::Unregister(nsIServiceWorkerUnregisterCallback* aCallback,
                                  const nsAString& aScope)
 {
   AssertIsOnMainThread();
   MOZ_ASSERT(aCallback);
 
+// This is not accessible by content, and callers should always ensure scope is
+// a correct URI, so this is wrapped in DEBUG
+#ifdef DEBUG
   nsCOMPtr<nsIURI> scopeURI;
   nsresult rv = NS_NewURI(getter_AddRefs(scopeURI), aScope, nullptr, nullptr);
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return NS_ERROR_DOM_SECURITY_ERR;
   }
-
-  /*
-   * Implements the async aspects of the unregister algorithm.
-   */
-  class UnregisterRunnable : public nsRunnable
-  {
-    nsCOMPtr<nsIServiceWorkerUnregisterCallback> mCallback;
-    nsCOMPtr<nsIURI> mScopeURI;
-
-  public:
-    UnregisterRunnable(nsIServiceWorkerUnregisterCallback* aCallback,
-                       nsIURI* aScopeURI)
-      : mCallback(aCallback), mScopeURI(aScopeURI)
-    {
-      AssertIsOnMainThread();
-    }
-
-    NS_IMETHODIMP
-    Run()
-    {
-      AssertIsOnMainThread();
-
-      nsRefPtr<ServiceWorkerManager> swm = ServiceWorkerManager::GetInstance();
-
-      nsRefPtr<ServiceWorkerManager::ServiceWorkerDomainInfo> domainInfo =
-        swm->GetDomainInfo(mScopeURI);
-      MOZ_ASSERT(domainInfo);
-
-      nsCString spec;
-      nsresult rv = mScopeURI->GetSpecIgnoringRef(spec);
-      if (NS_WARN_IF(NS_FAILED(rv))) {
-        return mCallback->UnregisterFailed();
-      }
+#endif
 
-      nsRefPtr<ServiceWorkerRegistrationInfo> registration;
-      if (!domainInfo->mServiceWorkerRegistrationInfos.Get(spec,
-                                                           getter_AddRefs(registration))) {
-        return mCallback->UnregisterSucceeded(false);
-      }
-
-      MOZ_ASSERT(registration);
-
-      registration->mPendingUninstall = true;
-      rv = mCallback->UnregisterSucceeded(true);
-      if (NS_WARN_IF(NS_FAILED(rv))) {
-        return rv;
-      }
+  NS_ConvertUTF16toUTF8 scope(aScope);
+  nsRefPtr<ServiceWorkerManager::ServiceWorkerDomainInfo> domainInfo =
+    GetDomainInfo(scope);
+  ServiceWorkerJobQueue* queue = domainInfo->GetOrCreateJobQueue(scope);
+  MOZ_ASSERT(queue);
 
-      // The "Wait until no document is using registration" can actually be
-      // handled by [[HandleDocumentUnload]] in Bug 1041340, so we simply check
-      // if the document is currently in use here.
-      if (!registration->IsControllingDocuments()) {
-        if (!registration->mPendingUninstall) {
-          return NS_OK;
-        }
-
-        registration->Clear();
-        domainInfo->RemoveRegistration(registration);
-      }
-
-      return NS_OK;
-    }
-  };
-
-  nsRefPtr<nsIRunnable> unregisterRunnable =
-    new UnregisterRunnable(aCallback, scopeURI);
-  return NS_DispatchToCurrentThread(unregisterRunnable);
+  nsRefPtr<ServiceWorkerUnregisterJob> job =
+    new ServiceWorkerUnregisterJob(queue, scope, aCallback);
+  queue->Append(job);
+  return NS_OK;
 }
 
 /* static */
 already_AddRefed<ServiceWorkerManager>
 ServiceWorkerManager::GetInstance()
 {
   nsCOMPtr<nsIServiceWorkerManager> swm = mozilla::services::GetServiceWorkerManager();
   nsRefPtr<ServiceWorkerManager> concrete = do_QueryObject(swm);
@@ -2188,18 +2220,18 @@ ServiceWorkerManager::Update(const nsASt
   }
 
   ServiceWorkerJobQueue* queue = domainInfo->GetOrCreateJobQueue(scope);
   MOZ_ASSERT(queue);
 
   nsRefPtr<ServiceWorkerUpdateFinishCallback> cb =
     new ServiceWorkerUpdateFinishCallback();
 
-  nsRefPtr<ServiceWorkerRegisterJob> job
-    = new ServiceWorkerRegisterJob(queue, registration, cb);
+  nsRefPtr<ServiceWorkerRegisterJob> job =
+    new ServiceWorkerRegisterJob(queue, registration, cb);
   queue->Append(job);
   return NS_OK;
 }
 
 namespace {
 
 class MOZ_STACK_CLASS FilterRegistrationData
 {
--- a/dom/workers/ServiceWorkerManager.h
+++ b/dom/workers/ServiceWorkerManager.h
@@ -270,17 +270,17 @@ class ServiceWorkerManager MOZ_FINAL : p
 {
   friend class ActivationRunnable;
   friend class ServiceWorkerRegistrationInfo;
   friend class ServiceWorkerRegisterJob;
   friend class GetReadyPromiseRunnable;
   friend class GetRegistrationsRunnable;
   friend class GetRegistrationRunnable;
   friend class QueueFireUpdateFoundRunnable;
-  friend class UnregisterRunnable;
+  friend class ServiceWorkerUnregisterJob;
 
 public:
   NS_DECL_ISUPPORTS
   NS_DECL_NSISERVICEWORKERMANAGER
   NS_DECLARE_STATIC_IID_ACCESSOR(NS_SERVICEWORKERMANAGER_IMPL_IID)
 
   static ServiceWorkerManager* FactoryCreate()
   {
--- a/dom/workers/ServiceWorkerRegistration.cpp
+++ b/dom/workers/ServiceWorkerRegistration.cpp
@@ -159,32 +159,33 @@ ServiceWorkerRegistration::Unregister(Er
   nsCOMPtr<nsIDocument> document = 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<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->GetSpec(uriSpec);
+  aRv = scopeURI->GetSpecIgnoringRef(uriSpec);
   if (NS_WARN_IF(aRv.Failed())) {
     return nullptr;
   }
 
   nsCOMPtr<nsIServiceWorkerManager> swm =
     do_GetService(SERVICEWORKERMANAGER_CONTRACTID, &rv);
   if (NS_WARN_IF(NS_FAILED(rv))) {
     aRv.Throw(rv);
--- a/dom/workers/test/serviceworkers/mochitest.ini
+++ b/dom/workers/test/serviceworkers/mochitest.ini
@@ -22,14 +22,13 @@ support-files =
 [test_unregister.html]
 skip-if = true # bug 1094375
 [test_installation_simple.html]
 skip-if = true # bug 1094375
 [test_get_serviced.html]
 [test_install_event.html]
 [test_navigator.html]
 [test_scopes.html]
-skip-if = true # bug 1124743
 [test_controller.html]
 [test_workerUpdate.html]
 skip-if = true # Enable after Bug 982726 postMessage is landed.
 [test_workerUnregister.html]
 skip-if = true # Enable after Bug 982726 postMessage is landed.
--- a/dom/workers/test/serviceworkers/test_unregister.html
+++ b/dom/workers/test/serviceworkers/test_unregister.html
@@ -81,35 +81,19 @@
 
     return testPromise.then(function() {
       div.removeChild(ifr);
     });
   }
 
   function runTest() {
     simpleRegister()
-      .then(function(v) {
-        info("simpleRegister() promise resolved");
-      })
       .then(testControlled)
-      .then(function(v) {
-        info("testControlled() promise resolved");
-      }, function(e) {
-        info("testControlled() promise rejected " + e);
-      })
       .then(unregister)
-      .then(function(v) {
-        info("unregister() promise resolved");
-      })
       .then(testUncontrolled)
-      .then(function(v) {
-        info("testUncontrolled() promise resolved");
-      }, function(e) {
-        info("testUncontrolled() promise rejected " + e);
-      })
       .then(function() {
         SimpleTest.finish();
       }).catch(function(e) {
         ok(false, "Some test failed with error " + e);
         SimpleTest.finish();
       });
   }