commit 7553059284a38f9aa4b1f91c8dbe723a9130a2cf
author TreeHugger Robot <treehugger-gerrit@google.com> Thu Jun 18 00:43:40 2020 +0000
committer Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com> Thu Jun 18 00:43:40 2020 +0000
tree 016aabee50f8ae4bca713303a2135d2e5395fa69
parent afde1b232549dafdcfc2281e5f432686aa1a0fe8
parent ec9b37a0ce4196024d32133b36cb333cab19614f

Merge "SF: avoid rearming Timer during cancel" into rvc-dev am: 0f083b8f33 am: 5f0d4ca567 am: 493368fa7a am: ec9b37a0ce

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

Change-Id: I5dc25837f2336c884f5cc539faf1ce9efbd01d31

services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.cpp

diff --git a/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.cpp b/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.cpp
index abeacfe..a596bce 100644
--- a/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.cpp
+++ b/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.cpp
@@ -338,10 +338,14 @@
     }
     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;

services/surfaceflinger/tests/unittests/VSyncDispatchTimerQueueTest.cpp

diff --git a/services/surfaceflinger/tests/unittests/VSyncDispatchTimerQueueTest.cpp b/services/surfaceflinger/tests/unittests/VSyncDispatchTimerQueueTest.cpp
index 793cb8b..d940dc5 100644
--- a/services/surfaceflinger/tests/unittests/VSyncDispatchTimerQueueTest.cpp
+++ b/services/surfaceflinger/tests/unittests/VSyncDispatchTimerQueueTest.cpp
@@ -701,6 +701,52 @@
     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;