SF: VSyncDispatch: correct vsync prediction drift

Refine VSyncDispatch::schedule implementation so that refining
the prediction by small amounts would not lead to skipped callbacks.
The current implementation did not account for a case where
a valid vsync callback would be skipped. (exposed
in unit testing). Like the rest of VSyncDispatch, this
code is flagged off (ie, latent, not production code yet)

Fixes: 145213786
Bug: 146050690
Test: 6 new unit tests, 3 unit test change
Test: validation via systrace

Change-Id: I400fc5e3c181b49ab237b0dd0da2a62e38522fa0
diff --git a/services/surfaceflinger/Scheduler/VSyncDispatch.h b/services/surfaceflinger/Scheduler/VSyncDispatch.h
index e001080..56b3252 100644
--- a/services/surfaceflinger/Scheduler/VSyncDispatch.h
+++ b/services/surfaceflinger/Scheduler/VSyncDispatch.h
@@ -26,7 +26,7 @@
 class TimeKeeper;
 class VSyncTracker;
 
-enum class ScheduleResult { Scheduled, ReScheduled, CannotSchedule, Error };
+enum class ScheduleResult { Scheduled, CannotSchedule, Error };
 enum class CancelResult { Cancelled, TooLate, Error };
 
 /*
@@ -83,7 +83,6 @@
      * \param [in] earliestVsync   The targeted display time. This will be snapped to the closest
      *                             predicted vsync time after earliestVsync.
      * \return                     A ScheduleResult::Scheduled if callback was scheduled.
-     *                             A ScheduleResult::ReScheduled if callback was rescheduled.
      *                             A ScheduleResult::CannotSchedule
      *                             if (workDuration - earliestVsync) is in the past, or
      *                             if a callback was dispatched for the predictedVsync already.
diff --git a/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.cpp b/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.cpp
index a79fe98..48f2abb 100644
--- a/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.cpp
+++ b/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.cpp
@@ -29,8 +29,13 @@
 TimeKeeper::~TimeKeeper() = default;
 
 VSyncDispatchTimerQueueEntry::VSyncDispatchTimerQueueEntry(std::string const& name,
-                                                           std::function<void(nsecs_t)> const& cb)
-      : mName(name), mCallback(cb), mWorkDuration(0), mEarliestVsync(0) {}
+                                                           std::function<void(nsecs_t)> const& cb,
+                                                           nsecs_t minVsyncDistance)
+      : mName(name),
+        mCallback(cb),
+        mWorkDuration(0),
+        mEarliestVsync(0),
+        mMinVsyncDistance(minVsyncDistance) {}
 
 std::optional<nsecs_t> VSyncDispatchTimerQueueEntry::lastExecutedVsyncTarget() const {
     return mLastDispatchTime;
@@ -49,18 +54,28 @@
 
 ScheduleResult VSyncDispatchTimerQueueEntry::schedule(nsecs_t workDuration, nsecs_t earliestVsync,
                                                       VSyncTracker& tracker, nsecs_t now) {
-    auto const nextVsyncTime =
+    auto nextVsyncTime =
             tracker.nextAnticipatedVSyncTimeFrom(std::max(earliestVsync, now + workDuration));
-    if (mLastDispatchTime >= nextVsyncTime) { // already dispatched a callback for this vsync
-        return ScheduleResult::CannotSchedule;
+
+    bool const wouldSkipAVsyncTarget =
+            mArmedInfo && (nextVsyncTime > (mArmedInfo->mActualVsyncTime + mMinVsyncDistance));
+    if (wouldSkipAVsyncTarget) {
+        return ScheduleResult::Scheduled;
+    }
+
+    bool const alreadyDispatchedForVsync = mLastDispatchTime &&
+            ((*mLastDispatchTime + mMinVsyncDistance) >= nextVsyncTime &&
+             (*mLastDispatchTime - mMinVsyncDistance) <= nextVsyncTime);
+    if (alreadyDispatchedForVsync) {
+        nextVsyncTime =
+                tracker.nextAnticipatedVSyncTimeFrom(*mLastDispatchTime + mMinVsyncDistance);
     }
 
     auto const nextWakeupTime = nextVsyncTime - workDuration;
-    auto result = mArmedInfo ? ScheduleResult::ReScheduled : ScheduleResult::Scheduled;
     mWorkDuration = workDuration;
     mEarliestVsync = earliestVsync;
     mArmedInfo = {nextWakeupTime, nextVsyncTime};
-    return result;
+    return ScheduleResult::Scheduled;
 }
 
 void VSyncDispatchTimerQueueEntry::update(VSyncTracker& tracker, nsecs_t now) {
@@ -101,8 +116,12 @@
 }
 
 VSyncDispatchTimerQueue::VSyncDispatchTimerQueue(std::unique_ptr<TimeKeeper> tk,
-                                                 VSyncTracker& tracker, nsecs_t timerSlack)
-      : mTimeKeeper(std::move(tk)), mTracker(tracker), mTimerSlack(timerSlack) {}
+                                                 VSyncTracker& tracker, nsecs_t timerSlack,
+                                                 nsecs_t minVsyncDistance)
+      : mTimeKeeper(std::move(tk)),
+        mTracker(tracker),
+        mTimerSlack(timerSlack),
+        mMinVsyncDistance(minVsyncDistance) {}
 
 VSyncDispatchTimerQueue::~VSyncDispatchTimerQueue() {
     std::lock_guard<decltype(mMutex)> lk(mMutex);
@@ -187,7 +206,8 @@
             mCallbacks
                     .emplace(++mCallbackToken,
                              std::make_shared<VSyncDispatchTimerQueueEntry>(callbackName,
-                                                                            callbackFn))
+                                                                            callbackFn,
+                                                                            mMinVsyncDistance))
                     .first->first};
 }
 
@@ -277,12 +297,16 @@
 }
 
 ScheduleResult VSyncCallbackRegistration::schedule(nsecs_t workDuration, nsecs_t earliestVsync) {
-    if (!mValidToken) return ScheduleResult::Error;
+    if (!mValidToken) {
+        return ScheduleResult::Error;
+    }
     return mDispatch.get().schedule(mToken, workDuration, earliestVsync);
 }
 
 CancelResult VSyncCallbackRegistration::cancel() {
-    if (!mValidToken) return CancelResult::Error;
+    if (!mValidToken) {
+        return CancelResult::Error;
+    }
     return mDispatch.get().cancel(mToken);
 }
 
diff --git a/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.h b/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.h
index fc78da3..0c9b4fe 100644
--- a/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.h
+++ b/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.h
@@ -36,7 +36,8 @@
     // Valid transition: disarmed -> armed ( when scheduled )
     // Valid transition: armed -> running -> disarmed ( when timer is called)
     // Valid transition: armed -> disarmed ( when cancelled )
-    VSyncDispatchTimerQueueEntry(std::string const& name, std::function<void(nsecs_t)> const& fn);
+    VSyncDispatchTimerQueueEntry(std::string const& name, std::function<void(nsecs_t)> const& fn,
+                                 nsecs_t minVsyncDistance);
     std::string_view name() const;
 
     // Start: functions that are not threadsafe.
@@ -72,6 +73,7 @@
 
     nsecs_t mWorkDuration;
     nsecs_t mEarliestVsync;
+    nsecs_t const mMinVsyncDistance;
 
     struct ArmingInfo {
         nsecs_t mActualWakeupTime;
@@ -91,8 +93,15 @@
  */
 class VSyncDispatchTimerQueue : public VSyncDispatch {
 public:
+    // Constructs a VSyncDispatchTimerQueue.
+    // \param[in] tk                    A timekeeper.
+    // \param[in] tracker               A tracker.
+    // \param[in] timerSlack            The threshold at which different similarly timed callbacks
+    //                                  should be grouped into one wakeup.
+    // \param[in] minVsyncDistance      The minimum distance between two vsync estimates before the
+    //                                  vsyncs are considered the same vsync event.
     explicit VSyncDispatchTimerQueue(std::unique_ptr<TimeKeeper> tk, VSyncTracker& tracker,
-                                     nsecs_t timerSlack);
+                                     nsecs_t timerSlack, nsecs_t minVsyncDistance);
     ~VSyncDispatchTimerQueue();
 
     CallbackToken registerCallback(std::function<void(nsecs_t)> const& callbackFn,
@@ -119,6 +128,7 @@
     std::unique_ptr<TimeKeeper> const mTimeKeeper;
     VSyncTracker& mTracker;
     nsecs_t const mTimerSlack;
+    nsecs_t const mMinVsyncDistance;
 
     std::mutex mutable mMutex;
     size_t mCallbackToken GUARDED_BY(mMutex) = 0;
diff --git a/services/surfaceflinger/Scheduler/VSyncReactor.cpp b/services/surfaceflinger/Scheduler/VSyncReactor.cpp
index 49ab6c1..47e3f4f 100644
--- a/services/surfaceflinger/Scheduler/VSyncReactor.cpp
+++ b/services/surfaceflinger/Scheduler/VSyncReactor.cpp
@@ -14,6 +14,8 @@
  * limitations under the License.
  */
 
+#undef LOG_TAG
+#define LOG_TAG "VSyncReactor"
 //#define LOG_NDEBUG 0
 #include "VSyncReactor.h"
 #include <log/log.h>
@@ -59,8 +61,9 @@
         mStopped = false;
         mOffset = offset;
 
-        // TODO: (b/145213786) check the return code here sensibly
-        mRegistration.schedule(calculateWorkload(), mLastCallTime);
+        auto const schedule_result = mRegistration.schedule(calculateWorkload(), mLastCallTime);
+        LOG_ALWAYS_FATAL_IF((schedule_result != ScheduleResult::Scheduled),
+                            "Error scheduling callback: rc %X", schedule_result);
     }
 
     void setPeriod(nsecs_t period) {
@@ -91,7 +94,9 @@
 
         {
             std::lock_guard<std::mutex> lk(mMutex);
-            mRegistration.schedule(calculateWorkload(), vsynctime);
+            auto const schedule_result = mRegistration.schedule(calculateWorkload(), vsynctime);
+            LOG_ALWAYS_FATAL_IF((schedule_result != ScheduleResult::Scheduled),
+                                "Error rescheduling callback: rc %X", schedule_result);
         }
     }
 
diff --git a/services/surfaceflinger/tests/unittests/VSyncDispatchRealtimeTest.cpp b/services/surfaceflinger/tests/unittests/VSyncDispatchRealtimeTest.cpp
index 484947d..5846c77 100644
--- a/services/surfaceflinger/tests/unittests/VSyncDispatchRealtimeTest.cpp
+++ b/services/surfaceflinger/tests/unittests/VSyncDispatchRealtimeTest.cpp
@@ -92,6 +92,7 @@
 
 struct VSyncDispatchRealtimeTest : testing::Test {
     static nsecs_t constexpr mDispatchGroupThreshold = toNs(100us);
+    static nsecs_t constexpr mVsyncMoveThreshold = toNs(500us);
     static size_t constexpr mIterations = 20;
 };
 
@@ -148,7 +149,8 @@
 
 TEST_F(VSyncDispatchRealtimeTest, triple_alarm) {
     FixedRateIdealStubTracker tracker;
-    VSyncDispatchTimerQueue dispatch(std::make_unique<Timer>(), tracker, mDispatchGroupThreshold);
+    VSyncDispatchTimerQueue dispatch(std::make_unique<Timer>(), tracker, mDispatchGroupThreshold,
+                                     mVsyncMoveThreshold);
 
     static size_t constexpr num_clients = 3;
     std::array<RepeatingCallbackReceiver, num_clients>
@@ -176,7 +178,8 @@
 TEST_F(VSyncDispatchRealtimeTest, vascillating_vrr) {
     auto next_vsync_interval = toNs(3ms);
     VRRStubTracker tracker(next_vsync_interval);
-    VSyncDispatchTimerQueue dispatch(std::make_unique<Timer>(), tracker, mDispatchGroupThreshold);
+    VSyncDispatchTimerQueue dispatch(std::make_unique<Timer>(), tracker, mDispatchGroupThreshold,
+                                     mVsyncMoveThreshold);
 
     RepeatingCallbackReceiver cb_receiver(dispatch, toNs(1ms));
 
@@ -193,7 +196,8 @@
 // starts at 333hz, jumps to 200hz at frame 10
 TEST_F(VSyncDispatchRealtimeTest, fixed_jump) {
     VRRStubTracker tracker(toNs(3ms));
-    VSyncDispatchTimerQueue dispatch(std::make_unique<Timer>(), tracker, mDispatchGroupThreshold);
+    VSyncDispatchTimerQueue dispatch(std::make_unique<Timer>(), tracker, mDispatchGroupThreshold,
+                                     mVsyncMoveThreshold);
 
     RepeatingCallbackReceiver cb_receiver(dispatch, toNs(1ms));
 
diff --git a/services/surfaceflinger/tests/unittests/VSyncDispatchTimerQueueTest.cpp b/services/surfaceflinger/tests/unittests/VSyncDispatchTimerQueueTest.cpp
index d668a41..5aff429 100644
--- a/services/surfaceflinger/tests/unittests/VSyncDispatchTimerQueueTest.cpp
+++ b/services/surfaceflinger/tests/unittests/VSyncDispatchTimerQueueTest.cpp
@@ -196,16 +196,18 @@
     NiceMock<ControllableClock> mMockClock;
     static nsecs_t constexpr mDispatchGroupThreshold = 5;
     nsecs_t const mPeriod = 1000;
+    nsecs_t const mVsyncMoveThreshold = 300;
     NiceMock<MockVSyncTracker> mStubTracker{mPeriod};
-    VSyncDispatchTimerQueue mDispatch{createTimeKeeper(), mStubTracker, mDispatchGroupThreshold};
+    VSyncDispatchTimerQueue mDispatch{createTimeKeeper(), mStubTracker, mDispatchGroupThreshold,
+                                      mVsyncMoveThreshold};
 };
 
 TEST_F(VSyncDispatchTimerQueueTest, unregistersSetAlarmOnDestruction) {
     EXPECT_CALL(mMockClock, alarmIn(_, 900));
     EXPECT_CALL(mMockClock, alarmCancel());
     {
-        VSyncDispatchTimerQueue mDispatch{createTimeKeeper(), mStubTracker,
-                                          mDispatchGroupThreshold};
+        VSyncDispatchTimerQueue mDispatch{createTimeKeeper(), mStubTracker, mDispatchGroupThreshold,
+                                          mVsyncMoveThreshold};
         CountingCallback cb(mDispatch);
         EXPECT_EQ(mDispatch.schedule(cb, 100, 1000), ScheduleResult::Scheduled);
     }
@@ -468,14 +470,24 @@
 
 TEST_F(VSyncDispatchTimerQueueTest, callbackReentrantWithPastWakeup) {
     VSyncDispatch::CallbackToken tmp;
+    std::optional<nsecs_t> lastTarget;
     tmp = mDispatch.registerCallback(
-            [&](auto) {
-                EXPECT_EQ(mDispatch.schedule(tmp, 400, 1000), ScheduleResult::CannotSchedule);
+            [&](auto timestamp) {
+                EXPECT_EQ(mDispatch.schedule(tmp, 400, timestamp - mVsyncMoveThreshold),
+                          ScheduleResult::Scheduled);
+                EXPECT_EQ(mDispatch.schedule(tmp, 400, timestamp), ScheduleResult::Scheduled);
+                EXPECT_EQ(mDispatch.schedule(tmp, 400, timestamp + mVsyncMoveThreshold),
+                          ScheduleResult::Scheduled);
+                lastTarget = timestamp;
             },
             "oo");
 
     mDispatch.schedule(tmp, 999, 1000);
     advanceToNextCallback();
+    EXPECT_THAT(lastTarget, Eq(1000));
+
+    advanceToNextCallback();
+    EXPECT_THAT(lastTarget, Eq(2000));
 }
 
 TEST_F(VSyncDispatchTimerQueueTest, modificationsAroundVsyncTime) {
@@ -547,10 +559,33 @@
     EXPECT_THAT(mDispatch.cancel(token), Eq(CancelResult::Error));
 }
 
-TEST_F(VSyncDispatchTimerQueueTest, distinguishesScheduleAndReschedule) {
+TEST_F(VSyncDispatchTimerQueueTest, canMoveCallbackBackwardsInTime) {
     CountingCallback cb0(mDispatch);
     EXPECT_EQ(mDispatch.schedule(cb0, 500, 1000), ScheduleResult::Scheduled);
-    EXPECT_EQ(mDispatch.schedule(cb0, 100, 1000), ScheduleResult::ReScheduled);
+    EXPECT_EQ(mDispatch.schedule(cb0, 100, 1000), ScheduleResult::Scheduled);
+}
+
+// b/1450138150
+TEST_F(VSyncDispatchTimerQueueTest, doesNotMoveCallbackBackwardsAndSkipAScheduledTargetVSync) {
+    EXPECT_CALL(mMockClock, alarmIn(_, 500));
+    CountingCallback cb(mDispatch);
+    EXPECT_EQ(mDispatch.schedule(cb, 500, 1000), ScheduleResult::Scheduled);
+    mMockClock.advanceBy(400);
+
+    EXPECT_EQ(mDispatch.schedule(cb, 800, 1000), ScheduleResult::Scheduled);
+    advanceToNextCallback();
+    ASSERT_THAT(cb.mCalls.size(), Eq(1));
+}
+
+TEST_F(VSyncDispatchTimerQueueTest, targetOffsetMovingBackALittleCanStillSchedule) {
+    EXPECT_CALL(mStubTracker, nextAnticipatedVSyncTimeFrom(1000))
+            .Times(2)
+            .WillOnce(Return(1000))
+            .WillOnce(Return(1002));
+    CountingCallback cb(mDispatch);
+    EXPECT_EQ(mDispatch.schedule(cb, 500, 1000), ScheduleResult::Scheduled);
+    mMockClock.advanceBy(400);
+    EXPECT_EQ(mDispatch.schedule(cb, 400, 1000), ScheduleResult::Scheduled);
 }
 
 TEST_F(VSyncDispatchTimerQueueTest, canScheduleNegativeOffsetAgainstDifferentPeriods) {
@@ -570,13 +605,15 @@
     EXPECT_EQ(mDispatch.schedule(cb0, 1900, 2000), ScheduleResult::Scheduled);
 }
 
-TEST_F(VSyncDispatchTimerQueueTest, cannotScheduleDoesNotAffectSchedulingState) {
+TEST_F(VSyncDispatchTimerQueueTest, scheduleUpdatesDoesNotAffectSchedulingState) {
     EXPECT_CALL(mMockClock, alarmIn(_, 600));
 
     CountingCallback cb(mDispatch);
     EXPECT_EQ(mDispatch.schedule(cb, 400, 1000), ScheduleResult::Scheduled);
+
+    EXPECT_EQ(mDispatch.schedule(cb, 1400, 1000), ScheduleResult::Scheduled);
+
     advanceToNextCallback();
-    EXPECT_EQ(mDispatch.schedule(cb, 100, 1000), ScheduleResult::CannotSchedule);
 }
 
 TEST_F(VSyncDispatchTimerQueueTest, helperMove) {
@@ -612,19 +649,22 @@
 class VSyncDispatchTimerQueueEntryTest : public testing::Test {
 protected:
     nsecs_t const mPeriod = 1000;
+    nsecs_t const mVsyncMoveThreshold = 200;
     NiceMock<MockVSyncTracker> mStubTracker{mPeriod};
 };
 
 TEST_F(VSyncDispatchTimerQueueEntryTest, stateAfterInitialization) {
     std::string name("basicname");
-    VSyncDispatchTimerQueueEntry entry(name, [](auto) {});
+    VSyncDispatchTimerQueueEntry entry(
+            name, [](auto) {}, mVsyncMoveThreshold);
     EXPECT_THAT(entry.name(), Eq(name));
     EXPECT_FALSE(entry.lastExecutedVsyncTarget());
     EXPECT_FALSE(entry.wakeupTime());
 }
 
 TEST_F(VSyncDispatchTimerQueueEntryTest, stateScheduling) {
-    VSyncDispatchTimerQueueEntry entry("test", [](auto) {});
+    VSyncDispatchTimerQueueEntry entry(
+            "test", [](auto) {}, mVsyncMoveThreshold);
 
     EXPECT_FALSE(entry.wakeupTime());
     EXPECT_THAT(entry.schedule(100, 500, mStubTracker, 0), Eq(ScheduleResult::Scheduled));
@@ -643,7 +683,8 @@
     EXPECT_CALL(mStubTracker, nextAnticipatedVSyncTimeFrom(now + duration))
             .Times(1)
             .WillOnce(Return(10000));
-    VSyncDispatchTimerQueueEntry entry("test", [](auto) {});
+    VSyncDispatchTimerQueueEntry entry(
+            "test", [](auto) {}, mVsyncMoveThreshold);
 
     EXPECT_FALSE(entry.wakeupTime());
     EXPECT_THAT(entry.schedule(500, 994, mStubTracker, now), Eq(ScheduleResult::Scheduled));
@@ -655,10 +696,13 @@
 TEST_F(VSyncDispatchTimerQueueEntryTest, runCallback) {
     auto callCount = 0;
     auto calledTime = 0;
-    VSyncDispatchTimerQueueEntry entry("test", [&](auto time) {
-        callCount++;
-        calledTime = time;
-    });
+    VSyncDispatchTimerQueueEntry entry(
+            "test",
+            [&](auto time) {
+                callCount++;
+                calledTime = time;
+            },
+            mVsyncMoveThreshold);
 
     EXPECT_THAT(entry.schedule(100, 500, mStubTracker, 0), Eq(ScheduleResult::Scheduled));
     auto const wakeup = entry.wakeupTime();
@@ -681,7 +725,8 @@
             .WillOnce(Return(1000))
             .WillOnce(Return(1020));
 
-    VSyncDispatchTimerQueueEntry entry("test", [](auto) {});
+    VSyncDispatchTimerQueueEntry entry(
+            "test", [](auto) {}, mVsyncMoveThreshold);
 
     EXPECT_FALSE(entry.wakeupTime());
     entry.update(mStubTracker, 0);
@@ -699,7 +744,8 @@
 }
 
 TEST_F(VSyncDispatchTimerQueueEntryTest, skipsUpdateIfJustScheduled) {
-    VSyncDispatchTimerQueueEntry entry("test", [](auto) {});
+    VSyncDispatchTimerQueueEntry entry(
+            "test", [](auto) {}, mVsyncMoveThreshold);
     EXPECT_THAT(entry.schedule(100, 500, mStubTracker, 0), Eq(ScheduleResult::Scheduled));
     entry.update(mStubTracker, 0);
 
@@ -708,19 +754,52 @@
     EXPECT_THAT(*wakeup, Eq(wakeup));
 }
 
-TEST_F(VSyncDispatchTimerQueueEntryTest, reportsCannotScheduleIfMissedOpportunity) {
-    VSyncDispatchTimerQueueEntry entry("test", [](auto) {});
+TEST_F(VSyncDispatchTimerQueueEntryTest, willSnapToNextTargettableVSync) {
+    VSyncDispatchTimerQueueEntry entry(
+            "test", [](auto) {}, mVsyncMoveThreshold);
     EXPECT_THAT(entry.schedule(100, 500, mStubTracker, 0), Eq(ScheduleResult::Scheduled));
-    entry.executing();
-    EXPECT_THAT(entry.schedule(200, 500, mStubTracker, 0), Eq(ScheduleResult::CannotSchedule));
-    EXPECT_THAT(entry.schedule(50, 500, mStubTracker, 0), Eq(ScheduleResult::CannotSchedule));
+    entry.executing(); // 1000 is executing
+    // had 1000 not been executing, this could have been scheduled for time 800.
+    EXPECT_THAT(entry.schedule(200, 500, mStubTracker, 0), Eq(ScheduleResult::Scheduled));
+    EXPECT_THAT(*entry.wakeupTime(), Eq(1800));
+
+    EXPECT_THAT(entry.schedule(50, 500, mStubTracker, 0), Eq(ScheduleResult::Scheduled));
+    EXPECT_THAT(*entry.wakeupTime(), Eq(1950));
+
     EXPECT_THAT(entry.schedule(200, 1001, mStubTracker, 0), Eq(ScheduleResult::Scheduled));
+    EXPECT_THAT(*entry.wakeupTime(), Eq(1800));
 }
 
-TEST_F(VSyncDispatchTimerQueueEntryTest, reportsReScheduleIfStillTime) {
-    VSyncDispatchTimerQueueEntry entry("test", [](auto) {});
+TEST_F(VSyncDispatchTimerQueueEntryTest,
+       willRequestNextEstimateWhenSnappingToNextTargettableVSync) {
+    VSyncDispatchTimerQueueEntry entry(
+            "test", [](auto) {}, mVsyncMoveThreshold);
+
+    Sequence seq;
+    EXPECT_CALL(mStubTracker, nextAnticipatedVSyncTimeFrom(500))
+            .InSequence(seq)
+            .WillOnce(Return(1000));
+    EXPECT_CALL(mStubTracker, nextAnticipatedVSyncTimeFrom(500))
+            .InSequence(seq)
+            .WillOnce(Return(1000));
+    EXPECT_CALL(mStubTracker, nextAnticipatedVSyncTimeFrom(1000 + mVsyncMoveThreshold))
+            .InSequence(seq)
+            .WillOnce(Return(2000));
+
     EXPECT_THAT(entry.schedule(100, 500, mStubTracker, 0), Eq(ScheduleResult::Scheduled));
-    EXPECT_THAT(entry.schedule(200, 500, mStubTracker, 0), Eq(ScheduleResult::ReScheduled));
+
+    entry.executing(); // 1000 is executing
+
+    EXPECT_THAT(entry.schedule(200, 500, mStubTracker, 0), Eq(ScheduleResult::Scheduled));
+}
+
+TEST_F(VSyncDispatchTimerQueueEntryTest, reportsScheduledIfStillTime) {
+    VSyncDispatchTimerQueueEntry entry(
+            "test", [](auto) {}, mVsyncMoveThreshold);
+    EXPECT_THAT(entry.schedule(100, 500, mStubTracker, 0), Eq(ScheduleResult::Scheduled));
+    EXPECT_THAT(entry.schedule(200, 500, mStubTracker, 0), Eq(ScheduleResult::Scheduled));
+    EXPECT_THAT(entry.schedule(50, 500, mStubTracker, 0), Eq(ScheduleResult::Scheduled));
+    EXPECT_THAT(entry.schedule(1200, 500, mStubTracker, 0), Eq(ScheduleResult::Scheduled));
 }
 
 } // namespace android::scheduler
diff --git a/services/surfaceflinger/tests/unittests/VSyncReactorTest.cpp b/services/surfaceflinger/tests/unittests/VSyncReactorTest.cpp
index 652a7b8..537cc80 100644
--- a/services/surfaceflinger/tests/unittests/VSyncReactorTest.cpp
+++ b/services/surfaceflinger/tests/unittests/VSyncReactorTest.cpp
@@ -416,6 +416,29 @@
     mReactor.addEventListener(mName, mPhase, &outerCb, lastCallbackTime);
 }
 
+TEST_F(VSyncReactorTest, offsetsAppliedOnNextOpportunity) {
+    Sequence seq;
+    EXPECT_CALL(*mMockDispatch, registerCallback(_, std::string(mName)))
+            .InSequence(seq)
+            .WillOnce(DoAll(SaveArg<0>(&innerCb), Return(mFakeToken)));
+    EXPECT_CALL(*mMockDispatch, schedule(mFakeToken, computeWorkload(period, mPhase), _))
+            .InSequence(seq)
+            .WillOnce(Return(ScheduleResult::Scheduled));
+
+    EXPECT_CALL(*mMockDispatch, schedule(mFakeToken, computeWorkload(period, mAnotherPhase), _))
+            .InSequence(seq)
+            .WillOnce(Return(ScheduleResult::Scheduled));
+
+    EXPECT_CALL(*mMockDispatch, schedule(mFakeToken, computeWorkload(period, mAnotherPhase), _))
+            .InSequence(seq)
+            .WillOnce(Return(ScheduleResult::Scheduled));
+
+    mReactor.addEventListener(mName, mPhase, &outerCb, lastCallbackTime);
+    mReactor.changePhaseOffset(&outerCb, mAnotherPhase);
+    ASSERT_TRUE(innerCb);
+    innerCb(mFakeCbTime);
+}
+
 TEST_F(VSyncReactorTest, negativeOffsetsApplied) {
     nsecs_t const negativePhase = -4000;
     Sequence seq;
@@ -446,4 +469,24 @@
     mReactor.changePhaseOffset(&outerCb, mPhase);
 }
 
+TEST_F(VSyncReactorDeathTest, cannotScheduleOnRegistration) {
+    ON_CALL(*mMockDispatch, schedule(_, _, _))
+            .WillByDefault(Return(ScheduleResult::CannotSchedule));
+    EXPECT_DEATH(mReactor.addEventListener(mName, mPhase, &outerCb, lastCallbackTime), ".*");
+}
+
+TEST_F(VSyncReactorDeathTest, cannotScheduleOnCallback) {
+    EXPECT_CALL(*mMockDispatch, registerCallback(_, std::string(mName)))
+            .WillOnce(DoAll(SaveArg<0>(&innerCb), Return(mFakeToken)));
+    EXPECT_CALL(*mMockDispatch, schedule(_, _, _)).WillOnce(Return(ScheduleResult::Scheduled));
+
+    mReactor.addEventListener(mName, mPhase, &outerCb, lastCallbackTime);
+    ASSERT_TRUE(innerCb);
+    Mock::VerifyAndClearExpectations(mMockDispatch.get());
+
+    ON_CALL(*mMockDispatch, schedule(_, _, _))
+            .WillByDefault(Return(ScheduleResult::CannotSchedule));
+    EXPECT_DEATH(innerCb(mFakeCbTime), ".*");
+}
+
 } // namespace android::scheduler