Bug 1189659 - Part 2 - Remove set of scopes being updated from ServiceWorkerManager. r=bkelly
authorCatalin Badea <catalin.badea392@gmail.com>
Thu, 26 Nov 2015 19:03:10 +0200
changeset 274368 8a417a349a612cf299b8618acad7413aefa9b6ec
parent 274367 48ea5c3f22148791466a906664376eeb6adcbe6a
child 274369 a4376ddc87b5642d3898ca702001302976981c55
push id68551
push usercatalin.badea392@gmail.com
push dateThu, 26 Nov 2015 17:03:30 +0000
treeherdermozilla-inbound@9424e969d623 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbkelly
bugs1189659
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 1189659 - Part 2 - Remove set of scopes being updated from ServiceWorkerManager. r=bkelly
dom/workers/ServiceWorkerManager.cpp
dom/workers/ServiceWorkerPrivate.cpp
dom/workers/ServiceWorkerPrivate.h
--- a/dom/workers/ServiceWorkerManager.cpp
+++ b/dom/workers/ServiceWorkerManager.cpp
@@ -128,18 +128,16 @@ struct ServiceWorkerManager::Registratio
   nsTArray<nsCString> mOrderedScopes;
 
   // Scope to registration.
   // The scope should be a fully qualified valid URL.
   nsRefPtrHashtable<nsCStringHashKey, ServiceWorkerRegistrationInfo> mInfos;
 
   // Maps scopes to job queues.
   nsClassHashtable<nsCStringHashKey, ServiceWorkerJobQueue> mJobQueues;
-
-  nsDataHashtable<nsCStringHashKey, bool> mSetOfScopesBeingUpdated;
 };
 
 struct ServiceWorkerManager::PendingOperation final
 {
   nsCOMPtr<nsIRunnable> mRunnable;
 
   ServiceWorkerJobQueue* mQueue;
   RefPtr<ServiceWorkerJob> mJob;
@@ -639,26 +637,34 @@ public:
 
   void
   UpdateFailed(ErrorResult& aStatus) override
   {
     mPromise->MaybeReject(aStatus);
   }
 };
 
-class ContinueUpdateRunnable final : public nsRunnable
+class ContinueUpdateRunnable final : public LifeCycleEventCallback
 {
   nsMainThreadPtrHandle<nsISupports> mJob;
+  bool mScriptEvaluationResult;
 public:
   explicit ContinueUpdateRunnable(const nsMainThreadPtrHandle<nsISupports> aJob)
     : mJob(aJob)
+    , mScriptEvaluationResult(false)
   {
     AssertIsOnMainThread();
   }
 
+  void
+  SetResult(bool aResult)
+  {
+    mScriptEvaluationResult = aResult;
+  }
+
   NS_IMETHOD Run();
 };
 
 namespace {
 
 /**
  * The spec mandates slightly different behaviors for computing the scope
  * prefix string in case a Service-Worker-Allowed header is specified versus
@@ -1050,58 +1056,35 @@ public:
       return Fail(NS_ERROR_FAILURE);
     }
 
     ServiceWorkerManager::RegistrationDataPerPrincipal* data;
     if (!swm->mRegistrationInfos.Get(scopeKey, &data)) {
       return Fail(NS_ERROR_FAILURE);
     }
 
-    nsAutoString cacheName;
-    // We have to create a ServiceWorker here simply to ensure there are no
-    // errors. Ideally we should just pass this worker on to ContinueInstall.
-    MOZ_ASSERT(!data->mSetOfScopesBeingUpdated.Contains(mRegistration->mScope));
-    data->mSetOfScopesBeingUpdated.Put(mRegistration->mScope, true);
-
-    // Call FailScopeUpdate on main thread if the SW script load fails below.
-    nsCOMPtr<nsIRunnable> failRunnable = NS_NewRunnableMethodWithArgs
-      <StorensRefPtrPassByPtr<ServiceWorkerManager>, nsCString>
-      (this, &ServiceWorkerRegisterJob::FailScopeUpdate, swm, scopeKey);
-
     MOZ_ASSERT(!mUpdateAndInstallInfo);
     mUpdateAndInstallInfo =
       new ServiceWorkerInfo(mRegistration, mRegistration->mScriptSpec,
                             aNewCacheName);
 
     RefPtr<ServiceWorkerJob> upcasted = this;
     nsMainThreadPtrHandle<nsISupports> handle(
         new nsMainThreadPtrHolder<nsISupports>(upcasted));
-    RefPtr<nsRunnable> callback = new ContinueUpdateRunnable(handle);
+    RefPtr<LifeCycleEventCallback> callback = new ContinueUpdateRunnable(handle);
 
     ServiceWorkerPrivate* workerPrivate =
       mUpdateAndInstallInfo->WorkerPrivate();
-    rv = workerPrivate->ContinueOnSuccessfulScriptEvaluation(callback);
+    rv = workerPrivate->CheckScriptEvaluation(callback);
 
     if (NS_WARN_IF(NS_FAILED(rv))) {
-      return FailScopeUpdate(swm, scopeKey);
+      Fail(NS_ERROR_DOM_ABORT_ERR);
     }
   }
 
-  void
-  FailScopeUpdate(ServiceWorkerManager* aSwm, const nsACString& aScopeKey)
-  {
-    AssertIsOnMainThread();
-    MOZ_ASSERT(aSwm);
-    ServiceWorkerManager::RegistrationDataPerPrincipal* data;
-    if (aSwm->mRegistrationInfos.Get(aScopeKey, &data)) {
-      data->mSetOfScopesBeingUpdated.Remove(aScopeKey);
-    }
-    Fail(NS_ERROR_DOM_ABORT_ERR);
-  }
-
   // 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.
   // Callers MUST hold a strong ref before calling this!
   void
   Fail(ErrorResult& aRv)
   {
     AssertIsOnMainThread();
@@ -1163,59 +1146,48 @@ public:
 
   void
   Fail(nsresult aRv)
   {
     ErrorResult rv(aRv);
     Fail(rv);
   }
 
-  // Public so our error handling code can continue with a successful worker.
+private:
   void
-  ContinueInstall()
+  ContinueInstall(bool aScriptEvaluationResult)
   {
     AssertIsOnMainThread();
-    // mRegistration will be null if we have already Fail()ed.
-    if (!mRegistration) {
-      return;
-    }
-
+    MOZ_ASSERT(mRegistration);
     mRegistration->mUpdating = false;
 
-    // Even if we are canceled, ensure integrity of mSetOfScopesBeingUpdated
-    // first.
-    RefPtr<ServiceWorkerManager> swm = ServiceWorkerManager::GetInstance();
-
-    nsAutoCString scopeKey;
-    nsresult rv = swm->PrincipalToScopeKey(mRegistration->mPrincipal, scopeKey);
-    if (NS_WARN_IF(NS_FAILED(rv))) {
-      return Fail(NS_ERROR_FAILURE);
-    }
-
-    ServiceWorkerManager::RegistrationDataPerPrincipal* data;
-    if (!swm->mRegistrationInfos.Get(scopeKey, &data)) {
-      return Fail(NS_ERROR_FAILURE);
-    }
-
-    MOZ_ASSERT(data->mSetOfScopesBeingUpdated.Contains(mRegistration->mScope));
-    data->mSetOfScopesBeingUpdated.Remove(mRegistration->mScope);
     // This is effectively the end of Step 4.3 of the [[Update]] algorithm.
     // The invocation of [[Install]] is not part of the atomic block.
-
     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 scope(mRegistration->mScope);
+      error.ThrowTypeError<MSG_SW_SCRIPT_THREW>(scriptSpec, scope);
+      return Fail(error);
+    }
+
     // Begin [[Install]] atomic step 4.
     if (mRegistration->mInstallingWorker) {
       mRegistration->mInstallingWorker->UpdateState(ServiceWorkerState::Redundant);
       mRegistration->mInstallingWorker->WorkerPrivate()->TerminateWorker();
     }
 
+    RefPtr<ServiceWorkerManager> swm = ServiceWorkerManager::GetInstance();
+
     swm->InvalidateServiceWorkerRegistrationWorker(mRegistration,
                                                    WhichServiceWorker::INSTALLING_WORKER);
 
     mRegistration->mInstallingWorker = mUpdateAndInstallInfo.forget();
     mRegistration->mInstallingWorker->UpdateState(ServiceWorkerState::Installing);
     mRegistration->NotifyListenersOnChange();
 
     Succeed();
@@ -1246,17 +1218,16 @@ public:
     rv = workerPrivate->SendLifeCycleEvent(NS_LITERAL_STRING("install"),
                                            callback, failRunnable);
 
     if (NS_WARN_IF(NS_FAILED(rv))) {
       ContinueAfterInstallEvent(false /* aSuccess */);
     }
   }
 
-private:
   void
   Update()
   {
     AssertIsOnMainThread();
 
     // Since Update() is called synchronously from Start(), we can assert this.
     MOZ_ASSERT(!mCanceled);
     MOZ_ASSERT(mRegistration);
@@ -1404,17 +1375,17 @@ ServiceWorkerJobQueue::CancelJobs()
 }
 
 NS_IMETHODIMP
 ContinueUpdateRunnable::Run()
 {
   AssertIsOnMainThread();
   RefPtr<ServiceWorkerJob> job = static_cast<ServiceWorkerJob*>(mJob.get());
   RefPtr<ServiceWorkerRegisterJob> upjob = static_cast<ServiceWorkerRegisterJob*>(job.get());
-  upjob->ContinueInstall();
+  upjob->ContinueInstall(mScriptEvaluationResult);
   return NS_OK;
 }
 
 void
 ContinueInstallTask::ContinueAfterWorkerEvent(bool aSuccess)
 {
   // This does not start the job immediately if there are other jobs in the
   // queue, which captures the "atomic" behaviour we want.
@@ -2565,38 +2536,16 @@ ServiceWorkerManager::HandleError(JSCont
     return;
   }
 
   ServiceWorkerManager::RegistrationDataPerPrincipal* data;
   if (NS_WARN_IF(!mRegistrationInfos.Get(scopeKey, &data))) {
     return;
   }
 
-  // If this is a failure, we may need to cancel an in-progress registration.
-  if (!JSREPORT_IS_WARNING(aFlags) &&
-      data->mSetOfScopesBeingUpdated.Contains(aScope)) {
-
-    data->mSetOfScopesBeingUpdated.Remove(aScope);
-
-    ServiceWorkerJobQueue* queue = data->mJobQueues.Get(aScope);
-    MOZ_ASSERT(queue);
-
-    ServiceWorkerJob* job = queue->Peek();
-    if (job) {
-      MOZ_ASSERT(job->IsRegisterJob());
-      RefPtr<ServiceWorkerRegisterJob> regJob =
-        static_cast<ServiceWorkerRegisterJob*>(job);
-
-      ErrorResult rv;
-      NS_ConvertUTF8toUTF16 scope(aScope);
-      rv.ThrowTypeError<MSG_SW_SCRIPT_THREW>(aWorkerURL, scope);
-      regJob->Fail(rv);
-    }
-  }
-
   // Always report any uncaught exceptions or errors to the console of
   // each client.
   ReportToAllClients(aScope, aMessage, aFilename, aLine, aLineNumber,
                      aColumnNumber, aFlags);
 }
 
 void
 ServiceWorkerRegistrationInfo::FinishActivate(bool aSuccess)
--- a/dom/workers/ServiceWorkerPrivate.cpp
+++ b/dom/workers/ServiceWorkerPrivate.cpp
@@ -99,56 +99,74 @@ ServiceWorkerPrivate::SendMessageEvent(J
   return rv.StealNSResult();
 }
 
 namespace {
 
 class CheckScriptEvaluationWithCallback final : public WorkerRunnable
 {
   nsMainThreadPtrHandle<KeepAliveToken> mKeepAliveToken;
-  RefPtr<nsRunnable> mCallback;
+  RefPtr<LifeCycleEventCallback> mCallback;
+  DebugOnly<bool> mDone;
 
 public:
   CheckScriptEvaluationWithCallback(WorkerPrivate* aWorkerPrivate,
                                     KeepAliveToken* aKeepAliveToken,
-                                    nsRunnable* aCallback)
+                                    LifeCycleEventCallback* aCallback)
     : WorkerRunnable(aWorkerPrivate, WorkerThreadModifyBusyCount)
     , mKeepAliveToken(new nsMainThreadPtrHolder<KeepAliveToken>(aKeepAliveToken))
     , mCallback(aCallback)
+    , mDone(false)
   {
     AssertIsOnMainThread();
   }
 
+  ~CheckScriptEvaluationWithCallback()
+  {
+    MOZ_ASSERT(mDone);
+  }
+
   bool
   WorkerRun(JSContext* aCx, WorkerPrivate* aWorkerPrivate) override
   {
     aWorkerPrivate->AssertIsOnWorkerThread();
-    if (aWorkerPrivate->WorkerScriptExecutedSuccessfully()) {
-      nsresult rv = NS_DispatchToMainThread(mCallback);
-      if (NS_FAILED(rv)) {
-        NS_WARNING("Failed to dispatch CheckScriptEvaluation callback.");
-      }
-    }
+    Done(aWorkerPrivate->WorkerScriptExecutedSuccessfully());
 
     return true;
   }
+
+  NS_IMETHOD
+  Cancel() override
+  {
+    Done(false);
+    return WorkerRunnable::Cancel();
+  }
+
+private:
+  void
+  Done(bool aResult)
+  {
+    mDone = true;
+    mCallback->SetResult(aResult);
+    MOZ_ALWAYS_TRUE(NS_SUCCEEDED(NS_DispatchToMainThread(mCallback)));
+  }
 };
 
 } // anonymous namespace
 
 nsresult
-ServiceWorkerPrivate::ContinueOnSuccessfulScriptEvaluation(nsRunnable* aCallback)
+ServiceWorkerPrivate::CheckScriptEvaluation(LifeCycleEventCallback* aCallback)
 {
   nsresult rv = SpawnWorkerIfNeeded(LifeCycleEvent, nullptr);
   NS_ENSURE_SUCCESS(rv, rv);
 
   MOZ_ASSERT(mKeepAliveToken);
   RefPtr<WorkerRunnable> r = new CheckScriptEvaluationWithCallback(mWorkerPrivate,
-                                                                     mKeepAliveToken,
-                                                                     aCallback);
+                                                                   mKeepAliveToken,
+                                                                   aCallback);
   AutoJSAPI jsapi;
   jsapi.Init();
   if (NS_WARN_IF(!r->Dispatch(jsapi.cx()))) {
     return NS_ERROR_FAILURE;
   }
 
   return NS_OK;
 }
--- a/dom/workers/ServiceWorkerPrivate.h
+++ b/dom/workers/ServiceWorkerPrivate.h
@@ -67,21 +67,19 @@ public:
   explicit ServiceWorkerPrivate(ServiceWorkerInfo* aInfo);
 
   nsresult
   SendMessageEvent(JSContext* aCx, JS::Handle<JS::Value> aMessage,
                    const Optional<Sequence<JS::Value>>& aTransferable,
                    UniquePtr<ServiceWorkerClientInfo>&& aClientInfo);
 
   // This is used to validate the worker script and continue the installation
-  // process. Note that the callback is dispatched to the main thread
-  // ONLY if the evaluation was successful. Failure is handled by the JS
-  // exception handler which will call ServiceWorkerManager::HandleError.
+  // process.
   nsresult
-  ContinueOnSuccessfulScriptEvaluation(nsRunnable* aCallback);
+  CheckScriptEvaluation(LifeCycleEventCallback* aCallback);
 
   nsresult
   SendLifeCycleEvent(const nsAString& aEventType,
                      LifeCycleEventCallback* aCallback,
                      nsIRunnable* aLoadFailure);
 
   nsresult
   SendPushEvent(const Maybe<nsTArray<uint8_t>>& aData,