Merge "Fix subtle bug in disableHardwareVsync" into udc-dev

2 files changed, 50 insertions, 3 deletions

diff --git a/services/surfaceflinger/Scheduler/VsyncSchedule.cpp b/services/surfaceflinger/Scheduler/VsyncSchedule.cpp
index 62e37db4e9..84671aea0d 100644
--- a/services/surfaceflinger/Scheduler/VsyncSchedule.cpp
+++ b/services/surfaceflinger/Scheduler/VsyncSchedule.cpp
@@ -176,10 +176,16 @@ void VsyncSchedule::enableHardwareVsyncLocked(ISchedulerCallback& callback) {
 
 void VsyncSchedule::disableHardwareVsync(ISchedulerCallback& callback, bool disallow) {
     std::lock_guard<std::mutex> lock(mHwVsyncLock);
-    if (mHwVsyncState == HwVsyncState::Enabled) {
-        callback.setVsyncEnabled(mId, false);
+    switch (mHwVsyncState) {
+        case HwVsyncState::Enabled:
+            callback.setVsyncEnabled(mId, false);
+            [[fallthrough]];
+        case HwVsyncState::Disabled:
+            mHwVsyncState = disallow ? HwVsyncState::Disallowed : HwVsyncState::Disabled;
+            break;
+        case HwVsyncState::Disallowed:
+            break;
     }
-    mHwVsyncState = disallow ? HwVsyncState::Disallowed : HwVsyncState::Disabled;
 }
 
 bool VsyncSchedule::isHardwareVsyncAllowed(bool makeAllowed) {
diff --git a/services/surfaceflinger/tests/unittests/VsyncScheduleTest.cpp b/services/surfaceflinger/tests/unittests/VsyncScheduleTest.cpp
index adf080471c..4010fa6a5b 100644
--- a/services/surfaceflinger/tests/unittests/VsyncScheduleTest.cpp
+++ b/services/surfaceflinger/tests/unittests/VsyncScheduleTest.cpp
@@ -86,6 +86,12 @@ TEST_F(VsyncScheduleTest, DisableDoesNothingWhenDisallowed) {
     mVsyncSchedule->disableHardwareVsync(mCallback, false /* disallow */);
 }
 
+TEST_F(VsyncScheduleTest, DisableDoesNothingWhenDisallowed2) {
+    EXPECT_CALL(mCallback, setVsyncEnabled(_, _)).Times(0);
+
+    mVsyncSchedule->disableHardwareVsync(mCallback, true /* disallow */);
+}
+
 TEST_F(VsyncScheduleTest, MakeAllowed) {
     ASSERT_TRUE(mVsyncSchedule->isHardwareVsyncAllowed(true /* makeAllowed */));
 }
@@ -97,6 +103,13 @@ TEST_F(VsyncScheduleTest, DisableDoesNothingWhenDisabled) {
     mVsyncSchedule->disableHardwareVsync(mCallback, false /* disallow */);
 }
 
+TEST_F(VsyncScheduleTest, DisableDoesNothingWhenDisabled2) {
+    ASSERT_TRUE(mVsyncSchedule->isHardwareVsyncAllowed(true /* makeAllowed */));
+    EXPECT_CALL(mCallback, setVsyncEnabled(_, _)).Times(0);
+
+    mVsyncSchedule->disableHardwareVsync(mCallback, true /* disallow */);
+}
+
 TEST_F(VsyncScheduleTest, EnableWorksWhenDisabled) {
     ASSERT_TRUE(mVsyncSchedule->isHardwareVsyncAllowed(true /* makeAllowed */));
     EXPECT_CALL(mCallback, setVsyncEnabled(DEFAULT_DISPLAY_ID, true));
@@ -129,6 +142,16 @@ TEST_F(VsyncScheduleTest, EnableDisable) {
     mVsyncSchedule->disableHardwareVsync(mCallback, false /* disallow */);
 }
 
+TEST_F(VsyncScheduleTest, EnableDisable2) {
+    ASSERT_TRUE(mVsyncSchedule->isHardwareVsyncAllowed(true /* makeAllowed */));
+    EXPECT_CALL(mCallback, setVsyncEnabled(DEFAULT_DISPLAY_ID, true));
+
+    mVsyncSchedule->enableHardwareVsync(mCallback);
+
+    EXPECT_CALL(mCallback, setVsyncEnabled(DEFAULT_DISPLAY_ID, false));
+    mVsyncSchedule->disableHardwareVsync(mCallback, true /* disallow */);
+}
+
 TEST_F(VsyncScheduleTest, StartPeriodTransition) {
     // Note: startPeriodTransition is only called when hardware vsyncs are
     // allowed.
@@ -225,5 +248,23 @@ TEST_F(VsyncScheduleTest, PendingState) FTL_FAKE_GUARD(kMainThreadContext) {
     ASSERT_FALSE(mVsyncSchedule->getPendingHardwareVsyncState());
 }
 
+TEST_F(VsyncScheduleTest, DisableDoesNotMakeAllowed) {
+    ASSERT_FALSE(mVsyncSchedule->isHardwareVsyncAllowed(false /* makeAllowed */));
+    mVsyncSchedule->disableHardwareVsync(mCallback, false /* disallow */);
+    ASSERT_FALSE(mVsyncSchedule->isHardwareVsyncAllowed(false /* makeAllowed */));
+}
+
+TEST_F(VsyncScheduleTest, DisallowMakesNotAllowed) {
+    ASSERT_TRUE(mVsyncSchedule->isHardwareVsyncAllowed(true /* makeAllowed */));
+    mVsyncSchedule->disableHardwareVsync(mCallback, true /* disallow */);
+    ASSERT_FALSE(mVsyncSchedule->isHardwareVsyncAllowed(false /* makeAllowed */));
+}
+
+TEST_F(VsyncScheduleTest, StillAllowedAfterDisable) {
+    ASSERT_TRUE(mVsyncSchedule->isHardwareVsyncAllowed(true /* makeAllowed */));
+    mVsyncSchedule->disableHardwareVsync(mCallback, false /* disallow */);
+    ASSERT_TRUE(mVsyncSchedule->isHardwareVsyncAllowed(false /* makeAllowed */));
+}
+
 } // namespace
 } // namespace android