Bug 1113631 - Remove registration when installation failure occurs. Fixes spec issue #547. r=baku
authorNikhil Marathe <nsm.nikhil@gmail.com>
Wed, 05 Nov 2014 14:43:51 -0800
changeset 239412 56339532dd96c7d2a7f4f109026d47828b1c6886
parent 239411 53a300073430e727fe1783df8897c555555eef51
child 239413 4056ebeecf61515b6fce5ae8a3f828870855e538
push id497
push usermleibovic@mozilla.com
push dateWed, 28 Jan 2015 16:43:37 +0000
reviewersbaku
bugs1113631
milestone38.0a1
Bug 1113631 - Remove registration when installation failure occurs. Fixes spec issue #547. r=baku
dom/workers/ServiceWorkerManager.cpp
dom/workers/test/serviceworkers/test_installation_simple.html
--- a/dom/workers/ServiceWorkerManager.cpp
+++ b/dom/workers/ServiceWorkerManager.cpp
@@ -527,18 +527,17 @@ public:
   }
 
   // Public so our error handling code can use it.
   void
   Fail(const ErrorEventInit& aError)
   {
     MOZ_ASSERT(mCallback);
     mCallback->UpdateFailed(aError);
-    mCallback = nullptr;
-    Done(NS_ERROR_DOM_JS_EXCEPTION);
+    FailCommon(NS_ERROR_DOM_JS_EXCEPTION);
   }
 
   // Public so our error handling code can continue with a successful worker.
   void
   ContinueInstall()
   {
     nsRefPtr<ServiceWorkerManager> swm = ServiceWorkerManager::GetInstance();
     nsRefPtr<ServiceWorkerManager::ServiceWorkerDomainInfo> domainInfo =
@@ -656,26 +655,49 @@ private:
   void
   Succeed()
   {
     MOZ_ASSERT(mCallback);
     mCallback->UpdateSucceeded(mRegistration);
     mCallback = nullptr;
   }
 
+  void
+  FailCommon(nsresult aRv)
+  {
+    mCallback = nullptr;
+    MaybeRemoveRegistration();
+    // Ensures that the job can't do anything useful from this point on.
+    mRegistration = nullptr;
+    Done(aRv);
+  }
+
   // This MUST only be called when the job is still performing actions related
   // to registration or update. After the spec resolves the update promise, use
   // Done() with the failure code instead.
   void
-  Fail(nsresult rv)
+  Fail(nsresult aRv)
   {
     MOZ_ASSERT(mCallback);
-    mCallback->UpdateFailed(rv);
-    mCallback = nullptr;
-    Done(rv);
+    mCallback->UpdateFailed(aRv);
+    FailCommon(aRv);
+  }
+
+  void
+  MaybeRemoveRegistration()
+  {
+    MOZ_ASSERT(mRegistration);
+    nsRefPtr<ServiceWorkerInfo> newest = mRegistration->Newest();
+    if (!newest) {
+      nsRefPtr<ServiceWorkerManager> swm = ServiceWorkerManager::GetInstance();
+      nsRefPtr<ServiceWorkerManager::ServiceWorkerDomainInfo> domainInfo =
+        swm->GetDomainInfo(mRegistration->mScope);
+      MOZ_ASSERT(domainInfo);
+      domainInfo->RemoveRegistration(mRegistration);
+    }
   }
 
   void
   ContinueAfterInstallEvent(bool aSuccess, bool aActivateImmediately)
   {
     // By this point the callback should've been notified about success or fail
     // and nulled.
     MOZ_ASSERT(!mCallback);
@@ -688,16 +710,17 @@ private:
     nsRefPtr<ServiceWorkerManager> swm = ServiceWorkerManager::GetInstance();
 
     // "If installFailed is true"
     if (!aSuccess) {
       mRegistration->mInstallingWorker->UpdateState(ServiceWorkerState::Redundant);
       mRegistration->mInstallingWorker = nullptr;
       swm->InvalidateServiceWorkerRegistrationWorker(mRegistration,
                                                      WhichServiceWorker::INSTALLING_WORKER);
+      MaybeRemoveRegistration();
       return Done(NS_ERROR_DOM_ABORT_ERR);
     }
 
     // "If registration's waiting worker is not null"
     if (mRegistration->mWaitingWorker) {
       // FIXME(nsm): Terminate
       mRegistration->mWaitingWorker->UpdateState(ServiceWorkerState::Redundant);
     }
--- a/dom/workers/test/serviceworkers/test_installation_simple.html
+++ b/dom/workers/test/serviceworkers/test_installation_simple.html
@@ -101,16 +101,20 @@
         ok(e.name === "NetworkError", "Should fail with NetworkError");
       });
   }
 
   function parseError() {
     var p = navigator.serviceWorker.register("parse_error_worker.js", { scope: "parse_error/" });
     return p.then(function(wr) {
       ok(false, "Registration should fail with parse error");
+      return navigator.serviceWorker.getRegistration("parse_error/").then(function(swr) {
+        // See https://github.com/slightlyoff/ServiceWorker/issues/547
+        is(swr, undefined, "A failed registration for a scope with no prior controllers should clear itself");
+      });
     }, function(e) {
     info("NSM " + e.name);
       ok(e instanceof Error, "Registration should fail with parse error");
     });
   }
 
   // FIXME(nsm): test for parse error when Update step doesn't happen (directly from register).