Merge "Change InputReader::mDevices collection type"

3 files changed, 61 insertions, 62 deletions

diff --git a/services/inputflinger/reader/InputReader.cpp b/services/inputflinger/reader/InputReader.cpp
index 3e23fa6c84..8327ed8e30 100644
--- a/services/inputflinger/reader/InputReader.cpp
+++ b/services/inputflinger/reader/InputReader.cpp
@@ -62,11 +62,7 @@ InputReader::InputReader(std::shared_ptr<EventHubInterface> eventHub,
     } // release lock
 }
 
-InputReader::~InputReader() {
-    for (auto& devicePair : mDevices) {
-        delete devicePair.second;
-    }
-}
+InputReader::~InputReader() {}
 
 status_t InputReader::start() {
     if (mThread) {
@@ -198,7 +194,8 @@ void InputReader::addDeviceLocked(nsecs_t when, int32_t deviceId) {
     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 @@ void InputReader::addDeviceLocked(nsecs_t when, int32_t deviceId) {
               device->getSources());
     }
 
-    mDevices.insert({deviceId, device});
+    mDevices.emplace(deviceId, device);
     bumpGenerationLocked();
 
     if (device->getClasses() & INPUT_DEVICE_CLASS_EXTERNAL_STYLUS) {
@@ -225,7 +222,7 @@ void InputReader::removeDeviceLocked(nsecs_t when, int32_t deviceId) {
         return;
     }
 
-    InputDevice* device = deviceIt->second;
+    std::shared_ptr<InputDevice> device = std::move(deviceIt->second);
     mDevices.erase(deviceIt);
     bumpGenerationLocked();
 
@@ -242,13 +239,13 @@ void InputReader::removeDeviceLocked(nsecs_t when, int32_t deviceId) {
     }
 
     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 @@ void InputReader::processEventsForDeviceLocked(int32_t deviceId, const RawEvent*
         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::processEventsForDeviceLocked(int32_t deviceId, const RawEvent*
 
 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 @@ void InputReader::refreshConfigurationLocked(uint32_t changes) {
             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 @@ void InputReader::updateGlobalMetaStateLocked() {
     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::notifyExternalStylusPresenceChanged() {
 
 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::getExternalStylusDevicesLocked(std::vector<InputDeviceInfo>& o
 
 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 @@ bool InputReader::shouldDropVirtualKeyLocked(nsecs_t now, int32_t keyCode, int32
 
 void InputReader::fadePointerLocked() {
     for (auto& devicePair : mDevices) {
-        InputDevice* device = devicePair.second;
+        std::shared_ptr<InputDevice>& device = devicePair.second;
         device->fadePointer();
     }
 }
@@ -386,7 +383,7 @@ void InputReader::getInputDevicesLocked(std::vector<InputDeviceInfo>& outInputDe
     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 @@ int32_t InputReader::getStateLocked(int32_t deviceId, uint32_t sourceMask, int32
     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 @@ void InputReader::toggleCapsLockState(int32_t deviceId) {
         return;
     }
 
-    InputDevice* device = deviceIt->second;
+    std::shared_ptr<InputDevice>& device = deviceIt->second;
     if (device->isIgnored()) {
         return;
     }
@@ -472,14 +469,14 @@ bool InputReader::markSupportedKeyCodesLocked(int32_t deviceId, uint32_t sourceM
     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 @@ void InputReader::vibrate(int32_t deviceId, const nsecs_t* pattern, size_t patte
     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 @@ void InputReader::cancelVibrate(int32_t deviceId, int32_t token) {
 
     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 @@ bool InputReader::isInputDeviceEnabled(int32_t deviceId) {
 
     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 @@ bool InputReader::canDispatchToDisplay(int32_t deviceId, int32_t displayId) {
         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 @@ void InputReader::dump(std::string& dump) {
     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 cf1af04417..4f5d2eabf3 100644
--- a/services/inputflinger/reader/include/InputReader.h
+++ b/services/inputflinger/reader/include/InputReader.h
@@ -84,9 +84,10 @@ public:
 
 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 @@ private:
     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 fc196400c9..b7f99422c4 100644
--- a/services/inputflinger/tests/InputReader_test.cpp
+++ b/services/inputflinger/tests/InputReader_test.cpp
@@ -1108,7 +1108,7 @@ private:
 // --- InstrumentedInputReader ---
 
 class InstrumentedInputReader : public InputReader {
-    InputDevice* mNextDevice;
+    std::shared_ptr<InputDevice> mNextDevice;
 
 public:
     InstrumentedInputReader(std::shared_ptr<EventHubInterface> eventHub,
@@ -1116,33 +1116,31 @@ public:
                             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;
         }
@@ -1385,7 +1383,8 @@ protected:
                                                   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);
@@ -1421,7 +1420,8 @@ TEST_F(InputReaderTest, GetInputDevices) {
 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);
@@ -1601,7 +1601,8 @@ TEST_F(InputReaderTest, LoopOnce_ForwardsRawEventsToMappers) {
 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);
@@ -1634,8 +1635,8 @@ TEST_F(InputReaderTest, Device_CanDispatchToDisplay) {
     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);
 
@@ -1801,7 +1802,7 @@ protected:
     sp<TestInputListener> mFakeListener;
     FakeInputReaderContext* mFakeContext;
 
-    InputDevice* mDevice;
+    std::shared_ptr<InputDevice> mDevice;
 
     virtual void SetUp() override {
         mFakeEventHub = std::make_unique<FakeEventHub>();
@@ -1813,13 +1814,13 @@ protected:
         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();