From 97dd056cfac1fcb42447f7c58d6e4248f5349dce Mon Sep 17 00:00:00 2001
From: Mason Chang <mchang@mozilla.com>
Date: Fri, 10 Apr 2015 07:59:21 -0700
Subject: [PATCH] Bug 1147390. Enable / disable vsync on the vsync thread only.
 r=kats

---
 gfx/thebes/SoftwareVsyncSource.cpp | 65 +++++++++++++++---------------
 gfx/thebes/SoftwareVsyncSource.h   |  7 ++--
 2 files changed, 35 insertions(+), 37 deletions(-)

diff --git a/gfx/thebes/SoftwareVsyncSource.cpp b/gfx/thebes/SoftwareVsyncSource.cpp
index ee2add763438..eda9202ca56a 100644
--- a/gfx/thebes/SoftwareVsyncSource.cpp
+++ b/gfx/thebes/SoftwareVsyncSource.cpp
@@ -17,14 +17,13 @@ SoftwareVsyncSource::SoftwareVsyncSource()
 SoftwareVsyncSource::~SoftwareVsyncSource()
 {
   MOZ_ASSERT(NS_IsMainThread());
-  // Ensure we disable vsync on the main thread here
-  mGlobalDisplay->DisableVsync();
+  mGlobalDisplay->Shutdown();
   mGlobalDisplay = nullptr;
 }
 
 SoftwareDisplay::SoftwareDisplay()
-  : mVsyncEnabled(false)
-  , mCurrentTaskMonitor("SoftwareVsyncCurrentTaskMonitor")
+  : mCurrentVsyncTask(nullptr)
+  , mVsyncEnabled(false)
 {
   // Mimic 60 fps
   MOZ_ASSERT(NS_IsMainThread());
@@ -34,41 +33,46 @@ SoftwareDisplay::SoftwareDisplay()
   MOZ_RELEASE_ASSERT(mVsyncThread->Start(), "Could not start software vsync thread");
 }
 
+SoftwareDisplay::~SoftwareDisplay() {}
+
 void
 SoftwareDisplay::EnableVsync()
 {
-  MOZ_ASSERT(NS_IsMainThread());
-  if (IsVsyncEnabled()) {
+  MOZ_ASSERT(mVsyncThread->IsRunning());
+  if (NS_IsMainThread()) {
+    if (mVsyncEnabled) {
+      return;
+    }
+    mVsyncEnabled = true;
+
+    mVsyncThread->message_loop()->PostTask(FROM_HERE,
+      NewRunnableMethod(this, &SoftwareDisplay::EnableVsync));
     return;
   }
 
-  { // scope lock
-    mozilla::MonitorAutoLock lock(mCurrentTaskMonitor);
-    mVsyncEnabled = true;
-    MOZ_ASSERT(mVsyncThread->IsRunning());
-    mCurrentVsyncTask = NewRunnableMethod(this,
-        &SoftwareDisplay::NotifyVsync,
-        mozilla::TimeStamp::Now());
-    mVsyncThread->message_loop()->PostTask(FROM_HERE, mCurrentVsyncTask);
-  }
+  MOZ_ASSERT(IsInSoftwareVsyncThread());
+  NotifyVsync(mozilla::TimeStamp::Now());
 }
 
 void
 SoftwareDisplay::DisableVsync()
 {
-  MOZ_ASSERT(NS_IsMainThread());
-  if (!IsVsyncEnabled()) {
+  MOZ_ASSERT(mVsyncThread->IsRunning());
+  if (NS_IsMainThread()) {
+    if (!mVsyncEnabled) {
+      return;
+    }
+    mVsyncEnabled = false;
+
+    mVsyncThread->message_loop()->PostTask(FROM_HERE,
+      NewRunnableMethod(this, &SoftwareDisplay::DisableVsync));
     return;
   }
 
-  MOZ_ASSERT(mVsyncThread->IsRunning());
-  { // scope lock
-    mozilla::MonitorAutoLock lock(mCurrentTaskMonitor);
-    mVsyncEnabled = false;
-    if (mCurrentVsyncTask) {
-      mCurrentVsyncTask->Cancel();
-      mCurrentVsyncTask = nullptr;
-    }
+  MOZ_ASSERT(IsInSoftwareVsyncThread());
+  if (mCurrentVsyncTask) {
+    mCurrentVsyncTask->Cancel();
+    mCurrentVsyncTask = nullptr;
   }
 }
 
@@ -76,7 +80,6 @@ bool
 SoftwareDisplay::IsVsyncEnabled()
 {
   MOZ_ASSERT(NS_IsMainThread());
-  mozilla::MonitorAutoLock lock(mCurrentTaskMonitor);
   return mVsyncEnabled;
 }
 
@@ -118,12 +121,6 @@ SoftwareDisplay::ScheduleNextVsync(mozilla::TimeStamp aVsyncTimestamp)
     delay = mozilla::TimeDuration::FromMilliseconds(0);
   }
 
-  mozilla::MonitorAutoLock lock(mCurrentTaskMonitor);
-  // We could've disabled vsync between this posted task and when it actually
-  // executes
-  if (!mVsyncEnabled) {
-    return;
-  }
   mCurrentVsyncTask = NewRunnableMethod(this,
       &SoftwareDisplay::NotifyVsync,
       nextVsync);
@@ -133,9 +130,11 @@ SoftwareDisplay::ScheduleNextVsync(mozilla::TimeStamp aVsyncTimestamp)
       delay.ToMilliseconds());
 }
 
-SoftwareDisplay::~SoftwareDisplay()
+void
+SoftwareDisplay::Shutdown()
 {
   MOZ_ASSERT(NS_IsMainThread());
+  DisableVsync();
   mVsyncThread->Stop();
   delete mVsyncThread;
 }
diff --git a/gfx/thebes/SoftwareVsyncSource.h b/gfx/thebes/SoftwareVsyncSource.h
index 2b0ee26308a5..f4923b274a61 100644
--- a/gfx/thebes/SoftwareVsyncSource.h
+++ b/gfx/thebes/SoftwareVsyncSource.h
@@ -28,6 +28,7 @@ public:
   bool IsInSoftwareVsyncThread();
   virtual void NotifyVsync(mozilla::TimeStamp aVsyncTimestamp) override;
   void ScheduleNextVsync(mozilla::TimeStamp aVsyncTimestamp);
+  void Shutdown();
 
 protected:
   ~SoftwareDisplay();
@@ -36,10 +37,8 @@ private:
   mozilla::TimeDuration mVsyncRate;
   // Use a chromium thread because nsITimers* fire on the main thread
   base::Thread* mVsyncThread;
-  bool mVsyncEnabled;
-  CancelableTask* mCurrentVsyncTask;
-  // Locks against both mCurrentVsyncTask and mVsyncEnabled
-  mozilla::Monitor mCurrentTaskMonitor;
+  CancelableTask* mCurrentVsyncTask; // only access on vsync thread
+  bool mVsyncEnabled; // Only access on main thread
 }; // SoftwareDisplay
 
 // Fallback option to use a software timer to mimic vsync. Useful for gtests