Merge "Prevent processing touch after receiving an invalid tracking id" into sc-dev am: 10d57c6cfa

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

Change-Id: Idf72d2b888c845f0675b68520140d96c4926ab72

4 files changed, 90 insertions, 10 deletions

diff --git a/services/inputflinger/reader/mapper/MultiTouchInputMapper.cpp b/services/inputflinger/reader/mapper/MultiTouchInputMapper.cpp
index ca43123d28..fab7f4ca14 100644
--- a/services/inputflinger/reader/mapper/MultiTouchInputMapper.cpp
+++ b/services/inputflinger/reader/mapper/MultiTouchInputMapper.cpp
@@ -104,36 +104,37 @@ void MultiTouchMotionAccumulator::process(const RawEvent* rawEvent) {
 #endif
         } else {
             Slot* slot = &mSlots[mCurrentSlot];
+            // If mUsingSlotsProtocol is true, it means the raw pointer has axis info of
+            // ABS_MT_TRACKING_ID and ABS_MT_SLOT, so driver should send a valid trackingId while
+            // updating the slot.
+            if (!mUsingSlotsProtocol) {
+                slot->mInUse = true;
+            }
 
             switch (rawEvent->code) {
                 case ABS_MT_POSITION_X:
-                    slot->mInUse = true;
                     slot->mAbsMTPositionX = rawEvent->value;
+                    warnIfNotInUse(*rawEvent, *slot);
                     break;
                 case ABS_MT_POSITION_Y:
-                    slot->mInUse = true;
                     slot->mAbsMTPositionY = rawEvent->value;
+                    warnIfNotInUse(*rawEvent, *slot);
                     break;
                 case ABS_MT_TOUCH_MAJOR:
-                    slot->mInUse = true;
                     slot->mAbsMTTouchMajor = rawEvent->value;
                     break;
                 case ABS_MT_TOUCH_MINOR:
-                    slot->mInUse = true;
                     slot->mAbsMTTouchMinor = rawEvent->value;
                     slot->mHaveAbsMTTouchMinor = true;
                     break;
                 case ABS_MT_WIDTH_MAJOR:
-                    slot->mInUse = true;
                     slot->mAbsMTWidthMajor = rawEvent->value;
                     break;
                 case ABS_MT_WIDTH_MINOR:
-                    slot->mInUse = true;
                     slot->mAbsMTWidthMinor = rawEvent->value;
                     slot->mHaveAbsMTWidthMinor = true;
                     break;
                 case ABS_MT_ORIENTATION:
-                    slot->mInUse = true;
                     slot->mAbsMTOrientation = rawEvent->value;
                     break;
                 case ABS_MT_TRACKING_ID:
@@ -147,15 +148,12 @@ void MultiTouchMotionAccumulator::process(const RawEvent* rawEvent) {
                     }
                     break;
                 case ABS_MT_PRESSURE:
-                    slot->mInUse = true;
                     slot->mAbsMTPressure = rawEvent->value;
                     break;
                 case ABS_MT_DISTANCE:
-                    slot->mInUse = true;
                     slot->mAbsMTDistance = rawEvent->value;
                     break;
                 case ABS_MT_TOOL_TYPE:
-                    slot->mInUse = true;
                     slot->mAbsMTToolType = rawEvent->value;
                     slot->mHaveAbsMTToolType = true;
                     break;
@@ -177,6 +175,13 @@ bool MultiTouchMotionAccumulator::hasStylus() const {
     return mHaveStylus;
 }
 
+void MultiTouchMotionAccumulator::warnIfNotInUse(const RawEvent& event, const Slot& slot) {
+    if (!slot.mInUse) {
+        ALOGW("Received unexpected event (0x%0x, 0x%0x) for slot %i with tracking id %i",
+              event.code, event.value, mCurrentSlot, slot.mAbsMTTrackingId);
+    }
+}
+
 // --- MultiTouchMotionAccumulator::Slot ---
 
 MultiTouchMotionAccumulator::Slot::Slot() {
diff --git a/services/inputflinger/reader/mapper/MultiTouchInputMapper.h b/services/inputflinger/reader/mapper/MultiTouchInputMapper.h
index ea6f2078d7..225ad49363 100644
--- a/services/inputflinger/reader/mapper/MultiTouchInputMapper.h
+++ b/services/inputflinger/reader/mapper/MultiTouchInputMapper.h
@@ -87,6 +87,7 @@ private:
     bool mHaveStylus;
 
     void clearSlots(int32_t initialSlot);
+    void warnIfNotInUse(const RawEvent& event, const Slot& slot);
 };
 
 class MultiTouchInputMapper : public TouchInputMapper {
diff --git a/services/inputflinger/reader/mapper/TouchInputMapper.cpp b/services/inputflinger/reader/mapper/TouchInputMapper.cpp
index 60502384a3..962d8d2935 100644
--- a/services/inputflinger/reader/mapper/TouchInputMapper.cpp
+++ b/services/inputflinger/reader/mapper/TouchInputMapper.cpp
@@ -1471,6 +1471,13 @@ void TouchInputMapper::sync(nsecs_t when, nsecs_t readTime) {
           next.rawPointerData.canceledIdBits.value);
 #endif
 
+    if (!next.rawPointerData.touchingIdBits.isEmpty() &&
+        !next.rawPointerData.hoveringIdBits.isEmpty() &&
+        last.rawPointerData.hoveringIdBits != next.rawPointerData.hoveringIdBits) {
+        ALOGI("Multi-touch contains some hovering ids 0x%08x",
+              next.rawPointerData.hoveringIdBits.value);
+    }
+
     processRawTouches(false /*timeout*/);
 }
 
diff --git a/services/inputflinger/tests/InputReader_test.cpp b/services/inputflinger/tests/InputReader_test.cpp
index 73198bc5a1..997cbe88a1 100644
--- a/services/inputflinger/tests/InputReader_test.cpp
+++ b/services/inputflinger/tests/InputReader_test.cpp
@@ -2276,6 +2276,7 @@ TEST_F(TouchIntegrationTest, InputEvent_ProcessSingleTouch) {
     const Point centerPoint = mDevice->getCenterPoint();
 
     // ACTION_DOWN
+    mDevice->sendTrackingId(FIRST_TRACKING_ID);
     mDevice->sendDown(centerPoint);
     ASSERT_NO_FATAL_FAILURE(mTestListener->assertNotifyMotionWasCalled(&args));
     ASSERT_EQ(AMOTION_EVENT_ACTION_DOWN, args.action);
@@ -2296,6 +2297,8 @@ TEST_F(TouchIntegrationTest, InputEvent_ProcessMultiTouch) {
     const Point centerPoint = mDevice->getCenterPoint();
 
     // ACTION_DOWN
+    mDevice->sendSlot(FIRST_SLOT);
+    mDevice->sendTrackingId(FIRST_TRACKING_ID);
     mDevice->sendDown(centerPoint);
     ASSERT_NO_FATAL_FAILURE(mTestListener->assertNotifyMotionWasCalled(&args));
     ASSERT_EQ(AMOTION_EVENT_ACTION_DOWN, args.action);
@@ -8217,6 +8220,70 @@ TEST_F(MultiTouchInputMapperTest, Process_ShouldHandlePalmToolType_KeepFirstPoin
     ASSERT_NE(AMOTION_EVENT_FLAG_CANCELED, motionArgs.flags);
 }
 
+/**
+ * Test multi-touch should sent ACTION_POINTER_UP/ACTION_UP when received the INVALID_TRACKING_ID,
+ * to prevent the driver side may send unexpected data after set tracking id as INVALID_TRACKING_ID
+ * cause slot be valid again.
+ */
+TEST_F(MultiTouchInputMapperTest, Process_MultiTouch_WithInvalidTrackingId) {
+    addConfigurationProperty("touch.deviceType", "touchScreen");
+    prepareDisplay(DISPLAY_ORIENTATION_0);
+    prepareAxes(POSITION | ID | SLOT | PRESSURE);
+    MultiTouchInputMapper& mapper = addMapperAndConfigure<MultiTouchInputMapper>();
+
+    NotifyMotionArgs motionArgs;
+
+    constexpr int32_t x1 = 100, y1 = 200, x2 = 0, y2 = 0;
+    // First finger down.
+    processId(mapper, FIRST_TRACKING_ID);
+    processPosition(mapper, x1, y1);
+    processPressure(mapper, RAW_PRESSURE_MAX);
+    processSync(mapper);
+    ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyMotionWasCalled(&motionArgs));
+    ASSERT_EQ(AMOTION_EVENT_ACTION_DOWN, motionArgs.action);
+    ASSERT_EQ(uint32_t(1), motionArgs.pointerCount);
+
+    // First finger move.
+    processId(mapper, FIRST_TRACKING_ID);
+    processPosition(mapper, x1 + 1, y1 + 1);
+    processPressure(mapper, RAW_PRESSURE_MAX);
+    processSync(mapper);
+    ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyMotionWasCalled(&motionArgs));
+    ASSERT_EQ(AMOTION_EVENT_ACTION_MOVE, motionArgs.action);
+    ASSERT_EQ(uint32_t(1), motionArgs.pointerCount);
+
+    // Second finger down.
+    processSlot(mapper, SECOND_SLOT);
+    processId(mapper, SECOND_TRACKING_ID);
+    processPosition(mapper, x2, y2);
+    processPressure(mapper, RAW_PRESSURE_MAX);
+    processSync(mapper);
+    ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyMotionWasCalled(&motionArgs));
+    ASSERT_EQ(AMOTION_EVENT_ACTION_POINTER_DOWN | (1 << AMOTION_EVENT_ACTION_POINTER_INDEX_SHIFT),
+              motionArgs.action);
+    ASSERT_EQ(uint32_t(2), motionArgs.pointerCount);
+
+    // second finger up with some unexpected data.
+    processSlot(mapper, SECOND_SLOT);
+    processId(mapper, INVALID_TRACKING_ID);
+    processPosition(mapper, x2, y2);
+    processSync(mapper);
+    ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyMotionWasCalled(&motionArgs));
+    ASSERT_EQ(AMOTION_EVENT_ACTION_POINTER_UP | (1 << AMOTION_EVENT_ACTION_POINTER_INDEX_SHIFT),
+              motionArgs.action);
+    ASSERT_EQ(uint32_t(2), motionArgs.pointerCount);
+
+    // first finger up with some unexpected data.
+    processSlot(mapper, FIRST_SLOT);
+    processId(mapper, INVALID_TRACKING_ID);
+    processPosition(mapper, x2, y2);
+    processPressure(mapper, RAW_PRESSURE_MAX);
+    processSync(mapper);
+    ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyMotionWasCalled(&motionArgs));
+    ASSERT_EQ(AMOTION_EVENT_ACTION_UP, motionArgs.action);
+    ASSERT_EQ(uint32_t(1), motionArgs.pointerCount);
+}
+
 // --- MultiTouchInputMapperTest_ExternalDevice ---
 
 class MultiTouchInputMapperTest_ExternalDevice : public MultiTouchInputMapperTest {