Bug 1227015 P5 Remove ServiceWorkerRegistrationInfo mScriptSpec. r=ehsan
authorBen Kelly <ben@wanderview.com>
Fri, 11 Dec 2015 14:53:10 -0500
changeset 310368 2834622813f8bab158970d0fcfcc5ea5188a6c9e
parent 310367 d24d444fcec1c8e8e9acf2252b5cb679f1fb3538
child 310369 3632028dfc0c55cb2b897bb284a4a0f82bf82bfa
push id5513
push userraliiev@mozilla.com
push dateMon, 25 Jan 2016 13:55:34 +0000
treeherdermozilla-beta@5ee97dd05b5c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersehsan
bugs1227015
milestone45.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 1227015 P5 Remove ServiceWorkerRegistrationInfo mScriptSpec. r=ehsan
dom/workers/ServiceWorkerManager.cpp
dom/workers/ServiceWorkerManager.h
dom/workers/test/serviceworkers/test_serviceworkerregistrationinfo.xul
--- a/dom/workers/ServiceWorkerManager.cpp
+++ b/dom/workers/ServiceWorkerManager.cpp
@@ -313,17 +313,23 @@ PopulateRegistrationData(nsIPrincipal* a
   }
 
   nsresult rv = PrincipalToPrincipalInfo(aPrincipal, &aData.principal());
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
 
   aData.scope() = aRegistration->mScope;
-  aData.scriptSpec() = aRegistration->mScriptSpec;
+
+  RefPtr<ServiceWorkerInfo> newest = aRegistration->Newest();
+  if (NS_WARN_IF(!newest)) {
+    return NS_ERROR_FAILURE;
+  }
+
+  aData.scriptSpec() = newest->ScriptSpec();
 
   if (aRegistration->mActiveWorker) {
     aData.currentWorkerURL() = aRegistration->mActiveWorker->ScriptSpec();
     aData.activeCacheName() = aRegistration->mActiveWorker->CacheName();
   }
 
   if (aRegistration->mWaitingWorker) {
     aData.waitingCacheName() = aRegistration->mWaitingWorker->CacheName();
@@ -453,17 +459,20 @@ ServiceWorkerRegistrationInfo::GetScope(
   CopyUTF8toUTF16(mScope, aScope);
   return NS_OK;
 }
 
 NS_IMETHODIMP
 ServiceWorkerRegistrationInfo::GetScriptSpec(nsAString& aScriptSpec)
 {
   AssertIsOnMainThread();
-  CopyUTF8toUTF16(mScriptSpec, aScriptSpec);
+  RefPtr<ServiceWorkerInfo> newest = Newest();
+  if (newest) {
+    CopyUTF8toUTF16(newest->ScriptSpec(), aScriptSpec);
+  }
   return NS_OK;
 }
 
 NS_IMETHODIMP
 ServiceWorkerRegistrationInfo::GetInstallingWorker(nsIServiceWorkerInfo **aResult)
 {
   AssertIsOnMainThread();
   nsCOMPtr<nsIServiceWorkerInfo> info = do_QueryInterface(mInstallingWorker);
@@ -1002,17 +1011,17 @@ protected:
 
     // Ensure that we only surface SecurityErr or TypeErr to script.
     if (aRv.Failed() && !aRv.ErrorCodeIs(NS_ERROR_DOM_SECURITY_ERR) &&
                         !aRv.ErrorCodeIs(NS_ERROR_DOM_TYPE_ERR)) {
 
       // Remove the old error code so we can replace it with a TypeError.
       aRv.SuppressException();
 
-      NS_ConvertUTF8toUTF16 scriptSpec(mRegistration->mScriptSpec);
+      NS_ConvertUTF8toUTF16 scriptSpec(mScriptSpec);
       NS_ConvertUTF8toUTF16 scope(mRegistration->mScope);
 
       // Throw the type error with a generic error message.
       aRv.ThrowTypeError<MSG_SW_INSTALL_ERROR>(scriptSpec, scope);
     }
 
     if (mCallback) {
       mCallback->UpdateFailed(aRv);
@@ -1241,18 +1250,17 @@ public:
     }
 
     if (mJobType == RegisterJob) {
       mRegistration = swm->GetRegistration(mPrincipal, mScope);
 
       if (mRegistration) {
         mRegistration->mPendingUninstall = false;
         RefPtr<ServiceWorkerInfo> newest = mRegistration->Newest();
-        if (newest && mScriptSpec.Equals(newest->ScriptSpec()) &&
-            mScriptSpec.Equals(mRegistration->mScriptSpec)) {
+        if (newest && mScriptSpec.Equals(newest->ScriptSpec())) {
           swm->StoreRegistration(mPrincipal, mRegistration);
           Succeed();
 
           // Done() must always be called async from Start()
           nsCOMPtr<nsIRunnable> runnable =
             NS_NewRunnableMethodWithArg<nsresult>(
               this,
               &ServiceWorkerRegisterJob::Done,
@@ -1260,18 +1268,16 @@ public:
           MOZ_ALWAYS_TRUE(NS_SUCCEEDED(NS_DispatchToCurrentThread(runnable)));
 
           return;
         }
       } else {
         mRegistration = swm->CreateNewRegistration(mScope, mPrincipal);
       }
 
-      mRegistration->mScriptSpec = mScriptSpec;
-      mRegistration->NotifyListenersOnChange();
       swm->StoreRegistration(mPrincipal, mRegistration);
     } else {
       MOZ_ASSERT(mJobType == UpdateJob);
     }
 
     Update();
   }
 
@@ -1298,17 +1304,17 @@ public:
     }
 
     AssertIsOnMainThread();
     Telemetry::Accumulate(Telemetry::SERVICE_WORKER_UPDATED, 1);
 
     RefPtr<ServiceWorkerManager> swm = ServiceWorkerManager::GetInstance();
 
     nsCOMPtr<nsIURI> scriptURI;
-    nsresult rv = NS_NewURI(getter_AddRefs(scriptURI), mRegistration->mScriptSpec);
+    nsresult rv = NS_NewURI(getter_AddRefs(scriptURI), mScriptSpec);
     if (NS_WARN_IF(NS_FAILED(rv))) {
       Fail(NS_ERROR_DOM_SECURITY_ERR);
       return;
     }
     nsCOMPtr<nsIURI> maxScopeURI;
     if (!aMaxScope.IsEmpty()) {
       rv = NS_NewURI(getter_AddRefs(maxScopeURI), aMaxScope,
                      nullptr, scriptURI);
@@ -1348,18 +1354,17 @@ public:
 
     ServiceWorkerManager::RegistrationDataPerPrincipal* data;
     if (!swm->mRegistrationInfos.Get(scopeKey, &data)) {
       return Fail(NS_ERROR_FAILURE);
     }
 
     MOZ_ASSERT(!mUpdateAndInstallInfo);
     mUpdateAndInstallInfo =
-      new ServiceWorkerInfo(mRegistration, mRegistration->mScriptSpec,
-                            aNewCacheName);
+      new ServiceWorkerInfo(mRegistration, mScriptSpec, aNewCacheName);
 
     RefPtr<ServiceWorkerJob> upcasted = this;
     nsMainThreadPtrHandle<nsISupports> handle(
         new nsMainThreadPtrHolder<nsISupports>(upcasted));
     RefPtr<LifeCycleEventCallback> callback = new ContinueUpdateRunnable(handle);
 
     ServiceWorkerPrivate* workerPrivate =
       mUpdateAndInstallInfo->WorkerPrivate();
@@ -1383,17 +1388,17 @@ private:
     RefPtr<ServiceWorkerRegisterJob> kungFuDeathGrip = this;
     if (mCanceled) {
       return Fail(NS_ERROR_DOM_ABORT_ERR);
     }
 
     if (NS_WARN_IF(!aScriptEvaluationResult)) {
       ErrorResult error;
 
-      NS_ConvertUTF8toUTF16 scriptSpec(mRegistration->mScriptSpec);
+      NS_ConvertUTF8toUTF16 scriptSpec(mScriptSpec);
       NS_ConvertUTF8toUTF16 scope(mRegistration->mScope);
       error.ThrowTypeError<MSG_SW_SCRIPT_THREW>(scriptSpec, scope);
       return Fail(error);
     }
 
     RefPtr<ServiceWorkerInstallJob> job =
       new ServiceWorkerInstallJob(mQueue, mCallback, mRegistration,
                                   mUpdateAndInstallInfo, mScriptSpec);
@@ -1434,24 +1439,24 @@ private:
     }
 
     RefPtr<ServiceWorkerInfo> workerInfo = mRegistration->Newest();
     nsAutoString cacheName;
 
     // 9.2.20 If newestWorker is not null, and newestWorker's script url is
     // equal to registration's registering script url and response is a
     // byte-for-byte match with the script resource of newestWorker...
-    if (workerInfo && workerInfo->ScriptSpec().Equals(mRegistration->mScriptSpec)) {
+    if (workerInfo && workerInfo->ScriptSpec().Equals(mScriptSpec)) {
       cacheName = workerInfo->CacheName();
     }
 
     nsresult rv =
       serviceWorkerScriptCache::Compare(mRegistration, mRegistration->mPrincipal, cacheName,
-                                        NS_ConvertUTF8toUTF16(mRegistration->mScriptSpec),
-                                        this, mLoadGroup);
+                                        NS_ConvertUTF8toUTF16(mScriptSpec), this,
+                                        mLoadGroup);
     if (NS_WARN_IF(NS_FAILED(rv))) {
       return Fail(rv);
     }
   }
 
   void
   Done(nsresult aStatus)
   {
@@ -2730,23 +2735,24 @@ ServiceWorkerManager::LoadRegistration(
   if (!principal) {
     return;
   }
 
   RefPtr<ServiceWorkerRegistrationInfo> registration =
     GetRegistration(principal, aRegistration.scope());
   if (!registration) {
     registration = CreateNewRegistration(aRegistration.scope(), principal);
-  } else if (registration->mScriptSpec == aRegistration.scriptSpec() &&
-             !!registration->mActiveWorker == aRegistration.currentWorkerURL().IsEmpty()) {
-    // No needs for updates.
-    return;
-  }
-
-  registration->mScriptSpec = aRegistration.scriptSpec();
+  } else {
+    RefPtr<ServiceWorkerInfo> newest = registration->Newest();
+    if (newest && newest->ScriptSpec() == aRegistration.scriptSpec() &&
+        !!registration->mActiveWorker == aRegistration.currentWorkerURL().IsEmpty()) {
+      // No needs for updates.
+      return;
+    }
+  }
 
   const nsCString& currentWorkerURL = aRegistration.currentWorkerURL();
   if (!currentWorkerURL.IsEmpty()) {
     registration->mActiveWorker =
       new ServiceWorkerInfo(registration, currentWorkerURL,
                             aRegistration.activeCacheName());
     registration->mActiveWorker->SetActivateStateUncheckedWithoutEvent(ServiceWorkerState::Activated);
   }
@@ -3622,19 +3628,16 @@ ServiceWorkerManager::SoftUpdate(const O
   // "Let newestWorker be the result of running Get Newest Worker algorithm
   // passing registration as its argument.
   // If newestWorker is null, abort these steps."
   RefPtr<ServiceWorkerInfo> newest = registration->Newest();
   if (!newest) {
     return;
   }
 
-  // "Set registration's registering script url to newestWorker's script url."
-  registration->mScriptSpec = newest->ScriptSpec();
-
   // "If the registration queue for registration is empty, invoke Update algorithm,
   // or its equivalent, with client, registration as its argument."
   // TODO(catalinb): We don't implement the force bypass cache flag.
   // See: https://github.com/slightlyoff/ServiceWorker/issues/759
   if (!registration->mUpdating) {
     ServiceWorkerJobQueue* queue = GetOrCreateJobQueue(scopeKey, aScope);
     MOZ_ASSERT(queue);
 
@@ -3679,19 +3682,16 @@ ServiceWorkerManager::Update(nsIPrincipa
     aCallback->UpdateFailed(error);
 
     // In case the callback does not consume the exception
     error.SuppressException();
 
     return;
   }
 
-  // "Set registration's registering script url to newestWorker's script url."
-  registration->mScriptSpec = newest->ScriptSpec();
-
   ServiceWorkerJobQueue* queue =
     GetOrCreateJobQueue(scopeKey, aScope);
   MOZ_ASSERT(queue);
 
   // "Invoke Update algorithm, or its equivalent, with client, registration as
   // its argument."
   RefPtr<ServiceWorkerRegisterJob> job =
     new ServiceWorkerRegisterJob(queue, registration, aCallback,
--- a/dom/workers/ServiceWorkerManager.h
+++ b/dom/workers/ServiceWorkerManager.h
@@ -59,19 +59,16 @@ class ServiceWorkerRegistrationInfo fina
 
   virtual ~ServiceWorkerRegistrationInfo();
 
 public:
   NS_DECL_ISUPPORTS
   NS_DECL_NSISERVICEWORKERREGISTRATIONINFO
 
   nsCString mScope;
-  // The scriptURL for the registration. This may be completely different from
-  // the URLs of the following three workers.
-  nsCString mScriptSpec;
 
   nsCOMPtr<nsIPrincipal> mPrincipal;
 
   RefPtr<ServiceWorkerInfo> mActiveWorker;
   RefPtr<ServiceWorkerInfo> mWaitingWorker;
   RefPtr<ServiceWorkerInfo> mInstallingWorker;
 
   nsTArray<nsCOMPtr<nsIServiceWorkerRegistrationInfoListener>> mListeners;
@@ -88,17 +85,17 @@ public:
   // removed since documents may be controlled. It is marked as
   // pendingUninstall and when all controlling documents go away, removed.
   bool mPendingUninstall;
 
   ServiceWorkerRegistrationInfo(const nsACString& aScope,
                                 nsIPrincipal* aPrincipal);
 
   already_AddRefed<ServiceWorkerInfo>
-  Newest()
+  Newest() const
   {
     RefPtr<ServiceWorkerInfo> newest;
     if (mInstallingWorker) {
       newest = mInstallingWorker;
     } else if (mWaitingWorker) {
       newest = mWaitingWorker;
     } else {
       newest = mActiveWorker;
--- a/dom/workers/test/serviceworkers/test_serviceworkerregistrationinfo.xul
+++ b/dom/workers/test/serviceworkers/test_serviceworkerregistrationinfo.xul
@@ -42,63 +42,58 @@
           promise = waitForRegister(EXAMPLE_URL, function (registration) {
             is(registration.scriptSpec, "");
             ok(registration.installingWorker === null);
             ok(registration.waitingWorker === null);
             ok(registration.activeWorker === null);
 
             return waitForServiceWorkerRegistrationChange(registration, function  () {
               is(registration.scriptSpec, EXAMPLE_URL + "worker.js");
+              ok(registration.installingWorker !== null);
+              is(registration.installingWorker.scriptSpec, EXAMPLE_URL + "worker.js");
+              ok(registration.waitingWorker === null);
+              ok(registration.activeWorker === null);
 
               return waitForServiceWorkerRegistrationChange(registration, function () {
-                ok(registration.installingWorker !== null);
-                ok(registration.waitingWorker === null);
+                ok(registration.installingWorker === null);
+                ok(registration.waitingWorker !== null);
                 ok(registration.activeWorker === null);
 
                 return waitForServiceWorkerRegistrationChange(registration, function () {
                   ok(registration.installingWorker === null);
-                  ok(registration.waitingWorker !== null);
-                  ok(registration.activeWorker === null);
+                  ok(registration.waitingWorker === null);
+                  ok(registration.activeWorker !== null);
 
-                  return waitForServiceWorkerRegistrationChange(registration, function () {
-                    ok(registration.installingWorker === null);
-                    ok(registration.waitingWorker === null);
-                    ok(registration.activeWorker !== null);
-
-                    return registration;
-                  });
+                  return registration;
                 });
               });
             });
           });
           iframe.contentWindow.postMessage("register", "*");
           let registration = yield promise;
 
           promise = waitForServiceWorkerRegistrationChange(registration, function () {
             is(registration.scriptSpec, EXAMPLE_URL + "worker2.js");
+            ok(registration.installingWorker !== null);
+            is(registration.installingWorker.scriptSpec, EXAMPLE_URL + "worker2.js");
+            ok(registration.waitingWorker === null);
+            ok(registration.activeWorker !== null);
 
             return waitForServiceWorkerRegistrationChange(registration, function () {
-              ok(registration.installingWorker !== null);
-              ok(registration.waitingWorker === null);
+              ok(registration.installingWorker === null);
+              ok(registration.waitingWorker !== null);
               ok(registration.activeWorker !== null);
 
               return waitForServiceWorkerRegistrationChange(registration, function () {
                 ok(registration.installingWorker === null);
-                ok(registration.waitingWorker !== null);
+                ok(registration.waitingWorker === null);
                 ok(registration.activeWorker !== null);
 
-                return waitForServiceWorkerRegistrationChange(registration, function () {
-                  ok(registration.installingWorker === null);
-                  ok(registration.waitingWorker === null);
-                  ok(registration.activeWorker !== null);
-
-                  return registration;
-                });
+                return registration;
               });
-
             });
           });
           iframe.contentWindow.postMessage("register", "*");
           yield promise;
 
           SimpleTest.finish();
         });
       });