diff options
| -rw-r--r-- | services/inputflinger/InputClassifier.cpp | 134 | ||||
| -rw-r--r-- | services/inputflinger/InputClassifier.h | 88 | ||||
| -rw-r--r-- | services/inputflinger/tests/InputClassifier_test.cpp | 33 |
3 files changed, 135 insertions, 120 deletions
diff --git a/services/inputflinger/InputClassifier.cpp b/services/inputflinger/InputClassifier.cpp index ef1a2247e9..6e51fb2717 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<int32_t> ClassifierEvent::getDeviceId() const { // --- MotionClassifier --- -MotionClassifier::MotionClassifier(sp<android::hardware::hidl_death_recipient> deathRecipient) : - mDeathRecipient(deathRecipient), mEvents(MAX_EVENTS) { - mHalThread = std::thread(&MotionClassifier::callInputClassifierHal, this); +MotionClassifier::MotionClassifier( + sp<android::hardware::input::classifier::V1_0::IInputClassifier> 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<MotionClassifierInterface> MotionClassifier::create( + sp<android::hardware::hidl_death_recipient> 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<android::hardware::input::classifier::V1_0::IInputClassifier> 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<android::hardware::hidl_death_recipient> 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<MotionClassifier>(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<android::hardware::hidl_death_recipient> 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; @@ -383,24 +350,30 @@ void MotionClassifier::dump(std::string& dump) { } } +// --- HalDeathRecipient -// --- InputClassifier --- +InputClassifier::HalDeathRecipient::HalDeathRecipient(InputClassifier& parent) : mParent(parent) {} -InputClassifier::InputClassifier(const sp<InputListenerInterface>& 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<android::hidl::base::V1_0::IBase>& who) { + sp<android::hidl::base::V1_0::IBase> 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<MotionClassifier>(this); +// --- InputClassifier --- + +InputClassifier::InputClassifier(const sp<InputListenerInterface>& 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) { @@ -441,15 +414,10 @@ void InputClassifier::notifyDeviceReset(const NotifyDeviceResetArgs* args) { mListener->notifyDeviceReset(args); } -void InputClassifier::serviceDied(uint64_t /*cookie*/, - const wp<android::hidl::base::V1_0::IBase>& who) { +void InputClassifier::setMotionClassifier( + std::unique_ptr<MotionClassifierInterface> motionClassifier) { std::scoped_lock lock(mLock); - ALOGE("InputClassifier HAL has died. Setting mMotionClassifier to null"); - mMotionClassifier = nullptr; - sp<android::hidl::base::V1_0::IBase> service = who.promote(); - if (service) { - service->unlinkToDeath(this); - } + mMotionClassifier = std::move(motionClassifier); } void InputClassifier::dump(std::string& dump) { @@ -465,4 +433,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 47e20dbf75..b1769b9d96 100644 --- a/services/inputflinger/InputClassifier.h +++ b/services/inputflinger/InputClassifier.h @@ -19,8 +19,8 @@ #include <android-base/thread_annotations.h> #include <utils/RefBase.h> -#include <unordered_map> #include <thread> +#include <unordered_map> #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<android::hardware::hidl_death_recipient> deathRecipient = nullptr); + static std::unique_ptr<MotionClassifierInterface> create( + sp<android::hardware::hidl_death_recipient> 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<android::hardware::hidl_death_recipient> mDeathRecipient; + friend class MotionClassifierTest; // to create MotionClassifier with a test HAL implementation + explicit MotionClassifier( + sp<android::hardware::input::classifier::V1_0::IInputClassifier> service); // The events that need to be sent to the HAL. BlockingQueue<ClassifierEvent> 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 @@ -223,19 +212,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<InputListenerInterface>& 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; @@ -243,17 +228,44 @@ public: virtual void notifySwitch(const NotifySwitchArgs* args) override; virtual void notifyDeviceReset(const NotifyDeviceResetArgs* args) override; - virtual void serviceDied(uint64_t cookie, - const wp<android::hidl::base::V1_0::IBase>& 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<MotionClassifierInterface> mMotionClassifier GUARDED_BY(mLock); // The next stage to pass input events to sp<InputListenerInterface> mListener; + + std::unique_ptr<MotionClassifierInterface> 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<MotionClassifierInterface> 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<android::hidl::base::V1_0::IBase>& 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<HalDeathRecipient> mHalDeathRecipient; }; } // namespace android diff --git a/services/inputflinger/tests/InputClassifier_test.cpp b/services/inputflinger/tests/InputClassifier_test.cpp index 7cc17a2215..9121d7ebba 100644 --- a/services/inputflinger/tests/InputClassifier_test.cpp +++ b/services/inputflinger/tests/InputClassifier_test.cpp @@ -22,6 +22,9 @@ #include <android/hardware/input/classifier/1.0/IInputClassifier.h> using namespace android::hardware::input; +using android::hardware::Return; +using android::hardware::Void; +using android::hardware::input::common::V1_0::Classification; namespace android { @@ -129,6 +132,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<Classification> classify( + const android::hardware::input::common::V1_0::MotionEvent& event) override { + return Classification::NONE; + }; + Return<void> reset() override { return Void(); }; + Return<void> 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<android::hidl::base::V1_0::IBase>& who) override{}; +}; + // --- MotionClassifierTest --- class MotionClassifierTest : public testing::Test { @@ -136,7 +160,14 @@ protected: std::unique_ptr<MotionClassifierInterface> mMotionClassifier; virtual void SetUp() override { - mMotionClassifier = std::make_unique<MotionClassifier>(); + 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<MotionClassifier>(new MotionClassifier(new TestHal())); + } } }; |