Bug 1731564: Introduce a motivated SpinEventLoopUntil for direct use from C++. r=xpcom-reviewers,nika,necko-reviewers

This patch:
- moves the AutoNestedEventLoopAnnotation into SpinEventLoopUntil.h
- introduces the motivated SpinEventLoopUntil
- maps remaining SpinEventLoopUntil instances to SpinEventLoopUntil with "Missing motivation."
- changes relevant uses in nsThread and nsThreadManager to the motivated SpinEventLoopUntil
- changes all uses with IgnoreAndContinue behavior to the motivated SpinEventLoopUntil

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

dom/media/gtest/TestAudioDecoderInputTrack.cpp

@@ -338,6 +338,7 @@ TEST_F(TestAudioDecoderInputTrack, VolumeChange) {
   expectedVolume = 0.1;
   mTrack->SetVolume(expectedVolume);
   SpinEventLoopUntil<ProcessFailureBehavior::IgnoreAndContinue>(
+      "TEST_F(TestAudioDecoderInputTrack, VolumeChange)"_ns,
       [&] { return mTrack->Volume() == expectedVolume; });
   start = end;
   end += 10;
@@ -366,6 +367,7 @@ TEST_F(TestAudioDecoderInputTrack, BatchedData) {
   // the track first time started to batch data, which we can't control here.
   // Therefore, we need to wait until the batched data gets cleared.
   SpinEventLoopUntil<ProcessFailureBehavior::IgnoreAndContinue>(
+      "TEST_F(TestAudioDecoderInputTrack, BatchedData)"_ns,
       [&] { return !mTrack->HasBatchedData(); });
 
   // Check that we received all the remainging data previously appended.
@@ -424,6 +426,7 @@ TEST_F(TestAudioDecoderInputTrack, PlaybackRateChange) {
   float expectedPlaybackRate = 2.0;
   mTrack->SetPlaybackRate(expectedPlaybackRate);
   SpinEventLoopUntil<ProcessFailureBehavior::IgnoreAndContinue>(
+      "TEST_F(TestAudioDecoderInputTrack, PlaybackRateChange)"_ns,
       [&] { return mTrack->PlaybackRate() == expectedPlaybackRate; });
 
   // Time stretcher in the track would usually need certain amount of data

dom/media/gtest/TestAudioTrackGraph.cpp

@@ -255,6 +255,7 @@ TEST(TestAudioTrackGraph, ErrorCallback)
   MediaEventListener initListener = cubeb->StreamInitEvent().Connect(
       AbstractThread::GetCurrent(), [&] { init = true; });
   SpinEventLoopUntil<ProcessFailureBehavior::IgnoreAndContinue>(
+      "TEST(TestAudioTrackGraph, ErrorCallback)"_ns,
       [&] { return errored && init; });
   errorListener.Disconnect();
   initListener.Disconnect();

dom/media/gtest/WaitFor.cpp

@@ -12,7 +12,7 @@ void WaitFor(MediaEventSource<void>& aEvent) {
   MediaEventListener listener =
       aEvent.Connect(AbstractThread::GetCurrent(), [&] { done = true; });
   SpinEventLoopUntil<ProcessFailureBehavior::IgnoreAndContinue>(
-      [&] { return done; });
+      "WaitFor(MediaEventSource<void>& aEvent)"_ns, [&] { return done; });
   listener.Disconnect();
 }
 

dom/media/gtest/WaitFor.h

@@ -30,6 +30,7 @@ T WaitFor(MediaEventSource<T>& aEvent) {
   MediaEventListener listener = aEvent.Connect(
       AbstractThread::GetCurrent(), [&](T aValue) { value = Some(aValue); });
   SpinEventLoopUntil<ProcessFailureBehavior::IgnoreAndContinue>(
+      "WaitFor(MediaEventSource<T>& aEvent)"_ns,
       [&] { return value.isSome(); });
   listener.Disconnect();
   return value.value();
@@ -53,6 +54,7 @@ Result<R, E> WaitFor(const RefPtr<MozPromise<R, E, Exc>>& aPromise) {
       [&](R aResult) { success = Some(aResult); },
       [&](E aError) { error = Some(aError); });
   SpinEventLoopUntil<ProcessFailureBehavior::IgnoreAndContinue>(
+      "WaitFor(const RefPtr<MozPromise<R, E, Exc>>& aPromise)"_ns,
       [&] { return success.isSome() || error.isSome(); });
   if (success.isSome()) {
     return success.extract();
@@ -74,6 +76,7 @@ void WaitUntil(MediaEventSource<T>& aEvent, const CallbackFunction& aF) {
         }
       });
   SpinEventLoopUntil<ProcessFailureBehavior::IgnoreAndContinue>(
+      "WaitUntil(MediaEventSource<T>& aEvent, const CallbackFunction& aF)"_ns,
       [&] { return done; });
   listener.Disconnect();
 }

media/webrtc/signaling/gtest/mediapipeline_unittest.cpp

@@ -392,7 +392,7 @@ void WaitFor(TimeDuration aDuration) {
       NS_NewRunnableFunction(__func__, [&] { done = true; }),
       aDuration.ToMilliseconds());
   SpinEventLoopUntil<ProcessFailureBehavior::IgnoreAndContinue>(
-      [&] { return done; });
+      "WaitFor(TimeDuration aDuration)"_ns, [&] { return done; });
 }
 
 class MediaPipelineTest : public ::testing::Test {

netwerk/test/gtest/TestStandardURL.cpp

@@ -246,7 +246,7 @@ TEST(TestStandardURL, From_test_standardurldotjs)
   }
 }
 
-#define COUNT 10000
+#define TEST_COUNT 10000
 
 MOZ_GTEST_BENCH(TestStandardURL, DISABLED_Perf, [] {
   nsCOMPtr<nsIURI> url;
@@ -255,7 +255,7 @@ MOZ_GTEST_BENCH(TestStandardURL, DISABLED_Perf, [] {
                        .Finalize(url));
 
   nsAutoCString out;
-  for (int i = COUNT; i; --i) {
+  for (int i = TEST_COUNT; i; --i) {
     ASSERT_EQ(NS_MutateURI(url).SetSpec("http://example.com"_ns).Finalize(url),
               NS_OK);
     ASSERT_EQ(url->GetSpec(out), NS_OK);

xpcom/base/AppShutdown.h

@@ -8,6 +8,8 @@
 #define AppShutdown_h
 
 #include <type_traits>
+#include "nsCOMPtr.h"
+#include "nsISupportsBase.h"
 #include "ShutdownPhase.h"
 
 namespace mozilla {

xpcom/threads/SpinEventLoopUntil.h

@@ -9,6 +9,9 @@
 
 #include "MainThreadUtils.h"
 #include "mozilla/Maybe.h"
+#include "mozilla/ProfilerLabels.h"
+#include "mozilla/ProfilerMarkers.h"
+#include "nsString.h"
 #include "nsThreadUtils.h"
 #include "xpcpublic.h"
 
@@ -75,10 +78,75 @@ enum class ProcessFailureBehavior {
   ReportToCaller,
 };
 
+// SpinEventLoopUntil is a dangerous operation that can result in hangs.
+// In particular during shutdown we want to know if we are hanging
+// inside a nested event loop on the main thread.
+// This is a helper annotation class to keep track of this.
+struct MOZ_STACK_CLASS AutoNestedEventLoopAnnotation {
+  explicit AutoNestedEventLoopAnnotation(const nsACString& aEntry)
+      : mPrev(nullptr) {
+    if (NS_IsMainThread()) {
+      mPrev = sCurrent;
+      sCurrent = this;
+      if (mPrev) {
+        mStack = mPrev->mStack + "|"_ns + aEntry;
+      } else {
+        mStack = aEntry;
+      }
+      AnnotateXPCOMSpinEventLoopStack(mStack);
+    }
+  }
+
+  ~AutoNestedEventLoopAnnotation() {
+    if (NS_IsMainThread()) {
+      MOZ_ASSERT(sCurrent == this);
+      sCurrent = mPrev;
+      if (mPrev) {
+        AnnotateXPCOMSpinEventLoopStack(mPrev->mStack);
+      } else {
+        AnnotateXPCOMSpinEventLoopStack(""_ns);
+      }
+    }
+  }
+
+ private:
+  AutoNestedEventLoopAnnotation(const AutoNestedEventLoopAnnotation&) = delete;
+  AutoNestedEventLoopAnnotation& operator=(
+      const AutoNestedEventLoopAnnotation&) = delete;
+
+  // The declaration of this static lives in nsThreadManager.cpp.
+  static AutoNestedEventLoopAnnotation* sCurrent;
+
+  // We need this to avoid the inclusion of nsExceptionHandler.h here
+  // which can include windows.h which disturbs some dom/media/gtest.
+  // The implementation lives in nsThreadManager.cpp.
+  static void AnnotateXPCOMSpinEventLoopStack(const nsACString& aStack);
+
+  AutoNestedEventLoopAnnotation* mPrev;
+  nsCString mStack;
+};
+
+// This is the preferred way to use SpinEventLoopUntil from C++.
+// Please see the above notes for the Behavior template parameter.
+//
+// aVeryGoodReasonToDoThis is usually a literal string unique to each
+//   caller that can be recognized in the XPCOMSpinEventLoopStack
+//   annotation.
+// aPredicate is the condition we wait for.
+// aThread can be used to specify a thread, see the above introduction.
+//   It defaults to the current thread.
 template <
     ProcessFailureBehavior Behavior = ProcessFailureBehavior::ReportToCaller,
     typename Pred>
-bool SpinEventLoopUntil(Pred&& aPredicate, nsIThread* aThread = nullptr) {
+bool SpinEventLoopUntil(const nsACString& aVeryGoodReasonToDoThis,
+                        Pred&& aPredicate, nsIThread* aThread = nullptr) {
+  // Prepare the annotations
+  AutoNestedEventLoopAnnotation annotation(aVeryGoodReasonToDoThis);
+  AUTO_PROFILER_LABEL_DYNAMIC_NSCSTRING_NONSENSITIVE(
+      "SpinEventLoopUntil", OTHER, aVeryGoodReasonToDoThis);
+  AUTO_PROFILER_MARKER_TEXT("SpinEventLoop", OTHER, MarkerStack::Capture(),
+                            aVeryGoodReasonToDoThis);
+
   nsIThread* thread = aThread ? aThread : NS_GetCurrentThread();
 
   // From a latency perspective, spinning the event loop is like leaving script
@@ -103,6 +171,13 @@ bool SpinEventLoopUntil(Pred&& aPredicate, nsIThread* aThread = nullptr) {
   return true;
 }
 
+// This is the "legacy" way of invoking SpinEventLoopUntil. We want to
+// get rid of this in favor of a motivated SpinEventLoopUntil.
+template <typename Pred>
+bool SpinEventLoopUntil(Pred&& aPredicate, nsIThread* aThread = nullptr) {
+  return SpinEventLoopUntil("Missing motivation."_ns, aPredicate, aThread);
+}
+
 }  // namespace mozilla
 
 #endif  // xpcom_threads_SpinEventLoopUntil_h__

xpcom/threads/nsThread.cpp

@@ -818,9 +818,10 @@ void nsThread::ShutdownComplete(NotNull<nsThreadShutdownContext*> aContext) {
 }
 
 void nsThread::WaitForAllAsynchronousShutdowns() {
-  // This is the motivating example for why SpinEventLoop has the template
-  // parameter we are providing here.
+  // This is the motivating example for why SpinEventLoopUntil
+  // has the template parameter we are providing here.
   SpinEventLoopUntil<ProcessFailureBehavior::IgnoreAndContinue>(
+      "nsThread::WaitForAllAsynchronousShutdowns"_ns,
       [&]() { return mRequestedShutdownContexts.IsEmpty(); }, this);
 }
 
@@ -837,8 +838,10 @@ nsThread::Shutdown() {
 
   // Process events on the current thread until we receive a shutdown ACK.
   // Allows waiting; ensure no locks are held that would deadlock us!
-  SpinEventLoopUntil([&, context]() { return !context->mAwaitingShutdownAck; },
-                     context->mJoiningThread);
+  SpinEventLoopUntil(
+      "nsThread::Shutdown"_ns,
+      [&, context]() { return !context->mAwaitingShutdownAck; },
+      context->mJoiningThread);
 
   ShutdownComplete(context);
 

xpcom/threads/nsThreadManager.cpp

@@ -9,6 +9,7 @@
 #include "nsThreadPool.h"
 #include "nsThreadUtils.h"
 #include "nsIClassInfoImpl.h"
+#include "nsExceptionHandler.h"
 #include "nsTArray.h"
 #include "nsXULAppAPI.h"
 #include "nsExceptionHandler.h"
@@ -388,7 +389,9 @@ void nsThreadManager::Shutdown() {
   // preventing the task queues from emptying, preventing the shutdown promises
   // from resolving, and prevent anything checking `taskQueuesShutdown` from
   // working.
-  ::SpinEventLoopUntil([&]() { return taskQueuesShutdown; }, mMainThread);
+  mozilla::SpinEventLoopUntil(
+      "nsThreadManager::Shutdown"_ns, [&]() { return taskQueuesShutdown; },
+      mMainThread);
 
   {
     // We gather the threads from the hashtable into a list, so that we avoid
@@ -519,7 +522,9 @@ void nsThreadManager::CancelBackgroundDelayedRunnables() {
   bool canceled = false;
   mBackgroundEventTarget->CancelBackgroundDelayedRunnables()->Then(
       GetMainThreadSerialEventTarget(), __func__, [&] { canceled = true; });
-  ::SpinEventLoopUntil([&]() { return canceled; });
+  mozilla::SpinEventLoopUntil(
+      "nsThreadManager::CancelBackgroundDelayedRunnables"_ns,
+      [&]() { return canceled; });
 }
 
 nsThread* nsThreadManager::GetCurrentThread() {
@@ -651,73 +656,29 @@ nsThreadManager::SpinEventLoopUntilOrQuit(
                                     ShutdownPhase::AppShutdownConfirmed);
 }
 
-struct MOZ_STACK_CLASS AutoNestedEventLoopAnnotation {
-  explicit AutoNestedEventLoopAnnotation(const nsACString& aEntry)
-      : mPrev(sCurrent) {
-    sCurrent = this;
-    if (mPrev) {
-      mStack = mPrev->mStack + "|"_ns + aEntry;
-    } else {
-      mStack = aEntry;
-    }
-    CrashReporter::AnnotateCrashReport(
-        CrashReporter::Annotation::XPCOMSpinEventLoopStack, mStack);
-  }
-
-  ~AutoNestedEventLoopAnnotation() {
-    MOZ_ASSERT(sCurrent == this);
-    sCurrent = mPrev;
-    if (mPrev) {
-      CrashReporter::AnnotateCrashReport(
-          CrashReporter::Annotation::XPCOMSpinEventLoopStack, mPrev->mStack);
-    } else {
-      CrashReporter::RemoveCrashReportAnnotation(
-          CrashReporter::Annotation::XPCOMSpinEventLoopStack);
-    }
-  }
-
- private:
-  AutoNestedEventLoopAnnotation(const AutoNestedEventLoopAnnotation&) = delete;
-  AutoNestedEventLoopAnnotation& operator=(
-      const AutoNestedEventLoopAnnotation&) = delete;
-
-  static AutoNestedEventLoopAnnotation* sCurrent;
-
-  AutoNestedEventLoopAnnotation* mPrev;
-  nsCString mStack;
-};
-
+// static from SpinEventLoopUntil.h
 AutoNestedEventLoopAnnotation* AutoNestedEventLoopAnnotation::sCurrent =
     nullptr;
 
+// static from SpinEventLoopUntil.h
+void AutoNestedEventLoopAnnotation::AnnotateXPCOMSpinEventLoopStack(
+    const nsACString& aStack) {
+  CrashReporter::AnnotateCrashReport(
+      CrashReporter::Annotation::XPCOMSpinEventLoopStack, aStack);
+}
+
 nsresult nsThreadManager::SpinEventLoopUntilInternal(
     const nsACString& aVeryGoodReasonToDoThis,
     nsINestedEventLoopCondition* aCondition,
-    ShutdownPhase aCheckingShutdownPhase) {
-  AutoNestedEventLoopAnnotation annotation(aVeryGoodReasonToDoThis);
-  AUTO_PROFILER_LABEL_DYNAMIC_NSCSTRING_NONSENSITIVE(
-      "nsThreadManager::SpinEventLoop", OTHER, aVeryGoodReasonToDoThis);
-  AUTO_PROFILER_MARKER_TEXT("SpinEventLoop", OTHER, MarkerStack::Capture(),
-                            aVeryGoodReasonToDoThis);
-
+    ShutdownPhase aShutdownPhaseToCheck) {
   // XXX: We would want to AssertIsOnMainThread(); but that breaks some GTest.
   nsCOMPtr<nsINestedEventLoopCondition> condition(aCondition);
   nsresult rv = NS_OK;
 
-  bool checkingShutdown =
-      (aCheckingShutdownPhase > ShutdownPhase::NotInShutdown);
-
-  // Nothing to do if already shutting down.
-  if (checkingShutdown &&
-      AppShutdown::GetCurrentShutdownPhase() >= aCheckingShutdownPhase) {
-    return NS_OK;
-  }
-
-  if (!mozilla::SpinEventLoopUntil([&]() -> bool {
-        // Check if shutting down reached our limits.
-        if (checkingShutdown &&
-            AppShutdown::GetCurrentShutdownPhase() >= aCheckingShutdownPhase) {
-          // This will make us return with NS_OK.
+  if (!mozilla::SpinEventLoopUntil(aVeryGoodReasonToDoThis, [&]() -> bool {
+        // Check if an ongoing shutdown reached our limits.
+        if (aShutdownPhaseToCheck > ShutdownPhase::NotInShutdown &&
+            AppShutdown::GetCurrentShutdownPhase() >= aShutdownPhaseToCheck) {
           return true;
         }
 

xpcom/threads/nsThreadManager.h

@@ -92,7 +92,7 @@ class nsThreadManager : public nsIThreadManager {
   nsresult SpinEventLoopUntilInternal(
       const nsACString& aVeryGoodReasonToDoThis,
       nsINestedEventLoopCondition* aCondition,
-      mozilla::ShutdownPhase aCheckingShutdownPhase);
+      mozilla::ShutdownPhase aShutdownPhaseToCheck);
 
   static void ReleaseThread(void* aData);