Do not resample when frameTime is not available

LegacyResampler still resamples MotionEvents even when requestedFrameTime is
std::nullopt. The problem is that if the parameter requestedFrameTime
is std::nullopt, it is reassigned inside consumeBatchedInputEvents to
std::numeric_limits<nsecs_t>::max(). In spite of using max() as a
placeholder to indicate that everything should be consumed, it is still a valid time, and resampler uses it to resample MotionEvent. To fix this issue, having a valid requestedFrameTime should also be a condition to resample MotionEvents.

Bug: 374375203
Flag: EXEMPT refactor
Test: TEST=libinput_tests; m $TEST && $ANDROID_HOST_OUT/nativetest64/$TEST/$TEST
Change-Id: I6e53bdd0a35d359da8b50f10dd4aad9bddc8aa3f

4 files changed, 50 insertions, 17 deletions

diff --git a/include/input/InputConsumerNoResampling.h b/include/input/InputConsumerNoResampling.h
index 228347d818..2e346bb7cd 100644
--- a/include/input/InputConsumerNoResampling.h
+++ b/include/input/InputConsumerNoResampling.h
@@ -211,16 +211,17 @@ private:
      * `consumeBatchedInputEvents`.
      */
     std::map<DeviceId, std::queue<InputMessage>> mBatches;
+
     /**
-     * Creates a MotionEvent by consuming samples from the provided queue. If one message has
-     * eventTime > adjustedFrameTime, all subsequent messages in the queue will be skipped. It is
-     * assumed that messages are queued in chronological order. In other words, only events that
-     * occurred prior to the adjustedFrameTime will be consumed.
-     * @param requestedFrameTime the time up to which to consume events.
-     * @param messages the queue of messages to consume from
+     * Creates a MotionEvent by consuming samples from the provided queue. Consumes all messages
+     * with eventTime <= requestedFrameTime - resampleLatency, where `resampleLatency` is latency
+     * introduced by the resampler. Assumes that messages are queued in chronological order.
+     * @param requestedFrameTime The time up to which consume messages, as given by the inequality
+     * above. If std::nullopt, everything in messages will be consumed.
+     * @param messages the queue of messages to consume from.
      */
     std::pair<std::unique_ptr<MotionEvent>, std::optional<uint32_t>> createBatchedMotionEvent(
-            const nsecs_t requestedFrameTime, std::queue<InputMessage>& messages);
+            const std::optional<nsecs_t> requestedFrameTime, std::queue<InputMessage>& messages);
 
     /**
      * Consumes the batched input events, optionally allowing the caller to specify a device id
diff --git a/libs/input/InputConsumerNoResampling.cpp b/libs/input/InputConsumerNoResampling.cpp
index 9665de799f..d3653cfc45 100644
--- a/libs/input/InputConsumerNoResampling.cpp
+++ b/libs/input/InputConsumerNoResampling.cpp
@@ -357,7 +357,8 @@ void InputConsumerNoResampling::handleMessages(std::vector<InputMessage>&& messa
                 mBatches[deviceId].emplace(msg);
             } else {
                 // consume all pending batches for this device immediately
-                consumeBatchedInputEvents(deviceId, /*requestedFrameTime=*/std::nullopt);
+                consumeBatchedInputEvents(deviceId, /*requestedFrameTime=*/
+                                          std::numeric_limits<nsecs_t>::max());
                 if (canResample &&
                     (action == AMOTION_EVENT_ACTION_UP || action == AMOTION_EVENT_ACTION_CANCEL)) {
                     LOG_IF(INFO, mResamplers.erase(deviceId) == 0)
@@ -480,7 +481,7 @@ void InputConsumerNoResampling::handleMessage(const InputMessage& msg) const {
 }
 
 std::pair<std::unique_ptr<MotionEvent>, std::optional<uint32_t>>
-InputConsumerNoResampling::createBatchedMotionEvent(const nsecs_t requestedFrameTime,
+InputConsumerNoResampling::createBatchedMotionEvent(const std::optional<nsecs_t> requestedFrameTime,
                                                     std::queue<InputMessage>& messages) {
     std::unique_ptr<MotionEvent> motionEvent;
     std::optional<uint32_t> firstSeqForBatch;
@@ -491,7 +492,11 @@ InputConsumerNoResampling::createBatchedMotionEvent(const nsecs_t requestedFrame
     const nanoseconds resampleLatency = (resampler != mResamplers.cend())
             ? resampler->second->getResampleLatency()
             : nanoseconds{0};
-    const nanoseconds adjustedFrameTime = nanoseconds{requestedFrameTime} - resampleLatency;
+    // When batching is not enabled, we want to consume all events. That's equivalent to having an
+    // infinite requestedFrameTime.
+    const nanoseconds adjustedFrameTime = (requestedFrameTime.has_value())
+            ? (nanoseconds{*requestedFrameTime} - resampleLatency)
+            : nanoseconds{std::numeric_limits<nsecs_t>::max()};
 
     while (!messages.empty() &&
            (messages.front().body.motion.eventTime <= adjustedFrameTime.count())) {
@@ -513,8 +518,9 @@ InputConsumerNoResampling::createBatchedMotionEvent(const nsecs_t requestedFrame
     if (!messages.empty()) {
         futureSample = &messages.front();
     }
-    if ((motionEvent != nullptr) && (resampler != mResamplers.cend())) {
-        resampler->second->resampleMotionEvent(nanoseconds{requestedFrameTime}, *motionEvent,
+    if ((motionEvent != nullptr) && (resampler != mResamplers.cend()) &&
+        (requestedFrameTime.has_value())) {
+        resampler->second->resampleMotionEvent(nanoseconds{*requestedFrameTime}, *motionEvent,
                                                futureSample);
     }
 
@@ -524,16 +530,13 @@ InputConsumerNoResampling::createBatchedMotionEvent(const nsecs_t requestedFrame
 bool InputConsumerNoResampling::consumeBatchedInputEvents(
         std::optional<DeviceId> deviceId, std::optional<nsecs_t> requestedFrameTime) {
     ensureCalledOnLooperThread(__func__);
-    // When batching is not enabled, we want to consume all events. That's equivalent to having an
-    // infinite requestedFrameTime.
-    requestedFrameTime = requestedFrameTime.value_or(std::numeric_limits<nsecs_t>::max());
     bool producedEvents = false;
 
     for (auto deviceIdIter = (deviceId.has_value()) ? (mBatches.find(*deviceId))
                                                     : (mBatches.begin());
          deviceIdIter != mBatches.cend(); ++deviceIdIter) {
         std::queue<InputMessage>& messages = deviceIdIter->second;
-        auto [motion, firstSeqForBatch] = createBatchedMotionEvent(*requestedFrameTime, messages);
+        auto [motion, firstSeqForBatch] = createBatchedMotionEvent(requestedFrameTime, messages);
         if (motion != nullptr) {
             LOG_ALWAYS_FATAL_IF(!firstSeqForBatch.has_value());
             mCallbacks.onMotionEvent(std::move(motion), *firstSeqForBatch);
diff --git a/libs/input/tests/InputConsumerResampling_test.cpp b/libs/input/tests/InputConsumerResampling_test.cpp
index 883ca82fe0..a7feeeb301 100644
--- a/libs/input/tests/InputConsumerResampling_test.cpp
+++ b/libs/input/tests/InputConsumerResampling_test.cpp
@@ -566,4 +566,32 @@ TEST_F(InputConsumerResamplingTest, OldEventReceivedAfterResampleOccurs) {
     mClientTestChannel->assertFinishMessage(/*seq=*/4, /*handled=*/true);
 }
 
+TEST_F(InputConsumerResamplingTest, DoNotResampleWhenFrameTimeIsNotAvailable) {
+    mClientTestChannel->enqueueMessage(nextPointerMessage(
+            {0ms, {Pointer{.id = 0, .x = 10.0f, .y = 20.0f}}, AMOTION_EVENT_ACTION_DOWN}));
+
+    invokeLooperCallback();
+    assertReceivedMotionEvent({InputEventEntry{0ms,
+                                               {Pointer{.id = 0, .x = 10.0f, .y = 20.0f}},
+                                               AMOTION_EVENT_ACTION_DOWN}});
+
+    mClientTestChannel->enqueueMessage(nextPointerMessage(
+            {10ms, {Pointer{.id = 0, .x = 20.0f, .y = 30.0f}}, AMOTION_EVENT_ACTION_MOVE}));
+    mClientTestChannel->enqueueMessage(nextPointerMessage(
+            {20ms, {Pointer{.id = 0, .x = 30.0f, .y = 30.0f}}, AMOTION_EVENT_ACTION_MOVE}));
+
+    invokeLooperCallback();
+    mConsumer->consumeBatchedInputEvents(std::nullopt);
+    assertReceivedMotionEvent({InputEventEntry{10ms,
+                                               {Pointer{.id = 0, .x = 20.0f, .y = 30.0f}},
+                                               AMOTION_EVENT_ACTION_MOVE},
+                               InputEventEntry{20ms,
+                                               {Pointer{.id = 0, .x = 30.0f, .y = 30.0f}},
+                                               AMOTION_EVENT_ACTION_MOVE}});
+
+    mClientTestChannel->assertFinishMessage(/*seq=*/1, /*handled=*/true);
+    mClientTestChannel->assertFinishMessage(/*seq=*/2, /*handled=*/true);
+    mClientTestChannel->assertFinishMessage(/*seq=*/3, /*handled=*/true);
+}
+
 } // namespace android
diff --git a/libs/input/tests/InputConsumer_test.cpp b/libs/input/tests/InputConsumer_test.cpp
index 6a3bbe5b36..06e19bb644 100644
--- a/libs/input/tests/InputConsumer_test.cpp
+++ b/libs/input/tests/InputConsumer_test.cpp
@@ -194,7 +194,7 @@ TEST_F(InputConsumerTest, MessageStreamBatchedInMotionEvent) {
     std::unique_ptr<MotionEvent> moveMotionEvent =
             assertReceivedMotionEvent(WithMotionAction(ACTION_MOVE));
     ASSERT_NE(moveMotionEvent, nullptr);
-    EXPECT_EQ(moveMotionEvent->getHistorySize() + 1, 3UL);
+    EXPECT_EQ(moveMotionEvent->getHistorySize() + 1, 2UL);
 
     mClientTestChannel->assertFinishMessage(/*seq=*/0, /*handled=*/true);
     mClientTestChannel->assertFinishMessage(/*seq=*/1, /*handled=*/true);
@@ -443,4 +443,5 @@ TEST_F(InputConsumerTest, MultiDeviceResampling) {
     mClientTestChannel->assertFinishMessage(/*seq=*/8, /*handled=*/true);
     mClientTestChannel->assertFinishMessage(/*seq=*/10, /*handled=*/true);
 }
+
 } // namespace android