Bug 1765399 - Move main thread observers to the vsync dispatcher. r=smaug

Main thread observers (previously "generic" observers) are only used
by Windows touchpad scrolling so far.

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

gfx/thebes/VsyncSource.cpp

@@ -19,8 +19,7 @@ namespace gfx {
 
 VsyncSource::VsyncSource()
     : mDispatcherLock("display dispatcher lock"),
-      mVsyncDispatcherNeedsVsync(false),
-      mHasGenericObservers(false) {
+      mVsyncDispatcherNeedsVsync(false) {
   MOZ_ASSERT(NS_IsMainThread());
   mVsyncDispatcher = new VsyncDispatcher(this);
 }
@@ -44,36 +43,9 @@ void VsyncSource::NotifyVsync(const TimeStamp& aVsyncTimestamp,
     return;
   }
 
-  // If the task posted to the main thread from the last NotifyVsync call
-  // hasn't been processed yet, then don't send another one. Otherwise we might
-  // end up flooding the main thread.
-  bool dispatchToMainThread =
-      mHasGenericObservers &&
-      (mLastVsyncIdSentToMainThread == mLastMainThreadProcessedVsyncId);
-
   mVsyncId = mVsyncId.Next();
   const VsyncEvent event(mVsyncId, aVsyncTimestamp, aOutputTimestamp);
-
   mVsyncDispatcher->NotifyVsync(event);
-
-  if (dispatchToMainThread) {
-    mLastVsyncIdSentToMainThread = mVsyncId;
-    NS_DispatchToMainThread(NewRunnableMethod<VsyncEvent>(
-        "VsyncSource::NotifyGenericObservers", this,
-        &VsyncSource::NotifyGenericObservers, event));
-  }
-}
-
-void VsyncSource::NotifyGenericObservers(VsyncEvent aEvent) {
-  MOZ_ASSERT(NS_IsMainThread());
-  for (size_t i = 0; i < mGenericObservers.Length(); i++) {
-    mGenericObservers[i]->NotifyVsync(aEvent);
-  }
-
-  {  // Scope lock
-    MutexAutoLock lock(mDispatcherLock);
-    mLastMainThreadProcessedVsyncId = aEvent.mId;
-  }
 }
 
 TimeDuration VsyncSource::GetVsyncRate() {
@@ -81,28 +53,11 @@ TimeDuration VsyncSource::GetVsyncRate() {
   return TimeDuration::FromMilliseconds(1000.0 / 60.0);
 }
 
-void VsyncSource::AddGenericObserver(VsyncObserver* aObserver) {
-  MOZ_ASSERT(NS_IsMainThread());
-  MOZ_ASSERT(aObserver);
-  mGenericObservers.AppendElement(aObserver);
-
-  UpdateVsyncStatus();
-}
-
-void VsyncSource::RemoveGenericObserver(VsyncObserver* aObserver) {
-  MOZ_ASSERT(NS_IsMainThread());
-  MOZ_ASSERT(aObserver);
-  mGenericObservers.RemoveElement(aObserver);
-
-  UpdateVsyncStatus();
-}
-
 void VsyncSource::MoveListenersToNewSource(
     const RefPtr<VsyncSource>& aNewSource) {
   MOZ_ASSERT(NS_IsMainThread());
   MutexAutoLock lock(mDispatcherLock);
   MutexAutoLock newLock(aNewSource->mDispatcherLock);
-  aNewSource->mGenericObservers.AppendElements(std::move(mGenericObservers));
 
   aNewSource->mVsyncDispatcher = mVsyncDispatcher;
   mVsyncDispatcher->MoveToSource(aNewSource);
@@ -125,8 +80,7 @@ void VsyncSource::UpdateVsyncStatus() {
   bool enableVsync = false;
   {  // scope lock
     MutexAutoLock lock(mDispatcherLock);
-    enableVsync = mVsyncDispatcherNeedsVsync || !mGenericObservers.IsEmpty();
-    mHasGenericObservers = !mGenericObservers.IsEmpty();
+    enableVsync = mVsyncDispatcherNeedsVsync;
   }
 
   if (enableVsync) {

gfx/thebes/VsyncSource.h

@@ -51,7 +51,6 @@ class VsyncSource {
   //     callback is called.
   virtual void NotifyVsync(const TimeStamp& aVsyncTimestamp,
                            const TimeStamp& aOutputTimestamp);
-  void NotifyGenericObservers(VsyncEvent aEvent);
 
   void NotifyVsyncDispatcherVsyncStatus(bool aEnable);
   virtual TimeDuration GetVsyncRate();
@@ -62,14 +61,6 @@ class VsyncSource {
   virtual bool IsVsyncEnabled() = 0;
   virtual void Shutdown() = 0;
 
-  // Add and remove a generic observer for vsync. Note that keeping an observer
-  // registered means vsync will keep firing, which may impact power usage. So
-  // this is intended only for "short term" vsync observers. These methods must
-  // be called on the parent process main thread, and the observer will likewise
-  // be notified on the parent process main thread.
-  void AddGenericObserver(VsyncObserver* aObserver);
-  void RemoveGenericObserver(VsyncObserver* aObserver);
-
   void MoveListenersToNewSource(const RefPtr<VsyncSource>& aNewSource);
 
   RefPtr<VsyncDispatcher> GetVsyncDispatcher();
@@ -86,12 +77,7 @@ class VsyncSource {
   Mutex mDispatcherLock MOZ_UNANNOTATED;
   bool mVsyncDispatcherNeedsVsync;
   RefPtr<VsyncDispatcher> mVsyncDispatcher;
-  nsTArray<RefPtr<VsyncObserver>>
-      mGenericObservers;  // can only be touched from the main thread
   VsyncId mVsyncId;
-  VsyncId mLastVsyncIdSentToMainThread;     // hold mDispatcherLock to touch
-  VsyncId mLastMainThreadProcessedVsyncId;  // hold mDispatcherLock to touch
-  bool mHasGenericObservers;                // hold mDispatcherLock to touch
 };
 
 }  // namespace gfx

widget/VsyncDispatcher.cpp

@@ -89,8 +89,7 @@ void CompositorVsyncDispatcher::Shutdown() {
 }
 
 VsyncDispatcher::VsyncDispatcher(gfx::VsyncSource* aVsyncSource)
-    : mVsyncSource(aVsyncSource),
-      mVsyncObservers("VsyncDispatcher::mVsyncObservers") {
+    : mVsyncSource(aVsyncSource), mState("VsyncDispatcher::mState") {
   MOZ_ASSERT(XRE_IsParentProcess());
   MOZ_ASSERT(NS_IsMainThread());
 }
@@ -106,19 +105,57 @@ void VsyncDispatcher::MoveToSource(gfx::VsyncSource* aVsyncSource) {
 }
 
 void VsyncDispatcher::NotifyVsync(const VsyncEvent& aVsync) {
-  auto observers = mVsyncObservers.Lock();
+  nsTArray<RefPtr<VsyncObserver>> observers;
+  bool shouldDispatchToMainThread = false;
+  {
+    // Copy out the observers so that we don't keep the mutex
+    // locked while notifying vsync.
+    auto state = mState.Lock();
+    observers = state->mObservers.Clone();
+    shouldDispatchToMainThread = !state->mMainThreadObservers.IsEmpty() &&
+                                 (state->mLastVsyncIdSentToMainThread ==
+                                  state->mLastMainThreadProcessedVsyncId);
+  }
 
-  for (const auto& observer : *observers) {
+  for (const auto& observer : observers) {
     observer->NotifyVsync(aVsync);
   }
+
+  if (shouldDispatchToMainThread) {
+    auto state = mState.Lock();
+    state->mLastVsyncIdSentToMainThread = aVsync.mId;
+    NS_DispatchToMainThread(NewRunnableMethod<VsyncEvent>(
+        "VsyncDispatcher::NotifyMainThreadObservers", this,
+        &VsyncDispatcher::NotifyMainThreadObservers, aVsync));
+  }
+}
+
+void VsyncDispatcher::NotifyMainThreadObservers(VsyncEvent aEvent) {
+  MOZ_ASSERT(NS_IsMainThread());
+  nsTArray<RefPtr<VsyncObserver>> observers;
+  {
+    // Copy out the main thread observers so that we don't keep the mutex
+    // locked while notifying vsync.
+    auto state = mState.Lock();
+    observers.AppendElements(state->mMainThreadObservers);
+  }
+
+  for (const auto& observer : observers) {
+    observer->NotifyVsync(aEvent);
+  }
+
+  {  // Scope lock
+    auto state = mState.Lock();
+    state->mLastMainThreadProcessedVsyncId = aEvent.mId;
+  }
 }
 
 void VsyncDispatcher::AddVsyncObserver(VsyncObserver* aVsyncObserver) {
   MOZ_ASSERT(aVsyncObserver);
   {  // scope lock - called on PBackground thread or main thread
-    auto observers = mVsyncObservers.Lock();
-    if (!observers->Contains(aVsyncObserver)) {
-      observers->AppendElement(aVsyncObserver);
+    auto state = mState.Lock();
+    if (!state->mObservers.Contains(aVsyncObserver)) {
+      state->mObservers.AppendElement(aVsyncObserver);
     }
   }
 
@@ -128,8 +165,30 @@ void VsyncDispatcher::AddVsyncObserver(VsyncObserver* aVsyncObserver) {
 void VsyncDispatcher::RemoveVsyncObserver(VsyncObserver* aVsyncObserver) {
   MOZ_ASSERT(aVsyncObserver);
   {  // scope lock - called on PBackground thread or main thread
-    auto observers = mVsyncObservers.Lock();
-    observers->RemoveElement(aVsyncObserver);
+    auto state = mState.Lock();
+    state->mObservers.RemoveElement(aVsyncObserver);
+  }
+
+  UpdateVsyncStatus();
+}
+
+void VsyncDispatcher::AddMainThreadObserver(VsyncObserver* aObserver) {
+  MOZ_ASSERT(NS_IsMainThread());
+  MOZ_ASSERT(aObserver);
+  {
+    auto state = mState.Lock();
+    state->mMainThreadObservers.AppendElement(aObserver);
+  }
+
+  UpdateVsyncStatus();
+}
+
+void VsyncDispatcher::RemoveMainThreadObserver(VsyncObserver* aObserver) {
+  MOZ_ASSERT(NS_IsMainThread());
+  MOZ_ASSERT(aObserver);
+  {
+    auto state = mState.Lock();
+    state->mMainThreadObservers.RemoveElement(aObserver);
   }
 
   UpdateVsyncStatus();
@@ -147,9 +206,8 @@ void VsyncDispatcher::UpdateVsyncStatus() {
 }
 
 bool VsyncDispatcher::NeedsVsync() {
-  MOZ_ASSERT(NS_IsMainThread());
-  auto observers = mVsyncObservers.Lock();
-  return !observers->IsEmpty();
+  auto state = mState.Lock();
+  return !state->mObservers.IsEmpty() || !state->mMainThreadObservers.IsEmpty();
 }
 
 }  // namespace mozilla

widget/VsyncDispatcher.h

@@ -105,16 +105,36 @@ class VsyncDispatcher final {
   // observer is not registered. Can be called from any thread.
   void RemoveVsyncObserver(VsyncObserver* aVsyncObserver);
 
+  // Add and remove an observer for vsync which can only be notified on the
+  // main thread. Note that keeping an observer registered means vsync will keep
+  // firing, which may impact power usage. So this is intended only for "short
+  // term" vsync observers.
+  // These methods must be called on the parent process main thread, and the
+  // observer will likewise be notified on the parent process main thread.
+  void AddMainThreadObserver(VsyncObserver* aObserver);
+  void RemoveMainThreadObserver(VsyncObserver* aObserver);
+
  private:
   virtual ~VsyncDispatcher();
   void UpdateVsyncStatus();
   bool NeedsVsync();
 
+  // Can only be called on the main thread.
+  void NotifyMainThreadObservers(VsyncEvent aEvent);
+
   // We need to hold a weak ref to the vsync source we belong to in order to
   // notify it of our vsync requirement. The vsync source holds a RefPtr to us,
   // so we can't hold a RefPtr back without causing a cyclic dependency.
   gfx::VsyncSource* mVsyncSource;
-  DataMutex<nsTArray<RefPtr<VsyncObserver>>> mVsyncObservers;
+
+  struct State {
+    nsTArray<RefPtr<VsyncObserver>> mObservers;
+    nsTArray<RefPtr<VsyncObserver>> mMainThreadObservers;
+    VsyncId mLastVsyncIdSentToMainThread;
+    VsyncId mLastMainThreadProcessedVsyncId;
+  };
+
+  DataMutex<State> mState;
 };
 
 }  // namespace mozilla

widget/windows/DirectManipulationOwner.cpp

@@ -362,14 +362,15 @@ DManipEventHandler::OnInteraction(
       mObserver = new VObserver(this);
     }
 
-    gfxWindowsPlatform::GetPlatform()->GetHardwareVsync()->AddGenericObserver(
-        mObserver);
+    gfxWindowsPlatform::GetPlatform()
+        ->GetGlobalVsyncDispatcher()
+        ->AddMainThreadObserver(mObserver);
   }
 
   if (mObserver && interaction == DIRECTMANIPULATION_INTERACTION_END) {
     gfxWindowsPlatform::GetPlatform()
-        ->GetHardwareVsync()
-        ->RemoveGenericObserver(mObserver);
+        ->GetGlobalVsyncDispatcher()
+        ->RemoveMainThreadObserver(mObserver);
   }
 
   return S_OK;
@@ -680,8 +681,8 @@ void DirectManipulationOwner::Destroy() {
     mDmHandler->mOwner = nullptr;
     if (mDmHandler->mObserver) {
       gfxWindowsPlatform::GetPlatform()
-          ->GetHardwareVsync()
-          ->RemoveGenericObserver(mDmHandler->mObserver);
+          ->GetGlobalVsyncDispatcher()
+          ->RemoveMainThreadObserver(mDmHandler->mObserver);
       mDmHandler->mObserver->ClearOwner();
       mDmHandler->mObserver = nullptr;
     }