Fix batching logic in InputConsumerNoResampling.cpp

Fixed batching logic in InputConsumerNoResampling.cpp because it was
violating resampling frameTime preconditions.

Bug: 297226446
Flag: EXEMPT bugfix
Test: TEST=libinput_tests; m $TEST && $ANDROID_HOST_OUT/nativetest64/$TEST/$TEST --gtest_filter="InputConsumerTest*"
Change-Id: I1b658d5b9ac7a56a8a3917a49c2b97b60641c0d5

4 files changed, 99 insertions, 18 deletions

diff --git a/include/input/Resampler.h b/include/input/Resampler.h
index 67d92bd3ad..dcb25b729f 100644
--- a/include/input/Resampler.h
+++ b/include/input/Resampler.h
@@ -47,6 +47,13 @@ struct Resampler {
      */
     virtual void resampleMotionEvent(std::chrono::nanoseconds frameTime, MotionEvent& motionEvent,
                                      const InputMessage* futureSample) = 0;
+
+    /**
+     * Returns resample latency. Resample latency is the time difference between frame time and
+     * resample time. More precisely, let frameTime and resampleTime be two timestamps, and
+     * frameTime > resampleTime. Resample latency is defined as frameTime - resampleTime.
+     */
+    virtual std::chrono::nanoseconds getResampleLatency() const = 0;
 };
 
 class LegacyResampler final : public Resampler {
@@ -63,6 +70,8 @@ public:
     void resampleMotionEvent(std::chrono::nanoseconds frameTime, MotionEvent& motionEvent,
                              const InputMessage* futureSample) override;
 
+    std::chrono::nanoseconds getResampleLatency() const override;
+
 private:
     struct Pointer {
         PointerProperties properties;
diff --git a/libs/input/InputConsumerNoResampling.cpp b/libs/input/InputConsumerNoResampling.cpp
index eb419180e7..de55828a39 100644
--- a/libs/input/InputConsumerNoResampling.cpp
+++ b/libs/input/InputConsumerNoResampling.cpp
@@ -37,6 +37,8 @@ namespace android {
 
 namespace {
 
+using std::chrono::nanoseconds;
+
 /**
  * Log debug messages relating to the consumer end of the transport channel.
  * Enable this via "adb shell setprop log.tag.InputTransportConsumer DEBUG" (requires restart)
@@ -485,7 +487,12 @@ InputConsumerNoResampling::createBatchedMotionEvent(const nsecs_t frameTime,
                                                     std::queue<InputMessage>& messages) {
     std::unique_ptr<MotionEvent> motionEvent;
     std::optional<uint32_t> firstSeqForBatch;
-    while (!messages.empty() && !(messages.front().body.motion.eventTime > frameTime)) {
+    const nanoseconds resampleLatency =
+            (mResampler != nullptr) ? mResampler->getResampleLatency() : nanoseconds{0};
+    const nanoseconds adjustedFrameTime = nanoseconds{frameTime} - resampleLatency;
+
+    while (!messages.empty() &&
+           (messages.front().body.motion.eventTime <= adjustedFrameTime.count())) {
         if (motionEvent == nullptr) {
             motionEvent = createMotionEvent(messages.front());
             firstSeqForBatch = messages.front().header.seq;
@@ -504,8 +511,7 @@ InputConsumerNoResampling::createBatchedMotionEvent(const nsecs_t frameTime,
         if (!messages.empty()) {
             futureSample = &messages.front();
         }
-        mResampler->resampleMotionEvent(static_cast<std::chrono::nanoseconds>(frameTime),
-                                        *motionEvent, futureSample);
+        mResampler->resampleMotionEvent(nanoseconds{frameTime}, *motionEvent, futureSample);
     }
     return std::make_pair(std::move(motionEvent), firstSeqForBatch);
 }
diff --git a/libs/input/Resampler.cpp b/libs/input/Resampler.cpp
index b535ff42c4..51fadf8ec1 100644
--- a/libs/input/Resampler.cpp
+++ b/libs/input/Resampler.cpp
@@ -241,6 +241,10 @@ inline void LegacyResampler::addSampleToMotionEvent(const Sample& sample,
                           motionEvent.getId());
 }
 
+nanoseconds LegacyResampler::getResampleLatency() const {
+    return RESAMPLE_LATENCY;
+}
+
 void LegacyResampler::resampleMotionEvent(nanoseconds frameTime, MotionEvent& motionEvent,
                                           const InputMessage* futureSample) {
     if (mPreviousDeviceId && *mPreviousDeviceId != motionEvent.getDeviceId()) {
diff --git a/libs/input/tests/InputConsumer_test.cpp b/libs/input/tests/InputConsumer_test.cpp
index c30f243398..55be453287 100644
--- a/libs/input/tests/InputConsumer_test.cpp
+++ b/libs/input/tests/InputConsumer_test.cpp
@@ -30,14 +30,21 @@
 
 namespace android {
 
+namespace {
+
+using std::chrono::nanoseconds;
+
+} // namespace
+
 class InputConsumerTest : public testing::Test, public InputConsumerCallbacks {
 protected:
     InputConsumerTest()
           : mClientTestChannel{std::make_shared<TestInputChannel>("TestChannel")},
             mTestLooper{std::make_shared<TestLooper>()} {
         Looper::setForThread(mTestLooper->getLooper());
-        mConsumer = std::make_unique<InputConsumerNoResampling>(mClientTestChannel, mTestLooper,
-                                                                *this, /*resampler=*/nullptr);
+        mConsumer =
+                std::make_unique<InputConsumerNoResampling>(mClientTestChannel, mTestLooper, *this,
+                                                            std::make_unique<LegacyResampler>());
     }
 
     void assertOnBatchedInputEventPendingWasCalled();
@@ -54,7 +61,7 @@ protected:
     BlockingQueue<std::unique_ptr<TouchModeEvent>> mTouchModeEvents;
 
 private:
-    size_t onBatchedInputEventPendingInvocationCount{0};
+    size_t mOnBatchedInputEventPendingInvocationCount{0};
 
     // InputConsumerCallbacks interface
     void onKeyEvent(std::unique_ptr<KeyEvent> event, uint32_t seq) override {
@@ -69,7 +76,7 @@ private:
         if (!mConsumer->probablyHasInput()) {
             ADD_FAILURE() << "should deterministically have input because there is a batch";
         }
-        ++onBatchedInputEventPendingInvocationCount;
+        ++mOnBatchedInputEventPendingInvocationCount;
     };
     void onFocusEvent(std::unique_ptr<FocusEvent> event, uint32_t seq) override {
         mFocusEvents.push(std::move(event));
@@ -90,18 +97,24 @@ private:
 };
 
 void InputConsumerTest::assertOnBatchedInputEventPendingWasCalled() {
-    ASSERT_GT(onBatchedInputEventPendingInvocationCount, 0UL)
+    ASSERT_GT(mOnBatchedInputEventPendingInvocationCount, 0UL)
             << "onBatchedInputEventPending has not been called.";
-    --onBatchedInputEventPendingInvocationCount;
+    --mOnBatchedInputEventPendingInvocationCount;
 }
 
 TEST_F(InputConsumerTest, MessageStreamBatchedInMotionEvent) {
-    mClientTestChannel->enqueueMessage(
-            InputMessageBuilder{InputMessage::Type::MOTION, /*seq=*/0}.build());
-    mClientTestChannel->enqueueMessage(
-            InputMessageBuilder{InputMessage::Type::MOTION, /*seq=*/1}.build());
-    mClientTestChannel->enqueueMessage(
-            InputMessageBuilder{InputMessage::Type::MOTION, /*seq=*/2}.build());
+    mClientTestChannel->enqueueMessage(InputMessageBuilder{InputMessage::Type::MOTION, /*seq=*/0}
+                                               .eventTime(nanoseconds{0ms}.count())
+                                               .action(AMOTION_EVENT_ACTION_DOWN)
+                                               .build());
+    mClientTestChannel->enqueueMessage(InputMessageBuilder{InputMessage::Type::MOTION, /*seq=*/1}
+                                               .eventTime(nanoseconds{5ms}.count())
+                                               .action(AMOTION_EVENT_ACTION_MOVE)
+                                               .build());
+    mClientTestChannel->enqueueMessage(InputMessageBuilder{InputMessage::Type::MOTION, /*seq=*/2}
+                                               .eventTime(nanoseconds{10ms}.count())
+                                               .action(AMOTION_EVENT_ACTION_MOVE)
+                                               .build());
 
     mClientTestChannel->assertNoSentMessages();
 
@@ -111,13 +124,62 @@ TEST_F(InputConsumerTest, MessageStreamBatchedInMotionEvent) {
 
     mConsumer->consumeBatchedInputEvents(std::nullopt);
 
-    std::unique_ptr<MotionEvent> batchedMotionEvent = mMotionEvents.pop();
-    ASSERT_NE(batchedMotionEvent, nullptr);
+    std::unique_ptr<MotionEvent> downMotionEvent = mMotionEvents.pop();
+    ASSERT_NE(downMotionEvent, nullptr);
+
+    std::unique_ptr<MotionEvent> moveMotionEvent = mMotionEvents.pop();
+    ASSERT_NE(moveMotionEvent, nullptr);
+    EXPECT_EQ(moveMotionEvent->getHistorySize() + 1, 3UL);
 
     mClientTestChannel->assertFinishMessage(/*seq=*/0, /*handled=*/true);
     mClientTestChannel->assertFinishMessage(/*seq=*/1, /*handled=*/true);
     mClientTestChannel->assertFinishMessage(/*seq=*/2, /*handled=*/true);
+}
+
+TEST_F(InputConsumerTest, LastBatchedSampleIsLessThanResampleTime) {
+    mClientTestChannel->enqueueMessage(InputMessageBuilder{InputMessage::Type::MOTION, /*seq=*/0}
+                                               .eventTime(nanoseconds{0ms}.count())
+                                               .action(AMOTION_EVENT_ACTION_DOWN)
+                                               .build());
+    mClientTestChannel->enqueueMessage(InputMessageBuilder{InputMessage::Type::MOTION, /*seq=*/1}
+                                               .eventTime(nanoseconds{5ms}.count())
+                                               .action(AMOTION_EVENT_ACTION_MOVE)
+                                               .build());
+    mClientTestChannel->enqueueMessage(InputMessageBuilder{InputMessage::Type::MOTION, /*seq=*/2}
+                                               .eventTime(nanoseconds{10ms}.count())
+                                               .action(AMOTION_EVENT_ACTION_MOVE)
+                                               .build());
+    mClientTestChannel->enqueueMessage(InputMessageBuilder{InputMessage::Type::MOTION, /*seq=*/3}
+                                               .eventTime(nanoseconds{15ms}.count())
+                                               .action(AMOTION_EVENT_ACTION_MOVE)
+                                               .build());
+
+    mClientTestChannel->assertNoSentMessages();
+
+    mTestLooper->invokeCallback(mClientTestChannel->getFd(), ALOOPER_EVENT_INPUT);
+
+    assertOnBatchedInputEventPendingWasCalled();
+
+    mConsumer->consumeBatchedInputEvents(16'000'000 /*ns*/);
+
+    std::unique_ptr<MotionEvent> downMotionEvent = mMotionEvents.pop();
+    ASSERT_NE(downMotionEvent, nullptr);
+
+    std::unique_ptr<MotionEvent> moveMotionEvent = mMotionEvents.pop();
+    ASSERT_NE(moveMotionEvent, nullptr);
+    const size_t numSamples = moveMotionEvent->getHistorySize() + 1;
+    EXPECT_LT(moveMotionEvent->getHistoricalEventTime(numSamples - 2),
+              moveMotionEvent->getEventTime());
+
+    // Consume all remaining events before ending the test. Otherwise, the smart pointer that owns
+    // consumer is set to null before destroying consumer. This leads to a member function call on a
+    // null object.
+    // TODO(b/332613662): Remove this workaround.
+    mConsumer->consumeBatchedInputEvents(std::nullopt);
 
-    EXPECT_EQ(batchedMotionEvent->getHistorySize() + 1, 3UL);
+    mClientTestChannel->assertFinishMessage(/*seq=*/0, true);
+    mClientTestChannel->assertFinishMessage(/*seq=*/1, true);
+    mClientTestChannel->assertFinishMessage(/*seq=*/2, true);
+    mClientTestChannel->assertFinishMessage(/*seq=*/3, true);
 }
 } // namespace android