Backed out changeset ef75f461147c (bug 1614212) for GTest failures
authorCoroiu Cristina <ccoroiu@mozilla.com>
Mon, 16 Mar 2020 03:52:15 +0200
changeset 518886 0fdeafcdcb41a6e7def0650ef1d93e3e5bef9299
parent 518885 10998b6bf6d86d1201bdc9c0027154cf42970315
child 518887 6199f7b91e8bde7b7965585842d7a72e2e406e3e
push id37218
push userrmaries@mozilla.com
push dateMon, 16 Mar 2020 09:28:04 +0000
treeherdermozilla-central@6199f7b91e8b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1614212
milestone76.0a1
backs outef75f461147c5148755e505ed8b8c2d9ca20f856
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
Backed out changeset ef75f461147c (bug 1614212) for GTest failures
gfx/thebes/VsyncSource.cpp
gfx/thebes/VsyncSource.h
layout/base/nsRefreshDriver.cpp
widget/VsyncDispatcher.cpp
widget/VsyncDispatcher.h
--- a/gfx/thebes/VsyncSource.cpp
+++ b/gfx/thebes/VsyncSource.cpp
@@ -7,54 +7,36 @@
 #include "nsThreadUtils.h"
 #include "nsXULAppAPI.h"
 #include "mozilla/VsyncDispatcher.h"
 #include "MainThreadUtils.h"
 
 namespace mozilla {
 namespace gfx {
 
-void VsyncSource::EnableCompositorVsyncDispatcher(
+void VsyncSource::AddCompositorVsyncDispatcher(
     CompositorVsyncDispatcher* aCompositorVsyncDispatcher) {
   MOZ_ASSERT(XRE_IsParentProcess());
   MOZ_ASSERT(NS_IsMainThread());
   // Just use the global display until we have enough information to get the
   // corresponding display for compositor.
-  GetGlobalDisplay().EnableCompositorVsyncDispatcher(
-      aCompositorVsyncDispatcher);
+  GetGlobalDisplay().AddCompositorVsyncDispatcher(aCompositorVsyncDispatcher);
 }
 
-void VsyncSource::DisableCompositorVsyncDispatcher(
+void VsyncSource::RemoveCompositorVsyncDispatcher(
     CompositorVsyncDispatcher* aCompositorVsyncDispatcher) {
   MOZ_ASSERT(XRE_IsParentProcess());
   MOZ_ASSERT(NS_IsMainThread());
-  // See also EnableCompositorVsyncDispatcher().
-  GetGlobalDisplay().DisableCompositorVsyncDispatcher(
+  // See also AddCompositorVsyncDispatcher().
+  GetGlobalDisplay().RemoveCompositorVsyncDispatcher(
       aCompositorVsyncDispatcher);
 }
 
-void VsyncSource::RegisterCompositorVsyncDispatcher(
-    CompositorVsyncDispatcher* aCompositorVsyncDispatcher) {
-  MOZ_ASSERT(XRE_IsParentProcess());
-  MOZ_ASSERT(NS_IsMainThread());
-  GetGlobalDisplay().RegisterCompositorVsyncDispatcher(
-      aCompositorVsyncDispatcher);
-}
-
-void VsyncSource::DeregisterCompositorVsyncDispatcher(
-    CompositorVsyncDispatcher* aCompositorVsyncDispatcher) {
-  MOZ_ASSERT(XRE_IsParentProcess());
-  MOZ_ASSERT(NS_IsMainThread());
-  GetGlobalDisplay().DeregisterCompositorVsyncDispatcher(
-      aCompositorVsyncDispatcher);
-}
-
-void VsyncSource::MoveListenersToNewSource(
-    const RefPtr<VsyncSource>& aNewSource) {
-  GetGlobalDisplay().MoveListenersToNewSource(aNewSource);
+void VsyncSource::MoveListenersToNewSource(VsyncSource* aNewSource) {
+  GetGlobalDisplay().MoveListenersToNewSource(aNewSource->GetGlobalDisplay());
 }
 
 RefPtr<RefreshTimerVsyncDispatcher>
 VsyncSource::GetRefreshTimerVsyncDispatcher() {
   MOZ_ASSERT(XRE_IsParentProcess());
   // See also AddCompositorVsyncDispatcher().
   return GetGlobalDisplay().GetRefreshTimerVsyncDispatcher();
 }
@@ -65,18 +47,17 @@ VsyncSource::Display::Display()
   MOZ_ASSERT(NS_IsMainThread());
   mRefreshTimerVsyncDispatcher = new RefreshTimerVsyncDispatcher(this);
 }
 
 VsyncSource::Display::~Display() {
   MOZ_ASSERT(NS_IsMainThread());
   MutexAutoLock lock(mDispatcherLock);
   mRefreshTimerVsyncDispatcher = nullptr;
-  MOZ_ASSERT(mRegisteredCompositorVsyncDispatchers.Length() == 0);
-  MOZ_ASSERT(mEnabledCompositorVsyncDispatchers.Length() == 0);
+  mCompositorVsyncDispatchers.Clear();
 }
 
 void VsyncSource::Display::NotifyVsync(TimeStamp aVsyncTimestamp) {
   // Called on the vsync thread
   MutexAutoLock lock(mDispatcherLock);
 
   // mRefreshTimerVsyncDispatcher might be null here if MoveListenersToNewSource
   // was called concurrently with this function and won the race to acquire
@@ -84,96 +65,61 @@ void VsyncSource::Display::NotifyVsync(T
   // one will handle notifications from now on, so we can abort.
   if (!mRefreshTimerVsyncDispatcher) {
     return;
   }
 
   mVsyncId = mVsyncId.Next();
   VsyncEvent event(mVsyncId, aVsyncTimestamp);
 
-  for (size_t i = 0; i < mEnabledCompositorVsyncDispatchers.Length(); i++) {
-    mEnabledCompositorVsyncDispatchers[i]->NotifyVsync(event);
+  for (size_t i = 0; i < mCompositorVsyncDispatchers.Length(); i++) {
+    mCompositorVsyncDispatchers[i]->NotifyVsync(event);
   }
 
   mRefreshTimerVsyncDispatcher->NotifyVsync(event);
 }
 
 TimeDuration VsyncSource::Display::GetVsyncRate() {
   // If hardware queries fail / are unsupported, we have to just guess.
   return TimeDuration::FromMilliseconds(1000.0 / 60.0);
 }
 
-void VsyncSource::Display::RegisterCompositorVsyncDispatcher(
+void VsyncSource::Display::AddCompositorVsyncDispatcher(
     CompositorVsyncDispatcher* aCompositorVsyncDispatcher) {
   MOZ_ASSERT(NS_IsMainThread());
   MOZ_ASSERT(aCompositorVsyncDispatcher);
   {  // scope lock
     MutexAutoLock lock(mDispatcherLock);
-    mRegisteredCompositorVsyncDispatchers.AppendElement(
-        aCompositorVsyncDispatcher);
+    if (!mCompositorVsyncDispatchers.Contains(aCompositorVsyncDispatcher)) {
+      mCompositorVsyncDispatchers.AppendElement(aCompositorVsyncDispatcher);
+    }
   }
+  UpdateVsyncStatus();
 }
 
-void VsyncSource::Display::DeregisterCompositorVsyncDispatcher(
+void VsyncSource::Display::RemoveCompositorVsyncDispatcher(
     CompositorVsyncDispatcher* aCompositorVsyncDispatcher) {
   MOZ_ASSERT(NS_IsMainThread());
   MOZ_ASSERT(aCompositorVsyncDispatcher);
   {  // Scope lock
     MutexAutoLock lock(mDispatcherLock);
-    mRegisteredCompositorVsyncDispatchers.RemoveElement(
-        aCompositorVsyncDispatcher);
-  }
-}
-
-void VsyncSource::Display::EnableCompositorVsyncDispatcher(
-    CompositorVsyncDispatcher* aCompositorVsyncDispatcher) {
-  MOZ_ASSERT(NS_IsMainThread());
-  MOZ_ASSERT(aCompositorVsyncDispatcher);
-  {  // scope lock
-    MutexAutoLock lock(mDispatcherLock);
-    if (!mEnabledCompositorVsyncDispatchers.Contains(
-            aCompositorVsyncDispatcher)) {
-      mEnabledCompositorVsyncDispatchers.AppendElement(
-          aCompositorVsyncDispatcher);
-    }
-  }
-  UpdateVsyncStatus();
-}
-
-void VsyncSource::Display::DisableCompositorVsyncDispatcher(
-    CompositorVsyncDispatcher* aCompositorVsyncDispatcher) {
-  MOZ_ASSERT(NS_IsMainThread());
-  MOZ_ASSERT(aCompositorVsyncDispatcher);
-  {  // Scope lock
-    MutexAutoLock lock(mDispatcherLock);
-    if (mEnabledCompositorVsyncDispatchers.Contains(
-            aCompositorVsyncDispatcher)) {
-      mEnabledCompositorVsyncDispatchers.RemoveElement(
-          aCompositorVsyncDispatcher);
+    if (mCompositorVsyncDispatchers.Contains(aCompositorVsyncDispatcher)) {
+      mCompositorVsyncDispatchers.RemoveElement(aCompositorVsyncDispatcher);
     }
   }
   UpdateVsyncStatus();
 }
 
 void VsyncSource::Display::MoveListenersToNewSource(
-    const RefPtr<VsyncSource>& aNewSource) {
+    VsyncSource::Display& aNewDisplay) {
   MOZ_ASSERT(NS_IsMainThread());
-  VsyncSource::Display& aNewDisplay = aNewSource->GetGlobalDisplay();
   MutexAutoLock lock(mDispatcherLock);
   MutexAutoLock newLock(aNewDisplay.mDispatcherLock);
-  aNewDisplay.mRegisteredCompositorVsyncDispatchers.AppendElements(
-      std::move(mRegisteredCompositorVsyncDispatchers));
-  aNewDisplay.mEnabledCompositorVsyncDispatchers.AppendElements(
-      std::move(mEnabledCompositorVsyncDispatchers));
-
-  for (size_t i = 0;
-       i < aNewDisplay.mRegisteredCompositorVsyncDispatchers.Length(); i++) {
-    aNewDisplay.mRegisteredCompositorVsyncDispatchers[i]->MoveToSource(
-        aNewSource);
-  }
+  aNewDisplay.mCompositorVsyncDispatchers.AppendElements(
+      std::move(mCompositorVsyncDispatchers));
 
   aNewDisplay.mRefreshTimerVsyncDispatcher = mRefreshTimerVsyncDispatcher;
   mRefreshTimerVsyncDispatcher->MoveToDisplay(&aNewDisplay);
   mRefreshTimerVsyncDispatcher = nullptr;
 }
 
 void VsyncSource::Display::NotifyRefreshTimerVsyncStatus(bool aEnable) {
   MOZ_ASSERT(NS_IsMainThread());
@@ -186,18 +132,18 @@ void VsyncSource::Display::UpdateVsyncSt
   // WARNING: This function SHOULD NOT BE CALLED WHILE HOLDING LOCKS
   // NotifyVsync grabs a lock to dispatch vsync events
   // When disabling vsync, we wait for the underlying thread to stop on some
   // platforms We can deadlock if we wait for the underlying vsync thread to
   // stop while the vsync thread is in NotifyVsync.
   bool enableVsync = false;
   {  // scope lock
     MutexAutoLock lock(mDispatcherLock);
-    enableVsync = !mEnabledCompositorVsyncDispatchers.IsEmpty() ||
-                  mRefreshTimerNeedsVsync;
+    enableVsync =
+        !mCompositorVsyncDispatchers.IsEmpty() || mRefreshTimerNeedsVsync;
   }
 
   if (enableVsync) {
     EnableVsync();
   } else {
     DisableVsync();
   }
 
--- a/gfx/thebes/VsyncSource.h
+++ b/gfx/thebes/VsyncSource.h
@@ -45,55 +45,46 @@ class VsyncSource {
     // Android: TODO
     // All platforms should normalize to the vsync that just occured.
     // Large parts of Gecko assume TimeStamps should not be in the future such
     // as animations
     virtual void NotifyVsync(TimeStamp aVsyncTimestamp);
 
     RefPtr<RefreshTimerVsyncDispatcher> GetRefreshTimerVsyncDispatcher();
 
-    void RegisterCompositorVsyncDispatcher(
-        CompositorVsyncDispatcher* aCompositorVsyncDispatcher);
-    void DeregisterCompositorVsyncDispatcher(
+    void AddCompositorVsyncDispatcher(
         CompositorVsyncDispatcher* aCompositorVsyncDispatcher);
-    void EnableCompositorVsyncDispatcher(
+    void RemoveCompositorVsyncDispatcher(
         CompositorVsyncDispatcher* aCompositorVsyncDispatcher);
-    void DisableCompositorVsyncDispatcher(
-        CompositorVsyncDispatcher* aCompositorVsyncDispatcher);
-    void MoveListenersToNewSource(const RefPtr<VsyncSource>& aNewSource);
+    void MoveListenersToNewSource(VsyncSource::Display& aNewDisplay);
     void NotifyRefreshTimerVsyncStatus(bool aEnable);
     virtual TimeDuration GetVsyncRate();
 
     // These should all only be called on the main thread
     virtual void EnableVsync() = 0;
     virtual void DisableVsync() = 0;
     virtual bool IsVsyncEnabled() = 0;
     virtual void Shutdown() = 0;
 
    private:
     void UpdateVsyncStatus();
 
     Mutex mDispatcherLock;
     bool mRefreshTimerNeedsVsync;
-    nsTArray<CompositorVsyncDispatcher*> mEnabledCompositorVsyncDispatchers;
-    nsTArray<CompositorVsyncDispatcher*> mRegisteredCompositorVsyncDispatchers;
+    nsTArray<RefPtr<CompositorVsyncDispatcher>> mCompositorVsyncDispatchers;
     RefPtr<RefreshTimerVsyncDispatcher> mRefreshTimerVsyncDispatcher;
     VsyncId mVsyncId;
   };
 
-  void EnableCompositorVsyncDispatcher(
-      CompositorVsyncDispatcher* aCompositorVsyncDispatcher);
-  void DisableCompositorVsyncDispatcher(
+  void AddCompositorVsyncDispatcher(
       CompositorVsyncDispatcher* aCompositorVsyncDispatcher);
-  void RegisterCompositorVsyncDispatcher(
-      CompositorVsyncDispatcher* aCompositorVsyncDispatcher);
-  void DeregisterCompositorVsyncDispatcher(
+  void RemoveCompositorVsyncDispatcher(
       CompositorVsyncDispatcher* aCompositorVsyncDispatcher);
 
-  void MoveListenersToNewSource(const RefPtr<VsyncSource>& aNewSource);
+  void MoveListenersToNewSource(VsyncSource* aNewSource);
 
   RefPtr<RefreshTimerVsyncDispatcher> GetRefreshTimerVsyncDispatcher();
   virtual Display& GetGlobalDisplay() = 0;  // Works across all displays
   void Shutdown();
 
  protected:
   virtual ~VsyncSource() = default;
 };
--- a/layout/base/nsRefreshDriver.cpp
+++ b/layout/base/nsRefreshDriver.cpp
@@ -462,19 +462,16 @@ class VsyncRefreshDriverTimer : public R
     MOZ_ASSERT(!XRE_IsParentProcess());
     MOZ_ASSERT(NS_IsMainThread());
     MOZ_ASSERT(mVsyncChild);
     mVsyncObserver = new RefreshDriverVsyncObserver(this);
     mVsyncChild->SetVsyncObserver(mVsyncObserver);
     mVsyncRate = mVsyncChild->GetVsyncRate();
   }
 
-  // Constructor for when we have a local vsync source. As it is local, we do
-  // not have to worry about it being re-inited by gfxPlatform on frame rate
-  // change on the global source.
   explicit VsyncRefreshDriverTimer(const RefPtr<gfx::VsyncSource>& aVsyncSource)
       : mVsyncChild(nullptr) {
     MOZ_ASSERT(XRE_IsParentProcess());
     MOZ_ASSERT(NS_IsMainThread());
     mVsyncSource = aVsyncSource;
     mVsyncObserver = new RefreshDriverVsyncObserver(this);
     MOZ_ALWAYS_TRUE(mVsyncDispatcher =
                         aVsyncSource->GetRefreshTimerVsyncDispatcher());
@@ -823,20 +820,18 @@ class VsyncRefreshDriverTimer : public R
     // Do nothing since we just wait for the next vsync from
     // RefreshDriverVsyncObserver.
   }
 
   void RunRefreshDrivers(VsyncId aId, TimeStamp aTimeStamp) {
     Tick(aId, aTimeStamp);
   }
 
-  // When using local vsync source, we keep a strong ref to it here to ensure
-  // that the weak ref in the vsync dispatcher does not end up dangling.
-  // As this is a local vsync source, it is not affected by gfxPlatform vsync
-  // source reinit.
+  // Used to hold external vsync sources alive. Must be destroyed *after*
+  // mVsyncDispatcher.
   RefPtr<gfx::VsyncSource> mVsyncSource;
   RefPtr<RefreshDriverVsyncObserver> mVsyncObserver;
   // Used for parent process.
   RefPtr<RefreshTimerVsyncDispatcher> mVsyncDispatcher;
   // Used for child process.
   // The mVsyncChild will be always available before VsncChild::ActorDestroy().
   // After ActorDestroy(), StartTimer() and StopTimer() calls will be non-op.
   RefPtr<VsyncChild> mVsyncChild;
--- a/widget/VsyncDispatcher.cpp
+++ b/widget/VsyncDispatcher.cpp
@@ -16,29 +16,25 @@ using namespace mozilla::layers;
 namespace mozilla {
 
 CompositorVsyncDispatcher::CompositorVsyncDispatcher()
     : mVsyncSource(gfxPlatform::GetPlatform()->GetHardwareVsync()),
       mCompositorObserverLock("CompositorObserverLock"),
       mDidShutdown(false) {
   MOZ_ASSERT(XRE_IsParentProcess());
   MOZ_ASSERT(NS_IsMainThread());
-
-  mVsyncSource->RegisterCompositorVsyncDispatcher(this);
 }
 
 CompositorVsyncDispatcher::CompositorVsyncDispatcher(
     RefPtr<gfx::VsyncSource> aVsyncSource)
     : mVsyncSource(std::move(aVsyncSource)),
       mCompositorObserverLock("CompositorObserverLock"),
       mDidShutdown(false) {
   MOZ_ASSERT(XRE_IsParentProcess());
   MOZ_ASSERT(NS_IsMainThread());
-
-  mVsyncSource->RegisterCompositorVsyncDispatcher(this);
 }
 
 CompositorVsyncDispatcher::~CompositorVsyncDispatcher() {
   MOZ_ASSERT(XRE_IsParentProcess());
   // We auto remove this vsync dispatcher from the vsync source in the
   // nsBaseWidget
 }
 
@@ -47,34 +43,27 @@ void CompositorVsyncDispatcher::NotifyVs
   layers::CompositorBridgeParent::PostInsertVsyncProfilerMarker(aVsync.mTime);
 
   MutexAutoLock lock(mCompositorObserverLock);
   if (mCompositorVsyncObserver) {
     mCompositorVsyncObserver->NotifyVsync(aVsync);
   }
 }
 
-void CompositorVsyncDispatcher::MoveToSource(
-    const RefPtr<gfx::VsyncSource>& aVsyncSource) {
-  MOZ_ASSERT(NS_IsMainThread());
-  MOZ_ASSERT(XRE_IsParentProcess());
-  mVsyncSource = aVsyncSource;
-}
-
 void CompositorVsyncDispatcher::ObserveVsync(bool aEnable) {
   MOZ_ASSERT(NS_IsMainThread());
   MOZ_ASSERT(XRE_IsParentProcess());
   if (mDidShutdown) {
     return;
   }
 
   if (aEnable) {
-    mVsyncSource->EnableCompositorVsyncDispatcher(this);
+    mVsyncSource->AddCompositorVsyncDispatcher(this);
   } else {
-    mVsyncSource->DisableCompositorVsyncDispatcher(this);
+    mVsyncSource->RemoveCompositorVsyncDispatcher(this);
   }
 }
 
 void CompositorVsyncDispatcher::SetCompositorVsyncObserver(
     VsyncObserver* aVsyncObserver) {
   // When remote compositing or running gtests, vsync observation is
   // initiated on the main thread. Otherwise, it is initiated from the
   // compositor thread.
@@ -94,25 +83,22 @@ void CompositorVsyncDispatcher::SetCompo
 }
 
 void CompositorVsyncDispatcher::Shutdown() {
   // Need to explicitly remove CompositorVsyncDispatcher when the nsBaseWidget
   // shuts down. Otherwise, we would get dead vsync notifications between when
   // the nsBaseWidget shuts down and the CompositorBridgeParent shuts down.
   MOZ_ASSERT(XRE_IsParentProcess());
   MOZ_ASSERT(NS_IsMainThread());
-  MOZ_ASSERT(!mDidShutdown);
   ObserveVsync(false);
   mDidShutdown = true;
   {  // scope lock
     MutexAutoLock lock(mCompositorObserverLock);
     mCompositorVsyncObserver = nullptr;
   }
-  mVsyncSource->DeregisterCompositorVsyncDispatcher(this);
-  mVsyncSource = nullptr;
 }
 
 RefreshTimerVsyncDispatcher::RefreshTimerVsyncDispatcher(
     gfx::VsyncSource::Display* aDisplay)
     : mDisplay(aDisplay),
       mDisplayLock("RefreshTimerVsyncDispatcherDisplayLock"),
       mRefreshTimersLock("RefreshTimers lock") {
   MOZ_ASSERT(XRE_IsParentProcess());
--- a/widget/VsyncDispatcher.h
+++ b/widget/VsyncDispatcher.h
@@ -48,18 +48,16 @@ class CompositorVsyncDispatcher final {
 
  public:
   CompositorVsyncDispatcher();
   explicit CompositorVsyncDispatcher(RefPtr<gfx::VsyncSource> aVsyncSource);
 
   // Called on the vsync thread when a hardware vsync occurs
   void NotifyVsync(const VsyncEvent& aVsync);
 
-  void MoveToSource(const RefPtr<gfx::VsyncSource>& aVsyncSource);
-
   // Compositor vsync observers must be added/removed on the compositor thread
   void SetCompositorVsyncObserver(VsyncObserver* aVsyncObserver);
   void Shutdown();
 
  private:
   virtual ~CompositorVsyncDispatcher();
   void ObserveVsync(bool aEnable);