Merge "inputflinger: fix format of parameter name comments" into main
diff --git a/libs/input/OWNERS b/libs/input/OWNERS
new file mode 100644
index 0000000..c88bfe9
--- /dev/null
+++ b/libs/input/OWNERS
@@ -0,0 +1 @@
+include platform/frameworks/base:/INPUT_OWNERS
diff --git a/services/inputflinger/InputDeviceMetricsCollector.cpp b/services/inputflinger/InputDeviceMetricsCollector.cpp
index 89e37db..7c99a1c 100644
--- a/services/inputflinger/InputDeviceMetricsCollector.cpp
+++ b/services/inputflinger/InputDeviceMetricsCollector.cpp
@@ -31,7 +31,7 @@
namespace {
-constexpr nanoseconds DEFAULT_USAGE_SESSION_TIMEOUT = std::chrono::seconds(5);
+constexpr nanoseconds DEFAULT_USAGE_SESSION_TIMEOUT = std::chrono::minutes(2);
/**
* Log debug messages about metrics events logged to statsd.
@@ -39,6 +39,8 @@
*/
const bool DEBUG = __android_log_is_loggable(ANDROID_LOG_DEBUG, LOG_TAG, ANDROID_LOG_INFO);
+constexpr size_t INTERACTIONS_QUEUE_CAPACITY = 500;
+
int32_t linuxBusToInputDeviceBusEnum(int32_t linuxBus) {
// When adding cases to this switch, also add them to the copy of this method in
// TouchpadInputMapper.cpp.
@@ -201,7 +203,10 @@
InputDeviceMetricsCollector::InputDeviceMetricsCollector(InputListenerInterface& listener,
InputDeviceMetricsLogger& logger,
nanoseconds usageSessionTimeout)
- : mNextListener(listener), mLogger(logger), mUsageSessionTimeout(usageSessionTimeout) {}
+ : mNextListener(listener),
+ mLogger(logger),
+ mUsageSessionTimeout(usageSessionTimeout),
+ mInteractionsQueue(INTERACTIONS_QUEUE_CAPACITY) {}
void InputDeviceMetricsCollector::notifyInputDevicesChanged(
const NotifyInputDevicesChangedArgs& args) {
@@ -262,6 +267,9 @@
void InputDeviceMetricsCollector::notifyDeviceInteraction(int32_t deviceId, nsecs_t timestamp,
const std::set<Uid>& uids) {
+ if (isIgnoredInputDeviceId(deviceId)) {
+ return;
+ }
mInteractionsQueue.push(DeviceId{deviceId}, timestamp, uids);
}
diff --git a/services/inputflinger/InputManager.cpp b/services/inputflinger/InputManager.cpp
index 09c88ba..c903564 100644
--- a/services/inputflinger/InputManager.cpp
+++ b/services/inputflinger/InputManager.cpp
@@ -34,7 +34,7 @@
namespace android {
static const bool ENABLE_INPUT_DEVICE_USAGE_METRICS =
- sysprop::InputProperties::enable_input_device_usage_metrics().value_or(false);
+ sysprop::InputProperties::enable_input_device_usage_metrics().value_or(true);
using gui::FocusRequest;
diff --git a/services/inputflinger/SyncQueue.h b/services/inputflinger/SyncQueue.h
index 62efd55..84ccace 100644
--- a/services/inputflinger/SyncQueue.h
+++ b/services/inputflinger/SyncQueue.h
@@ -27,6 +27,10 @@
template <class T>
class SyncQueue {
public:
+ SyncQueue() = default;
+
+ SyncQueue(size_t capacity) : mCapacity(capacity) {}
+
/** Retrieve and remove the oldest object. Returns std::nullopt if the queue is empty. */
std::optional<T> pop() {
std::scoped_lock lock(mLock);
@@ -38,14 +42,23 @@
return t;
};
- /** Add a new object to the queue. */
+ /**
+ * Add a new object to the queue.
+ * Return true if an element was successfully added.
+ * Return false if the queue is full.
+ */
template <class... Args>
- void push(Args&&... args) {
+ bool push(Args&&... args) {
std::scoped_lock lock(mLock);
+ if (mCapacity && mQueue.size() == mCapacity) {
+ return false;
+ }
mQueue.emplace_back(args...);
+ return true;
};
private:
+ const std::optional<size_t> mCapacity;
std::mutex mLock;
std::list<T> mQueue GUARDED_BY(mLock);
};
diff --git a/services/inputflinger/reader/InputDevice.cpp b/services/inputflinger/reader/InputDevice.cpp
index 2aaddf5..035d96d 100644
--- a/services/inputflinger/reader/InputDevice.cpp
+++ b/services/inputflinger/reader/InputDevice.cpp
@@ -66,23 +66,38 @@
return enabled;
}
-std::list<NotifyArgs> InputDevice::setEnabled(bool enabled, nsecs_t when) {
- std::list<NotifyArgs> out;
- if (enabled && mAssociatedDisplayPort && !mAssociatedViewport) {
- ALOGW("Cannot enable input device %s because it is associated with port %" PRIu8 ", "
- "but the corresponding viewport is not found",
- getName().c_str(), *mAssociatedDisplayPort);
- enabled = false;
+std::list<NotifyArgs> InputDevice::updateEnableState(nsecs_t when,
+ const InputReaderConfiguration& readerConfig,
+ bool forceEnable) {
+ bool enable = true;
+ 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 (isEnabled() == enabled) {
+ std::list<NotifyArgs> out;
+ if (isEnabled() == enable) {
return out;
}
// When resetting some devices, the driver needs to be queried to ensure that a proper reset is
// performed. The querying must happen when the device is enabled, so we reset after enabling
// but before disabling the device. See MultiTouchMotionAccumulator::reset for more information.
- if (enabled) {
+ if (enable) {
for_each_subdevice([](auto& context) { context.enableDevice(); });
out += reset(when);
} else {
@@ -158,18 +173,23 @@
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
+ addEmptyEventHubDevice(eventHubId);
+ std::list<NotifyArgs> out = configure(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) {
@@ -182,7 +202,7 @@
std::list<NotifyArgs> InputDevice::configure(nsecs_t when,
const InputReaderConfiguration& readerConfig,
- ConfigurationChanges changes) {
+ ConfigurationChanges changes, bool forceEnable) {
std::list<NotifyArgs> out;
mSources = 0;
mClasses = ftl::Flags<InputDeviceClass>(0);
@@ -252,15 +272,6 @@
}
}
- if (changes.test(Change::ENABLED_STATE)) {
- // Do not execute this code on the first configure, because 'setEnabled' would call
- // InputMapper::reset, and you can't reset a mapper before it has been configured.
- // The mappers are configured for the first time at the bottom of this function.
- auto it = readerConfig.disabledDevices.find(mId);
- bool enabled = it == readerConfig.disabledDevices.end();
- out += setEnabled(enabled, when);
- }
-
if (!changes.any() || changes.test(Change::DISPLAY_INFO)) {
// In most situations, no port or name will be specified.
mAssociatedDisplayPort = std::nullopt;
@@ -284,12 +295,8 @@
}
}
- // If the device was explicitly disabled by the user, it would be present in the
- // "disabledDevices" list. If it is associated with a specific display, and it was not
- // explicitly disabled, then enable/disable the device based on whether we can find the
- // corresponding viewport.
- bool enabled =
- (readerConfig.disabledDevices.find(mId) == readerConfig.disabledDevices.end());
+ // If it is associated with a specific display, then find the corresponding viewport
+ // which will be used to enable/disable the device.
if (mAssociatedDisplayPort) {
mAssociatedViewport =
readerConfig.getDisplayViewportByPort(*mAssociatedDisplayPort);
@@ -297,7 +304,6 @@
ALOGW("Input device %s should be associated with display on port %" PRIu8 ", "
"but the corresponding viewport is not found.",
getName().c_str(), *mAssociatedDisplayPort);
- enabled = false;
}
} else if (mAssociatedDisplayUniqueId != std::nullopt) {
mAssociatedViewport =
@@ -306,30 +312,21 @@
ALOGW("Input device %s should be associated with display %s but the "
"corresponding viewport cannot be found",
getName().c_str(), mAssociatedDisplayUniqueId->c_str());
- enabled = false;
}
}
+ }
- if (changes.any()) {
- // For first-time configuration, only allow device to be disabled after mappers have
- // finished configuring. This is because we need to read some of the properties from
- // the device's open fd.
- out += setEnabled(enabled, when);
- }
+ 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);
}
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 += setEnabled(readerConfig.disabledDevices.find(mId) ==
- readerConfig.disabledDevices.end(),
- when);
- }
}
return out;
}
@@ -522,9 +519,9 @@
classes.test(InputDeviceClass::TOUCH_MT) && !isSonyDualShock4Touchpad) {
mappers.push_back(createInputMapper<TouchpadInputMapper>(contextPtr, readerConfig));
} else if (classes.test(InputDeviceClass::TOUCH_MT)) {
- mappers.push_back(std::make_unique<MultiTouchInputMapper>(contextPtr, readerConfig));
+ mappers.push_back(createInputMapper<MultiTouchInputMapper>(contextPtr, readerConfig));
} else if (classes.test(InputDeviceClass::TOUCH)) {
- mappers.push_back(std::make_unique<SingleTouchInputMapper>(contextPtr, readerConfig));
+ mappers.push_back(createInputMapper<SingleTouchInputMapper>(contextPtr, readerConfig));
}
// Joystick-like devices.
diff --git a/services/inputflinger/reader/InputReader.cpp b/services/inputflinger/reader/InputReader.cpp
index ea95f78..7d0f28b 100644
--- a/services/inputflinger/reader/InputReader.cpp
+++ b/services/inputflinger/reader/InputReader.cpp
@@ -234,7 +234,7 @@
}
InputDeviceIdentifier identifier = mEventHub->getDeviceIdentifier(eventHubId);
- std::shared_ptr<InputDevice> device = createDeviceLocked(eventHubId, identifier);
+ std::shared_ptr<InputDevice> device = createDeviceLocked(when, eventHubId, identifier);
notifyAll(device->configure(when, mConfig, /*changes=*/{}));
notifyAll(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);
+ notifyAll(device->addEventHubDevice(when, eventHubId, mConfig));
return device;
}
diff --git a/services/inputflinger/reader/include/InputDevice.h b/services/inputflinger/reader/include/InputDevice.h
index aae3fe7..d92f4a2 100644
--- a/services/inputflinger/reader/include/InputDevice.h
+++ b/services/inputflinger/reader/include/InputDevice.h
@@ -77,15 +77,16 @@
inline bool isIgnored() { return !getMapperCount() && !mController; }
bool isEnabled();
- [[nodiscard]] std::list<NotifyArgs> setEnabled(bool enabled, nsecs_t when);
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,
- ConfigurationChanges changes);
+ ConfigurationChanges changes,
+ bool forceEnable = false);
[[nodiscard]] std::list<NotifyArgs> reset(nsecs_t when);
[[nodiscard]] std::list<NotifyArgs> process(const RawEvent* rawEvents, size_t count);
[[nodiscard]] std::list<NotifyArgs> timeoutExpired(nsecs_t when);
@@ -206,6 +207,9 @@
std::vector<std::unique_ptr<InputMapper>> createMappers(
InputDeviceContext& contextPtr, const InputReaderConfiguration& readerConfig);
+ [[nodiscard]] std::list<NotifyArgs> updateEnableState(
+ nsecs_t when, const InputReaderConfiguration& readerConfig, bool forceEnable = false);
+
PropertyMap mConfiguration;
// Runs logic post a `process` call. This can be used to update the generated `NotifyArgs` as
diff --git a/services/inputflinger/reader/include/InputReader.h b/services/inputflinger/reader/include/InputReader.h
index 9112913..391a889 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/reader/mapper/MultiTouchInputMapper.h b/services/inputflinger/reader/mapper/MultiTouchInputMapper.h
index f300ee1..1d788df 100644
--- a/services/inputflinger/reader/mapper/MultiTouchInputMapper.h
+++ b/services/inputflinger/reader/mapper/MultiTouchInputMapper.h
@@ -27,8 +27,6 @@
friend std::unique_ptr<T> createInputMapper(InputDeviceContext& deviceContext,
const InputReaderConfiguration& readerConfig,
Args... args);
- explicit MultiTouchInputMapper(InputDeviceContext& deviceContext,
- const InputReaderConfiguration& readerConfig);
~MultiTouchInputMapper() override;
@@ -41,6 +39,8 @@
bool hasStylus() const override;
private:
+ explicit MultiTouchInputMapper(InputDeviceContext& deviceContext,
+ const InputReaderConfiguration& readerConfig);
// simulate_stylus_with_touch is a debug mode that converts all finger pointers reported by this
// mapper's touchscreen into stylus pointers, and adds SOURCE_STYLUS to the input device.
// It is used to simulate stylus events for debugging and testing on a device that does not
diff --git a/services/inputflinger/reader/mapper/SingleTouchInputMapper.h b/services/inputflinger/reader/mapper/SingleTouchInputMapper.h
index dac53cf..7726bfb 100644
--- a/services/inputflinger/reader/mapper/SingleTouchInputMapper.h
+++ b/services/inputflinger/reader/mapper/SingleTouchInputMapper.h
@@ -27,8 +27,6 @@
friend std::unique_ptr<T> createInputMapper(InputDeviceContext& deviceContext,
const InputReaderConfiguration& readerConfig,
Args... args);
- explicit SingleTouchInputMapper(InputDeviceContext& deviceContext,
- const InputReaderConfiguration& readerConfig);
~SingleTouchInputMapper() override;
@@ -42,6 +40,8 @@
private:
SingleTouchMotionAccumulator mSingleTouchMotionAccumulator;
+ explicit SingleTouchInputMapper(InputDeviceContext& deviceContext,
+ const InputReaderConfiguration& readerConfig);
};
} // namespace android
diff --git a/services/inputflinger/tests/InputDeviceMetricsCollector_test.cpp b/services/inputflinger/tests/InputDeviceMetricsCollector_test.cpp
index 0aa5e23..484e7d6 100644
--- a/services/inputflinger/tests/InputDeviceMetricsCollector_test.cpp
+++ b/services/inputflinger/tests/InputDeviceMetricsCollector_test.cpp
@@ -421,6 +421,7 @@
// Device was used.
mMetricsCollector.notifyMotion(generateMotionArgs(ignoredDeviceId));
mTestListener.assertNotifyMotionWasCalled();
+ mMetricsCollector.notifyDeviceInteraction(ignoredDeviceId, TIME.count(), uids({0, 1, 2}));
ASSERT_NO_FATAL_FAILURE(assertUsageNotLogged());
// Device was used again after the usage timeout expired, but we still don't log usage.
diff --git a/services/inputflinger/tests/InputReader_test.cpp b/services/inputflinger/tests/InputReader_test.cpp
index e751c89..f4e471d 100644
--- a/services/inputflinger/tests/InputReader_test.cpp
+++ b/services/inputflinger/tests/InputReader_test.cpp
@@ -2553,7 +2553,7 @@
// A single input device is associated with a specific display. Check that:
// 1. Device is disabled if the viewport corresponding to the associated display is not found
-// 2. Device is disabled when setEnabled API is called
+// 2. Device is disabled when configure API is called
TEST_F(InputDeviceTest, Configure_AssignsDisplayPort) {
mDevice->addMapper<FakeInputMapper>(EVENTHUB_ID, mFakePolicy->getReaderConfiguration(),
AINPUT_SOURCE_TOUCHSCREEN);
@@ -2660,7 +2660,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 7f8d556..e6bf3f9 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:
diff --git a/services/inputflinger/tests/SyncQueue_test.cpp b/services/inputflinger/tests/SyncQueue_test.cpp
index af2f961..b57ccc2 100644
--- a/services/inputflinger/tests/SyncQueue_test.cpp
+++ b/services/inputflinger/tests/SyncQueue_test.cpp
@@ -50,6 +50,18 @@
}
}
+// Make sure the queue has strict capacity limits.
+TEST(SyncQueueTest, QueueReachesCapacity) {
+ constexpr size_t capacity = 3;
+ SyncQueue<int> queue(capacity);
+
+ // First 3 elements should be added successfully
+ ASSERT_TRUE(queue.push(1));
+ ASSERT_TRUE(queue.push(2));
+ ASSERT_TRUE(queue.push(3));
+ ASSERT_FALSE(queue.push(4)) << "Queue should reach capacity at size " << capacity;
+}
+
TEST(SyncQueueTest, AllowsMultipleThreads) {
SyncQueue<int> queue;