commit 889c78e6e75bd41c13a39edce67e85d89465b526
author Leon Scroggins <scroggo@google.com> Mon Mar 06 15:20:55 2023 +0000
committer Android (Google) Code Review <android-gerrit@google.com> Mon Mar 06 15:20:55 2023 +0000
tree cee505170b6da3c07734f06ff074c1fff9c916bf
parent 7f247abaa9ba0a180a5313bf6b2bac58a98d9adc
parent aadc62d761cfda80a5aedab7643767292a7fcca9

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

services/surfaceflinger/Scheduler/VsyncSchedule.cpp

diff --git a/services/surfaceflinger/Scheduler/VsyncSchedule.cpp b/services/surfaceflinger/Scheduler/VsyncSchedule.cpp
index 62e37db..84671ae 100644
--- a/services/surfaceflinger/Scheduler/VsyncSchedule.cpp
+++ b/services/surfaceflinger/Scheduler/VsyncSchedule.cpp
@@ -176,10 +176,16 @@
 
 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) {

services/surfaceflinger/tests/unittests/VsyncScheduleTest.cpp

diff --git a/services/surfaceflinger/tests/unittests/VsyncScheduleTest.cpp b/services/surfaceflinger/tests/unittests/VsyncScheduleTest.cpp
index adf0804..4010fa6 100644
--- a/services/surfaceflinger/tests/unittests/VsyncScheduleTest.cpp
+++ b/services/surfaceflinger/tests/unittests/VsyncScheduleTest.cpp
@@ -86,6 +86,12 @@
     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 @@
     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 @@
     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 @@
     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