Bug 1217367 - Service workers update algorithm optimization. r=bkelly

dom/workers/ServiceWorkerManager.cpp

@@ -865,7 +865,7 @@ class ServiceWorkerRegisterJob final : public ServiceWorkerJob,
   nsCString mScope;
   nsCString mScriptSpec;
   RefPtr<ServiceWorkerRegistrationInfo> mRegistration;
-  RefPtr<ServiceWorkerUpdateFinishCallback> mCallback;
+  nsTArray<RefPtr<ServiceWorkerUpdateFinishCallback>> mCallbacks;
   nsCOMPtr<nsIPrincipal> mPrincipal;
   RefPtr<ServiceWorkerInfo> mUpdateAndInstallInfo;
   nsCOMPtr<nsILoadGroup> mLoadGroup;
@@ -894,13 +894,16 @@ public:
     : ServiceWorkerJob(aQueue)
     , mScope(aScope)
     , mScriptSpec(aScriptSpec)
-    , mCallback(aCallback)
     , mPrincipal(aPrincipal)
     , mLoadGroup(aLoadGroup)
     , mJobType(REGISTER_JOB)
     , mCanceled(false)
   {
+    AssertIsOnMainThread();
     MOZ_ASSERT(mLoadGroup);
+    MOZ_ASSERT(aCallback);
+
+    mCallbacks.AppendElement(aCallback);
   }
 
   // [[Update]]
@@ -909,10 +912,14 @@ public:
                            ServiceWorkerUpdateFinishCallback* aCallback)
     : ServiceWorkerJob(aQueue)
     , mRegistration(aRegistration)
-    , mCallback(aCallback)
     , mJobType(UPDATE_JOB)
     , mCanceled(false)
-  { }
+  {
+    AssertIsOnMainThread();
+    MOZ_ASSERT(aCallback);
+
+    mCallbacks.AppendElement(aCallback);
+  }
 
   bool
   IsRegisterJob() const override
@@ -920,6 +927,16 @@ public:
     return true;
   }
 
+  void
+  AppendCallback(ServiceWorkerUpdateFinishCallback* aCallback)
+  {
+    AssertIsOnMainThread();
+    MOZ_ASSERT(aCallback);
+    MOZ_ASSERT(!mCallbacks.Contains(aCallback));
+
+    mCallbacks.AppendElement(aCallback);
+  }
+
   void
   Cancel()
   {
@@ -971,6 +988,10 @@ public:
       swm->StoreRegistration(mPrincipal, mRegistration);
     } else {
       MOZ_ASSERT(mJobType == UPDATE_JOB);
+      MOZ_ASSERT(mRegistration);
+      MOZ_ASSERT(mRegistration->mUpdateJob == nullptr);
+
+      mRegistration->mUpdateJob = this;
     }
 
     Update();
@@ -1101,8 +1122,9 @@ public:
   void
   Fail(ErrorResult& aRv)
   {
-    MOZ_ASSERT(mCallback);
-    RefPtr<ServiceWorkerUpdateFinishCallback> callback = mCallback.forget();
+    AssertIsOnMainThread();
+    MOZ_ASSERT(mCallbacks.Length());
+
     // With cancellation support, we may only be running with one reference
     // from another object like a stream loader or something.
     RefPtr<ServiceWorkerRegisterJob> kungFuDeathGrip = this;
@@ -1135,7 +1157,14 @@ public:
       aRv.ThrowTypeError<MSG_SW_INSTALL_ERROR>(&scriptSpec, &scope);
     }
 
-    callback->UpdateFailed(aRv);
+    for (uint32_t i = 1; i < mCallbacks.Length(); ++i) {
+      ErrorResult rv;
+      aRv.CloneTo(rv);
+      mCallbacks[i]->UpdateFailed(rv);
+      rv.SuppressException();
+    }
+
+    mCallbacks[0]->UpdateFailed(aRv);
 
     // In case the callback does not consume the exception
     aRv.SuppressException();
@@ -1152,6 +1181,7 @@ public:
     RefPtr<ServiceWorkerManager> swm = ServiceWorkerManager::GetInstance();
     swm->MaybeRemoveRegistration(mRegistration);
     // Ensures that the job can't do anything useful from this point on.
+    mRegistration->mUpdateJob = nullptr;
     mRegistration = nullptr;
     Done(origStatus);
   }
@@ -1294,9 +1324,13 @@ private:
   void
   Succeed()
   {
-    MOZ_ASSERT(mCallback);
-    mCallback->UpdateSucceeded(mRegistration);
-    mCallback = nullptr;
+    AssertIsOnMainThread();
+    MOZ_ASSERT(mCallbacks.Length());
+
+    for (uint32_t i = 0; i < mCallbacks.Length(); ++i) {
+      mCallbacks[i]->UpdateSucceeded(mRegistration);
+    }
+    mCallbacks.Clear();
   }
 
   void
@@ -1350,6 +1384,18 @@ private:
     // Activate() is invoked out of band of atomic.
     mRegistration->TryToActivate();
   }
+
+  void
+  Done(nsresult aStatus)
+  {
+    ServiceWorkerJob::Done(aStatus);
+
+    if (mJobType == UPDATE_JOB && mRegistration) {
+      MOZ_ASSERT(NS_IsMainThread());
+      MOZ_ASSERT(mRegistration->mUpdateJob);
+      mRegistration->mUpdateJob = nullptr;
+    }
+  }
 };
 
 NS_IMPL_ISUPPORTS_INHERITED0(ServiceWorkerRegisterJob, ServiceWorkerJob);
@@ -2620,6 +2666,23 @@ ServiceWorkerRegistrationInfo::NotifyListenersOnChange()
   }
 }
 
+bool
+ServiceWorkerRegistrationInfo::IsUpdating() const
+{
+  MOZ_ASSERT(NS_IsMainThread());
+  return mUpdateJob != nullptr;
+}
+
+void
+ServiceWorkerRegistrationInfo::AppendUpdateCallback(ServiceWorkerUpdateFinishCallback* aCallback)
+{
+  MOZ_ASSERT(NS_IsMainThread());
+  MOZ_ASSERT(aCallback);
+  MOZ_ASSERT(mUpdateJob);
+
+  mUpdateJob->AppendCallback(aCallback);
+}
+
 void
 ServiceWorkerManager::LoadRegistration(
                              const ServiceWorkerRegistrationData& aRegistration)
@@ -3551,9 +3614,17 @@ ServiceWorkerManager::SoftUpdate(const nsACString& aScopeKey,
 
   // "Invoke Update algorithm, or its equivalent, with client, registration as
   // its argument."
-  RefPtr<ServiceWorkerRegisterJob> job =
-    new ServiceWorkerRegisterJob(queue, registration, cb);
-  queue->Append(job);
+  if (registration->IsUpdating()) {
+    // This is used to reduce burst of update events. If there is an update
+    // job in queue when we try to create a new one, drop current one and
+    // merge the callback function to existing update job.
+    // See. https://github.com/slightlyoff/ServiceWorker/issues/759
+    registration->AppendUpdateCallback(cb);
+  } else {
+    RefPtr<ServiceWorkerRegisterJob> job =
+      new ServiceWorkerRegisterJob(queue, registration, cb);
+    queue->Append(job);
+  }
 }
 
 namespace {

dom/workers/ServiceWorkerManager.h

@@ -47,9 +47,11 @@ class ServiceWorker;
 class ServiceWorkerClientInfo;
 class ServiceWorkerInfo;
 class ServiceWorkerJob;
+class ServiceWorkerRegisterJob;
 class ServiceWorkerJobQueue;
 class ServiceWorkerManagerChild;
 class ServiceWorkerPrivate;
+class ServiceWorkerUpdateFinishCallback;
 
 class ServiceWorkerRegistrationInfo final : public nsIServiceWorkerRegistrationInfo
 {
@@ -76,6 +78,8 @@ public:
 
   uint64_t mLastUpdateCheckTime;
 
+  RefPtr<ServiceWorkerRegisterJob> mUpdateJob;
+
   // When unregister() is called on a registration, it is not immediately
   // removed since documents may be controlled. It is marked as
   // pendingUninstall and when all controlling documents go away, removed.
@@ -143,6 +147,12 @@ public:
 
   void
   NotifyListenersOnChange();
+
+  bool
+  IsUpdating() const;
+
+  void
+  AppendUpdateCallback(ServiceWorkerUpdateFinishCallback* aCallback);
 };
 
 class ServiceWorkerUpdateFinishCallback

testing/web-platform/mozilla/tests/service-workers/service-worker/update-after-oneday.https.html

@@ -11,11 +11,11 @@
 promise_test(function(t) {
     var script = 'resources/update-nocookie-worker.py';
     var scope = 'resources/update/update-after-oneday.https.html';
-    var parsed_url = normalizeURL(script);
+    var expected_url = normalizeURL(script);
     var registration;
     var frame;
 
-    return service_worker_unregister_and_register(t, parsed_url, scope)
+    return service_worker_unregister_and_register(t, expected_url, scope)
       .then(function(r) {
           registration = r;
           return wait_for_state(t, registration.installing, 'activated');
@@ -24,8 +24,17 @@ promise_test(function(t) {
       .then(function(f) {
           frame = f;
           return wait_for_update(t, registration);
-       })
-       .then(function() {
+        })
+      .then(function() {
+          assert_equals(registration.installing.scriptURL, expected_url,
+                        'new installing should be set after update resolves.');
+          assert_equals(registration.waiting, null,
+                        'waiting should still be null after update resolves.');
+          assert_equals(registration.active.scriptURL, expected_url,
+                        'active should still exist after update found.');
+          return wait_for_state(t, registration.installing, 'installed');
+        })
+      .then(function() {
           // Trigger a non-navigation fetch event
           frame.contentWindow.load_image(normalizeURL('resources/update/dummy'));
           return wait_for_update(t, registration);