Bug 1631304 - Use thread observers for the tail dispatcher. r=jya
authorBobby Holley <bobbyholley@gmail.com>
Tue, 28 Apr 2020 21:18:24 +0000
changeset 526569 658d78ebfd03ce9500c08a25a8a949450f6ae448
parent 526568 05287026736e9ba13d27a419298ba62ad437faa6
child 526570 3dcd50f429d93356a9b0b1a01d55dee689c597fe
push id37358
push useropoprus@mozilla.com
push dateWed, 29 Apr 2020 03:05:14 +0000
treeherdermozilla-central@6bb8423186c1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjya
bugs1631304
milestone77.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 1631304 - Use thread observers for the tail dispatcher. r=jya Differential Revision: https://phabricator.services.mozilla.com/D71710
xpcom/threads/AbstractThread.cpp
--- a/xpcom/threads/AbstractThread.cpp
+++ b/xpcom/threads/AbstractThread.cpp
@@ -23,32 +23,25 @@
 namespace mozilla {
 
 LazyLogModule gMozPromiseLog("MozPromise");
 LazyLogModule gStateWatchingLog("StateWatching");
 
 StaticRefPtr<AbstractThread> sMainThread;
 MOZ_THREAD_LOCAL(AbstractThread*) AbstractThread::sCurrentThreadTLS;
 
-class XPCOMThreadWrapper : public AbstractThread {
+class XPCOMThreadWrapper final : public AbstractThread,
+                                 public nsIThreadObserver {
  public:
   explicit XPCOMThreadWrapper(nsIThreadInternal* aThread,
                               bool aRequireTailDispatch)
-      : AbstractThread(aRequireTailDispatch), mThread(aThread) {
-    // Our current mechanism of implementing tail dispatch is appshell-specific.
-    // This is because a very similar mechanism already exists on the main
-    // thread, and we want to avoid making event dispatch on the main thread
-    // more complicated than it already is.
-    //
-    // If you need to use tail dispatch on other XPCOM threads, you'll need to
-    // implement an nsIThreadObserver to fire the tail dispatcher at the
-    // appropriate times. You will also need to modify this assertion.
-    MOZ_ASSERT_IF(aRequireTailDispatch,
-                  NS_IsMainThread() && aThread->IsOnCurrentThread());
-  }
+      : AbstractThread(aRequireTailDispatch), mThread(aThread) {}
+
+  NS_DECL_ISUPPORTS_INHERITED
+  NS_DECL_NSITHREADOBSERVER
 
   nsresult Dispatch(already_AddRefed<nsIRunnable> aRunnable,
                     DispatchReason aReason = NormalDispatch) override {
     nsCOMPtr<nsIRunnable> r = aRunnable;
     AbstractThread* currentThread;
     if (aReason != TailDispatch && (currentThread = GetCurrent()) &&
         RequiresTailDispatch(currentThread) &&
         currentThread->IsTailDispatcherAvailable()) {
@@ -83,32 +76,22 @@ class XPCOMThreadWrapper : public Abstra
 
   // Prevent a GCC warning about the other overload of Dispatch being hidden.
   using AbstractThread::Dispatch;
 
   bool IsCurrentThreadIn() const override {
     return mThread->IsOnCurrentThread();
   }
 
-  void FireTailDispatcher() {
-    MOZ_DIAGNOSTIC_ASSERT(mTailDispatcher.isSome());
-    mTailDispatcher.ref().DrainDirectTasks();
-    mTailDispatcher.reset();
-  }
-
   TaskDispatcher& TailDispatcher() override {
     MOZ_ASSERT(IsCurrentThreadIn());
     MOZ_ASSERT(IsTailDispatcherAvailable());
     if (!mTailDispatcher.isSome()) {
       mTailDispatcher.emplace(/* aIsTailDispatcher = */ true);
-
-      nsCOMPtr<nsIRunnable> event =
-          NewRunnableMethod("XPCOMThreadWrapper::FireTailDispatcher", this,
-                            &XPCOMThreadWrapper::FireTailDispatcher);
-      nsContentUtils::RunInStableState(event.forget());
+      mThread->AddObserver(this);
     }
 
     return mTailDispatcher.ref();
   }
 
   bool IsTailDispatcherAvailable() override {
     // Our tail dispatching implementation relies on nsIThreadObserver
     // callbacks. If we're not doing event processing, it won't work.
@@ -120,16 +103,26 @@ class XPCOMThreadWrapper : public Abstra
   bool MightHaveTailTasks() override { return mTailDispatcher.isSome(); }
 
   nsIEventTarget* AsEventTarget() override { return mThread; }
 
  private:
   const RefPtr<nsIThreadInternal> mThread;
   Maybe<AutoTaskDispatcher> mTailDispatcher;
 
+  ~XPCOMThreadWrapper() = default;
+
+  void MaybeFireTailDispatcher() {
+    if (mTailDispatcher.isSome()) {
+      mTailDispatcher.ref().DrainDirectTasks();
+      mThread->RemoveObserver(this);
+      mTailDispatcher.reset();
+    }
+  }
+
   class Runner : public Runnable {
    public:
     explicit Runner(XPCOMThreadWrapper* aThread,
                     already_AddRefed<nsIRunnable> aRunnable)
         : Runnable("XPCOMThreadWrapper::Runner"),
           mThread(aThread),
           mRunnable(aRunnable) {}
 
@@ -301,9 +294,35 @@ already_AddRefed<AbstractThread> Abstrac
         MOZ_ASSERT(!sCurrentThreadTLS.get(),
                    "There can only be a single XPCOMThreadWrapper available on "
                    "a thread");
         sCurrentThreadTLS.set(wrapper);
       });
   aThread->Dispatch(r.forget(), NS_DISPATCH_NORMAL);
   return wrapper.forget();
 }
+
+NS_IMPL_ISUPPORTS_INHERITED(XPCOMThreadWrapper, AbstractThread,
+                            nsIThreadObserver);
+
+NS_IMETHODIMP
+XPCOMThreadWrapper::OnDispatchedEvent() { return NS_OK; }
+
+NS_IMETHODIMP
+XPCOMThreadWrapper::AfterProcessNextEvent(nsIThreadInternal* thread,
+                                          bool eventWasProcessed) {
+  // This is the primary case.
+  MaybeFireTailDispatcher();
+  return NS_OK;
+}
+
+NS_IMETHODIMP
+XPCOMThreadWrapper::OnProcessNextEvent(nsIThreadInternal* thread,
+                                       bool mayWait) {
+  // In general, the tail dispatcher is handled at the end of the current in
+  // AfterProcessNextEvent() above. However, if start spinning a nested event
+  // loop, it's generally better to fire the tail dispatcher before the first
+  // nested event, rather than after it. This check handles that case.
+  MaybeFireTailDispatcher();
+  return NS_OK;
+}
+
 }  // namespace mozilla