Fix subtle bug in disableHardwareVsync
I54a1304a3428968134cc707b24d5b325927c31df introduced a bug in
disableHardwareVsync. If you pass false for `disallow`, it can switch
from the `Disallowed` state to the `Disabled` state, which is not
intended. Correct this by leaving the state unchanged if it's already
`Disallowed`.
Add tests. DisableDoesNotMakeAllowed catches the failure. While we're at
it, add a test that verifies that it disallows when it's intended to,
versions of the existing tests calling disableHardwareVsync that
pass true for `disallow`, and a test verifying that disable leaves it
allowed when passing `false`.
Bug: 241286146
Test: VsyncScheduleTest
Change-Id: I0036ba97b28bef64f9bae7c1c93f3c31e8733f48
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) {