From 9b5a821d1e214cac16a7d03b590964375fd2614a Mon Sep 17 00:00:00 2001 From: Siarhei Vishniakou Date: Mon, 1 Jul 2019 14:25:40 -0700 Subject: Do not use moved-from object We are currently reading a field from a moved-from object. Remove this read. Bug: none Test: none Change-Id: I0d33636f653b7f7ae0b98e4645ee8779bf9da2ea --- services/inputflinger/InputClassifier.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'services/inputflinger/InputClassifier.cpp') diff --git a/services/inputflinger/InputClassifier.cpp b/services/inputflinger/InputClassifier.cpp index 6a7f2797f4..7c061c5857 100644 --- a/services/inputflinger/InputClassifier.cpp +++ b/services/inputflinger/InputClassifier.cpp @@ -276,7 +276,7 @@ void MotionClassifier::enqueueEvent(ClassifierEvent&& event) { bool eventAdded = mEvents.push(std::move(event)); if (!eventAdded) { // If the queue is full, suspect the HAL is slow in processing the events. - ALOGE("Dropped event with eventTime %" PRId64, event.args->eventTime); + ALOGE("Could not add the event to the queue. Resetting"); reset(); } } -- cgit v1.2.3-59-g8ed1b From e3021d703130f9f28a94bef0e7179e75794a59cf Mon Sep 17 00:00:00 2001 From: Siarhei Vishniakou Date: Fri, 28 Feb 2020 15:25:41 -0800 Subject: Remove device state when device is reset When device reset is received in MotionClassifier, don't just set the classification for that device to 'none'. Instead, remove all state related to that device. This prevents growing indefinitely the unordered_maps that contain device-related state. Also, log the feature state to the InputClassifier dump. Bug: 150419367 Test: run several touch integration tests several times, and then do 'dumpsys input'. Ensure that the list doesn't grow as the tests are executed multiple times. Change-Id: I2e1ac359458a321f87e4599bb19350623afc9b2b --- services/inputflinger/BlockingQueue.h | 5 +---- services/inputflinger/InputClassifier.cpp | 9 ++++++++- services/inputflinger/InputClassifier.h | 2 ++ 3 files changed, 11 insertions(+), 5 deletions(-) (limited to 'services/inputflinger/InputClassifier.cpp') diff --git a/services/inputflinger/BlockingQueue.h b/services/inputflinger/BlockingQueue.h index db9f26ec12..b612ca77ce 100644 --- a/services/inputflinger/BlockingQueue.h +++ b/services/inputflinger/BlockingQueue.h @@ -45,10 +45,7 @@ public: T pop() { std::unique_lock lock(mLock); android::base::ScopedLockAssertion assumeLock(mLock); - mHasElements.wait(lock, [this]{ - android::base::ScopedLockAssertion assumeLock(mLock); - return !this->mQueue.empty(); - }); + mHasElements.wait(lock, [this]() REQUIRES(mLock) { return !this->mQueue.empty(); }); T t = std::move(mQueue.front()); mQueue.erase(mQueue.begin()); return t; diff --git a/services/inputflinger/InputClassifier.cpp b/services/inputflinger/InputClassifier.cpp index ae9a34882e..e5e83d752c 100644 --- a/services/inputflinger/InputClassifier.cpp +++ b/services/inputflinger/InputClassifier.cpp @@ -250,7 +250,7 @@ void MotionClassifier::callInputClassifierHal() { case ClassifierEventType::DEVICE_RESET: { const int32_t deviceId = *(event.getDeviceId()); halResponseOk = mService->resetDevice(deviceId).isOk(); - setClassification(deviceId, MotionClassification::NONE); + clearDeviceState(deviceId); break; } case ClassifierEventType::HAL_RESET: { @@ -321,6 +321,12 @@ void MotionClassifier::updateLastDownTime(int32_t deviceId, nsecs_t downTime) { mClassifications[deviceId] = MotionClassification::NONE; } +void MotionClassifier::clearDeviceState(int32_t deviceId) { + std::scoped_lock lock(mLock); + mClassifications.erase(deviceId); + mLastDownTimes.erase(deviceId); +} + MotionClassification MotionClassifier::classify(const NotifyMotionArgs& args) { if ((args.action & AMOTION_EVENT_ACTION_MASK) == AMOTION_EVENT_ACTION_DOWN) { updateLastDownTime(args.deviceId, args.downTime); @@ -455,6 +461,7 @@ void InputClassifier::serviceDied(uint64_t /*cookie*/, void InputClassifier::dump(std::string& dump) { std::scoped_lock lock(mLock); dump += "Input Classifier State:\n"; + dump += StringPrintf(INDENT1 "Deep press: %s\n", deepPressEnabled() ? "enabled" : "disabled"); dump += INDENT1 "Motion Classifier:\n"; if (mMotionClassifier) { diff --git a/services/inputflinger/InputClassifier.h b/services/inputflinger/InputClassifier.h index 47e20dbf75..96923526da 100644 --- a/services/inputflinger/InputClassifier.h +++ b/services/inputflinger/InputClassifier.h @@ -212,6 +212,8 @@ private: void updateLastDownTime(int32_t deviceId, nsecs_t downTime); + void clearDeviceState(int32_t deviceId); + /** * Exit the InputClassifier HAL thread. * Useful for tests to ensure proper cleanup. -- cgit v1.2.3-59-g8ed1b From 1652397ab77904092509a0315ad026f0b4f10a18 Mon Sep 17 00:00:00 2001 From: Siarhei Vishniakou Date: Wed, 4 Mar 2020 17:48:39 -0800 Subject: Properly initialize MotionClassifier InputClassifier will now play a more active role in managing the lifecycle of MotionClassifier. In the previous version of the code, we had a circular reference: MotionClassifier owned a wp, that sometimes got promoted to sp<>. But the InputClassifier was the official owner of MotionClassifier, and owned a unique_ptr. The owner of InputClassifier in real code is InputManager, and in test code, it's the test class. When the owner of InputClassifier destroyed InputClassifier, this could sometimes happen at the time when the MotionClassifier also held a reference to the InputClassifier. That meant that the proper owner of InputClassifier was not invoking the destructor. Instead, the MotionClassifier was invoking the InputClassifier destructor. To fix the situation, we now do the following: 1. InputClassifier will never die before MotionClassifier. 2. MotionClassifier constructor is now allowed to block. It would block for some time because calling getService takes a while. To account for this, InputClassifier launches a new thread to create MotionClassifier. 3. When MotionClassifier is ready to process events, InputClassifier updates mMotionClassifier, which makes it non-null. 4. We now create a separate death recipient, which is co-owned by InputClassifier and MotionClassifier. This is done so that the refcount of the deathrecipient does not affect the refcount of InputClassifier, and thus enforces the ownership of InputClassifier by the InputManager. Now, no one can call ~InputClassifier except for its real owner. 5. MotionClassifier will subscribe the death recipient to the death of the HAL. InputClassifier will delete MotionClassifier if HAL dies. MotionClassifier no longer holds on to the death recipient. 6. We move the loop of the MotionClassifier thread to focus only on processing events. That thread will no longer do any initialization. 7. Remove the thread check inside MotionClassifier. It isn't really useful, now that there's only 1 function for the entire thread. Ownership summary: Both InputClassifier and MotionClassifier own DeathRecipient. DeathRecipient has a reference to InputClassifier. Thus, we must guarantee that DeathRecipient dies before InputClassifier. InputClassifier owns MotionClassifier. This is OK, since even if InputClassifier dies, it will first delete MotionClassifier. That will cause MotionClassifier to release 1 refCount from DeathRecipient. That means the only reference remaining to DeathRecipient will be inside InputClassifier, so InputClassifier will always be alive until DeathRecipient is dead. Similar argument applies if MotionClassifier and DeathRecipient die in different order (as observed from InputClassifier). Tests: 1) Manually running inputflinger_tests on cuttlefish: build/launch cuttlefish using go/acloud m inputflinger_tests adb push out/target/product/vsoc_x86/data/nativetest/inputflinger_tests/inputflinger_tests /data/nativetest/inputflinger_tests/inputflinger_tests adb shell /data/nativetest/inputflinger_tests # ./inputflinger_tests --gtest_filter=*InputClassifierTest* --gtest_repeat=1000 --gtest_break_on_failure 2) Boot flame and open logcat. Observe in logcat: StartInputManagerService took to complete: 2ms Previously, in synchronous approach ( b/130184032) it was about 100 ms (so we did not regress). 3) Kill the HAL while system_server is running, and dumpsys input before and after: adb shell dumpsys input (observe that MotionClassifier is non-null on flame) adb shell -t killall android.hardware.input.classifier@1.0-service adb shell dumpsys input (observe that MotionCLassifier is null) Bug: 149155998 Test: see "Tests" section above Change-Id: Ic76b82bd5f2cd374e3b001400eb495ca36de7353 --- services/inputflinger/InputClassifier.cpp | 134 ++++++++------------- services/inputflinger/InputClassifier.h | 88 ++++++++------ .../inputflinger/tests/InputClassifier_test.cpp | 33 ++++- 3 files changed, 135 insertions(+), 120 deletions(-) (limited to 'services/inputflinger/InputClassifier.cpp') diff --git a/services/inputflinger/InputClassifier.cpp b/services/inputflinger/InputClassifier.cpp index e5e83d752c..8ba1f7f8c1 100644 --- a/services/inputflinger/InputClassifier.cpp +++ b/services/inputflinger/InputClassifier.cpp @@ -46,8 +46,6 @@ using namespace android::hardware::input; namespace android { -static constexpr bool DEBUG = false; - // Category (=namespace) name for the input settings that are applied at boot time static const char* INPUT_NATIVE_BOOT = "input_native_boot"; // Feature flag name for the deep press feature @@ -141,53 +139,46 @@ std::optional ClassifierEvent::getDeviceId() const { // --- MotionClassifier --- -MotionClassifier::MotionClassifier(sp deathRecipient) : - mDeathRecipient(deathRecipient), mEvents(MAX_EVENTS) { - mHalThread = std::thread(&MotionClassifier::callInputClassifierHal, this); +MotionClassifier::MotionClassifier( + sp service) + : mEvents(MAX_EVENTS), mService(service) { + // Under normal operation, we do not need to reset the HAL here. But in the case where system + // crashed, but HAL didn't, we may be connecting to an existing HAL process that might already + // have received events in the past. That means, that HAL could be in an inconsistent state + // once it receives events from the newly created MotionClassifier. + mEvents.push(ClassifierEvent::createHalResetEvent()); + + mHalThread = std::thread(&MotionClassifier::processEvents, this); #if defined(__linux__) // Set the thread name for debugging pthread_setname_np(mHalThread.native_handle(), "InputClassifier"); #endif } -/** - * This function may block for some time to initialize the HAL, so it should only be called - * from the "InputClassifier HAL" thread. - */ -bool MotionClassifier::init() { - ensureHalThread(__func__); +std::unique_ptr MotionClassifier::create( + sp deathRecipient) { + if (!deepPressEnabled()) { + // If feature is not enabled, MotionClassifier should stay null to avoid unnecessary work. + // When MotionClassifier is null, InputClassifier will forward all events + // to the next InputListener, unmodified. + return nullptr; + } sp service = classifier::V1_0::IInputClassifier::getService(); if (!service) { // Not really an error, maybe the device does not have this HAL, // but somehow the feature flag is flipped ALOGI("Could not obtain InputClassifier HAL"); - return false; + return nullptr; } - sp recipient = mDeathRecipient.promote(); - if (recipient != nullptr) { - const bool linked = service->linkToDeath(recipient, 0 /* cookie */).withDefault(false); - if (!linked) { - ALOGE("Could not link MotionClassifier to the HAL death"); - return false; - } - } - - // Under normal operation, we do not need to reset the HAL here. But in the case where system - // crashed, but HAL didn't, we may be connecting to an existing HAL process that might already - // have received events in the past. That means, that HAL could be in an inconsistent state - // once it receives events from the newly created MotionClassifier. - mEvents.push(ClassifierEvent::createHalResetEvent()); - - { - std::scoped_lock lock(mLock); - if (mService) { - ALOGE("MotionClassifier::%s should only be called once", __func__); - } - mService = service; + const bool linked = service->linkToDeath(deathRecipient, 0 /* cookie */).withDefault(false); + if (!linked) { + ALOGE("Could not link death recipient to the HAL death"); + return nullptr; } - return true; + // Using 'new' to access a non-public constructor + return std::unique_ptr(new MotionClassifier(service)); } MotionClassifier::~MotionClassifier() { @@ -195,14 +186,6 @@ MotionClassifier::~MotionClassifier() { mHalThread.join(); } -void MotionClassifier::ensureHalThread(const char* function) { - if (DEBUG) { - if (std::this_thread::get_id() != mHalThread.get_id()) { - LOG_FATAL("Function %s should only be called from InputClassifier thread", function); - } - } -} - /** * Obtain the classification from the HAL for a given MotionEvent. * Should only be called from the InputClassifier thread (mHalThread). @@ -213,23 +196,7 @@ void MotionClassifier::ensureHalThread(const char* function) { * To remove any possibility of negatively affecting the touch latency, the HAL * is called from a dedicated thread. */ -void MotionClassifier::callInputClassifierHal() { - ensureHalThread(__func__); - const bool initialized = init(); - if (!initialized) { - // MotionClassifier no longer useful. - // Deliver death notification from a separate thread - // because ~MotionClassifier may be invoked, which calls mHalThread.join() - std::thread([deathRecipient = mDeathRecipient](){ - sp recipient = deathRecipient.promote(); - if (recipient != nullptr) { - recipient->serviceDied(0 /*cookie*/, nullptr); - } - }).detach(); - return; - } - // From this point on, mService is guaranteed to be non-null. - +void MotionClassifier::processEvents() { while (true) { ClassifierEvent event = mEvents.pop(); bool halResponseOk = true; @@ -389,24 +356,30 @@ void MotionClassifier::dump(std::string& dump) { } } +// --- HalDeathRecipient -// --- InputClassifier --- +InputClassifier::HalDeathRecipient::HalDeathRecipient(InputClassifier& parent) : mParent(parent) {} -InputClassifier::InputClassifier(const sp& listener) : - mListener(listener) { - // The rest of the initialization is done in onFirstRef, because we need to obtain - // an sp to 'this' in order to register for HAL death notifications +void InputClassifier::HalDeathRecipient::serviceDied( + uint64_t cookie, const wp& who) { + sp service = who.promote(); + if (service) { + service->unlinkToDeath(this); + } + mParent.setMotionClassifier(nullptr); } -void InputClassifier::onFirstRef() { - if (!deepPressEnabled()) { - // If feature is not enabled, MotionClassifier should stay null to avoid unnecessary work. - // When MotionClassifier is null, InputClassifier will forward all events - // to the next InputListener, unmodified. - return; - } - std::scoped_lock lock(mLock); - mMotionClassifier = std::make_unique(this); +// --- InputClassifier --- + +InputClassifier::InputClassifier(const sp& listener) + : mListener(listener), mHalDeathRecipient(new HalDeathRecipient(*this)) { + mInitializeMotionClassifierThread = std::thread( + [this] { setMotionClassifier(MotionClassifier::create(mHalDeathRecipient)); }); +#if defined(__linux__) + // Set the thread name for debugging + pthread_setname_np(mInitializeMotionClassifierThread.native_handle(), + "Create MotionClassifier"); +#endif } void InputClassifier::notifyConfigurationChanged(const NotifyConfigurationChangedArgs* args) { @@ -447,15 +420,10 @@ void InputClassifier::notifyDeviceReset(const NotifyDeviceResetArgs* args) { mListener->notifyDeviceReset(args); } -void InputClassifier::serviceDied(uint64_t /*cookie*/, - const wp& who) { +void InputClassifier::setMotionClassifier( + std::unique_ptr motionClassifier) { std::scoped_lock lock(mLock); - ALOGE("InputClassifier HAL has died. Setting mMotionClassifier to null"); - mMotionClassifier = nullptr; - sp service = who.promote(); - if (service) { - service->unlinkToDeath(this); - } + mMotionClassifier = std::move(motionClassifier); } void InputClassifier::dump(std::string& dump) { @@ -472,4 +440,8 @@ void InputClassifier::dump(std::string& dump) { dump += "\n"; } +InputClassifier::~InputClassifier() { + mInitializeMotionClassifierThread.join(); +} + } // namespace android diff --git a/services/inputflinger/InputClassifier.h b/services/inputflinger/InputClassifier.h index 96923526da..8f586956e5 100644 --- a/services/inputflinger/InputClassifier.h +++ b/services/inputflinger/InputClassifier.h @@ -19,8 +19,8 @@ #include #include -#include #include +#include #include "BlockingQueue.h" #include "InputListener.h" @@ -113,23 +113,23 @@ protected: */ class MotionClassifier final : public MotionClassifierInterface { public: - /** - * The deathRecipient will be subscribed to the HAL death. If the death recipient - * owns MotionClassifier and receives HAL death, it should delete its copy of it. - * The callback serviceDied will also be sent if the MotionClassifier itself fails - * to initialize. If the MotionClassifier fails to initialize, it is not useful, and - * should be deleted. - * If no death recipient is supplied, then the registration step will be skipped, so there will - * be no listeners registered for the HAL death. This is useful for testing - * MotionClassifier in isolation. + /* + * Create an instance of MotionClassifier. + * The death recipient, if provided, will be subscribed to the HAL death. + * The death recipient could be used to destroy MotionClassifier. + * + * This function should be called asynchronously, because getService takes a long time. */ - explicit MotionClassifier(sp deathRecipient = nullptr); + static std::unique_ptr create( + sp deathRecipient); + ~MotionClassifier(); /** * Classifies events asynchronously; that is, it doesn't block events on a classification, - * but instead sends them over to the classifier HAL and after a classification is - * determined, it then marks the next event it sees in the stream with it. + * but instead sends them over to the classifier HAL. After a classification of a specific + * event is determined, MotionClassifier then marks the next event in the stream with this + * classification. * * Therefore, it is acceptable to have the classifications be delayed by 1-2 events * in a particular gesture. @@ -141,15 +141,9 @@ public: virtual void dump(std::string& dump) override; private: - /** - * Initialize MotionClassifier. - * Return true if initializaion is successful. - */ - bool init(); - /** - * Entity that will be notified of the HAL death (most likely InputClassifier). - */ - wp mDeathRecipient; + friend class MotionClassifierTest; // to create MotionClassifier with a test HAL implementation + explicit MotionClassifier( + sp service); // The events that need to be sent to the HAL. BlockingQueue mEvents; @@ -164,14 +158,9 @@ private: */ std::thread mHalThread; /** - * Print an error message if the caller is not on the InputClassifier thread. - * Caller must supply the name of the calling function as __func__ + * Process events and call the InputClassifier HAL */ - void ensureHalThread(const char* function); - /** - * Call the InputClassifier HAL - */ - void callInputClassifierHal(); + void processEvents(); /** * Access to the InputClassifier HAL. May be null if init() hasn't completed yet. * When init() successfully completes, mService is guaranteed to remain non-null and to not @@ -225,19 +214,15 @@ private: const char* getServiceStatus() REQUIRES(mLock); }; - /** * Implementation of the InputClassifierInterface. * Represents a separate stage of input processing. All of the input events go through this stage. * Acts as a passthrough for all input events except for motion events. * The events of motion type are sent to MotionClassifier. */ -class InputClassifier : public InputClassifierInterface, - public android::hardware::hidl_death_recipient { +class InputClassifier : public InputClassifierInterface { public: explicit InputClassifier(const sp& listener); - // Some of the constructor logic is finished in onFirstRef - virtual void onFirstRef() override; virtual void notifyConfigurationChanged(const NotifyConfigurationChangedArgs* args) override; virtual void notifyKey(const NotifyKeyArgs* args) override; @@ -245,17 +230,44 @@ public: virtual void notifySwitch(const NotifySwitchArgs* args) override; virtual void notifyDeviceReset(const NotifyDeviceResetArgs* args) override; - virtual void serviceDied(uint64_t cookie, - const wp& who) override; - virtual void dump(std::string& dump) override; + ~InputClassifier(); + private: // Protect access to mMotionClassifier, since it may become null via a hidl callback std::mutex mLock; - std::unique_ptr mMotionClassifier GUARDED_BY(mLock); // The next stage to pass input events to sp mListener; + + std::unique_ptr mMotionClassifier GUARDED_BY(mLock); + std::thread mInitializeMotionClassifierThread; + /** + * Set the value of mMotionClassifier. + * This is called from 2 different threads: + * 1) mInitializeMotionClassifierThread, when we have constructed a MotionClassifier + * 2) A binder thread of the HalDeathRecipient, which is created when HAL dies. This would cause + * mMotionClassifier to become nullptr. + */ + void setMotionClassifier(std::unique_ptr motionClassifier); + + /** + * The deathRecipient will call setMotionClassifier(null) when the HAL dies. + */ + class HalDeathRecipient : public android::hardware::hidl_death_recipient { + public: + explicit HalDeathRecipient(InputClassifier& parent); + virtual void serviceDied(uint64_t cookie, + const wp& who) override; + + private: + InputClassifier& mParent; + }; + // We retain a reference to death recipient, because the death recipient will be calling + // ~MotionClassifier if the HAL dies. + // If we don't retain a reference, and MotionClassifier is the only owner of the death + // recipient, the serviceDied call will cause death recipient to call its own destructor. + sp mHalDeathRecipient; }; } // namespace android diff --git a/services/inputflinger/tests/InputClassifier_test.cpp b/services/inputflinger/tests/InputClassifier_test.cpp index 40086ef708..b4e755a595 100644 --- a/services/inputflinger/tests/InputClassifier_test.cpp +++ b/services/inputflinger/tests/InputClassifier_test.cpp @@ -22,6 +22,9 @@ #include using namespace android::hardware::input; +using android::hardware::Return; +using android::hardware::Void; +using android::hardware::input::common::V1_0::Classification; namespace android { @@ -132,6 +135,27 @@ TEST_F(InputClassifierTest, SendToNextStage_NotifyDeviceResetArgs) { ASSERT_EQ(args, outArgs); } +/** + * A minimal implementation of IInputClassifier. + */ +struct TestHal : public android::hardware::input::classifier::V1_0::IInputClassifier { + Return classify( + const android::hardware::input::common::V1_0::MotionEvent& event) override { + return Classification::NONE; + }; + Return reset() override { return Void(); }; + Return resetDevice(int32_t deviceId) override { return Void(); }; +}; + +/** + * An entity that will be subscribed to the HAL death. + */ +class TestDeathRecipient : public android::hardware::hidl_death_recipient { +public: + virtual void serviceDied(uint64_t cookie, + const wp& who) override{}; +}; + // --- MotionClassifierTest --- class MotionClassifierTest : public testing::Test { @@ -139,7 +163,14 @@ protected: std::unique_ptr mMotionClassifier; virtual void SetUp() override { - mMotionClassifier = std::make_unique(); + mMotionClassifier = MotionClassifier::create(new TestDeathRecipient()); + if (mMotionClassifier == nullptr) { + // If the device running this test does not have IInputClassifier service, + // use the test HAL instead. + // Using 'new' to access non-public constructor + mMotionClassifier = + std::unique_ptr(new MotionClassifier(new TestHal())); + } } }; -- cgit v1.2.3-59-g8ed1b From c60da19f2a012a15971dcd8f4fa3b7c22a90a670 Mon Sep 17 00:00:00 2001 From: Siarhei Vishniakou Date: Thu, 19 Mar 2020 11:55:01 -0700 Subject: Disable deep press when long press timeout is long If the user sets the long press timeout to anything > default value, then we should disable deep press. That means, if the user is triggering long press too easily, it is likely that the user will trigger deep press too easily as well. To prevent unwanted long press invocations, we disable deep press in such situations. Bug: 148311342 Test: Add logging to both cases where deep press is enabled and disabled. Go to accessibility -> touch & hold delay, and change the value. Ensure that the logging matches expectation (short -> enable, medium/long -> disable). The state should persist across the reboot. Change-Id: Ic631c684609c4436e9eaa6f9389939c5a6e4e1cd Merged-In: Ic631c684609c4436e9eaa6f9389939c5a6e4e1cd (cherry picked from commit c9ac19eb3d3c3f185fea95906541249dd4c3baf8) --- services/inputflinger/Android.bp | 1 - services/inputflinger/InputClassifier.cpp | 55 ++++++++-------------- services/inputflinger/InputClassifier.h | 4 ++ services/inputflinger/InputManager.cpp | 4 ++ services/inputflinger/InputManager.h | 2 + .../inputflinger/tests/InputClassifier_test.cpp | 22 +++++++++ 6 files changed, 52 insertions(+), 36 deletions(-) (limited to 'services/inputflinger/InputClassifier.cpp') diff --git a/services/inputflinger/Android.bp b/services/inputflinger/Android.bp index 4ec4e25010..f67c9d006b 100644 --- a/services/inputflinger/Android.bp +++ b/services/inputflinger/Android.bp @@ -52,7 +52,6 @@ cc_defaults { "libstatslog", "libutils", "libui", - "server_configurable_flags", ], } diff --git a/services/inputflinger/InputClassifier.cpp b/services/inputflinger/InputClassifier.cpp index 8ba1f7f8c1..77a0716269 100644 --- a/services/inputflinger/InputClassifier.cpp +++ b/services/inputflinger/InputClassifier.cpp @@ -27,7 +27,6 @@ #if defined(__linux__) #include #endif -#include #include #include @@ -46,11 +45,6 @@ using namespace android::hardware::input; namespace android { -// Category (=namespace) name for the input settings that are applied at boot time -static const char* INPUT_NATIVE_BOOT = "input_native_boot"; -// Feature flag name for the deep press feature -static const char* DEEP_PRESS_ENABLED = "deep_press_enabled"; - //Max number of elements to store in mEvents. static constexpr size_t MAX_EVENTS = 5; @@ -77,20 +71,6 @@ static bool isTouchEvent(const NotifyMotionArgs& args) { return args.source == AINPUT_SOURCE_TOUCHPAD || args.source == AINPUT_SOURCE_TOUCHSCREEN; } -// Check if the "deep touch" feature is on. -static bool deepPressEnabled() { - std::string flag_value = server_configurable_flags::GetServerConfigurableFlag( - INPUT_NATIVE_BOOT, DEEP_PRESS_ENABLED, "true"); - std::transform(flag_value.begin(), flag_value.end(), flag_value.begin(), ::tolower); - if (flag_value == "1" || flag_value == "true") { - ALOGI("Deep press feature enabled."); - return true; - } - ALOGI("Deep press feature is not enabled."); - return false; -} - - // --- ClassifierEvent --- ClassifierEvent::ClassifierEvent(std::unique_ptr args) : @@ -157,12 +137,6 @@ MotionClassifier::MotionClassifier( std::unique_ptr MotionClassifier::create( sp deathRecipient) { - if (!deepPressEnabled()) { - // If feature is not enabled, MotionClassifier should stay null to avoid unnecessary work. - // When MotionClassifier is null, InputClassifier will forward all events - // to the next InputListener, unmodified. - return nullptr; - } sp service = classifier::V1_0::IInputClassifier::getService(); if (!service) { @@ -372,14 +346,25 @@ void InputClassifier::HalDeathRecipient::serviceDied( // --- InputClassifier --- InputClassifier::InputClassifier(const sp& listener) - : mListener(listener), mHalDeathRecipient(new HalDeathRecipient(*this)) { - mInitializeMotionClassifierThread = std::thread( - [this] { setMotionClassifier(MotionClassifier::create(mHalDeathRecipient)); }); + : mListener(listener), mHalDeathRecipient(new HalDeathRecipient(*this)) {} + +void InputClassifier::setMotionClassifierEnabled(bool enabled) { + if (enabled) { + ALOGI("Enabling motion classifier"); + if (mInitializeMotionClassifierThread.joinable()) { + mInitializeMotionClassifierThread.join(); + } + mInitializeMotionClassifierThread = std::thread( + [this] { setMotionClassifier(MotionClassifier::create(mHalDeathRecipient)); }); #if defined(__linux__) - // Set the thread name for debugging - pthread_setname_np(mInitializeMotionClassifierThread.native_handle(), - "Create MotionClassifier"); + // Set the thread name for debugging + pthread_setname_np(mInitializeMotionClassifierThread.native_handle(), + "Create MotionClassifier"); #endif + } else { + ALOGI("Disabling motion classifier"); + setMotionClassifier(nullptr); + } } void InputClassifier::notifyConfigurationChanged(const NotifyConfigurationChangedArgs* args) { @@ -429,8 +414,6 @@ void InputClassifier::setMotionClassifier( void InputClassifier::dump(std::string& dump) { std::scoped_lock lock(mLock); dump += "Input Classifier State:\n"; - dump += StringPrintf(INDENT1 "Deep press: %s\n", deepPressEnabled() ? "enabled" : "disabled"); - dump += INDENT1 "Motion Classifier:\n"; if (mMotionClassifier) { mMotionClassifier->dump(dump); @@ -441,7 +424,9 @@ void InputClassifier::dump(std::string& dump) { } InputClassifier::~InputClassifier() { - mInitializeMotionClassifierThread.join(); + if (mInitializeMotionClassifierThread.joinable()) { + mInitializeMotionClassifierThread.join(); + } } } // namespace android diff --git a/services/inputflinger/InputClassifier.h b/services/inputflinger/InputClassifier.h index 8f586956e5..03510a623c 100644 --- a/services/inputflinger/InputClassifier.h +++ b/services/inputflinger/InputClassifier.h @@ -90,6 +90,7 @@ public: */ class InputClassifierInterface : public virtual RefBase, public InputListenerInterface { public: + virtual void setMotionClassifierEnabled(bool enabled) = 0; /** * Dump the state of the input classifier. * This method may be called on any thread (usually by the input manager). @@ -234,6 +235,9 @@ public: ~InputClassifier(); + // Called from InputManager + virtual void setMotionClassifierEnabled(bool enabled) override; + private: // Protect access to mMotionClassifier, since it may become null via a hidl callback std::mutex mLock; diff --git a/services/inputflinger/InputManager.cpp b/services/inputflinger/InputManager.cpp index c7c61cf1ef..fc771a2c58 100644 --- a/services/inputflinger/InputManager.cpp +++ b/services/inputflinger/InputManager.cpp @@ -132,4 +132,8 @@ void InputManager::unregisterInputChannel(const sp& channel) { mDispatcher->unregisterInputChannel(channel); } +void InputManager::setMotionClassifierEnabled(bool enabled) { + mClassifier->setMotionClassifierEnabled(enabled); +} + } // namespace android diff --git a/services/inputflinger/InputManager.h b/services/inputflinger/InputManager.h index 586097f6a3..0158441fd1 100644 --- a/services/inputflinger/InputManager.h +++ b/services/inputflinger/InputManager.h @@ -100,6 +100,8 @@ public: virtual void registerInputChannel(const sp& channel); virtual void unregisterInputChannel(const sp& channel); + void setMotionClassifierEnabled(bool enabled); + private: sp mReader; diff --git a/services/inputflinger/tests/InputClassifier_test.cpp b/services/inputflinger/tests/InputClassifier_test.cpp index b4e755a595..ab74a0498d 100644 --- a/services/inputflinger/tests/InputClassifier_test.cpp +++ b/services/inputflinger/tests/InputClassifier_test.cpp @@ -135,6 +135,28 @@ TEST_F(InputClassifierTest, SendToNextStage_NotifyDeviceResetArgs) { ASSERT_EQ(args, outArgs); } +TEST_F(InputClassifierTest, SetMotionClassifier_Enabled) { + mClassifier->setMotionClassifierEnabled(true); +} + +TEST_F(InputClassifierTest, SetMotionClassifier_Disabled) { + mClassifier->setMotionClassifierEnabled(false); +} + +/** + * Try to break it by calling setMotionClassifierEnabled multiple times. + */ +TEST_F(InputClassifierTest, SetMotionClassifier_Multiple) { + mClassifier->setMotionClassifierEnabled(true); + mClassifier->setMotionClassifierEnabled(true); + mClassifier->setMotionClassifierEnabled(true); + mClassifier->setMotionClassifierEnabled(false); + mClassifier->setMotionClassifierEnabled(false); + mClassifier->setMotionClassifierEnabled(true); + mClassifier->setMotionClassifierEnabled(true); + mClassifier->setMotionClassifierEnabled(true); +} + /** * A minimal implementation of IInputClassifier. */ -- cgit v1.2.3-59-g8ed1b