Merge changes from topics "alt+meta=caps", "hasKeycode_bugfix" into main

Vaibhav Devmurari · Android (Google) Code Review · commit 6a3c7c9bf62e · 2024-10-09T18:20:46.000Z
* changes:
  Allow toggle caps lock even if device doesn't a have caps lock key
  hasKeycode API should take into account key remapping
diff --git a/include/input/KeyCharacterMap.h b/include/input/KeyCharacterMap.h
@@ -137,6 +137,9 @@ class KeyCharacterMap {
     /* Returns keycode after applying Android key code remapping defined in mKeyRemapping */
     int32_t applyKeyRemapping(int32_t fromKeyCode) const;
 
+    /** Returns list of keycodes that remap to provided keycode (@see setKeyRemapping()) */
+    std::vector<int32_t> findKeyCodesMappedToKeyCode(int32_t toKeyCode) const;
+
     /* Returns the <keyCode, metaState> pair after applying key behavior defined in the kcm file,
      * that tries to find a replacement key code based on current meta state */
     std::pair<int32_t /*keyCode*/, int32_t /*metaState*/> applyKeyBehavior(int32_t keyCode,
diff --git a/libs/input/KeyCharacterMap.cpp b/libs/input/KeyCharacterMap.cpp
@@ -365,6 +365,17 @@ int32_t KeyCharacterMap::applyKeyRemapping(int32_t fromKeyCode) const {
     return toKeyCode;
 }
 
+std::vector<int32_t> KeyCharacterMap::findKeyCodesMappedToKeyCode(int32_t toKeyCode) const {
+    std::vector<int32_t> fromKeyCodes;
+
+    for (const auto& [from, to] : mKeyRemapping) {
+        if (toKeyCode == to) {
+            fromKeyCodes.push_back(from);
+        }
+    }
+    return fromKeyCodes;
+}
+
 std::pair<int32_t, int32_t> KeyCharacterMap::applyKeyBehavior(int32_t fromKeyCode,
                                                               int32_t fromMetaState) const {
     int32_t toKeyCode = fromKeyCode;
diff --git a/services/inputflinger/reader/EventHub.cpp b/services/inputflinger/reader/EventHub.cpp
@@ -659,6 +659,19 @@ void EventHub::Device::populateAbsoluteAxisStates() {
 }
 
 bool EventHub::Device::hasKeycodeLocked(int keycode) const {
+    if (hasKeycodeInternalLocked(keycode)) {
+        return true;
+    }
+
+    for (auto& fromKey : getKeyCharacterMap()->findKeyCodesMappedToKeyCode(keycode)) {
+        if (hasKeycodeInternalLocked(fromKey)) {
+            return true;
+        }
+    }
+    return false;
+}
+
+bool EventHub::Device::hasKeycodeInternalLocked(int keycode) const {
     if (!keyMap.haveKeyLayout()) {
         return false;
     }
@@ -676,7 +689,6 @@ bool EventHub::Device::hasKeycodeLocked(int keycode) const {
     if (usageCodes.size() > 0 && mscBitmask.test(MSC_SCAN)) {
         return true;
     }
-
     return false;
 }
 
diff --git a/services/inputflinger/reader/InputDevice.cpp b/services/inputflinger/reader/InputDevice.cpp
@@ -691,16 +691,6 @@ int32_t InputDevice::getMetaState() {
     return result;
 }
 
-void InputDevice::updateMetaState(int32_t keyCode) {
-    first_in_mappers<bool>([keyCode](InputMapper& mapper) {
-        if (sourcesMatchMask(mapper.getSources(), AINPUT_SOURCE_KEYBOARD) &&
-            mapper.updateMetaState(keyCode)) {
-            return std::make_optional(true);
-        }
-        return std::optional<bool>();
-    });
-}
-
 void InputDevice::bumpGeneration() {
     mGeneration = mContext->bumpGeneration();
 }
diff --git a/services/inputflinger/reader/InputReader.cpp b/services/inputflinger/reader/InputReader.cpp
@@ -584,18 +584,9 @@ int32_t InputReader::getStateLocked(int32_t deviceId, uint32_t sourceMask, int32
 
 void InputReader::toggleCapsLockState(int32_t deviceId) {
     std::scoped_lock _l(mLock);
-    InputDevice* device = findInputDeviceLocked(deviceId);
-    if (!device) {
-        ALOGW("Ignoring toggleCapsLock for unknown deviceId %" PRId32 ".", deviceId);
-        return;
-    }
-
-    if (device->isIgnored()) {
-        ALOGW("Ignoring toggleCapsLock for ignored deviceId %" PRId32 ".", deviceId);
-        return;
+    if (mKeyboardClassifier->getKeyboardType(deviceId) == KeyboardType::ALPHABETIC) {
+        updateLedMetaStateLocked(mLedMetaState ^ AMETA_CAPS_LOCK_ON);
     }
-
-    device->updateMetaState(AKEYCODE_CAPS_LOCK);
 }
 
 bool InputReader::hasKeys(int32_t deviceId, uint32_t sourceMask,
diff --git a/services/inputflinger/reader/include/EventHub.h b/services/inputflinger/reader/include/EventHub.h
@@ -680,6 +680,7 @@ class EventHub : public EventHubInterface {
         void configureFd();
         void populateAbsoluteAxisStates();
         bool hasKeycodeLocked(int keycode) const;
+        bool hasKeycodeInternalLocked(int keycode) const;
         void loadConfigurationLocked();
         bool loadVirtualKeyMapLocked();
         status_t loadKeyMapLocked();
diff --git a/services/inputflinger/reader/include/InputDevice.h b/services/inputflinger/reader/include/InputDevice.h
@@ -122,8 +122,6 @@ class InputDevice {
     std::optional<int32_t> getLightPlayerId(int32_t lightId);
 
     int32_t getMetaState();
-    void updateMetaState(int32_t keyCode);
-
     void setKeyboardType(KeyboardType keyboardType);
 
     void bumpGeneration();
diff --git a/services/inputflinger/reader/mapper/InputMapper.cpp b/services/inputflinger/reader/mapper/InputMapper.cpp
@@ -109,10 +109,6 @@ int32_t InputMapper::getMetaState() {
     return 0;
 }
 
-bool InputMapper::updateMetaState(int32_t keyCode) {
-    return false;
-}
-
 std::list<NotifyArgs> InputMapper::updateExternalStylusState(const StylusState& state) {
     return {};
 }
diff --git a/services/inputflinger/reader/mapper/InputMapper.h b/services/inputflinger/reader/mapper/InputMapper.h
@@ -111,11 +111,6 @@ class InputMapper {
     virtual std::optional<int32_t> getLightPlayerId(int32_t lightId) { return std::nullopt; }
 
     virtual int32_t getMetaState();
-    /**
-     * Process the meta key and update the global meta state when changed.
-     * Return true if the meta key could be handled by the InputMapper.
-     */
-    virtual bool updateMetaState(int32_t keyCode);
 
     [[nodiscard]] virtual std::list<NotifyArgs> updateExternalStylusState(const StylusState& state);
 
diff --git a/services/inputflinger/reader/mapper/KeyboardInputMapper.cpp b/services/inputflinger/reader/mapper/KeyboardInputMapper.cpp
@@ -390,15 +390,6 @@ int32_t KeyboardInputMapper::getMetaState() {
     return mMetaState;
 }
 
-bool KeyboardInputMapper::updateMetaState(int32_t keyCode) {
-    if (!android::isMetaKey(keyCode) || !getDeviceContext().hasKeyCode(keyCode)) {
-        return false;
-    }
-
-    updateMetaStateIfNeeded(keyCode, false);
-    return true;
-}
-
 bool KeyboardInputMapper::updateMetaStateIfNeeded(int32_t keyCode, bool down) {
     int32_t oldMetaState = mMetaState;
     int32_t newMetaState = android::updateMetaState(keyCode, down, oldMetaState);
@@ -434,17 +425,21 @@ void KeyboardInputMapper::updateLedState(bool reset) {
     mMetaState &= ~(AMETA_CAPS_LOCK_ON | AMETA_NUM_LOCK_ON | AMETA_SCROLL_LOCK_ON);
     mMetaState |= getContext()->getLedMetaState();
 
-    constexpr int32_t META_NUM = 3;
-    const std::vector<int32_t> keyCodes{AKEYCODE_CAPS_LOCK, AKEYCODE_NUM_LOCK,
-                                        AKEYCODE_SCROLL_LOCK};
-    const std::array<int32_t, META_NUM> metaCodes = {AMETA_CAPS_LOCK_ON, AMETA_NUM_LOCK_ON,
-                                                     AMETA_SCROLL_LOCK_ON};
-    std::array<uint8_t, META_NUM> flags = {0, 0, 0};
-    bool hasKeyLayout = getDeviceContext().markSupportedKeyCodes(keyCodes, flags.data());
+    std::vector<int32_t> keyCodesToCheck{AKEYCODE_NUM_LOCK, AKEYCODE_SCROLL_LOCK};
+    std::vector<int32_t> metaCodes{AMETA_NUM_LOCK_ON, AMETA_SCROLL_LOCK_ON};
+    // Check for physical CapsLock key only for non-alphabetic keyboards. For Alphabetic
+    // keyboards, we will allow Caps Lock even if there is no physical CapsLock key.
+    if (getDeviceContext().getKeyboardType() != KeyboardType::ALPHABETIC) {
+        keyCodesToCheck.push_back(AKEYCODE_CAPS_LOCK);
+        metaCodes.push_back(AMETA_CAPS_LOCK_ON);
+    }
+    size_t size = keyCodesToCheck.size();
+    std::vector<uint8_t> flags(size, 0);
+    bool hasKeyLayout = getDeviceContext().markSupportedKeyCodes(keyCodesToCheck, flags.data());
     // If the device doesn't have the physical meta key it shouldn't generate the corresponding
     // meta state.
     if (hasKeyLayout) {
-        for (int i = 0; i < META_NUM; i++) {
+        for (size_t i = 0; i < size; i++) {
             if (!flags[i]) {
                 mMetaState &= ~metaCodes[i];
             }
diff --git a/services/inputflinger/reader/mapper/KeyboardInputMapper.h b/services/inputflinger/reader/mapper/KeyboardInputMapper.h
@@ -45,7 +45,6 @@ class KeyboardInputMapper : public InputMapper {
     int32_t getKeyCodeForKeyLocation(int32_t locationKeyCode) const override;
 
     int32_t getMetaState() override;
-    bool updateMetaState(int32_t keyCode) override;
     std::optional<ui::LogicalDisplayId> getAssociatedDisplayId() override;
     void updateLedState(bool reset) override;
 
diff --git a/services/inputflinger/tests/InputReader_test.cpp b/services/inputflinger/tests/InputReader_test.cpp
@@ -3671,40 +3671,6 @@ TEST_F(KeyboardInputMapperTest, Process_LockedKeysShouldToggleMetaStateAndLeds)
     ASSERT_EQ(AMETA_NONE, mapper.getMetaState());
 }
 
-TEST_F(KeyboardInputMapperTest, NoMetaStateWhenMetaKeysNotPresent) {
-    mFakeEventHub->addKey(EVENTHUB_ID, BTN_A, 0, AKEYCODE_BUTTON_A, 0);
-    mFakeEventHub->addKey(EVENTHUB_ID, BTN_B, 0, AKEYCODE_BUTTON_B, 0);
-    mFakeEventHub->addKey(EVENTHUB_ID, BTN_X, 0, AKEYCODE_BUTTON_X, 0);
-    mFakeEventHub->addKey(EVENTHUB_ID, BTN_Y, 0, AKEYCODE_BUTTON_Y, 0);
-
-    KeyboardInputMapper& mapper =
-            constructAndAddMapper<KeyboardInputMapper>(AINPUT_SOURCE_KEYBOARD);
-
-    // Meta state should be AMETA_NONE after reset
-    std::list<NotifyArgs> unused = mapper.reset(ARBITRARY_TIME);
-    ASSERT_EQ(AMETA_NONE, mapper.getMetaState());
-    // Meta state should be AMETA_NONE with update, as device doesn't have the keys.
-    mapper.updateMetaState(AKEYCODE_NUM_LOCK);
-    ASSERT_EQ(AMETA_NONE, mapper.getMetaState());
-
-    NotifyKeyArgs args;
-    // Press button "A"
-    process(mapper, ARBITRARY_TIME, READ_TIME, EV_KEY, BTN_A, 1);
-    ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyKeyWasCalled(&args));
-    ASSERT_EQ(AMETA_NONE, args.metaState);
-    ASSERT_EQ(AMETA_NONE, mapper.getMetaState());
-    ASSERT_EQ(AKEY_EVENT_ACTION_DOWN, args.action);
-    ASSERT_EQ(AKEYCODE_BUTTON_A, args.keyCode);
-
-    // Button up.
-    process(mapper, ARBITRARY_TIME + 2, READ_TIME, EV_KEY, BTN_A, 0);
-    ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyKeyWasCalled(&args));
-    ASSERT_EQ(AMETA_NONE, args.metaState);
-    ASSERT_EQ(AMETA_NONE, mapper.getMetaState());
-    ASSERT_EQ(AKEY_EVENT_ACTION_UP, args.action);
-    ASSERT_EQ(AKEYCODE_BUTTON_A, args.keyCode);
-}
-
 TEST_F(KeyboardInputMapperTest, Configure_AssignsDisplayPort) {
     // keyboard 1.
     mFakeEventHub->addKey(EVENTHUB_ID, KEY_UP, 0, AKEYCODE_DPAD_UP, 0);
diff --git a/services/inputflinger/tests/InterfaceMocks.h b/services/inputflinger/tests/InterfaceMocks.h
@@ -246,8 +246,6 @@ class MockInputDevice : public InputDevice {
     MOCK_METHOD(std::optional<int32_t>, getLightPlayerId, (int32_t lightId), ());
 
     MOCK_METHOD(int32_t, getMetaState, (), ());
-    MOCK_METHOD(void, updateMetaState, (int32_t keyCode), ());
-
     MOCK_METHOD(void, setKeyboardType, (KeyboardType keyboardType), ());
 
     MOCK_METHOD(void, bumpGeneration, (), ());
diff --git a/services/inputflinger/tests/fuzzers/KeyboardInputFuzzer.cpp b/services/inputflinger/tests/fuzzers/KeyboardInputFuzzer.cpp
@@ -99,7 +99,6 @@ extern "C" int LLVMFuzzerTestOneInput(uint8_t* data, size_t size) {
                                                  nullptr);
                 },
                 [&]() -> void { mapper.getMetaState(); },
-                [&]() -> void { mapper.updateMetaState(fdp->ConsumeIntegral<int32_t>()); },
                 [&]() -> void { mapper.getAssociatedDisplayId(); },
         })();
     }

Original file line number	Diff line number	Diff line change
`@@ -109,10 +109,6 @@ int32_t InputMapper::getMetaState() {`
`109`	`109`	`return 0;`
`110`	`110`	`}`
`111`	`111`
`112`		`-bool InputMapper::updateMetaState(int32_t keyCode) {`
`113`		`- return false;`
`114`		`-}`
`115`		`-`
`116`	`112`	`std::list<NotifyArgs> InputMapper::updateExternalStylusState(const StylusState& state) {`
`117`	`113`	`return {};`
`118`	`114`	`}`