InputReader: prevent merging pointer sub-devices

If input devices have the same identifier, they are usually merged, on
the assumption that they are separate sub-devices that likely got split
out by the kernel.

This stops the merging of multiple pointer-like input devices, as
InputReader is not designed to cope with these properly. This situation
is commonly seen with integrated touchscreen/stylus ICs that report
touchscreen and stylus input events via distinct HID collections/reports
on a single HID device.

Bug: 389689566
Test: Target booted, confirmed unmerged devices in InputReaderState via dumpsys inputflinger
Test: atest inputflinger_tests # on target, with flag enabled & disabled
Test: TEST=inputflinger_tests; m $TEST && $ANDROID_HOST_OUT/nativetest64/$TEST/$TEST
Flag: com.android.input.flags.prevent_merging_input_pointer_devices
Change-Id: Iac36f29ed5aa1bf915bf9ad665b237378e967a4f

6 files changed, 114 insertions, 12 deletions

diff --git a/libs/input/input_flags.aconfig b/libs/input/input_flags.aconfig
index 4e187ed333..5bb30db772 100644
--- a/libs/input/input_flags.aconfig
+++ b/libs/input/input_flags.aconfig
@@ -255,3 +255,12 @@ flag {
   bug: "277261245"
 }
 
+flag {
+  name: "prevent_merging_input_pointer_devices"
+  namespace: "desktop_input"
+  description: "Prevent merging input sub-devices that provide pointer input streams"
+  bug: "389689566"
+  metadata {
+    purpose: PURPOSE_BUGFIX
+  }
+}
diff --git a/services/inputflinger/reader/InputReader.cpp b/services/inputflinger/reader/InputReader.cpp
index 207806d891..7ab000bbd0 100644
--- a/services/inputflinger/reader/InputReader.cpp
+++ b/services/inputflinger/reader/InputReader.cpp
@@ -62,6 +62,28 @@ bool isSubDevice(const InputDeviceIdentifier& identifier1,
             identifier1.location == identifier2.location);
 }
 
+/**
+ * Determines if the device classes passed for two devices represent incompatible combinations
+ * that should not be merged into into a single InputDevice.
+ */
+
+bool isCompatibleSubDevice(ftl::Flags<InputDeviceClass> classes1,
+                           ftl::Flags<InputDeviceClass> classes2) {
+    if (!com::android::input::flags::prevent_merging_input_pointer_devices()) {
+        return true;
+    }
+
+    const ftl::Flags<InputDeviceClass> pointerFlags =
+            ftl::Flags<InputDeviceClass>{InputDeviceClass::TOUCH, InputDeviceClass::TOUCH_MT,
+                                         InputDeviceClass::CURSOR, InputDeviceClass::TOUCHPAD};
+
+    // Do not merge devices that both have any type of pointer event.
+    if (classes1.any(pointerFlags) && classes2.any(pointerFlags)) return false;
+
+    // Safe to merge
+    return true;
+}
+
 bool isStylusPointerGestureStart(const NotifyMotionArgs& motionArgs) {
     const auto actionMasked = MotionEvent::getActionMasked(motionArgs.action);
     if (actionMasked != AMOTION_EVENT_ACTION_HOVER_ENTER &&
@@ -271,7 +293,8 @@ void InputReader::addDeviceLocked(nsecs_t when, int32_t eventHubId) {
     }
 
     InputDeviceIdentifier identifier = mEventHub->getDeviceIdentifier(eventHubId);
-    std::shared_ptr<InputDevice> device = createDeviceLocked(when, eventHubId, identifier);
+    ftl::Flags<InputDeviceClass> classes = mEventHub->getDeviceClasses(eventHubId);
+    std::shared_ptr<InputDevice> device = createDeviceLocked(when, eventHubId, identifier, classes);
 
     mPendingArgs += device->configure(when, mConfig, /*changes=*/{});
     mPendingArgs += device->reset(when);
@@ -354,12 +377,16 @@ void InputReader::removeDeviceLocked(nsecs_t when, int32_t eventHubId) {
 }
 
 std::shared_ptr<InputDevice> InputReader::createDeviceLocked(
-        nsecs_t when, int32_t eventHubId, const InputDeviceIdentifier& identifier) {
-    auto deviceIt = std::find_if(mDevices.begin(), mDevices.end(), [identifier](auto& devicePair) {
-        const InputDeviceIdentifier identifier2 =
-                devicePair.second->getDeviceInfo().getIdentifier();
-        return isSubDevice(identifier, identifier2);
-    });
+        nsecs_t when, int32_t eventHubId, const InputDeviceIdentifier& identifier,
+        ftl::Flags<InputDeviceClass> classes) {
+    auto deviceIt =
+            std::find_if(mDevices.begin(), mDevices.end(), [identifier, classes](auto& devicePair) {
+                const InputDeviceIdentifier identifier2 =
+                        devicePair.second->getDeviceInfo().getIdentifier();
+                const ftl::Flags<InputDeviceClass> classes2 = devicePair.second->getClasses();
+                return isSubDevice(identifier, identifier2) &&
+                        isCompatibleSubDevice(classes, classes2);
+            });
 
     std::shared_ptr<InputDevice> device;
     if (deviceIt != mDevices.end()) {
diff --git a/services/inputflinger/reader/include/InputReader.h b/services/inputflinger/reader/include/InputReader.h
index 931766bc1d..0d6e1020c1 100644
--- a/services/inputflinger/reader/include/InputReader.h
+++ b/services/inputflinger/reader/include/InputReader.h
@@ -25,6 +25,7 @@
 #include <vector>
 
 #include "EventHub.h"
+#include "InputDevice.h"
 #include "InputListener.h"
 #include "InputReaderBase.h"
 #include "InputReaderContext.h"
@@ -127,7 +128,8 @@ public:
 protected:
     // These members are protected so they can be instrumented by test cases.
     virtual std::shared_ptr<InputDevice> createDeviceLocked(nsecs_t when, int32_t deviceId,
-                                                            const InputDeviceIdentifier& identifier)
+                                                            const InputDeviceIdentifier& identifier,
+                                                            ftl::Flags<InputDeviceClass> classes)
             REQUIRES(mLock);
 
     // With each iteration of the loop, InputReader reads and processes one incoming message from
diff --git a/services/inputflinger/tests/InputReader_test.cpp b/services/inputflinger/tests/InputReader_test.cpp
index d9a75a5309..43d2378f61 100644
--- a/services/inputflinger/tests/InputReader_test.cpp
+++ b/services/inputflinger/tests/InputReader_test.cpp
@@ -1405,6 +1405,68 @@ TEST_F(InputReaderTest, SetPowerWakeUp) {
     ASSERT_EQ(mFakeEventHub->fakeReadKernelWakeup(3), false);
 }
 
+TEST_F(InputReaderTest, MergeableInputDevices) {
+    constexpr int32_t eventHubIds[2] = {END_RESERVED_ID, END_RESERVED_ID + 1};
+
+    // By default, all of the default-created eventhub devices will have the same identifier
+    // (implicitly vid 0, pid 0, etc.), which is why we expect them to be merged.
+    ASSERT_NO_FATAL_FAILURE(addDevice(eventHubIds[0], "1st", InputDeviceClass::KEYBOARD, nullptr));
+    ASSERT_NO_FATAL_FAILURE(addDevice(eventHubIds[1], "2nd", InputDeviceClass::JOYSTICK, nullptr));
+
+    // The two devices will be merged to one input device as they have same identifier, and none are
+    // pointer devices.
+    ASSERT_EQ(1U, mFakePolicy->getInputDevices().size());
+}
+
+TEST_F(InputReaderTest, MergeableDevicesWithTouch) {
+    constexpr int32_t eventHubIds[3] = {END_RESERVED_ID, END_RESERVED_ID + 1, END_RESERVED_ID + 2};
+
+    // By default, all of the default-created eventhub devices will have the same identifier
+    // (implicitly vid 0, pid 0, etc.), which is why we expect them to be merged.
+    ASSERT_NO_FATAL_FAILURE(addDevice(eventHubIds[0], "1st", InputDeviceClass::TOUCH_MT, nullptr));
+    ASSERT_NO_FATAL_FAILURE(addDevice(eventHubIds[1], "2nd", InputDeviceClass::KEYBOARD, nullptr));
+    ASSERT_NO_FATAL_FAILURE(addDevice(eventHubIds[2], "3rd", InputDeviceClass::GAMEPAD, nullptr));
+
+    // The three devices will be merged to one input device as they have same identifier, and only
+    // one is a pointer device.
+    ASSERT_EQ(1U, mFakePolicy->getInputDevices().size());
+}
+
+TEST_F(InputReaderTest, UnmergeableTouchDevices) {
+    SCOPED_FLAG_OVERRIDE(prevent_merging_input_pointer_devices, true);
+
+    constexpr int32_t eventHubIds[3] = {END_RESERVED_ID, END_RESERVED_ID + 1, END_RESERVED_ID + 2};
+
+    // By default, all of the default-created eventhub devices will have the same identifier
+    // (implicitly vid 0, pid 0, etc.), which is why they can potentially be merged.
+    ASSERT_NO_FATAL_FAILURE(addDevice(eventHubIds[0], "1st", InputDeviceClass::TOUCH, nullptr));
+    ASSERT_NO_FATAL_FAILURE(addDevice(eventHubIds[1], "2nd", InputDeviceClass::TOUCH_MT, nullptr));
+    ASSERT_NO_FATAL_FAILURE(addDevice(eventHubIds[2], "2nd", InputDeviceClass::CURSOR, nullptr));
+
+    // The three devices will not be merged, as they have same identifier, but are all pointer
+    // devices.
+    ASSERT_EQ(3U, mFakePolicy->getInputDevices().size());
+}
+
+TEST_F(InputReaderTest, MergeableMixedDevices) {
+    SCOPED_FLAG_OVERRIDE(prevent_merging_input_pointer_devices, true);
+
+    constexpr int32_t eventHubIds[4] = {END_RESERVED_ID, END_RESERVED_ID + 1, END_RESERVED_ID + 2,
+                                        END_RESERVED_ID + 3};
+
+    // By default, all of the default-created eventhub devices will have the same identifier
+    // (implicitly vid 0, pid 0, etc.), which is why they can potentially be merged.
+    ASSERT_NO_FATAL_FAILURE(addDevice(eventHubIds[0], "1st", InputDeviceClass::TOUCH, nullptr));
+    ASSERT_NO_FATAL_FAILURE(addDevice(eventHubIds[1], "2nd", InputDeviceClass::TOUCH_MT, nullptr));
+    ASSERT_NO_FATAL_FAILURE(addDevice(eventHubIds[2], "3rd", InputDeviceClass::DPAD, nullptr));
+    ASSERT_NO_FATAL_FAILURE(addDevice(eventHubIds[3], "4th", InputDeviceClass::JOYSTICK, nullptr));
+
+    // Non-touch devices can be merged with one of the touch devices, as they have same identifier,
+    // but the two touch devices will not combine with each other. It is not specified which touch
+    // device the non-touch devices merge with.
+    ASSERT_EQ(2U, mFakePolicy->getInputDevices().size());
+}
+
 // --- InputReaderIntegrationTest ---
 
 // These tests create and interact with the InputReader only through its interface.
diff --git a/services/inputflinger/tests/InstrumentedInputReader.cpp b/services/inputflinger/tests/InstrumentedInputReader.cpp
index 110ca5fab0..53fc8a1f58 100644
--- a/services/inputflinger/tests/InstrumentedInputReader.cpp
+++ b/services/inputflinger/tests/InstrumentedInputReader.cpp
@@ -38,13 +38,14 @@ std::shared_ptr<InputDevice> InstrumentedInputReader::newDevice(int32_t deviceId
 }
 
 std::shared_ptr<InputDevice> InstrumentedInputReader::createDeviceLocked(
-        nsecs_t when, int32_t eventHubId, const InputDeviceIdentifier& identifier) REQUIRES(mLock) {
+        nsecs_t when, int32_t eventHubId, const InputDeviceIdentifier& identifier,
+        ftl::Flags<InputDeviceClass> classes) REQUIRES(mLock) {
     if (!mNextDevices.empty()) {
         std::shared_ptr<InputDevice> device(std::move(mNextDevices.front()));
         mNextDevices.pop();
         return device;
     }
-    return InputReader::createDeviceLocked(when, eventHubId, identifier);
+    return InputReader::createDeviceLocked(when, eventHubId, identifier, classes);
 }
 
 } // namespace android
diff --git a/services/inputflinger/tests/InstrumentedInputReader.h b/services/inputflinger/tests/InstrumentedInputReader.h
index e9c7bb44e8..9abf30c569 100644
--- a/services/inputflinger/tests/InstrumentedInputReader.h
+++ b/services/inputflinger/tests/InstrumentedInputReader.h
@@ -43,8 +43,9 @@ public:
     using InputReader::loopOnce;
 
 protected:
-    virtual std::shared_ptr<InputDevice> createDeviceLocked(
-            nsecs_t when, int32_t eventHubId, const InputDeviceIdentifier& identifier);
+    virtual std::shared_ptr<InputDevice> createDeviceLocked(nsecs_t when, int32_t eventHubId,
+                                                            const InputDeviceIdentifier& identifier,
+                                                            ftl::Flags<InputDeviceClass> classes);
 
     class FakeInputReaderContext : public ContextImpl {
     public: