Change InputReader::mDevices collection type
Convert InputReader::mDevices from unordered_map of bare pointers
to unordered_map of shared_ptr's. This removes the need for manual
deletion of InputDevice instances and prepares for a future patch
which will have multiple device ids pointing to the same InputDevice.
Cherry-picked from pa/1497945.
Bug: 38511270
Test: atest inputflinger_tests libinput_tests
Change-Id: I0f3bfd96bfe5904ce1a8d96813e45f8467cee0fa
diff --git a/services/inputflinger/reader/InputReader.cpp b/services/inputflinger/reader/InputReader.cpp
index 3e23fa6..8327ed8 100644
--- a/services/inputflinger/reader/InputReader.cpp
+++ b/services/inputflinger/reader/InputReader.cpp
@@ -62,11 +62,7 @@
} // release lock
}
-InputReader::~InputReader() {
- for (auto& devicePair : mDevices) {
- delete devicePair.second;
- }
-}
+InputReader::~InputReader() {}
status_t InputReader::start() {
if (mThread) {
@@ -198,7 +194,8 @@
uint32_t classes = mEventHub->getDeviceClasses(deviceId);
int32_t controllerNumber = mEventHub->getDeviceControllerNumber(deviceId);
- InputDevice* device = createDeviceLocked(deviceId, controllerNumber, identifier, classes);
+ std::shared_ptr<InputDevice> device =
+ createDeviceLocked(deviceId, controllerNumber, identifier, classes);
device->configure(when, &mConfig, 0);
device->reset(when);
@@ -210,7 +207,7 @@
device->getSources());
}
- mDevices.insert({deviceId, device});
+ mDevices.emplace(deviceId, device);
bumpGenerationLocked();
if (device->getClasses() & INPUT_DEVICE_CLASS_EXTERNAL_STYLUS) {
@@ -225,7 +222,7 @@
return;
}
- InputDevice* device = deviceIt->second;
+ std::shared_ptr<InputDevice> device = std::move(deviceIt->second);
mDevices.erase(deviceIt);
bumpGenerationLocked();
@@ -242,13 +239,13 @@
}
device->reset(when);
- delete device;
}
-InputDevice* InputReader::createDeviceLocked(int32_t deviceId, int32_t controllerNumber,
- const InputDeviceIdentifier& identifier,
- uint32_t classes) {
- InputDevice* device = new InputDevice(&mContext, deviceId, bumpGenerationLocked(),
+std::shared_ptr<InputDevice> InputReader::createDeviceLocked(
+ int32_t deviceId, int32_t controllerNumber, const InputDeviceIdentifier& identifier,
+ uint32_t classes) {
+ std::shared_ptr<InputDevice> device =
+ std::make_shared<InputDevice>(&mContext, deviceId, bumpGenerationLocked(),
controllerNumber, identifier, classes);
device->populateMappers();
return device;
@@ -262,7 +259,7 @@
return;
}
- InputDevice* device = deviceIt->second;
+ std::shared_ptr<InputDevice>& device = deviceIt->second;
if (device->isIgnored()) {
// ALOGD("Discarding event for ignored deviceId %d.", deviceId);
return;
@@ -273,7 +270,7 @@
void InputReader::timeoutExpiredLocked(nsecs_t when) {
for (auto& devicePair : mDevices) {
- InputDevice* device = devicePair.second;
+ std::shared_ptr<InputDevice>& device = devicePair.second;
if (!device->isIgnored()) {
device->timeoutExpired(when);
}
@@ -302,7 +299,7 @@
mEventHub->requestReopenDevices();
} else {
for (auto& devicePair : mDevices) {
- InputDevice* device = devicePair.second;
+ std::shared_ptr<InputDevice>& device = devicePair.second;
device->configure(now, &mConfig, changes);
}
}
@@ -313,7 +310,7 @@
mGlobalMetaState = 0;
for (auto& devicePair : mDevices) {
- InputDevice* device = devicePair.second;
+ std::shared_ptr<InputDevice>& device = devicePair.second;
mGlobalMetaState |= device->getMetaState();
}
}
@@ -328,7 +325,7 @@
void InputReader::getExternalStylusDevicesLocked(std::vector<InputDeviceInfo>& outDevices) {
for (auto& devicePair : mDevices) {
- InputDevice* device = devicePair.second;
+ std::shared_ptr<InputDevice>& device = devicePair.second;
if (device->getClasses() & INPUT_DEVICE_CLASS_EXTERNAL_STYLUS && !device->isIgnored()) {
InputDeviceInfo info;
device->getDeviceInfo(&info);
@@ -339,7 +336,7 @@
void InputReader::dispatchExternalStylusState(const StylusState& state) {
for (auto& devicePair : mDevices) {
- InputDevice* device = devicePair.second;
+ std::shared_ptr<InputDevice>& device = devicePair.second;
device->updateExternalStylusState(state);
}
}
@@ -361,7 +358,7 @@
void InputReader::fadePointerLocked() {
for (auto& devicePair : mDevices) {
- InputDevice* device = devicePair.second;
+ std::shared_ptr<InputDevice>& device = devicePair.second;
device->fadePointer();
}
}
@@ -386,7 +383,7 @@
outInputDevices.clear();
for (auto& devicePair : mDevices) {
- InputDevice* device = devicePair.second;
+ std::shared_ptr<InputDevice>& device = devicePair.second;
if (!device->isIgnored()) {
InputDeviceInfo info;
device->getDeviceInfo(&info);
@@ -419,18 +416,18 @@
if (deviceId >= 0) {
auto deviceIt = mDevices.find(deviceId);
if (deviceIt != mDevices.end()) {
- InputDevice* device = deviceIt->second;
+ std::shared_ptr<InputDevice>& device = deviceIt->second;
if (!device->isIgnored() && sourcesMatchMask(device->getSources(), sourceMask)) {
- result = (device->*getStateFunc)(sourceMask, code);
+ result = (device.get()->*getStateFunc)(sourceMask, code);
}
}
} else {
for (auto& devicePair : mDevices) {
- InputDevice* device = devicePair.second;
+ std::shared_ptr<InputDevice>& device = devicePair.second;
if (!device->isIgnored() && sourcesMatchMask(device->getSources(), sourceMask)) {
// If any device reports AKEY_STATE_DOWN or AKEY_STATE_VIRTUAL, return that
// value. Otherwise, return AKEY_STATE_UP as long as one device reports it.
- int32_t currentResult = (device->*getStateFunc)(sourceMask, code);
+ int32_t currentResult = (device.get()->*getStateFunc)(sourceMask, code);
if (currentResult >= AKEY_STATE_DOWN) {
return currentResult;
} else if (currentResult == AKEY_STATE_UP) {
@@ -449,7 +446,7 @@
return;
}
- InputDevice* device = deviceIt->second;
+ std::shared_ptr<InputDevice>& device = deviceIt->second;
if (device->isIgnored()) {
return;
}
@@ -472,14 +469,14 @@
if (deviceId >= 0) {
auto deviceIt = mDevices.find(deviceId);
if (deviceIt != mDevices.end()) {
- InputDevice* device = deviceIt->second;
+ std::shared_ptr<InputDevice>& device = deviceIt->second;
if (!device->isIgnored() && sourcesMatchMask(device->getSources(), sourceMask)) {
result = device->markSupportedKeyCodes(sourceMask, numCodes, keyCodes, outFlags);
}
}
} else {
for (auto& devicePair : mDevices) {
- InputDevice* device = devicePair.second;
+ std::shared_ptr<InputDevice>& device = devicePair.second;
if (!device->isIgnored() && sourcesMatchMask(device->getSources(), sourceMask)) {
result |= device->markSupportedKeyCodes(sourceMask, numCodes, keyCodes, outFlags);
}
@@ -506,7 +503,7 @@
AutoMutex _l(mLock);
auto deviceIt = mDevices.find(deviceId);
if (deviceIt != mDevices.end()) {
- InputDevice* device = deviceIt->second;
+ std::shared_ptr<InputDevice>& device = deviceIt->second;
device->vibrate(pattern, patternSize, repeat, token);
}
}
@@ -516,7 +513,7 @@
auto deviceIt = mDevices.find(deviceId);
if (deviceIt != mDevices.end()) {
- InputDevice* device = deviceIt->second;
+ std::shared_ptr<InputDevice>& device = deviceIt->second;
device->cancelVibrate(token);
}
}
@@ -526,7 +523,7 @@
auto deviceIt = mDevices.find(deviceId);
if (deviceIt != mDevices.end()) {
- InputDevice* device = deviceIt->second;
+ std::shared_ptr<InputDevice>& device = deviceIt->second;
return device->isEnabled();
}
ALOGW("Ignoring invalid device id %" PRId32 ".", deviceId);
@@ -542,7 +539,7 @@
return false;
}
- InputDevice* device = deviceIt->second;
+ std::shared_ptr<InputDevice>& device = deviceIt->second;
if (!device->isEnabled()) {
ALOGW("Ignoring disabled device %s", device->getName().c_str());
return false;
@@ -571,7 +568,7 @@
dump += "Input Reader State:\n";
for (const auto& devicePair : mDevices) {
- InputDevice* const device = devicePair.second;
+ const std::shared_ptr<InputDevice>& device = devicePair.second;
device->dump(dump);
}
diff --git a/services/inputflinger/reader/include/InputReader.h b/services/inputflinger/reader/include/InputReader.h
index cf1af04..4f5d2ea 100644
--- a/services/inputflinger/reader/include/InputReader.h
+++ b/services/inputflinger/reader/include/InputReader.h
@@ -84,9 +84,10 @@
protected:
// These members are protected so they can be instrumented by test cases.
- virtual InputDevice* createDeviceLocked(int32_t deviceId, int32_t controllerNumber,
- const InputDeviceIdentifier& identifier,
- uint32_t classes);
+ virtual std::shared_ptr<InputDevice> createDeviceLocked(int32_t deviceId,
+ int32_t controllerNumber,
+ const InputDeviceIdentifier& identifier,
+ uint32_t classes);
// With each iteration of the loop, InputReader reads and processes one incoming message from
// the EventHub.
@@ -138,7 +139,7 @@
static const int EVENT_BUFFER_SIZE = 256;
RawEvent mEventBuffer[EVENT_BUFFER_SIZE];
- std::unordered_map<int32_t /*deviceId*/, InputDevice*> mDevices;
+ std::unordered_map<int32_t /*deviceId*/, std::shared_ptr<InputDevice>> mDevices;
// low-level input event decoding and device management
void processEventsLocked(const RawEvent* rawEvents, size_t count);
diff --git a/services/inputflinger/tests/InputReader_test.cpp b/services/inputflinger/tests/InputReader_test.cpp
index 01bd9db..d870a01 100644
--- a/services/inputflinger/tests/InputReader_test.cpp
+++ b/services/inputflinger/tests/InputReader_test.cpp
@@ -1091,7 +1091,7 @@
// --- InstrumentedInputReader ---
class InstrumentedInputReader : public InputReader {
- InputDevice* mNextDevice;
+ std::shared_ptr<InputDevice> mNextDevice;
public:
InstrumentedInputReader(std::shared_ptr<EventHubInterface> eventHub,
@@ -1099,33 +1099,31 @@
const sp<InputListenerInterface>& listener)
: InputReader(eventHub, policy, listener), mNextDevice(nullptr) {}
- virtual ~InstrumentedInputReader() {
- if (mNextDevice) {
- delete mNextDevice;
- }
- }
+ virtual ~InstrumentedInputReader() {}
- void setNextDevice(InputDevice* device) { mNextDevice = device; }
+ void setNextDevice(std::shared_ptr<InputDevice> device) { mNextDevice = device; }
- InputDevice* newDevice(int32_t deviceId, int32_t controllerNumber, const std::string& name,
- uint32_t classes, const std::string& location = "") {
+ std::shared_ptr<InputDevice> newDevice(int32_t deviceId, int32_t controllerNumber,
+ const std::string& name, uint32_t classes,
+ const std::string& location = "") {
InputDeviceIdentifier identifier;
identifier.name = name;
identifier.location = location;
int32_t generation = deviceId + 1;
- return new InputDevice(&mContext, deviceId, generation, controllerNumber, identifier,
- classes);
+ return std::make_shared<InputDevice>(&mContext, deviceId, generation, controllerNumber,
+ identifier, classes);
}
// Make the protected loopOnce method accessible to tests.
using InputReader::loopOnce;
protected:
- virtual InputDevice* createDeviceLocked(int32_t deviceId, int32_t controllerNumber,
- const InputDeviceIdentifier& identifier,
- uint32_t classes) {
+ virtual std::shared_ptr<InputDevice> createDeviceLocked(int32_t deviceId,
+ int32_t controllerNumber,
+ const InputDeviceIdentifier& identifier,
+ uint32_t classes) {
if (mNextDevice) {
- InputDevice* device = mNextDevice;
+ std::shared_ptr<InputDevice> device(mNextDevice);
mNextDevice = nullptr;
return device;
}
@@ -1368,7 +1366,8 @@
const std::string& name, uint32_t classes,
uint32_t sources,
const PropertyMap* configuration) {
- InputDevice* device = mReader->newDevice(deviceId, controllerNumber, name, classes);
+ std::shared_ptr<InputDevice> device =
+ mReader->newDevice(deviceId, controllerNumber, name, classes);
FakeInputMapper& mapper = device->addMapper<FakeInputMapper>(sources);
mReader->setNextDevice(device);
addDevice(deviceId, name, classes, configuration);
@@ -1404,7 +1403,8 @@
TEST_F(InputReaderTest, WhenEnabledChanges_SendsDeviceResetNotification) {
constexpr int32_t deviceId = 1;
constexpr uint32_t deviceClass = INPUT_DEVICE_CLASS_KEYBOARD;
- InputDevice* device = mReader->newDevice(deviceId, 0 /*controllerNumber*/, "fake", deviceClass);
+ std::shared_ptr<InputDevice> device =
+ mReader->newDevice(deviceId, 0 /*controllerNumber*/, "fake", deviceClass);
// Must add at least one mapper or the device will be ignored!
device->addMapper<FakeInputMapper>(AINPUT_SOURCE_KEYBOARD);
mReader->setNextDevice(device);
@@ -1584,7 +1584,8 @@
TEST_F(InputReaderTest, DeviceReset_IncrementsSequenceNumber) {
constexpr int32_t deviceId = 1;
constexpr uint32_t deviceClass = INPUT_DEVICE_CLASS_KEYBOARD;
- InputDevice* device = mReader->newDevice(deviceId, 0 /*controllerNumber*/, "fake", deviceClass);
+ std::shared_ptr<InputDevice> device =
+ mReader->newDevice(deviceId, 0 /*controllerNumber*/, "fake", deviceClass);
// Must add at least one mapper or the device will be ignored!
device->addMapper<FakeInputMapper>(AINPUT_SOURCE_KEYBOARD);
mReader->setNextDevice(device);
@@ -1617,8 +1618,8 @@
constexpr int32_t deviceId = 1;
constexpr uint32_t deviceClass = INPUT_DEVICE_CLASS_KEYBOARD;
const char* DEVICE_LOCATION = "USB1";
- InputDevice* device = mReader->newDevice(deviceId, 0 /*controllerNumber*/, "fake", deviceClass,
- DEVICE_LOCATION);
+ std::shared_ptr<InputDevice> device = mReader->newDevice(deviceId, 0 /*controllerNumber*/,
+ "fake", deviceClass, DEVICE_LOCATION);
FakeInputMapper& mapper = device->addMapper<FakeInputMapper>(AINPUT_SOURCE_TOUCHSCREEN);
mReader->setNextDevice(device);
@@ -1671,7 +1672,7 @@
sp<TestInputListener> mFakeListener;
FakeInputReaderContext* mFakeContext;
- InputDevice* mDevice;
+ std::shared_ptr<InputDevice> mDevice;
virtual void SetUp() override {
mFakeEventHub = std::make_unique<FakeEventHub>();
@@ -1683,13 +1684,13 @@
InputDeviceIdentifier identifier;
identifier.name = DEVICE_NAME;
identifier.location = DEVICE_LOCATION;
- mDevice = new InputDevice(mFakeContext, DEVICE_ID, DEVICE_GENERATION,
- DEVICE_CONTROLLER_NUMBER, identifier, DEVICE_CLASSES);
+ mDevice =
+ std::make_shared<InputDevice>(mFakeContext, DEVICE_ID, DEVICE_GENERATION,
+ DEVICE_CONTROLLER_NUMBER, identifier, DEVICE_CLASSES);
}
virtual void TearDown() override {
- delete mDevice;
-
+ mDevice = nullptr;
delete mFakeContext;
mFakeListener.clear();
mFakePolicy.clear();