InputMapper refactor: Configure empty InputDevice

Configure the Device prior to populating mappers for mappers to
receive correct property map

Test: atest inputflinger_tests
Bug: 256009910
Change-Id: I0a7d2bf4d64b3d120d9050e9154b3f3a0f54657a
diff --git a/services/inputflinger/reader/InputDevice.cpp b/services/inputflinger/reader/InputDevice.cpp
index 49998a0..586d44c 100644
--- a/services/inputflinger/reader/InputDevice.cpp
+++ b/services/inputflinger/reader/InputDevice.cpp
@@ -67,21 +67,26 @@
 }
 
 std::list<NotifyArgs> InputDevice::updateEnableState(nsecs_t when,
-                                                     const InputReaderConfiguration& readerConfig) {
-    // If the device was explicitly disabled by the user, it would be present in the
-    // "disabledDevices" list. This device should be disabled.
-    bool enable = readerConfig.disabledDevices.find(mId) == readerConfig.disabledDevices.end();
+                                                     const InputReaderConfiguration& readerConfig,
+                                                     bool forceEnable) {
+    bool enable = forceEnable;
+    if (!forceEnable) {
+        // If the device was explicitly disabled by the user, it would be present in the
+        // "disabledDevices" list. This device should be disabled.
+        enable = readerConfig.disabledDevices.find(mId) == readerConfig.disabledDevices.end();
 
-    // If a device is associated with a specific display but there is no
-    // associated DisplayViewport, don't enable the device.
-    if (enable && (mAssociatedDisplayPort || mAssociatedDisplayUniqueId) && !mAssociatedViewport) {
-        const std::string desc = mAssociatedDisplayPort
-                ? "port " + std::to_string(*mAssociatedDisplayPort)
-                : "uniqueId " + *mAssociatedDisplayUniqueId;
-        ALOGW("Cannot enable input device %s because it is associated "
-              "with %s, but the corresponding viewport is not found",
-              getName().c_str(), desc.c_str());
-        enable = false;
+        // If a device is associated with a specific display but there is no
+        // associated DisplayViewport, don't enable the device.
+        if (enable && (mAssociatedDisplayPort || mAssociatedDisplayUniqueId) &&
+            !mAssociatedViewport) {
+            const std::string desc = mAssociatedDisplayPort
+                    ? "port " + std::to_string(*mAssociatedDisplayPort)
+                    : "uniqueId " + *mAssociatedDisplayUniqueId;
+            ALOGW("Cannot enable input device %s because it is associated "
+                  "with %s, but the corresponding viewport is not found",
+                  getName().c_str(), desc.c_str());
+            enable = false;
+        }
     }
 
     std::list<NotifyArgs> out;
@@ -168,18 +173,25 @@
     mDevices.insert({eventHubId, std::make_pair(std::move(contextPtr), std::move(mappers))});
 }
 
-void InputDevice::addEventHubDevice(int32_t eventHubId,
-                                    const InputReaderConfiguration& readerConfig) {
+[[nodiscard]] std::list<NotifyArgs> InputDevice::addEventHubDevice(
+        nsecs_t when, int32_t eventHubId, const InputReaderConfiguration& readerConfig) {
     if (mDevices.find(eventHubId) != mDevices.end()) {
-        return;
+        return {};
     }
-    std::unique_ptr<InputDeviceContext> contextPtr(new InputDeviceContext(*this, eventHubId));
-    std::vector<std::unique_ptr<InputMapper>> mappers = createMappers(*contextPtr, readerConfig);
 
-    // insert the context into the devices set
-    mDevices.insert({eventHubId, std::make_pair(std::move(contextPtr), std::move(mappers))});
+    // Add an empty device configure and keep it enabled to allow mapper population with correct
+    // configuration/context,
+    // Note: we need to ensure device is kept enabled till mappers are configured
+    // TODO: b/281852638 refactor tests to remove this flag and reliance on the empty device
+    addEmptyEventHubDevice(eventHubId);
+    std::list<NotifyArgs> out = configureInternal(when, readerConfig, {}, /*forceEnable=*/true);
+
+    DevicePair& devicePair = mDevices[eventHubId];
+    devicePair.second = createMappers(*devicePair.first, readerConfig);
+
     // Must change generation to flag this device as changed
     bumpGeneration();
+    return out;
 }
 
 void InputDevice::removeEventHubDevice(int32_t eventHubId) {
@@ -193,6 +205,12 @@
 std::list<NotifyArgs> InputDevice::configure(nsecs_t when,
                                              const InputReaderConfiguration& readerConfig,
                                              ConfigurationChanges changes) {
+    return configureInternal(when, readerConfig, changes);
+}
+std::list<NotifyArgs> InputDevice::configureInternal(nsecs_t when,
+                                                     const InputReaderConfiguration& readerConfig,
+                                                     ConfigurationChanges changes,
+                                                     bool forceEnable) {
     std::list<NotifyArgs> out;
     mSources = 0;
     mClasses = ftl::Flags<InputDeviceClass>(0);
@@ -289,24 +307,16 @@
             }
         }
 
-        if (changes.test(Change::ENABLED_STATE) || changes.test(Change::DISPLAY_INFO)) {
-            // Whether a device is enabled can depend on the display association,
-            // so update the enabled state when there is a change in display info.
-            // NOTE: The first configuration of a mapper must happen with the device enabled.
-            // Do not execute this code on the first configure to prevent mappers
-            // from being configured with the device disabled.
-            out += updateEnableState(when, readerConfig);
-        }
-
         for_each_mapper([this, when, &readerConfig, changes, &out](InputMapper& mapper) {
             out += mapper.reconfigure(when, readerConfig, changes);
             mSources |= mapper.getSources();
         });
 
-        // If a device is just plugged but it might be disabled, we need to update some info like
-        // axis range of touch from each InputMapper first, then disable it.
-        if (!changes.any()) {
-            out += updateEnableState(when, readerConfig);
+        if (!changes.any() || changes.test(Change::ENABLED_STATE) ||
+            changes.test(Change::DISPLAY_INFO)) {
+            // Whether a device is enabled can depend on the display association,
+            // so update the enabled state when there is a change in display info.
+            out += updateEnableState(when, readerConfig, forceEnable);
         }
     }
     return out;
diff --git a/services/inputflinger/reader/InputReader.cpp b/services/inputflinger/reader/InputReader.cpp
index 7f63355..0aea0b3 100644
--- a/services/inputflinger/reader/InputReader.cpp
+++ b/services/inputflinger/reader/InputReader.cpp
@@ -236,7 +236,7 @@
     }
 
     InputDeviceIdentifier identifier = mEventHub->getDeviceIdentifier(eventHubId);
-    std::shared_ptr<InputDevice> device = createDeviceLocked(eventHubId, identifier);
+    std::shared_ptr<InputDevice> device = createDeviceLocked(when, eventHubId, identifier);
 
     mPendingArgs += device->configure(when, mConfig, /*changes=*/{});
     mPendingArgs += device->reset(when);
@@ -319,7 +319,7 @@
 }
 
 std::shared_ptr<InputDevice> InputReader::createDeviceLocked(
-        int32_t eventHubId, const InputDeviceIdentifier& identifier) {
+        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();
@@ -334,7 +334,7 @@
         device = std::make_shared<InputDevice>(&mContext, deviceId, bumpGenerationLocked(),
                                                identifier);
     }
-    device->addEventHubDevice(eventHubId, mConfig);
+    mPendingArgs += device->addEventHubDevice(when, eventHubId, mConfig);
     return device;
 }
 
diff --git a/services/inputflinger/reader/include/InputDevice.h b/services/inputflinger/reader/include/InputDevice.h
index ec37cdc..31dcb2e 100644
--- a/services/inputflinger/reader/include/InputDevice.h
+++ b/services/inputflinger/reader/include/InputDevice.h
@@ -80,7 +80,8 @@
 
     void dump(std::string& dump, const std::string& eventHubDevStr);
     void addEmptyEventHubDevice(int32_t eventHubId);
-    void addEventHubDevice(int32_t eventHubId, const InputReaderConfiguration& readerConfig);
+    [[nodiscard]] std::list<NotifyArgs> addEventHubDevice(
+            nsecs_t when, int32_t eventHubId, const InputReaderConfiguration& readerConfig);
     void removeEventHubDevice(int32_t eventHubId);
     [[nodiscard]] std::list<NotifyArgs> configure(nsecs_t when,
                                                   const InputReaderConfiguration& readerConfig,
@@ -205,8 +206,12 @@
     std::vector<std::unique_ptr<InputMapper>> createMappers(
             InputDeviceContext& contextPtr, const InputReaderConfiguration& readerConfig);
 
+    [[nodiscard]] std::list<NotifyArgs> configureInternal(
+            nsecs_t when, const InputReaderConfiguration& readerConfig,
+            ConfigurationChanges changes, bool forceEnable = false);
+
     [[nodiscard]] std::list<NotifyArgs> updateEnableState(
-            nsecs_t when, const InputReaderConfiguration& readerConfig);
+            nsecs_t when, const InputReaderConfiguration& readerConfig, bool forceEnable = false);
 
     PropertyMap mConfiguration;
 
diff --git a/services/inputflinger/reader/include/InputReader.h b/services/inputflinger/reader/include/InputReader.h
index e21715e..9a297c9 100644
--- a/services/inputflinger/reader/include/InputReader.h
+++ b/services/inputflinger/reader/include/InputReader.h
@@ -121,7 +121,7 @@
 
 protected:
     // These members are protected so they can be instrumented by test cases.
-    virtual std::shared_ptr<InputDevice> createDeviceLocked(int32_t deviceId,
+    virtual std::shared_ptr<InputDevice> createDeviceLocked(nsecs_t when, int32_t deviceId,
                                                             const InputDeviceIdentifier& identifier)
             REQUIRES(mLock);
 
diff --git a/services/inputflinger/tests/InputReader_test.cpp b/services/inputflinger/tests/InputReader_test.cpp
index 9d2aea2..bce0937 100644
--- a/services/inputflinger/tests/InputReader_test.cpp
+++ b/services/inputflinger/tests/InputReader_test.cpp
@@ -2835,7 +2835,8 @@
     mFakeEventHub->addDevice(TEST_EVENTHUB_ID, "Test EventHub device", InputDeviceClass::BATTERY);
 
     InputDevice device(mReader->getContext(), /*id=*/1, /*generation=*/2, /*identifier=*/{});
-    device.addEventHubDevice(TEST_EVENTHUB_ID, mFakePolicy->getReaderConfiguration());
+    auto _ = device.addEventHubDevice(ARBITRARY_TIME, TEST_EVENTHUB_ID,
+                                      mFakePolicy->getReaderConfiguration());
     device.removeEventHubDevice(TEST_EVENTHUB_ID);
     std::string dumpStr, eventHubDevStr;
     device.dump(dumpStr, eventHubDevStr);
diff --git a/services/inputflinger/tests/InstrumentedInputReader.cpp b/services/inputflinger/tests/InstrumentedInputReader.cpp
index 1f8cd12..110ca5f 100644
--- a/services/inputflinger/tests/InstrumentedInputReader.cpp
+++ b/services/inputflinger/tests/InstrumentedInputReader.cpp
@@ -38,13 +38,13 @@
 }
 
 std::shared_ptr<InputDevice> InstrumentedInputReader::createDeviceLocked(
-        int32_t eventHubId, const InputDeviceIdentifier& identifier) REQUIRES(mLock) {
+        nsecs_t when, int32_t eventHubId, const InputDeviceIdentifier& identifier) REQUIRES(mLock) {
     if (!mNextDevices.empty()) {
         std::shared_ptr<InputDevice> device(std::move(mNextDevices.front()));
         mNextDevices.pop();
         return device;
     }
-    return InputReader::createDeviceLocked(eventHubId, identifier);
+    return InputReader::createDeviceLocked(when, eventHubId, identifier);
 }
 
 } // namespace android
diff --git a/services/inputflinger/tests/InstrumentedInputReader.h b/services/inputflinger/tests/InstrumentedInputReader.h
index fef58ec..ca85558 100644
--- a/services/inputflinger/tests/InstrumentedInputReader.h
+++ b/services/inputflinger/tests/InstrumentedInputReader.h
@@ -44,7 +44,7 @@
 
 protected:
     virtual std::shared_ptr<InputDevice> createDeviceLocked(
-            int32_t eventHubId, const InputDeviceIdentifier& identifier);
+            nsecs_t when, int32_t eventHubId, const InputDeviceIdentifier& identifier);
 
     class FakeInputReaderContext : public ContextImpl {
     public: