Merge "SF: avoid rearming Timer during cancel" into rvc-dev am: 0f083b8f33

Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/native/+/11881628

Change-Id: Ic6d7ffd2a6cd16b0d2696ae1b582911d4c728191

2 files changed, 53 insertions, 3 deletions

diff --git a/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.cpp b/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.cpp
index abeacfe66e..a596bce002 100644
--- a/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.cpp
+++ b/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.cpp
@@ -338,10 +338,14 @@ CancelResult VSyncDispatchTimerQueue::cancel(CallbackToken token) {
     }
     auto& callback = it->second;
 
-    if (callback->wakeupTime()) {
+    auto const wakeupTime = callback->wakeupTime();
+    if (wakeupTime) {
         callback->disarm();
-        mIntendedWakeupTime = kInvalidTime;
-        rearmTimer(mTimeKeeper->now());
+
+        if (*wakeupTime == mIntendedWakeupTime) {
+            mIntendedWakeupTime = kInvalidTime;
+            rearmTimer(mTimeKeeper->now());
+        }
         return CancelResult::Cancelled;
     }
     return CancelResult::TooLate;
diff --git a/services/surfaceflinger/tests/unittests/VSyncDispatchTimerQueueTest.cpp b/services/surfaceflinger/tests/unittests/VSyncDispatchTimerQueueTest.cpp
index 793cb8bcca..d940dc5870 100644
--- a/services/surfaceflinger/tests/unittests/VSyncDispatchTimerQueueTest.cpp
+++ b/services/surfaceflinger/tests/unittests/VSyncDispatchTimerQueueTest.cpp
@@ -701,6 +701,52 @@ TEST_F(VSyncDispatchTimerQueueTest, skipsSchedulingIfTimerReschedulingIsImminent
     EXPECT_THAT(cb.mCalls.size(), Eq(1));
 }
 
+// b/154303580.
+TEST_F(VSyncDispatchTimerQueueTest, skipsRearmingWhenNotNextScheduled) {
+    Sequence seq;
+    EXPECT_CALL(mMockClock, alarmIn(_, 600)).InSequence(seq);
+    EXPECT_CALL(mMockClock, alarmCancel()).InSequence(seq);
+    CountingCallback cb1(mDispatch);
+    CountingCallback cb2(mDispatch);
+
+    EXPECT_EQ(mDispatch.schedule(cb1, 400, 1000), ScheduleResult::Scheduled);
+    EXPECT_EQ(mDispatch.schedule(cb2, 100, 2000), ScheduleResult::Scheduled);
+
+    mMockClock.setLag(100);
+    mMockClock.advanceBy(620);
+
+    EXPECT_EQ(mDispatch.cancel(cb2), CancelResult::Cancelled);
+
+    mMockClock.advanceBy(80);
+
+    EXPECT_THAT(cb1.mCalls.size(), Eq(1));
+    EXPECT_THAT(cb2.mCalls.size(), Eq(0));
+}
+
+TEST_F(VSyncDispatchTimerQueueTest, rearmsWhenCancelledAndIsNextScheduled) {
+    Sequence seq;
+    EXPECT_CALL(mMockClock, alarmIn(_, 600)).InSequence(seq);
+    EXPECT_CALL(mMockClock, alarmIn(_, 1280)).InSequence(seq);
+    EXPECT_CALL(mMockClock, alarmCancel()).InSequence(seq);
+    CountingCallback cb1(mDispatch);
+    CountingCallback cb2(mDispatch);
+
+    EXPECT_EQ(mDispatch.schedule(cb1, 400, 1000), ScheduleResult::Scheduled);
+    EXPECT_EQ(mDispatch.schedule(cb2, 100, 2000), ScheduleResult::Scheduled);
+
+    mMockClock.setLag(100);
+    mMockClock.advanceBy(620);
+
+    EXPECT_EQ(mDispatch.cancel(cb1), CancelResult::Cancelled);
+
+    EXPECT_THAT(cb1.mCalls.size(), Eq(0));
+    EXPECT_THAT(cb2.mCalls.size(), Eq(0));
+    mMockClock.advanceToNextCallback();
+
+    EXPECT_THAT(cb1.mCalls.size(), Eq(0));
+    EXPECT_THAT(cb2.mCalls.size(), Eq(1));
+}
+
 class VSyncDispatchTimerQueueEntryTest : public testing::Test {
 protected:
     nsecs_t const mPeriod = 1000;