Skip to content

Commit 4498107

Browse files
prabirmspCherrypicker Worker
authored andcommitted
InputReader: Use shared keyboard source for all key events
When multiple EventHub devices are merged into a single InputDevice, it's possible that there are more than one KeyboardInputMappers created for the device. In this case, each mapper could be configured with a different source. This can lead to situations where a device generates key events that have different sources, depending on the mapper from which it originated. To make sure all key events use a consistent source for each InputDevice, use the shared keyboard source when generating events from a KeyboardInputMapper. Bug: 354270482 Test: atest inputflinger_tests Flag: EXEMPT bugfix (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:38636652797f16d84f8b5672be4d7d95d8b31947) Merged-In: I586424b5e8d31d17cbe635d9f91a889aee906d40 Change-Id: I586424b5e8d31d17cbe635d9f91a889aee906d40
1 parent 9cfc94c commit 4498107

6 files changed

Lines changed: 78 additions & 15 deletions

File tree

services/inputflinger/reader/include/InputDevice.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -299,6 +299,7 @@ class InputDeviceContext {
299299
inline ftl::Flags<InputDeviceClass> getDeviceClasses() const {
300300
return mEventHub->getDeviceClasses(mId);
301301
}
302+
inline uint32_t getDeviceSources() const { return mDevice.getSources(); }
302303
inline InputDeviceIdentifier getDeviceIdentifier() const {
303304
return mEventHub->getDeviceIdentifier(mId);
304305
}

services/inputflinger/reader/mapper/KeyboardInputMapper.cpp

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -98,10 +98,10 @@ static bool isMediaKey(int32_t keyCode) {
9898
KeyboardInputMapper::KeyboardInputMapper(InputDeviceContext& deviceContext,
9999
const InputReaderConfiguration& readerConfig,
100100
uint32_t source)
101-
: InputMapper(deviceContext, readerConfig), mSource(source) {}
101+
: InputMapper(deviceContext, readerConfig), mMapperSource(source) {}
102102

103103
uint32_t KeyboardInputMapper::getSources() const {
104-
return mSource;
104+
return mMapperSource;
105105
}
106106

107107
ui::Rotation KeyboardInputMapper::getOrientation() {
@@ -351,8 +351,8 @@ std::list<NotifyArgs> KeyboardInputMapper::processKey(nsecs_t when, nsecs_t read
351351
policyFlags |= POLICY_FLAG_DISABLE_KEY_REPEAT;
352352
}
353353

354-
out.emplace_back(NotifyKeyArgs(getContext()->getNextId(), when, readTime, deviceId, mSource,
355-
getDisplayId(), policyFlags,
354+
out.emplace_back(NotifyKeyArgs(getContext()->getNextId(), when, readTime, deviceId,
355+
getEventSource(), getDisplayId(), policyFlags,
356356
down ? AKEY_EVENT_ACTION_DOWN : AKEY_EVENT_ACTION_UP, flags,
357357
keyCode, scanCode, keyMetaState, downTime));
358358
return out;
@@ -478,12 +478,12 @@ std::list<NotifyArgs> KeyboardInputMapper::cancelAllDownKeys(nsecs_t when) {
478478
std::list<NotifyArgs> out;
479479
size_t n = mKeyDowns.size();
480480
for (size_t i = 0; i < n; i++) {
481-
out.emplace_back(NotifyKeyArgs(getContext()->getNextId(), when,
482-
systemTime(SYSTEM_TIME_MONOTONIC), getDeviceId(), mSource,
483-
getDisplayId(), /*policyFlags=*/0, AKEY_EVENT_ACTION_UP,
484-
mKeyDowns[i].flags | AKEY_EVENT_FLAG_CANCELED,
485-
mKeyDowns[i].keyCode, mKeyDowns[i].scanCode, AMETA_NONE,
486-
mKeyDowns[i].downTime));
481+
out.emplace_back(
482+
NotifyKeyArgs(getContext()->getNextId(), when, systemTime(SYSTEM_TIME_MONOTONIC),
483+
getDeviceId(), getEventSource(), getDisplayId(), /*policyFlags=*/0,
484+
AKEY_EVENT_ACTION_UP, mKeyDowns[i].flags | AKEY_EVENT_FLAG_CANCELED,
485+
mKeyDowns[i].keyCode, mKeyDowns[i].scanCode, AMETA_NONE,
486+
mKeyDowns[i].downTime));
487487
}
488488
mKeyDowns.clear();
489489
mMetaState = AMETA_NONE;
@@ -501,4 +501,14 @@ void KeyboardInputMapper::onKeyDownProcessed(nsecs_t downTime) {
501501
}
502502
}
503503

504+
uint32_t KeyboardInputMapper::getEventSource() const {
505+
// For all input events generated by this mapper, use the source that's shared across all
506+
// KeyboardInputMappers for this device in case there are more than one.
507+
static constexpr auto ALL_KEYBOARD_SOURCES =
508+
AINPUT_SOURCE_KEYBOARD | AINPUT_SOURCE_DPAD | AINPUT_SOURCE_GAMEPAD;
509+
const auto deviceSources = getDeviceContext().getDeviceSources();
510+
LOG_ALWAYS_FATAL_IF((deviceSources & mMapperSource) != mMapperSource);
511+
return deviceSources & ALL_KEYBOARD_SOURCES;
512+
}
513+
504514
} // namespace android

services/inputflinger/reader/mapper/KeyboardInputMapper.h

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,10 @@ class KeyboardInputMapper : public InputMapper {
6060
int32_t flags{};
6161
};
6262

63-
uint32_t mSource{};
63+
// The keyboard source for this mapper. Events generated should use the source shared
64+
// by all KeyboardInputMappers for this input device.
65+
uint32_t mMapperSource{};
66+
6467
std::optional<KeyboardLayoutInfo> mKeyboardLayoutInfo;
6568

6669
std::vector<KeyDown> mKeyDowns{}; // keys that are down
@@ -106,6 +109,7 @@ class KeyboardInputMapper : public InputMapper {
106109
std::optional<DisplayViewport> findViewport(const InputReaderConfiguration& readerConfig);
107110
[[nodiscard]] std::list<NotifyArgs> cancelAllDownKeys(nsecs_t when);
108111
void onKeyDownProcessed(nsecs_t downTime);
112+
uint32_t getEventSource() const;
109113
};
110114

111115
} // namespace android

services/inputflinger/tests/InputMapperTest.h

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -121,11 +121,12 @@ class InputMapperTest : public testing::Test {
121121
T& constructAndAddMapper(Args... args) {
122122
// ensure a device entry exists for this eventHubId
123123
mDevice->addEmptyEventHubDevice(EVENTHUB_ID);
124-
// configure the empty device
125-
configureDevice(/*changes=*/{});
126124

127-
return mDevice->constructAndAddMapper<T>(EVENTHUB_ID, mFakePolicy->getReaderConfiguration(),
128-
args...);
125+
auto& mapper =
126+
mDevice->constructAndAddMapper<T>(EVENTHUB_ID,
127+
mFakePolicy->getReaderConfiguration(), args...);
128+
configureDevice(/*changes=*/{});
129+
return mapper;
129130
}
130131

131132
void setDisplayInfoAndReconfigure(ui::LogicalDisplayId displayId, int32_t width, int32_t height,

services/inputflinger/tests/InputReader_test.cpp

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4179,6 +4179,51 @@ TEST_F(KeyboardInputMapperTest, Process_GesureEventToSetFlagKeepTouchMode) {
41794179
ASSERT_EQ(AKEY_EVENT_FLAG_FROM_SYSTEM | AKEY_EVENT_FLAG_KEEP_TOUCH_MODE, args.flags);
41804180
}
41814181

4182+
/**
4183+
* When there is more than one KeyboardInputMapper for an InputDevice, each mapper should produce
4184+
* events that use the shared keyboard source across all mappers. This is to ensure that each
4185+
* input device generates key events in a consistent manner, regardless of which mapper produces
4186+
* the event.
4187+
*/
4188+
TEST_F(KeyboardInputMapperTest, UsesSharedKeyboardSource) {
4189+
mFakeEventHub->addKey(EVENTHUB_ID, KEY_HOME, 0, AKEYCODE_HOME, POLICY_FLAG_WAKE);
4190+
4191+
// Add a mapper with SOURCE_KEYBOARD
4192+
KeyboardInputMapper& keyboardMapper =
4193+
constructAndAddMapper<KeyboardInputMapper>(AINPUT_SOURCE_KEYBOARD);
4194+
4195+
process(keyboardMapper, ARBITRARY_TIME, 0, EV_KEY, KEY_HOME, 1);
4196+
ASSERT_NO_FATAL_FAILURE(
4197+
mFakeListener->assertNotifyKeyWasCalled(WithSource(AINPUT_SOURCE_KEYBOARD)));
4198+
process(keyboardMapper, ARBITRARY_TIME, 0, EV_KEY, KEY_HOME, 0);
4199+
ASSERT_NO_FATAL_FAILURE(
4200+
mFakeListener->assertNotifyKeyWasCalled(WithSource(AINPUT_SOURCE_KEYBOARD)));
4201+
4202+
// Add a mapper with SOURCE_DPAD
4203+
KeyboardInputMapper& dpadMapper =
4204+
constructAndAddMapper<KeyboardInputMapper>(AINPUT_SOURCE_DPAD);
4205+
for (auto* mapper : {&keyboardMapper, &dpadMapper}) {
4206+
process(*mapper, ARBITRARY_TIME, 0, EV_KEY, KEY_HOME, 1);
4207+
ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyKeyWasCalled(
4208+
WithSource(AINPUT_SOURCE_KEYBOARD | AINPUT_SOURCE_DPAD)));
4209+
process(*mapper, ARBITRARY_TIME, 0, EV_KEY, KEY_HOME, 0);
4210+
ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyKeyWasCalled(
4211+
WithSource(AINPUT_SOURCE_KEYBOARD | AINPUT_SOURCE_DPAD)));
4212+
}
4213+
4214+
// Add a mapper with SOURCE_GAMEPAD
4215+
KeyboardInputMapper& gamepadMapper =
4216+
constructAndAddMapper<KeyboardInputMapper>(AINPUT_SOURCE_GAMEPAD);
4217+
for (auto* mapper : {&keyboardMapper, &dpadMapper, &gamepadMapper}) {
4218+
process(*mapper, ARBITRARY_TIME, 0, EV_KEY, KEY_HOME, 1);
4219+
ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyKeyWasCalled(
4220+
WithSource(AINPUT_SOURCE_KEYBOARD | AINPUT_SOURCE_DPAD | AINPUT_SOURCE_GAMEPAD)));
4221+
process(*mapper, ARBITRARY_TIME, 0, EV_KEY, KEY_HOME, 0);
4222+
ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyKeyWasCalled(
4223+
WithSource(AINPUT_SOURCE_KEYBOARD | AINPUT_SOURCE_DPAD | AINPUT_SOURCE_GAMEPAD)));
4224+
}
4225+
}
4226+
41824227
// --- KeyboardInputMapperTest_ExternalAlphabeticDevice ---
41834228

41844229
class KeyboardInputMapperTest_ExternalAlphabeticDevice : public InputMapperTest {

services/inputflinger/tests/KeyboardInputMapper_test.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,8 @@ class KeyboardInputMapperUnitTest : public InputMapperUnitTest {
6666
mFakePolicy = sp<FakeInputReaderPolicy>::make();
6767
EXPECT_CALL(mMockInputReaderContext, getPolicy).WillRepeatedly(Return(mFakePolicy.get()));
6868

69+
ON_CALL((*mDevice), getSources).WillByDefault(Return(AINPUT_SOURCE_KEYBOARD));
70+
6971
mMapper = createInputMapper<KeyboardInputMapper>(*mDeviceContext, mReaderConfiguration,
7072
AINPUT_SOURCE_KEYBOARD);
7173
}

0 commit comments

Comments
 (0)