Backed out changeset bc9a43cbbdfa (bug 1227932)
authorCarsten "Tomcat" Book <cbook@mozilla.com>
Thu, 26 Nov 2015 12:02:57 +0100
changeset 308468 8680266bbc0a5bb4b3508de5065b57ae1cfb7e49
parent 308467 d10c07e6542a79b71cd08859e1df1f500a519972
child 308469 2b986da70ee7a1ebdbd8f95c1f024649638b2fbe
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)
bugs1227932
milestone45.0a1
backs outbc9a43cbbdfad05a533c43dcecb59527e3b22b01
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
Backed out changeset bc9a43cbbdfa (bug 1227932)
dom/workers/ServiceWorkerManager.cpp
dom/workers/ServiceWorkerManager.h
dom/workers/ServiceWorkerManagerChild.cpp
dom/workers/ServiceWorkerRegistration.cpp
--- a/dom/workers/ServiceWorkerManager.cpp
+++ b/dom/workers/ServiceWorkerManager.cpp
@@ -370,17 +370,16 @@ ServiceWorkerRegistrationInfo::Clear()
 }
 
 ServiceWorkerRegistrationInfo::ServiceWorkerRegistrationInfo(const nsACString& aScope,
                                                              nsIPrincipal* aPrincipal)
   : mControlledDocumentsCounter(0)
   , mScope(aScope)
   , mPrincipal(aPrincipal)
   , mLastUpdateCheckTime(0)
-  , mUpdating(false)
   , mPendingUninstall(false)
 {}
 
 ServiceWorkerRegistrationInfo::~ServiceWorkerRegistrationInfo()
 {
   if (IsControllingDocuments()) {
     NS_WARNING("ServiceWorkerRegistrationInfo is still controlling documents. This can be a bug or a leak in ServiceWorker API or in any other API that takes the document alive.");
   }
@@ -859,17 +858,17 @@ private:
 class ServiceWorkerRegisterJob final : public ServiceWorkerJob,
                                        public serviceWorkerScriptCache::CompareCallback
 {
   friend class ContinueInstallTask;
 
   nsCString mScope;
   nsCString mScriptSpec;
   RefPtr<ServiceWorkerRegistrationInfo> mRegistration;
-  RefPtr<ServiceWorkerUpdateFinishCallback> mCallback;
+  nsTArray<RefPtr<ServiceWorkerUpdateFinishCallback>> mCallbacks;
   nsCOMPtr<nsIPrincipal> mPrincipal;
   RefPtr<ServiceWorkerInfo> mUpdateAndInstallInfo;
   nsCOMPtr<nsILoadGroup> mLoadGroup;
 
   ~ServiceWorkerRegisterJob()
   {}
 
   enum
@@ -888,47 +887,60 @@ public:
                            const nsCString& aScope,
                            const nsCString& aScriptSpec,
                            ServiceWorkerUpdateFinishCallback* aCallback,
                            nsIPrincipal* aPrincipal,
                            nsILoadGroup* aLoadGroup)
     : 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]]
   ServiceWorkerRegisterJob(ServiceWorkerJobQueue* aQueue,
                            ServiceWorkerRegistrationInfo* aRegistration,
                            ServiceWorkerUpdateFinishCallback* aCallback)
     : ServiceWorkerJob(aQueue)
     , mRegistration(aRegistration)
-    , mCallback(aCallback)
     , mJobType(UPDATE_JOB)
     , mCanceled(false)
   {
     AssertIsOnMainThread();
+    MOZ_ASSERT(aCallback);
+
+    mCallbacks.AppendElement(aCallback);
   }
 
   bool
   IsRegisterJob() const override
   {
     return true;
   }
 
   void
+  AppendCallback(ServiceWorkerUpdateFinishCallback* aCallback)
+  {
+    AssertIsOnMainThread();
+    MOZ_ASSERT(aCallback);
+    MOZ_ASSERT(!mCallbacks.Contains(aCallback));
+
+    mCallbacks.AppendElement(aCallback);
+  }
+
+  void
   Cancel()
   {
     mQueue = nullptr;
     mCanceled = true;
   }
 
   void
   Start() override
@@ -969,16 +981,20 @@ public:
         mRegistration = swm->CreateNewRegistration(mScope, mPrincipal);
       }
 
       mRegistration->mScriptSpec = mScriptSpec;
       mRegistration->NotifyListenersOnChange();
       swm->StoreRegistration(mPrincipal, mRegistration);
     } else {
       MOZ_ASSERT(mJobType == UPDATE_JOB);
+      MOZ_ASSERT(mRegistration);
+      MOZ_ASSERT(mRegistration->mUpdateJob == nullptr);
+
+      mRegistration->mUpdateJob = this;
     }
 
     Update();
   }
 
   void
   ComparisonResult(nsresult aStatus, bool aInCacheAndEqual,
                    const nsAString& aNewCacheName,
@@ -1100,16 +1116,18 @@ public:
   // 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();
+    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;
 
     // Save off the plain error code to pass to Done() where its logged to
     // stderr as a warning.
     nsresult origStatus = static_cast<nsresult>(aRv.ErrorCodeAsInt());
 
@@ -1132,59 +1150,61 @@ public:
         CopyUTF8toUTF16(mScriptSpec, scriptSpec);
         CopyUTF8toUTF16(mScope, scope);
       }
 
       // Throw the type error with a generic error message.
       aRv.ThrowTypeError<MSG_SW_INSTALL_ERROR>(scriptSpec, scope);
     }
 
-    if (mCallback) {
-      mCallback->UpdateFailed(aRv);
-      mCallback = nullptr;
+    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();
 
     mUpdateAndInstallInfo = nullptr;
     if (mRegistration->mInstallingWorker) {
       nsresult rv = serviceWorkerScriptCache::PurgeCache(mRegistration->mPrincipal,
                                                          mRegistration->mInstallingWorker->CacheName());
       if (NS_FAILED(rv)) {
         NS_WARNING("Failed to purge the installing worker cache.");
       }
     }
 
     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);
   }
 
   void
   Fail(nsresult aRv)
   {
     ErrorResult rv(aRv);
     Fail(rv);
   }
 
   // Public so our error handling code can continue with a successful worker.
   void
   ContinueInstall()
   {
-    AssertIsOnMainThread();
     // mRegistration will be null if we have already Fail()ed.
     if (!mRegistration) {
       return;
     }
 
-    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);
@@ -1250,26 +1270,22 @@ public:
       ContinueAfterInstallEvent(false /* aSuccess */);
     }
   }
 
 private:
   void
   Update()
   {
-    AssertIsOnMainThread();
-
     // Since Update() is called synchronously from Start(), we can assert this.
     MOZ_ASSERT(!mCanceled);
     MOZ_ASSERT(mRegistration);
     nsCOMPtr<nsIRunnable> r =
       NS_NewRunnableMethod(this, &ServiceWorkerRegisterJob::ContinueUpdate);
     NS_DispatchToMainThread(r);
-
-    mRegistration->mUpdating = true;
   }
 
   // Aspects of (actually the whole algorithm) of [[Update]] after
   // "Run the following steps in parallel."
   void
   ContinueUpdate()
   {
     AssertIsOnMainThread();
@@ -1302,21 +1318,22 @@ private:
       return Fail(rv);
     }
   }
 
   void
   Succeed()
   {
     AssertIsOnMainThread();
-    // We don't have a callback for soft updates.
-    if (mCallback) {
-      mCallback->UpdateSucceeded(mRegistration);
-      mCallback = nullptr;
+    MOZ_ASSERT(mCallbacks.Length());
+
+    for (uint32_t i = 0; i < mCallbacks.Length(); ++i) {
+      mCallbacks[i]->UpdateSucceeded(mRegistration);
     }
+    mCallbacks.Clear();
   }
 
   void
   ContinueAfterInstallEvent(bool aInstallEventSuccess)
   {
     if (mCanceled) {
       return Done(NS_ERROR_DOM_ABORT_ERR);
     }
@@ -1364,23 +1381,23 @@ private:
     Done(NS_OK);
     // Activate() is invoked out of band of atomic.
     mRegistration->TryToActivate();
   }
 
   void
   Done(nsresult aStatus)
   {
-    AssertIsOnMainThread();
-
-    if (mRegistration) {
-      mRegistration->mUpdating = false;
+    ServiceWorkerJob::Done(aStatus);
+
+    if (mJobType == UPDATE_JOB && mRegistration) {
+      MOZ_ASSERT(NS_IsMainThread());
+      MOZ_ASSERT(mRegistration->mUpdateJob);
+      mRegistration->mUpdateJob = nullptr;
     }
-
-    ServiceWorkerJob::Done(aStatus);
   }
 };
 
 NS_IMPL_ISUPPORTS_INHERITED0(ServiceWorkerRegisterJob, ServiceWorkerJob);
 
 void
 ServiceWorkerJobQueue::CancelJobs()
 {
@@ -2642,16 +2659,33 @@ void
 ServiceWorkerRegistrationInfo::NotifyListenersOnChange()
 {
   nsTArray<nsCOMPtr<nsIServiceWorkerRegistrationInfoListener>> listeners(mListeners);
   for (size_t index = 0; index < listeners.Length(); ++index) {
     listeners[index]->OnChange();
   }
 }
 
+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)
 {
   AssertIsOnMainThread();
 
   nsCOMPtr<nsIPrincipal> principal =
     PrincipalInfoToPrincipal(aRegistration.principal());
@@ -3488,25 +3522,65 @@ ServiceWorkerManager::InvalidateServiceW
 
     if (utf8Scope.Equals(aRegistration->mScope)) {
       target->InvalidateWorkers(aWhichOnes);
     }
   }
 }
 
 void
+ServiceWorkerManager::SoftUpdate(nsIPrincipal* aPrincipal,
+                                 const nsACString& aScope,
+                                 ServiceWorkerUpdateFinishCallback* aCallback)
+{
+  MOZ_ASSERT(aPrincipal);
+
+  nsAutoCString scopeKey;
+  nsresult rv = PrincipalToScopeKey(aPrincipal, scopeKey);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return;
+  }
+
+  SoftUpdate(scopeKey, aScope, aCallback);
+}
+
+void
 ServiceWorkerManager::SoftUpdate(const OriginAttributes& aOriginAttributes,
-                                 const nsACString& aScope)
+                                 const nsACString& aScope,
+                                 ServiceWorkerUpdateFinishCallback* aCallback)
 {
-  AssertIsOnMainThread();
   nsAutoCString scopeKey;
   aOriginAttributes.CreateSuffix(scopeKey);
-
+  SoftUpdate(scopeKey, aScope, aCallback);
+}
+
+namespace {
+
+// Empty callback.  Only use when you really want to ignore errors.
+class EmptyUpdateFinishCallback final : public ServiceWorkerUpdateFinishCallback
+{
+public:
+  void
+  UpdateSucceeded(ServiceWorkerRegistrationInfo* aInfo) override
+  {}
+
+  void
+  UpdateFailed(ErrorResult& aStatus) override
+  {}
+};
+
+} // anonymous namespace
+
+void
+ServiceWorkerManager::SoftUpdate(const nsACString& aScopeKey,
+                                 const nsACString& aScope,
+                                 ServiceWorkerUpdateFinishCallback* aCallback)
+{
   RefPtr<ServiceWorkerRegistrationInfo> registration =
-    GetRegistration(scopeKey, aScope);
+    GetRegistration(aScopeKey, aScope);
   if (NS_WARN_IF(!registration)) {
     return;
   }
 
   // "If registration's uninstalling flag is set, abort these steps."
   if (registration->mPendingUninstall) {
     return;
   }
@@ -3522,81 +3596,38 @@ ServiceWorkerManager::SoftUpdate(const O
   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);
-
-    RefPtr<ServiceWorkerRegisterJob> job =
-      new ServiceWorkerRegisterJob(queue, registration, nullptr);
-    queue->Append(job);
-  }
-}
-
-void
-ServiceWorkerManager::Update(nsIPrincipal* aPrincipal,
-                             const nsACString& aScope,
-                             ServiceWorkerUpdateFinishCallback* aCallback)
-{
-  MOZ_ASSERT(aPrincipal);
-  MOZ_ASSERT(aCallback);
-
-  nsAutoCString scopeKey;
-  nsresult rv = PrincipalToScopeKey(aPrincipal, scopeKey);
-  if (NS_WARN_IF(NS_FAILED(rv))) {
-    return;
-  }
-
-  RefPtr<ServiceWorkerRegistrationInfo> registration =
-    GetRegistration(scopeKey, aScope);
-  if (NS_WARN_IF(!registration)) {
-    return;
-  }
-
-  // "If registration's uninstalling flag is set, abort these steps."
-  if (registration->mPendingUninstall) {
-    return;
-  }
-
-  // "Let newestWorker be the result of running Get Newest Worker algorithm
-  // passing registration as its argument.
-  // If newestWorker is null, return a promise rejected with "InvalidStateError"
-  RefPtr<ServiceWorkerInfo> newest = registration->Newest();
-  if (!newest) {
-    ErrorResult error(NS_ERROR_DOM_INVALID_STATE_ERR);
-    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);
+    GetOrCreateJobQueue(aScopeKey, aScope);
   MOZ_ASSERT(queue);
 
+  RefPtr<ServiceWorkerUpdateFinishCallback> cb(aCallback);
+  if (!cb) {
+    cb = new EmptyUpdateFinishCallback();
+  }
+
   // "Invoke Update algorithm, or its equivalent, with client, registration as
   // its argument."
-  RefPtr<ServiceWorkerRegisterJob> job =
-    new ServiceWorkerRegisterJob(queue, registration, aCallback);
-  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 {
 
 static void
 FireControllerChangeOnDocument(nsIDocument* aDocument)
 {
   AssertIsOnMainThread();
--- a/dom/workers/ServiceWorkerManager.h
+++ b/dom/workers/ServiceWorkerManager.h
@@ -42,19 +42,21 @@ namespace dom {
 class ServiceWorkerRegistrationListener;
 
 namespace workers {
 
 class ServiceWorker;
 class ServiceWorkerClientInfo;
 class ServiceWorkerInfo;
 class ServiceWorkerJob;
+class ServiceWorkerRegisterJob;
 class ServiceWorkerJobQueue;
 class ServiceWorkerManagerChild;
 class ServiceWorkerPrivate;
+class ServiceWorkerUpdateFinishCallback;
 
 class ServiceWorkerRegistrationInfo final
   : public nsIServiceWorkerRegistrationInfo
 {
   uint32_t mControlledDocumentsCounter;
 
   virtual ~ServiceWorkerRegistrationInfo();
 
@@ -72,21 +74,17 @@ public:
   RefPtr<ServiceWorkerInfo> mActiveWorker;
   RefPtr<ServiceWorkerInfo> mWaitingWorker;
   RefPtr<ServiceWorkerInfo> mInstallingWorker;
 
   nsTArray<nsCOMPtr<nsIServiceWorkerRegistrationInfoListener>> mListeners;
 
   uint64_t mLastUpdateCheckTime;
 
-  // According to the spec, Soft Update shouldn't queue an update job
-  // if the registration queue is not empty. Because our job queue
-  // works slightly different, we use a flag to determine if the registration
-  // is already updating.
-  bool mUpdating;
+  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.
   bool mPendingUninstall;
 
   ServiceWorkerRegistrationInfo(const nsACString& aScope,
                                 nsIPrincipal* aPrincipal);
@@ -146,16 +144,22 @@ public:
   void
   RefreshLastUpdateCheckTime();
 
   bool
   IsLastUpdateCheckTimeOverOneDay() const;
 
   void
   NotifyListenersOnChange();
+
+  bool
+  IsUpdating() const;
+
+  void
+  AppendUpdateCallback(ServiceWorkerUpdateFinishCallback* aCallback);
 };
 
 class ServiceWorkerUpdateFinishCallback
 {
 protected:
   virtual ~ServiceWorkerUpdateFinishCallback()
   {}
 
@@ -364,23 +368,24 @@ public:
                     ErrorResult& aRv);
 
   void
   DispatchPreparedFetchEvent(nsIInterceptedChannel* aChannel,
                              nsIRunnable* aPreparedRunnable,
                              ErrorResult& aRv);
 
   void
-  Update(nsIPrincipal* aPrincipal,
-         const nsACString& aScope,
-         ServiceWorkerUpdateFinishCallback* aCallback);
+  SoftUpdate(nsIPrincipal* aPrincipal,
+             const nsACString& aScope,
+             ServiceWorkerUpdateFinishCallback* aCallback = nullptr);
 
   void
   SoftUpdate(const OriginAttributes& aOriginAttributes,
-             const nsACString& aScope);
+             const nsACString& aScope,
+             ServiceWorkerUpdateFinishCallback* aCallback = nullptr);
 
   void
   PropagateSoftUpdate(const OriginAttributes& aOriginAttributes,
                       const nsAString& aScope);
 
   void
   PropagateRemove(const nsACString& aHost);
 
@@ -479,16 +484,21 @@ private:
 
   ServiceWorkerJobQueue*
   GetOrCreateJobQueue(const nsACString& aOriginSuffix,
                       const nsACString& aScope);
 
   void
   MaybeRemoveRegistrationInfo(const nsACString& aScopeKey);
 
+  void
+  SoftUpdate(const nsACString& aScopeKey,
+             const nsACString& aScope,
+             ServiceWorkerUpdateFinishCallback* aCallback = nullptr);
+
   already_AddRefed<ServiceWorkerRegistrationInfo>
   GetRegistration(const nsACString& aScopeKey,
                   const nsACString& aScope) const;
 
   void
   AbortCurrentUpdate(ServiceWorkerRegistrationInfo* aRegistration);
 
   nsresult
--- a/dom/workers/ServiceWorkerManagerChild.cpp
+++ b/dom/workers/ServiceWorkerManagerChild.cpp
@@ -37,17 +37,17 @@ ServiceWorkerManagerChild::RecvNotifySof
 {
   if (mShuttingDown) {
     return true;
   }
 
   RefPtr<ServiceWorkerManager> swm = ServiceWorkerManager::GetInstance();
   MOZ_ASSERT(swm);
 
-  swm->SoftUpdate(aOriginAttributes, NS_ConvertUTF16toUTF8(aScope));
+  swm->SoftUpdate(aOriginAttributes, NS_ConvertUTF16toUTF8(aScope), nullptr);
   return true;
 }
 
 bool
 ServiceWorkerManagerChild::RecvNotifyUnregister(const PrincipalInfo& aPrincipalInfo,
                                                 const nsString& aScope)
 {
   if (mShuttingDown) {
--- a/dom/workers/ServiceWorkerRegistration.cpp
+++ b/dom/workers/ServiceWorkerRegistration.cpp
@@ -261,17 +261,18 @@ UpdateInternal(nsIPrincipal* aPrincipal,
 {
   AssertIsOnMainThread();
   MOZ_ASSERT(aPrincipal);
   MOZ_ASSERT(aCallback);
 
   RefPtr<ServiceWorkerManager> swm = ServiceWorkerManager::GetInstance();
   MOZ_ASSERT(swm);
 
-  swm->Update(aPrincipal, NS_ConvertUTF16toUTF8(aScope), aCallback);
+  // The spec defines ServiceWorkerRegistration.update() exactly as Soft Update.
+  swm->SoftUpdate(aPrincipal, NS_ConvertUTF16toUTF8(aScope), aCallback);
 }
 
 class MainThreadUpdateCallback final : public ServiceWorkerUpdateFinishCallback
 {
   RefPtr<Promise> mPromise;
 
   ~MainThreadUpdateCallback()
   { }
@@ -387,32 +388,24 @@ public:
   {}
 
   NS_IMETHOD
   Run() override
   {
     AssertIsOnMainThread();
     ErrorResult result;
 
-    nsCOMPtr<nsIPrincipal> principal;
-    // UpdateInternal may try to reject the promise synchronously leading
-    // to a deadlock.
-    {
-      MutexAutoLock lock(mPromiseProxy->Lock());
-      if (mPromiseProxy->CleanedUp()) {
-        return NS_OK;
-      }
-
-      principal = mPromiseProxy->GetWorkerPrivate()->GetPrincipal();
+    MutexAutoLock lock(mPromiseProxy->Lock());
+    if (mPromiseProxy->CleanedUp()) {
+      return NS_OK;
     }
-    MOZ_ASSERT(principal);
 
     RefPtr<WorkerThreadUpdateCallback> cb =
       new WorkerThreadUpdateCallback(mPromiseProxy);
-    UpdateInternal(principal, mScope, cb);
+    UpdateInternal(mPromiseProxy->GetWorkerPrivate()->GetPrincipal(), mScope, cb);
     return NS_OK;
   }
 
 private:
   ~UpdateRunnable()
   {}
 
   RefPtr<PromiseWorkerProxy> mPromiseProxy;