Bug 1803062 - Switch from ordinary bool errors to nsresult errors in code for handling sync loops; r=dom-worker-reviewers,smaug

Differential Revision: https://phabricator.services.mozilla.com/D163559

dom/file/FileReaderSync.cpp

@@ -329,7 +329,7 @@ class ReadReadyRunnable final : public WorkerSyncRunnable {
     nsCOMPtr<nsIEventTarget> syncLoopTarget;
     mSyncLoopTarget.swap(syncLoopTarget);
 
-    aWorkerPrivate->StopSyncLoop(syncLoopTarget, true);
+    aWorkerPrivate->StopSyncLoop(syncLoopTarget, NS_OK);
     return true;
   }
 
@@ -443,7 +443,7 @@ nsresult FileReaderSync::SyncRead(nsIInputStream* aStream, char* aBuffer,
       return rv;
     }
 
-    if (!syncLoop.Run()) {
+    if (NS_WARN_IF(NS_FAILED(syncLoop.Run()))) {
       return NS_ERROR_DOM_INVALID_STATE_ERR;
     }
   }

dom/fs/api/FileSystemSyncAccessHandle.cpp

@@ -327,14 +327,14 @@ uint64_t FileSystemSyncAccessHandle::ReadOrWrite(
 
                  workerPrivate->AssertIsOnWorkerThread();
 
-                 workerPrivate->StopSyncLoop(syncLoopTarget, true);
+                 workerPrivate->StopSyncLoop(syncLoopTarget, NS_OK);
 
                  return BoolPromise::CreateAndResolve(true, __func__);
                });
              })),
          throwAndReturn);
 
-  MOZ_ALWAYS_TRUE(syncLoop.Run());
+  MOZ_ALWAYS_SUCCEEDS(syncLoop.Run());
 
   return totalCount;
 }

dom/workers/ScriptLoader.cpp

@@ -946,7 +946,8 @@ void WorkerScriptLoader::ShutdownScriptLoader(bool aResult, bool aMutedError) {
     }
 
     mWorkerRef->Private()->AssertIsOnWorkerThread();
-    mWorkerRef->Private()->StopSyncLoop(mSyncLoopTarget, aResult);
+    mWorkerRef->Private()->StopSyncLoop(mSyncLoopTarget,
+                                        aResult ? NS_OK : NS_ERROR_FAILURE);
 
     // Signal cleanup
     mCleanedUp = true;

dom/workers/WorkerPrivate.cpp

@@ -1328,8 +1328,8 @@ WorkerPrivate::MemoryReporter::FinishCollectRunnable::Run() {
 
 WorkerPrivate::SyncLoopInfo::SyncLoopInfo(EventTarget* aEventTarget)
     : mEventTarget(aEventTarget),
-      mCompleted(false),
-      mResult(false)
+      mResult(NS_ERROR_FAILURE),
+      mCompleted(false)
 #ifdef DEBUG
       ,
       mHasRun(false)
@@ -4142,7 +4142,7 @@ already_AddRefed<nsISerialEventTarget> WorkerPrivate::CreateNewSyncLoop(
   return workerEventTarget.forget();
 }
 
-bool WorkerPrivate::RunCurrentSyncLoop() {
+nsresult WorkerPrivate::RunCurrentSyncLoop() {
   AssertIsOnWorkerThread();
   RefPtr<WorkerThread> thread;
   JSContext* cx = GetJSContext();
@@ -4240,7 +4240,7 @@ bool WorkerPrivate::RunCurrentSyncLoop() {
   return DestroySyncLoop(currentLoopIndex);
 }
 
-bool WorkerPrivate::DestroySyncLoop(uint32_t aLoopIndex) {
+nsresult WorkerPrivate::DestroySyncLoop(uint32_t aLoopIndex) {
   MOZ_ASSERT(!mSyncLoopStack.IsEmpty());
   MOZ_ASSERT(mSyncLoopStack.Length() - 1 == aLoopIndex);
 
@@ -4252,7 +4252,7 @@ bool WorkerPrivate::DestroySyncLoop(uint32_t aLoopIndex) {
       loopInfo->mEventTarget->GetWeakNestedEventTarget();
   MOZ_ASSERT(nestedEventTarget);
 
-  bool result = loopInfo->mResult;
+  nsresult result = loopInfo->mResult;
 
   {
     MutexAutoLock lock(mMutex);
@@ -4374,7 +4374,7 @@ void WorkerPrivate::ReportUseCounters() {
 }
 
 void WorkerPrivate::StopSyncLoop(nsIEventTarget* aSyncLoopTarget,
-                                 bool aResult) {
+                                 nsresult aResult) {
   AssertIsOnWorkerThread();
   AssertValidSyncLoop(aSyncLoopTarget);
 

dom/workers/WorkerPrivate.h

@@ -440,7 +440,7 @@ class WorkerPrivate final
     return data->mOnLine;
   }
 
-  void StopSyncLoop(nsIEventTarget* aSyncLoopTarget, bool aResult);
+  void StopSyncLoop(nsIEventTarget* aSyncLoopTarget, nsresult aResult);
 
   bool AllPendingRunnablesShouldBeCanceled() const {
     return mCancelAllPendingRunnables;
@@ -1118,9 +1118,9 @@ class WorkerPrivate final
   already_AddRefed<nsISerialEventTarget> CreateNewSyncLoop(
       WorkerStatus aFailStatus);
 
-  bool RunCurrentSyncLoop();
+  nsresult RunCurrentSyncLoop();
 
-  bool DestroySyncLoop(uint32_t aLoopIndex);
+  nsresult DestroySyncLoop(uint32_t aLoopIndex);
 
   void InitializeGCTimers();
 
@@ -1255,8 +1255,8 @@ class WorkerPrivate final
     explicit SyncLoopInfo(EventTarget* aEventTarget);
 
     RefPtr<EventTarget> mEventTarget;
+    nsresult mResult;
     bool mCompleted;
-    bool mResult;
 #ifdef DEBUG
     bool mHasRun;
 #endif
@@ -1490,12 +1490,12 @@ class AutoSyncLoopHolder {
   ~AutoSyncLoopHolder() {
     if (mWorkerPrivate && mTarget) {
       mWorkerPrivate->AssertIsOnWorkerThread();
-      mWorkerPrivate->StopSyncLoop(mTarget, false);
+      mWorkerPrivate->StopSyncLoop(mTarget, NS_ERROR_FAILURE);
       mWorkerPrivate->DestroySyncLoop(mIndex);
     }
   }
 
-  bool Run() {
+  nsresult Run() {
     CheckedUnsafePtr<WorkerPrivate> workerPrivate = mWorkerPrivate;
     mWorkerPrivate = nullptr;
 

dom/workers/WorkerRunnable.cpp

@@ -456,7 +456,7 @@ void MainThreadWorkerSyncRunnable::PostDispatch(WorkerPrivate* aWorkerPrivate,
 
 MainThreadStopSyncLoopRunnable::MainThreadStopSyncLoopRunnable(
     WorkerPrivate* aWorkerPrivate, nsCOMPtr<nsIEventTarget>&& aSyncLoopTarget,
-    bool aResult)
+    nsresult aResult)
     : WorkerSyncRunnable(aWorkerPrivate, std::move(aSyncLoopTarget)),
       mResult(aResult) {
   AssertIsOnMainThread();
@@ -567,7 +567,7 @@ void WorkerMainThreadRunnable::Dispatch(WorkerStatus aFailStatus,
       NS_SUCCEEDED(rv),
       "Should only fail after xpcom-shutdown-threads and we're gone by then");
 
-  bool success = syncLoop.Run();
+  bool success = NS_SUCCEEDED(syncLoop.Run());
 
   Telemetry::Accumulate(
       Telemetry::SYNC_WORKER_OPERATION, mTelemetryKey,
@@ -595,7 +595,8 @@ WorkerMainThreadRunnable::Run() {
 
   RefPtr<MainThreadStopSyncLoopRunnable> response =
       new MainThreadStopSyncLoopRunnable(mWorkerPrivate,
-                                         std::move(mSyncLoopTarget), runResult);
+                                         std::move(mSyncLoopTarget),
+                                         runResult ? NS_OK : NS_ERROR_FAILURE);
 
   MOZ_ALWAYS_TRUE(response->Dispatch());
 

dom/workers/WorkerRunnable.h

@@ -425,13 +425,13 @@ class WorkerProxyToMainThreadRunnable : public Runnable {
 // they run this runnable does not modify the busy count
 // in any way.
 class MainThreadStopSyncLoopRunnable : public WorkerSyncRunnable {
-  bool mResult;
+  nsresult mResult;
 
  public:
   // Passing null for aSyncLoopTarget is not allowed.
   MainThreadStopSyncLoopRunnable(WorkerPrivate* aWorkerPrivate,
                                  nsCOMPtr<nsIEventTarget>&& aSyncLoopTarget,
-                                 bool aResult);
+                                 nsresult aResult);
 
   // By default StopSyncLoopRunnables cannot be canceled since they could leave
   // a sync loop spinning forever.

dom/xhr/XMLHttpRequestWorker.cpp

@@ -391,7 +391,7 @@ class LoadStartDetectionRunnable final : public Runnable,
       }
 
       if (mSyncLoopTarget) {
-        aWorkerPrivate->StopSyncLoop(mSyncLoopTarget, true);
+        aWorkerPrivate->StopSyncLoop(mSyncLoopTarget, NS_OK);
       }
 
       if (mXMLHttpRequestPrivate->SendInProgress()) {
@@ -826,7 +826,7 @@ void Proxy::Teardown(bool aSendUnpin) {
         // We have an unclosed sync loop.  Fix that now.
         RefPtr<MainThreadStopSyncLoopRunnable> runnable =
             new MainThreadStopSyncLoopRunnable(
-                mWorkerPrivate, std::move(mSyncLoopTarget), false);
+                mWorkerPrivate, std::move(mSyncLoopTarget), NS_ERROR_FAILURE);
         MOZ_ALWAYS_TRUE(runnable->Dispatch());
       }
 
@@ -1692,7 +1692,7 @@ void XMLHttpRequestWorker::SendInternal(const BodyExtractorBase* aBody,
 
   autoUnpin.Clear();
 
-  bool succeeded = autoSyncLoop->Run();
+  bool succeeded = NS_SUCCEEDED(autoSyncLoop->Run());
   mStateData->mFlagSend = false;
 
   // Don't clobber an existing exception that we may have thrown on aRv