Bug 1632036 - check for null global when freezing/thawing WorkerPrivate r=dom-workers-and-storage-reviewers,sg
authorPerry Jiang <perry@mozilla.com>
Thu, 09 Jul 2020 07:25:16 +0000
changeset 540494 1bdbc884e289f2656391c85a1fb35126c4e21ad8
parent 540493 4e76e9b8d9e528770ef88248e8714d40b28df8a9
child 540495 1a1da1cacf42a503775603e640ca6d0b87262d46
push id121754
push userpjiang@mozilla.com
push dateWed, 15 Jul 2020 06:08:31 +0000
treeherderautoland@1bdbc884e289 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdom-workers-and-storage-reviewers, sg
bugs1632036
milestone80.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 1632036 - check for null global when freezing/thawing WorkerPrivate r=dom-workers-and-storage-reviewers,sg - Apply pointer guidelines to ClientSource getter. - When freezing/thawing WorkerPrivate, check if the client/global is null. It seems possible that the client is null because the WorkerPrivate can remain registered to the RuntimeService even if it failed to create its global, and freezing/thawing is done through the RuntimeService. The other two callsites to get the client won't execute unless CompileScriptRunnable successfully creates the client. Differential Revision: https://phabricator.services.mozilla.com/D82482
dom/workers/WorkerPrivate.cpp
dom/workers/WorkerScope.h
--- a/dom/workers/WorkerPrivate.cpp
+++ b/dom/workers/WorkerPrivate.cpp
@@ -1366,17 +1366,17 @@ nsresult WorkerPrivate::SetCSPFromHeader
   }
   return NS_OK;
 }
 
 void WorkerPrivate::StoreCSPOnClient() {
   auto data = mWorkerThreadAccessible.Access();
   MOZ_ASSERT(data->mScope);
   if (mLoadInfo.mCSPInfo) {
-    data->mScope->GetClientSource()->SetCspInfo(*mLoadInfo.mCSPInfo);
+    data->mScope->MutableClientSourceRef().SetCspInfo(*mLoadInfo.mCSPInfo);
   }
 }
 
 void WorkerPrivate::UpdateReferrerInfoFromHeader(
     const nsACString& aReferrerPolicyHeaderValue) {
   NS_ConvertUTF8toUTF16 headerValue(aReferrerPolicyHeaderValue);
 
   if (headerValue.IsEmpty()) {
@@ -3224,20 +3224,17 @@ void WorkerPrivate::ExecutionReady() {
   auto data = mWorkerThreadAccessible.Access();
   {
     MutexAutoLock lock(mMutex);
     if (mStatus >= Canceling) {
       return;
     }
   }
 
-  MOZ_ASSERT(data->mScope);
-  auto& clientSource = data->mScope->GetClientSource();
-  MOZ_DIAGNOSTIC_ASSERT(clientSource);
-  clientSource->WorkerExecutionReady(this);
+  data->mScope->MutableClientSourceRef().WorkerExecutionReady(this);
 }
 
 void WorkerPrivate::InitializeGCTimers() {
   auto data = mWorkerThreadAccessible.Access();
 
   // We need a timer for GC. The basic plan is to run a non-shrinking GC
   // periodically (PERIODIC_GC_TIMER_DELAY_SEC) while the worker is running.
   // Once the worker goes idle we set a short (IDLE_GC_TIMER_DELAY_SEC) timer to
@@ -3589,49 +3586,47 @@ void WorkerPrivate::ClearDebuggerEventQu
     mDebuggerQueue.Pop(runnable);
     // It should be ok to simply release the runnable, without running it.
     runnable->Release();
   }
 }
 
 bool WorkerPrivate::FreezeInternal() {
   auto data = mWorkerThreadAccessible.Access();
-  MOZ_ASSERT(data->mScope);
   NS_ASSERTION(!data->mFrozen, "Already frozen!");
 
   AutoYieldJSThreadExecution yield;
 
-  auto& clientSource = data->mScope->GetClientSource();
-  if (clientSource) {
-    clientSource->Freeze();
+  // The worker can freeze even if it failed to run (and doesn't have a global).
+  if (data->mScope) {
+    data->mScope->MutableClientSourceRef().Freeze();
   }
 
   data->mFrozen = true;
 
   for (uint32_t index = 0; index < data->mChildWorkers.Length(); index++) {
     data->mChildWorkers[index]->Freeze(nullptr);
   }
 
   return true;
 }
 
 bool WorkerPrivate::ThawInternal() {
   auto data = mWorkerThreadAccessible.Access();
-  MOZ_ASSERT(data->mScope);
   NS_ASSERTION(data->mFrozen, "Not yet frozen!");
 
   for (uint32_t index = 0; index < data->mChildWorkers.Length(); index++) {
     data->mChildWorkers[index]->Thaw(nullptr);
   }
 
   data->mFrozen = false;
 
-  auto& clientSource = data->mScope->GetClientSource();
-  if (clientSource) {
-    clientSource->Thaw();
+  // The worker can thaw even if it failed to run (and doesn't have a global).
+  if (data->mScope) {
+    data->mScope->MutableClientSourceRef().Thaw();
   }
 
   return true;
 }
 
 void WorkerPrivate::PropagateStorageAccessPermissionGrantedInternal() {
   auto data = mWorkerThreadAccessible.Access();
 
--- a/dom/workers/WorkerScope.h
+++ b/dom/workers/WorkerScope.h
@@ -125,19 +125,17 @@ class WorkerGlobalScopeBase : public DOM
   already_AddRefed<Console> GetConsole(ErrorResult& aRv);
 
   Console* GetConsoleIfExists() const { return mConsole; }
 
   uint64_t WindowID() const;
 
   void NoteTerminating() { StartDying(); }
 
-  const UniquePtr<ClientSource>& GetClientSource() const {
-    return mClientSource;
-  }
+  ClientSource& MutableClientSourceRef() const { return *mClientSource; }
 
   // WorkerPrivate wants to be able to forbid script when its state machine
   // demands it.
   void WorkerPrivateSaysForbidScript() { StartForbiddingScript(); }
   void WorkerPrivateSaysAllowScript() { StopForbiddingScript(); }
 
  protected:
   ~WorkerGlobalScopeBase() = default;