From 3f6626e257046005dd5e4a070e7279e8ed83d294 Mon Sep 17 00:00:00 2001 From: Prabir Pradhan Date: Thu, 4 Nov 2021 16:40:47 -0700 Subject: Change PointerController to display space PointerController used to work in the logical display space, so TouchInputMapper and CursorInputMapper would need to transform the coordinates before interacting with it. This CL makes PointerController work in the display space. It will transform incoming and outgoing coordinates to stay in the display space using the DisplayInfo provided by SurfaceFlinger. Using info provided by SF also means that there will be better synchonization between the pointers and display changes like rotation. Bug: 188939842 Bug: 144544464 Test: manual: ensure mouse and touch spots work in different display orientations and sizes set using "adb shell wm size" Change-Id: Ic2e05f06c70f4aaf5c104af9c9723e48c545de05 Change-Id: I5e9e19c3678766985ca2193cfe045a11f812fa2b --- libs/input/PointerController.cpp | 86 +++++++++++++++++++++++++++++++++------- 1 file changed, 71 insertions(+), 15 deletions(-) (limited to 'libs/input/PointerController.cpp') diff --git a/libs/input/PointerController.cpp b/libs/input/PointerController.cpp index 8f04cfb70469..6ea303c0163e 100644 --- a/libs/input/PointerController.cpp +++ b/libs/input/PointerController.cpp @@ -17,24 +17,29 @@ #define LOG_TAG "PointerController" //#define LOG_NDEBUG 0 -// Log debug messages about pointer updates -#define DEBUG_POINTER_UPDATES 0 - #include "PointerController.h" -#include "MouseCursorController.h" #include "PointerControllerContext.h" -#include "TouchSpotController.h" - -#include -#include #include #include #include -#include namespace android { +namespace { + +const ui::Transform kIdentityTransform; + +} // namespace + +// --- PointerController::DisplayInfoListener --- + +void PointerController::DisplayInfoListener::onWindowInfosChanged( + const std::vector&, + const std::vector& displayInfo) { + mPointerController.onDisplayInfosChanged(displayInfo); +} + // --- PointerController --- std::shared_ptr PointerController::create( @@ -63,9 +68,12 @@ std::shared_ptr PointerController::create( PointerController::PointerController(const sp& policy, const sp& looper, const sp& spriteController) - : mContext(policy, looper, spriteController, *this), mCursorController(mContext) { + : mContext(policy, looper, spriteController, *this), + mCursorController(mContext), + mDisplayInfoListener(new DisplayInfoListener(*this)) { std::scoped_lock lock(mLock); mLocked.presentation = Presentation::SPOT; + SurfaceComposerClient::getDefault()->addWindowInfosListener(mDisplayInfoListener); } bool PointerController::getBounds(float* outMinX, float* outMinY, float* outMaxX, @@ -74,7 +82,14 @@ bool PointerController::getBounds(float* outMinX, float* outMinY, float* outMaxX } void PointerController::move(float deltaX, float deltaY) { - mCursorController.move(deltaX, deltaY); + const int32_t displayId = mCursorController.getDisplayId(); + vec2 transformed; + { + std::scoped_lock lock(mLock); + const auto& transform = getTransformForDisplayLocked(displayId); + transformed = transformWithoutTranslation(transform, {deltaX, deltaY}); + } + mCursorController.move(transformed.x, transformed.y); } void PointerController::setButtonState(int32_t buttonState) { @@ -86,12 +101,26 @@ int32_t PointerController::getButtonState() const { } void PointerController::setPosition(float x, float y) { - std::scoped_lock lock(mLock); - mCursorController.setPosition(x, y); + const int32_t displayId = mCursorController.getDisplayId(); + vec2 transformed; + { + std::scoped_lock lock(mLock); + const auto& transform = getTransformForDisplayLocked(displayId); + transformed = transform.transform(x, y); + } + mCursorController.setPosition(transformed.x, transformed.y); } void PointerController::getPosition(float* outX, float* outY) const { + const int32_t displayId = mCursorController.getDisplayId(); mCursorController.getPosition(outX, outY); + { + std::scoped_lock lock(mLock); + const auto& transform = getTransformForDisplayLocked(displayId); + const auto xy = transform.inverse().transform(*outX, *outY); + *outX = xy.x; + *outY = xy.y; + } } int32_t PointerController::getDisplayId() const { @@ -130,11 +159,25 @@ void PointerController::setPresentation(Presentation presentation) { void PointerController::setSpots(const PointerCoords* spotCoords, const uint32_t* spotIdToIndex, BitSet32 spotIdBits, int32_t displayId) { std::scoped_lock lock(mLock); + std::array outSpotCoords{}; + const ui::Transform& transform = getTransformForDisplayLocked(displayId); + + for (BitSet32 idBits(spotIdBits); !idBits.isEmpty();) { + const uint32_t index = spotIdToIndex[idBits.clearFirstMarkedBit()]; + + const vec2 xy = transform.transform(spotCoords[index].getXYValue()); + outSpotCoords[index].setAxisValue(AMOTION_EVENT_AXIS_X, xy.x); + outSpotCoords[index].setAxisValue(AMOTION_EVENT_AXIS_Y, xy.y); + + float pressure = spotCoords[index].getAxisValue(AMOTION_EVENT_AXIS_PRESSURE); + outSpotCoords[index].setAxisValue(AMOTION_EVENT_AXIS_PRESSURE, pressure); + } + auto it = mLocked.spotControllers.find(displayId); if (it == mLocked.spotControllers.end()) { mLocked.spotControllers.try_emplace(displayId, displayId, mContext); } - mLocked.spotControllers.at(displayId).setSpots(spotCoords, spotIdToIndex, spotIdBits); + mLocked.spotControllers.at(displayId).setSpots(outSpotCoords.data(), spotIdToIndex, spotIdBits); } void PointerController::clearSpots() { @@ -194,7 +237,7 @@ void PointerController::doInactivityTimeout() { void PointerController::onDisplayViewportsUpdated(std::vector& viewports) { std::unordered_set displayIdSet; - for (DisplayViewport viewport : viewports) { + for (const DisplayViewport& viewport : viewports) { displayIdSet.insert(viewport.displayId); } @@ -214,4 +257,17 @@ void PointerController::onDisplayViewportsUpdated(std::vector& } } +void PointerController::onDisplayInfosChanged(const std::vector& displayInfo) { + std::scoped_lock lock(mLock); + mLocked.mDisplayInfos = displayInfo; +} + +const ui::Transform& PointerController::getTransformForDisplayLocked(int displayId) const { + const auto& di = mLocked.mDisplayInfos; + auto it = std::find_if(di.begin(), di.end(), [displayId](const gui::DisplayInfo& info) { + return info.displayId == displayId; + }); + return it != di.end() ? it->transform : kIdentityTransform; +} + } // namespace android -- cgit v1.2.3-59-g8ed1b From 02b0545d02001ad152eeaf1ff35465345cbd5d4d Mon Sep 17 00:00:00 2001 From: Prabir Pradhan Date: Wed, 17 Nov 2021 21:48:11 +0000 Subject: Revert "Change PointerController to display space" Revert "Change PointerController to display space" Revert submission 16194643-pointer-controller-in-display-space Reason for revert: b/206817973 Reverted Changes: I764c070ad:Change PointerController to display space I5e9e19c36:Change PointerController to display space Change-Id: I615d343968b818f498e905bab7963106b4e62651 --- libs/input/PointerController.cpp | 86 +++++++--------------------------------- libs/input/PointerController.h | 16 -------- 2 files changed, 15 insertions(+), 87 deletions(-) (limited to 'libs/input/PointerController.cpp') diff --git a/libs/input/PointerController.cpp b/libs/input/PointerController.cpp index 6ea303c0163e..8f04cfb70469 100644 --- a/libs/input/PointerController.cpp +++ b/libs/input/PointerController.cpp @@ -17,29 +17,24 @@ #define LOG_TAG "PointerController" //#define LOG_NDEBUG 0 +// Log debug messages about pointer updates +#define DEBUG_POINTER_UPDATES 0 + #include "PointerController.h" +#include "MouseCursorController.h" #include "PointerControllerContext.h" +#include "TouchSpotController.h" + +#include +#include #include #include #include +#include namespace android { -namespace { - -const ui::Transform kIdentityTransform; - -} // namespace - -// --- PointerController::DisplayInfoListener --- - -void PointerController::DisplayInfoListener::onWindowInfosChanged( - const std::vector&, - const std::vector& displayInfo) { - mPointerController.onDisplayInfosChanged(displayInfo); -} - // --- PointerController --- std::shared_ptr PointerController::create( @@ -68,12 +63,9 @@ std::shared_ptr PointerController::create( PointerController::PointerController(const sp& policy, const sp& looper, const sp& spriteController) - : mContext(policy, looper, spriteController, *this), - mCursorController(mContext), - mDisplayInfoListener(new DisplayInfoListener(*this)) { + : mContext(policy, looper, spriteController, *this), mCursorController(mContext) { std::scoped_lock lock(mLock); mLocked.presentation = Presentation::SPOT; - SurfaceComposerClient::getDefault()->addWindowInfosListener(mDisplayInfoListener); } bool PointerController::getBounds(float* outMinX, float* outMinY, float* outMaxX, @@ -82,14 +74,7 @@ bool PointerController::getBounds(float* outMinX, float* outMinY, float* outMaxX } void PointerController::move(float deltaX, float deltaY) { - const int32_t displayId = mCursorController.getDisplayId(); - vec2 transformed; - { - std::scoped_lock lock(mLock); - const auto& transform = getTransformForDisplayLocked(displayId); - transformed = transformWithoutTranslation(transform, {deltaX, deltaY}); - } - mCursorController.move(transformed.x, transformed.y); + mCursorController.move(deltaX, deltaY); } void PointerController::setButtonState(int32_t buttonState) { @@ -101,26 +86,12 @@ int32_t PointerController::getButtonState() const { } void PointerController::setPosition(float x, float y) { - const int32_t displayId = mCursorController.getDisplayId(); - vec2 transformed; - { - std::scoped_lock lock(mLock); - const auto& transform = getTransformForDisplayLocked(displayId); - transformed = transform.transform(x, y); - } - mCursorController.setPosition(transformed.x, transformed.y); + std::scoped_lock lock(mLock); + mCursorController.setPosition(x, y); } void PointerController::getPosition(float* outX, float* outY) const { - const int32_t displayId = mCursorController.getDisplayId(); mCursorController.getPosition(outX, outY); - { - std::scoped_lock lock(mLock); - const auto& transform = getTransformForDisplayLocked(displayId); - const auto xy = transform.inverse().transform(*outX, *outY); - *outX = xy.x; - *outY = xy.y; - } } int32_t PointerController::getDisplayId() const { @@ -159,25 +130,11 @@ void PointerController::setPresentation(Presentation presentation) { void PointerController::setSpots(const PointerCoords* spotCoords, const uint32_t* spotIdToIndex, BitSet32 spotIdBits, int32_t displayId) { std::scoped_lock lock(mLock); - std::array outSpotCoords{}; - const ui::Transform& transform = getTransformForDisplayLocked(displayId); - - for (BitSet32 idBits(spotIdBits); !idBits.isEmpty();) { - const uint32_t index = spotIdToIndex[idBits.clearFirstMarkedBit()]; - - const vec2 xy = transform.transform(spotCoords[index].getXYValue()); - outSpotCoords[index].setAxisValue(AMOTION_EVENT_AXIS_X, xy.x); - outSpotCoords[index].setAxisValue(AMOTION_EVENT_AXIS_Y, xy.y); - - float pressure = spotCoords[index].getAxisValue(AMOTION_EVENT_AXIS_PRESSURE); - outSpotCoords[index].setAxisValue(AMOTION_EVENT_AXIS_PRESSURE, pressure); - } - auto it = mLocked.spotControllers.find(displayId); if (it == mLocked.spotControllers.end()) { mLocked.spotControllers.try_emplace(displayId, displayId, mContext); } - mLocked.spotControllers.at(displayId).setSpots(outSpotCoords.data(), spotIdToIndex, spotIdBits); + mLocked.spotControllers.at(displayId).setSpots(spotCoords, spotIdToIndex, spotIdBits); } void PointerController::clearSpots() { @@ -237,7 +194,7 @@ void PointerController::doInactivityTimeout() { void PointerController::onDisplayViewportsUpdated(std::vector& viewports) { std::unordered_set displayIdSet; - for (const DisplayViewport& viewport : viewports) { + for (DisplayViewport viewport : viewports) { displayIdSet.insert(viewport.displayId); } @@ -257,17 +214,4 @@ void PointerController::onDisplayViewportsUpdated(std::vector& } } -void PointerController::onDisplayInfosChanged(const std::vector& displayInfo) { - std::scoped_lock lock(mLock); - mLocked.mDisplayInfos = displayInfo; -} - -const ui::Transform& PointerController::getTransformForDisplayLocked(int displayId) const { - const auto& di = mLocked.mDisplayInfos; - auto it = std::find_if(di.begin(), di.end(), [displayId](const gui::DisplayInfo& info) { - return info.displayId == displayId; - }); - return it != di.end() ? it->transform : kIdentityTransform; -} - } // namespace android diff --git a/libs/input/PointerController.h b/libs/input/PointerController.h index 58bb01466d25..97567bab202b 100644 --- a/libs/input/PointerController.h +++ b/libs/input/PointerController.h @@ -72,8 +72,6 @@ public: void reloadPointerResources(); void onDisplayViewportsUpdated(std::vector& viewports); - void onDisplayInfosChanged(const std::vector& displayInfos); - private: friend PointerControllerContext::LooperCallback; friend PointerControllerContext::MessageHandler; @@ -87,23 +85,9 @@ private: struct Locked { Presentation presentation; - std::vector mDisplayInfos; std::unordered_map spotControllers; } mLocked GUARDED_BY(mLock); - class DisplayInfoListener : public gui::WindowInfosListener { - public: - explicit DisplayInfoListener(PointerController& pc) : mPointerController(pc){}; - void onWindowInfosChanged(const std::vector&, - const std::vector&) override; - - private: - PointerController& mPointerController; - }; - sp mDisplayInfoListener; - - const ui::Transform& getTransformForDisplayLocked(int displayId) const REQUIRES(mLock); - PointerController(const sp& policy, const sp& looper, const sp& spriteController); void clearSpotsLocked(); -- cgit v1.2.3-59-g8ed1b From f97fac36a498f42eb05472869142a2d3200c8aeb Mon Sep 17 00:00:00 2001 From: Prabir Pradhan Date: Thu, 18 Nov 2021 16:40:34 +0000 Subject: Reland "Change PointerController to display space" 02b0545d02001ad152eeaf1ff35465345cbd5d4d Changes since the first time it landed: - Unregister the WindowInfosLisntener in PointerController's destructor. Bug: 188939842 Bug: 144544464 Bug: 206817973 Test: forrest run - CtsHardwareTestsCases Change-Id: I92a3f128545e73c85d2a5079ee914e2f890c4308 --- libs/input/PointerController.cpp | 90 +++++++++++++++++++++++++++++++++------- libs/input/PointerController.h | 18 +++++++- 2 files changed, 92 insertions(+), 16 deletions(-) (limited to 'libs/input/PointerController.cpp') diff --git a/libs/input/PointerController.cpp b/libs/input/PointerController.cpp index 8f04cfb70469..f43586f8d9d0 100644 --- a/libs/input/PointerController.cpp +++ b/libs/input/PointerController.cpp @@ -17,24 +17,29 @@ #define LOG_TAG "PointerController" //#define LOG_NDEBUG 0 -// Log debug messages about pointer updates -#define DEBUG_POINTER_UPDATES 0 - #include "PointerController.h" -#include "MouseCursorController.h" #include "PointerControllerContext.h" -#include "TouchSpotController.h" - -#include -#include #include #include #include -#include namespace android { +namespace { + +const ui::Transform kIdentityTransform; + +} // namespace + +// --- PointerController::DisplayInfoListener --- + +void PointerController::DisplayInfoListener::onWindowInfosChanged( + const std::vector&, + const std::vector& displayInfo) { + mPointerController.onDisplayInfosChanged(displayInfo); +} + // --- PointerController --- std::shared_ptr PointerController::create( @@ -63,9 +68,16 @@ std::shared_ptr PointerController::create( PointerController::PointerController(const sp& policy, const sp& looper, const sp& spriteController) - : mContext(policy, looper, spriteController, *this), mCursorController(mContext) { + : mContext(policy, looper, spriteController, *this), + mCursorController(mContext), + mDisplayInfoListener(new DisplayInfoListener(*this)) { std::scoped_lock lock(mLock); mLocked.presentation = Presentation::SPOT; + SurfaceComposerClient::getDefault()->addWindowInfosListener(mDisplayInfoListener); +} + +PointerController::~PointerController() { + SurfaceComposerClient::getDefault()->removeWindowInfosListener(mDisplayInfoListener); } bool PointerController::getBounds(float* outMinX, float* outMinY, float* outMaxX, @@ -74,7 +86,14 @@ bool PointerController::getBounds(float* outMinX, float* outMinY, float* outMaxX } void PointerController::move(float deltaX, float deltaY) { - mCursorController.move(deltaX, deltaY); + const int32_t displayId = mCursorController.getDisplayId(); + vec2 transformed; + { + std::scoped_lock lock(mLock); + const auto& transform = getTransformForDisplayLocked(displayId); + transformed = transformWithoutTranslation(transform, {deltaX, deltaY}); + } + mCursorController.move(transformed.x, transformed.y); } void PointerController::setButtonState(int32_t buttonState) { @@ -86,12 +105,26 @@ int32_t PointerController::getButtonState() const { } void PointerController::setPosition(float x, float y) { - std::scoped_lock lock(mLock); - mCursorController.setPosition(x, y); + const int32_t displayId = mCursorController.getDisplayId(); + vec2 transformed; + { + std::scoped_lock lock(mLock); + const auto& transform = getTransformForDisplayLocked(displayId); + transformed = transform.transform(x, y); + } + mCursorController.setPosition(transformed.x, transformed.y); } void PointerController::getPosition(float* outX, float* outY) const { + const int32_t displayId = mCursorController.getDisplayId(); mCursorController.getPosition(outX, outY); + { + std::scoped_lock lock(mLock); + const auto& transform = getTransformForDisplayLocked(displayId); + const auto xy = transform.inverse().transform(*outX, *outY); + *outX = xy.x; + *outY = xy.y; + } } int32_t PointerController::getDisplayId() const { @@ -130,11 +163,25 @@ void PointerController::setPresentation(Presentation presentation) { void PointerController::setSpots(const PointerCoords* spotCoords, const uint32_t* spotIdToIndex, BitSet32 spotIdBits, int32_t displayId) { std::scoped_lock lock(mLock); + std::array outSpotCoords{}; + const ui::Transform& transform = getTransformForDisplayLocked(displayId); + + for (BitSet32 idBits(spotIdBits); !idBits.isEmpty();) { + const uint32_t index = spotIdToIndex[idBits.clearFirstMarkedBit()]; + + const vec2 xy = transform.transform(spotCoords[index].getXYValue()); + outSpotCoords[index].setAxisValue(AMOTION_EVENT_AXIS_X, xy.x); + outSpotCoords[index].setAxisValue(AMOTION_EVENT_AXIS_Y, xy.y); + + float pressure = spotCoords[index].getAxisValue(AMOTION_EVENT_AXIS_PRESSURE); + outSpotCoords[index].setAxisValue(AMOTION_EVENT_AXIS_PRESSURE, pressure); + } + auto it = mLocked.spotControllers.find(displayId); if (it == mLocked.spotControllers.end()) { mLocked.spotControllers.try_emplace(displayId, displayId, mContext); } - mLocked.spotControllers.at(displayId).setSpots(spotCoords, spotIdToIndex, spotIdBits); + mLocked.spotControllers.at(displayId).setSpots(outSpotCoords.data(), spotIdToIndex, spotIdBits); } void PointerController::clearSpots() { @@ -194,7 +241,7 @@ void PointerController::doInactivityTimeout() { void PointerController::onDisplayViewportsUpdated(std::vector& viewports) { std::unordered_set displayIdSet; - for (DisplayViewport viewport : viewports) { + for (const DisplayViewport& viewport : viewports) { displayIdSet.insert(viewport.displayId); } @@ -214,4 +261,17 @@ void PointerController::onDisplayViewportsUpdated(std::vector& } } +void PointerController::onDisplayInfosChanged(const std::vector& displayInfo) { + std::scoped_lock lock(mLock); + mLocked.mDisplayInfos = displayInfo; +} + +const ui::Transform& PointerController::getTransformForDisplayLocked(int displayId) const { + const auto& di = mLocked.mDisplayInfos; + auto it = std::find_if(di.begin(), di.end(), [displayId](const gui::DisplayInfo& info) { + return info.displayId == displayId; + }); + return it != di.end() ? it->transform : kIdentityTransform; +} + } // namespace android diff --git a/libs/input/PointerController.h b/libs/input/PointerController.h index 97567bab202b..796077f6c38c 100644 --- a/libs/input/PointerController.h +++ b/libs/input/PointerController.h @@ -47,7 +47,7 @@ public: const sp& policy, const sp& looper, const sp& spriteController); - virtual ~PointerController() = default; + ~PointerController() override; virtual bool getBounds(float* outMinX, float* outMinY, float* outMaxX, float* outMaxY) const; virtual void move(float deltaX, float deltaY); @@ -72,6 +72,8 @@ public: void reloadPointerResources(); void onDisplayViewportsUpdated(std::vector& viewports); + void onDisplayInfosChanged(const std::vector& displayInfos); + private: friend PointerControllerContext::LooperCallback; friend PointerControllerContext::MessageHandler; @@ -85,9 +87,23 @@ private: struct Locked { Presentation presentation; + std::vector mDisplayInfos; std::unordered_map spotControllers; } mLocked GUARDED_BY(mLock); + class DisplayInfoListener : public gui::WindowInfosListener { + public: + explicit DisplayInfoListener(PointerController& pc) : mPointerController(pc){}; + void onWindowInfosChanged(const std::vector&, + const std::vector&) override; + + private: + PointerController& mPointerController; + }; + sp mDisplayInfoListener; + + const ui::Transform& getTransformForDisplayLocked(int displayId) const REQUIRES(mLock); + PointerController(const sp& policy, const sp& looper, const sp& spriteController); void clearSpotsLocked(); -- cgit v1.2.3-59-g8ed1b From 5693cee2f91e72ac2cbbb958cbb7cd6b58a479f3 Mon Sep 17 00:00:00 2001 From: Prabir Pradhan Date: Fri, 31 Dec 2021 06:51:15 -0800 Subject: Allow its WindowInfosListener to outlive PointerController A strong pointer to PointerController::DisplayInfoListener is given away when the listener is registered, so PC cannot guarantee that the listener is destroyed when it is destroyed. This means the listener can outlive PC, so there is a race condition between PC's destruction and an update to the listener. While it could be argued that it is the caller's responsibility to ensure that the listener is not updated after it is unregistered, there is no way to guarantee that using strong pointers. Here, we can be defensive and protect against this case. Bug: 212672261 Test: atest libinputservice_test Change-Id: I358a725980cc8c7d6ad0483a9b2a8b8715a03424 --- libs/input/PointerController.cpp | 78 ++++++++++++++++++++--------- libs/input/PointerController.h | 40 +++++++++++---- libs/input/tests/PointerController_test.cpp | 32 ++++++++++++ 3 files changed, 118 insertions(+), 32 deletions(-) (limited to 'libs/input/PointerController.cpp') diff --git a/libs/input/PointerController.cpp b/libs/input/PointerController.cpp index f43586f8d9d0..1dc74e5f7740 100644 --- a/libs/input/PointerController.cpp +++ b/libs/input/PointerController.cpp @@ -18,11 +18,13 @@ //#define LOG_NDEBUG 0 #include "PointerController.h" -#include "PointerControllerContext.h" #include #include #include +#include + +#include "PointerControllerContext.h" namespace android { @@ -36,8 +38,18 @@ const ui::Transform kIdentityTransform; void PointerController::DisplayInfoListener::onWindowInfosChanged( const std::vector&, - const std::vector& displayInfo) { - mPointerController.onDisplayInfosChanged(displayInfo); + const std::vector& displayInfos) { + std::scoped_lock lock(mLock); + if (mPointerController == nullptr) return; + + // PointerController uses DisplayInfoListener's lock. + base::ScopedLockAssertion assumeLocked(mPointerController->getLock()); + mPointerController->onDisplayInfosChangedLocked(displayInfos); +} + +void PointerController::DisplayInfoListener::onPointerControllerDestroyed() { + std::scoped_lock lock(mLock); + mPointerController = nullptr; } // --- PointerController --- @@ -68,16 +80,36 @@ std::shared_ptr PointerController::create( PointerController::PointerController(const sp& policy, const sp& looper, const sp& spriteController) + : PointerController( + policy, looper, spriteController, + [](const sp& listener) { + SurfaceComposerClient::getDefault()->addWindowInfosListener(listener); + }, + [](const sp& listener) { + SurfaceComposerClient::getDefault()->removeWindowInfosListener(listener); + }) {} + +PointerController::PointerController(const sp& policy, + const sp& looper, + const sp& spriteController, + WindowListenerConsumer registerListener, + WindowListenerConsumer unregisterListener) : mContext(policy, looper, spriteController, *this), mCursorController(mContext), - mDisplayInfoListener(new DisplayInfoListener(*this)) { - std::scoped_lock lock(mLock); + mDisplayInfoListener(new DisplayInfoListener(this)), + mUnregisterWindowInfosListener(std::move(unregisterListener)) { + std::scoped_lock lock(getLock()); mLocked.presentation = Presentation::SPOT; - SurfaceComposerClient::getDefault()->addWindowInfosListener(mDisplayInfoListener); + registerListener(mDisplayInfoListener); } PointerController::~PointerController() { - SurfaceComposerClient::getDefault()->removeWindowInfosListener(mDisplayInfoListener); + mDisplayInfoListener->onPointerControllerDestroyed(); + mUnregisterWindowInfosListener(mDisplayInfoListener); +} + +std::mutex& PointerController::getLock() const { + return mDisplayInfoListener->mLock; } bool PointerController::getBounds(float* outMinX, float* outMinY, float* outMaxX, @@ -89,7 +121,7 @@ void PointerController::move(float deltaX, float deltaY) { const int32_t displayId = mCursorController.getDisplayId(); vec2 transformed; { - std::scoped_lock lock(mLock); + std::scoped_lock lock(getLock()); const auto& transform = getTransformForDisplayLocked(displayId); transformed = transformWithoutTranslation(transform, {deltaX, deltaY}); } @@ -108,7 +140,7 @@ void PointerController::setPosition(float x, float y) { const int32_t displayId = mCursorController.getDisplayId(); vec2 transformed; { - std::scoped_lock lock(mLock); + std::scoped_lock lock(getLock()); const auto& transform = getTransformForDisplayLocked(displayId); transformed = transform.transform(x, y); } @@ -119,7 +151,7 @@ void PointerController::getPosition(float* outX, float* outY) const { const int32_t displayId = mCursorController.getDisplayId(); mCursorController.getPosition(outX, outY); { - std::scoped_lock lock(mLock); + std::scoped_lock lock(getLock()); const auto& transform = getTransformForDisplayLocked(displayId); const auto xy = transform.inverse().transform(*outX, *outY); *outX = xy.x; @@ -132,17 +164,17 @@ int32_t PointerController::getDisplayId() const { } void PointerController::fade(Transition transition) { - std::scoped_lock lock(mLock); + std::scoped_lock lock(getLock()); mCursorController.fade(transition); } void PointerController::unfade(Transition transition) { - std::scoped_lock lock(mLock); + std::scoped_lock lock(getLock()); mCursorController.unfade(transition); } void PointerController::setPresentation(Presentation presentation) { - std::scoped_lock lock(mLock); + std::scoped_lock lock(getLock()); if (mLocked.presentation == presentation) { return; @@ -162,7 +194,7 @@ void PointerController::setPresentation(Presentation presentation) { void PointerController::setSpots(const PointerCoords* spotCoords, const uint32_t* spotIdToIndex, BitSet32 spotIdBits, int32_t displayId) { - std::scoped_lock lock(mLock); + std::scoped_lock lock(getLock()); std::array outSpotCoords{}; const ui::Transform& transform = getTransformForDisplayLocked(displayId); @@ -185,11 +217,11 @@ void PointerController::setSpots(const PointerCoords* spotCoords, const uint32_t } void PointerController::clearSpots() { - std::scoped_lock lock(mLock); + std::scoped_lock lock(getLock()); clearSpotsLocked(); } -void PointerController::clearSpotsLocked() REQUIRES(mLock) { +void PointerController::clearSpotsLocked() { for (auto& [displayID, spotController] : mLocked.spotControllers) { spotController.clearSpots(); } @@ -200,7 +232,7 @@ void PointerController::setInactivityTimeout(InactivityTimeout inactivityTimeout } void PointerController::reloadPointerResources() { - std::scoped_lock lock(mLock); + std::scoped_lock lock(getLock()); for (auto& [displayID, spotController] : mLocked.spotControllers) { spotController.reloadSpotResources(); @@ -216,7 +248,7 @@ void PointerController::reloadPointerResources() { } void PointerController::setDisplayViewport(const DisplayViewport& viewport) { - std::scoped_lock lock(mLock); + std::scoped_lock lock(getLock()); bool getAdditionalMouseResources = false; if (mLocked.presentation == PointerController::Presentation::POINTER) { @@ -226,12 +258,12 @@ void PointerController::setDisplayViewport(const DisplayViewport& viewport) { } void PointerController::updatePointerIcon(int32_t iconId) { - std::scoped_lock lock(mLock); + std::scoped_lock lock(getLock()); mCursorController.updatePointerIcon(iconId); } void PointerController::setCustomPointerIcon(const SpriteIcon& icon) { - std::scoped_lock lock(mLock); + std::scoped_lock lock(getLock()); mCursorController.setCustomPointerIcon(icon); } @@ -245,7 +277,7 @@ void PointerController::onDisplayViewportsUpdated(std::vector& displayIdSet.insert(viewport.displayId); } - std::scoped_lock lock(mLock); + std::scoped_lock lock(getLock()); for (auto it = mLocked.spotControllers.begin(); it != mLocked.spotControllers.end();) { int32_t displayID = it->first; if (!displayIdSet.count(displayID)) { @@ -261,8 +293,8 @@ void PointerController::onDisplayViewportsUpdated(std::vector& } } -void PointerController::onDisplayInfosChanged(const std::vector& displayInfo) { - std::scoped_lock lock(mLock); +void PointerController::onDisplayInfosChangedLocked( + const std::vector& displayInfo) { mLocked.mDisplayInfos = displayInfo; } diff --git a/libs/input/PointerController.h b/libs/input/PointerController.h index 796077f6c38c..2e6e851ee15a 100644 --- a/libs/input/PointerController.h +++ b/libs/input/PointerController.h @@ -72,13 +72,31 @@ public: void reloadPointerResources(); void onDisplayViewportsUpdated(std::vector& viewports); - void onDisplayInfosChanged(const std::vector& displayInfos); + void onDisplayInfosChangedLocked(const std::vector& displayInfos) + REQUIRES(getLock()); + +protected: + using WindowListenerConsumer = + std::function&)>; + + // Constructor used to test WindowInfosListener registration. + PointerController(const sp& policy, const sp& looper, + const sp& spriteController, + WindowListenerConsumer registerListener, + WindowListenerConsumer unregisterListener); private: + PointerController(const sp& policy, const sp& looper, + const sp& spriteController); + friend PointerControllerContext::LooperCallback; friend PointerControllerContext::MessageHandler; - mutable std::mutex mLock; + // PointerController's DisplayInfoListener can outlive the PointerController because when the + // listener is registered, a strong pointer to the listener (which can extend its lifecycle) + // is given away. To avoid the small overhead of using two separate locks in these two objects, + // we use the DisplayInfoListener's lock in PointerController. + std::mutex& getLock() const; PointerControllerContext mContext; @@ -89,24 +107,28 @@ private: std::vector mDisplayInfos; std::unordered_map spotControllers; - } mLocked GUARDED_BY(mLock); + } mLocked GUARDED_BY(getLock()); class DisplayInfoListener : public gui::WindowInfosListener { public: - explicit DisplayInfoListener(PointerController& pc) : mPointerController(pc){}; + explicit DisplayInfoListener(PointerController* pc) : mPointerController(pc){}; void onWindowInfosChanged(const std::vector&, const std::vector&) override; + void onPointerControllerDestroyed(); + + // This lock is also used by PointerController. See PointerController::getLock(). + std::mutex mLock; private: - PointerController& mPointerController; + PointerController* mPointerController GUARDED_BY(mLock); }; + sp mDisplayInfoListener; + const WindowListenerConsumer mUnregisterWindowInfosListener; - const ui::Transform& getTransformForDisplayLocked(int displayId) const REQUIRES(mLock); + const ui::Transform& getTransformForDisplayLocked(int displayId) const REQUIRES(getLock()); - PointerController(const sp& policy, const sp& looper, - const sp& spriteController); - void clearSpotsLocked(); + void clearSpotsLocked() REQUIRES(getLock()); }; } // namespace android diff --git a/libs/input/tests/PointerController_test.cpp b/libs/input/tests/PointerController_test.cpp index b67088a389b6..dae1fccec804 100644 --- a/libs/input/tests/PointerController_test.cpp +++ b/libs/input/tests/PointerController_test.cpp @@ -255,4 +255,36 @@ TEST_F(PointerControllerTest, doesNotGetResourcesBeforeSettingViewport) { ensureDisplayViewportIsSet(); } +class PointerControllerWindowInfoListenerTest : public Test {}; + +class TestPointerController : public PointerController { +public: + TestPointerController(sp& registeredListener, + const sp& looper) + : PointerController( + new MockPointerControllerPolicyInterface(), looper, + new NiceMock(looper), + [®isteredListener](const sp& listener) { + // Register listener + registeredListener = listener; + }, + [®isteredListener](const sp& listener) { + // Unregister listener + if (registeredListener == listener) registeredListener = nullptr; + }) {} +}; + +TEST_F(PointerControllerWindowInfoListenerTest, + doesNotCrashIfListenerCalledAfterPointerControllerDestroyed) { + sp registeredListener; + sp localListenerCopy; + { + TestPointerController pointerController(registeredListener, new Looper(false)); + ASSERT_NE(nullptr, registeredListener) << "WindowInfosListener was not registered"; + localListenerCopy = registeredListener; + } + EXPECT_EQ(nullptr, registeredListener) << "WindowInfosListener was not unregistered"; + localListenerCopy->onWindowInfosChanged({}, {}); +} + } // namespace android -- cgit v1.2.3-59-g8ed1b From d4fd3a112b4cb46da17dc996daf745131664cc65 Mon Sep 17 00:00:00 2001 From: Prabir Pradhan Date: Thu, 10 Mar 2022 14:39:46 +0000 Subject: Synchronize pointer display change requests Previously, when InputManagerService requests for PointerController to change the pointer display, there was no way to know when the request was completed or whether it succeeded. This could lead to a few issues: - WM's MousePositionTracker's coordinates would not be updated until the next mouse event was generated, meaning the position would be out of sync. - The creation of a virtual mouse device moves the pointer to a specific displayId. In order to test this behavior, we would need to sleep in the test code to wait for the system to update the pointer display and position, resulting in generally flaky tests. Here, we add a way to synchonize changes to the pointer display so that InputMangerService can know the current pointer display with certainty. PointerController, which is updated in the InputReader thread, is the source of truth of the pointer display. We add a policy call to notify IMS when the pointer display changes. When the pointer display is changed, the cursor position on the updated display is also updated so that the VirtualMouse#getCursorPosition() API is synchronized to the pointer display change. Bug: 216792538 Test: atest FrameworksServicesTests:InputManagerServiceTests Test: atest PointerIconTest Change-Id: I578fd1aba9335e2e078d749321e55a6d05299f3b Merged-In: I578fd1aba9335e2e078d749321e55a6d05299f3b --- .../hardware/input/InputManagerInternal.java | 9 +- libs/input/PointerController.cpp | 7 + libs/input/PointerController.h | 1 + libs/input/PointerControllerContext.h | 1 + libs/input/tests/PointerController_test.cpp | 40 +++- .../android/server/input/InputManagerService.java | 135 +++++++++++-- .../server/input/NativeInputManagerService.java | 6 + .../android/server/wm/InputManagerCallback.java | 16 ++ .../android/server/wm/WindowManagerService.java | 44 +++- ...om_android_server_input_InputManagerService.cpp | 69 +++---- .../companion/virtual/InputControllerTest.java | 3 +- .../virtual/VirtualDeviceManagerServiceTest.java | 2 +- .../server/input/InputManagerServiceTests.kt | 222 +++++++++++++++++++++ 13 files changed, 488 insertions(+), 67 deletions(-) create mode 100644 services/tests/servicestests/src/com/android/server/input/InputManagerServiceTests.kt (limited to 'libs/input/PointerController.cpp') diff --git a/core/java/android/hardware/input/InputManagerInternal.java b/core/java/android/hardware/input/InputManagerInternal.java index b37c27c2a9e7..fc6bc555e823 100644 --- a/core/java/android/hardware/input/InputManagerInternal.java +++ b/core/java/android/hardware/input/InputManagerInternal.java @@ -75,8 +75,15 @@ public abstract class InputManagerInternal { /** * Sets the display id that the MouseCursorController will be forced to target. Pass * {@link android.view.Display#INVALID_DISPLAY} to clear the override. + * + * Note: This method generally blocks until the pointer display override has propagated. + * When setting a new override, the caller should ensure that an input device that can control + * the mouse pointer is connected. If a new override is set when no such input device is + * connected, the caller may be blocked for an arbitrary period of time. + * + * @return true if the pointer displayId was set successfully, or false if it fails. */ - public abstract void setVirtualMousePointerDisplayId(int pointerDisplayId); + public abstract boolean setVirtualMousePointerDisplayId(int pointerDisplayId); /** * Gets the display id that the MouseCursorController is being forced to target. Returns diff --git a/libs/input/PointerController.cpp b/libs/input/PointerController.cpp index 1dc74e5f7740..10ea6512c724 100644 --- a/libs/input/PointerController.cpp +++ b/libs/input/PointerController.cpp @@ -106,6 +106,7 @@ PointerController::PointerController(const sp& PointerController::~PointerController() { mDisplayInfoListener->onPointerControllerDestroyed(); mUnregisterWindowInfosListener(mDisplayInfoListener); + mContext.getPolicy()->onPointerDisplayIdChanged(ADISPLAY_ID_NONE, 0, 0); } std::mutex& PointerController::getLock() const { @@ -255,6 +256,12 @@ void PointerController::setDisplayViewport(const DisplayViewport& viewport) { getAdditionalMouseResources = true; } mCursorController.setDisplayViewport(viewport, getAdditionalMouseResources); + if (viewport.displayId != mLocked.pointerDisplayId) { + float xPos, yPos; + mCursorController.getPosition(&xPos, &yPos); + mContext.getPolicy()->onPointerDisplayIdChanged(viewport.displayId, xPos, yPos); + mLocked.pointerDisplayId = viewport.displayId; + } } void PointerController::updatePointerIcon(int32_t iconId) { diff --git a/libs/input/PointerController.h b/libs/input/PointerController.h index 2e6e851ee15a..eab030f71e1a 100644 --- a/libs/input/PointerController.h +++ b/libs/input/PointerController.h @@ -104,6 +104,7 @@ private: struct Locked { Presentation presentation; + int32_t pointerDisplayId = ADISPLAY_ID_NONE; std::vector mDisplayInfos; std::unordered_map spotControllers; diff --git a/libs/input/PointerControllerContext.h b/libs/input/PointerControllerContext.h index 26a65a47471d..c2bc1e020279 100644 --- a/libs/input/PointerControllerContext.h +++ b/libs/input/PointerControllerContext.h @@ -79,6 +79,7 @@ public: std::map* outAnimationResources, int32_t displayId) = 0; virtual int32_t getDefaultPointerIconId() = 0; virtual int32_t getCustomPointerIconId() = 0; + virtual void onPointerDisplayIdChanged(int32_t displayId, float xPos, float yPos) = 0; }; /* diff --git a/libs/input/tests/PointerController_test.cpp b/libs/input/tests/PointerController_test.cpp index dae1fccec804..f9752ed155df 100644 --- a/libs/input/tests/PointerController_test.cpp +++ b/libs/input/tests/PointerController_test.cpp @@ -56,9 +56,11 @@ public: std::map* outAnimationResources, int32_t displayId) override; virtual int32_t getDefaultPointerIconId() override; virtual int32_t getCustomPointerIconId() override; + virtual void onPointerDisplayIdChanged(int32_t displayId, float xPos, float yPos) override; bool allResourcesAreLoaded(); bool noResourcesAreLoaded(); + std::optional getLastReportedPointerDisplayId() { return latestPointerDisplayId; } private: void loadPointerIconForType(SpriteIcon* icon, int32_t cursorType); @@ -66,6 +68,7 @@ private: bool pointerIconLoaded{false}; bool pointerResourcesLoaded{false}; bool additionalMouseResourcesLoaded{false}; + std::optional latestPointerDisplayId; }; void MockPointerControllerPolicyInterface::loadPointerIcon(SpriteIcon* icon, int32_t) { @@ -126,12 +129,19 @@ void MockPointerControllerPolicyInterface::loadPointerIconForType(SpriteIcon* ic icon->hotSpotX = hotSpot.first; icon->hotSpotY = hotSpot.second; } + +void MockPointerControllerPolicyInterface::onPointerDisplayIdChanged(int32_t displayId, + float /*xPos*/, + float /*yPos*/) { + latestPointerDisplayId = displayId; +} + class PointerControllerTest : public Test { protected: PointerControllerTest(); ~PointerControllerTest(); - void ensureDisplayViewportIsSet(); + void ensureDisplayViewportIsSet(int32_t displayId = ADISPLAY_ID_DEFAULT); sp mPointerSprite; sp mPolicy; @@ -168,9 +178,9 @@ PointerControllerTest::~PointerControllerTest() { mThread.join(); } -void PointerControllerTest::ensureDisplayViewportIsSet() { +void PointerControllerTest::ensureDisplayViewportIsSet(int32_t displayId) { DisplayViewport viewport; - viewport.displayId = ADISPLAY_ID_DEFAULT; + viewport.displayId = displayId; viewport.logicalRight = 1600; viewport.logicalBottom = 1200; viewport.physicalRight = 800; @@ -255,6 +265,30 @@ TEST_F(PointerControllerTest, doesNotGetResourcesBeforeSettingViewport) { ensureDisplayViewportIsSet(); } +TEST_F(PointerControllerTest, notifiesPolicyWhenPointerDisplayChanges) { + EXPECT_FALSE(mPolicy->getLastReportedPointerDisplayId()) + << "A pointer display change does not occur when PointerController is created."; + + ensureDisplayViewportIsSet(ADISPLAY_ID_DEFAULT); + + const auto lastReportedPointerDisplayId = mPolicy->getLastReportedPointerDisplayId(); + ASSERT_TRUE(lastReportedPointerDisplayId) + << "The policy is notified of a pointer display change when the viewport is first set."; + EXPECT_EQ(ADISPLAY_ID_DEFAULT, *lastReportedPointerDisplayId) + << "Incorrect pointer display notified."; + + ensureDisplayViewportIsSet(42); + + EXPECT_EQ(42, *mPolicy->getLastReportedPointerDisplayId()) + << "The policy is notified when the pointer display changes."; + + // Release the PointerController. + mPointerController = nullptr; + + EXPECT_EQ(ADISPLAY_ID_NONE, *mPolicy->getLastReportedPointerDisplayId()) + << "The pointer display changes to invalid when PointerController is destroyed."; +} + class PointerControllerWindowInfoListenerTest : public Test {}; class TestPointerController : public PointerController { diff --git a/services/core/java/com/android/server/input/InputManagerService.java b/services/core/java/com/android/server/input/InputManagerService.java index 8ab0b931be11..9b78068497c8 100644 --- a/services/core/java/com/android/server/input/InputManagerService.java +++ b/services/core/java/com/android/server/input/InputManagerService.java @@ -164,6 +164,7 @@ public class InputManagerService extends IInputManager.Stub private static final int MSG_UPDATE_KEYBOARD_LAYOUTS = 4; private static final int MSG_RELOAD_DEVICE_ALIASES = 5; private static final int MSG_DELIVER_TABLET_MODE_CHANGED = 6; + private static final int MSG_POINTER_DISPLAY_ID_CHANGED = 7; private static final int DEFAULT_VIBRATION_MAGNITUDE = 192; @@ -276,11 +277,24 @@ public class InputManagerService extends IInputManager.Stub @GuardedBy("mAssociationLock") private final Map mUniqueIdAssociations = new ArrayMap<>(); + // Guards per-display input properties and properties relating to the mouse pointer. + // Threads can wait on this lock to be notified the next time the display on which the mouse + // pointer is shown has changed. private final Object mAdditionalDisplayInputPropertiesLock = new Object(); - // Forces the MouseCursorController to target a specific display id. + // Forces the PointerController to target a specific display id. @GuardedBy("mAdditionalDisplayInputPropertiesLock") private int mOverriddenPointerDisplayId = Display.INVALID_DISPLAY; + + // PointerController is the source of truth of the pointer display. This is the value of the + // latest pointer display id reported by PointerController. + @GuardedBy("mAdditionalDisplayInputPropertiesLock") + private int mAcknowledgedPointerDisplayId = Display.INVALID_DISPLAY; + // This is the latest display id that IMS has requested PointerController to use. If there are + // no devices that can control the pointer, PointerController may end up disregarding this + // value. + @GuardedBy("mAdditionalDisplayInputPropertiesLock") + private int mRequestedPointerDisplayId = Display.INVALID_DISPLAY; @GuardedBy("mAdditionalDisplayInputPropertiesLock") private final SparseArray mAdditionalDisplayInputProperties = new SparseArray<>(); @@ -289,7 +303,6 @@ public class InputManagerService extends IInputManager.Stub @GuardedBy("mAdditionalDisplayInputPropertiesLock") private PointerIcon mIcon; - // Holds all the registered gesture monitors that are implemented as spy windows. The spy // windows are mapped by their InputChannel tokens. @GuardedBy("mInputMonitors") @@ -383,6 +396,10 @@ public class InputManagerService extends IInputManager.Stub NativeInputManagerService getNativeService(InputManagerService service) { return new NativeInputManagerService.NativeImpl(service, mContext, mLooper.getQueue()); } + + void registerLocalService(InputManagerInternal localService) { + LocalServices.addService(InputManagerInternal.class, localService); + } } public InputManagerService(Context context) { @@ -406,7 +423,7 @@ public class InputManagerService extends IInputManager.Stub mDoubleTouchGestureEnableFile = TextUtils.isEmpty(doubleTouchGestureEnablePath) ? null : new File(doubleTouchGestureEnablePath); - LocalServices.addService(InputManagerInternal.class, new LocalService()); + injector.registerLocalService(new LocalService()); } public void setWindowManagerCallbacks(WindowManagerCallbacks callbacks) { @@ -556,6 +573,8 @@ public class InputManagerService extends IInputManager.Stub vArray[i] = viewports.get(i); } mNative.setDisplayViewports(vArray); + // Always attempt to update the pointer display when viewports change. + updatePointerDisplayId(); if (mOverriddenPointerDisplayId != Display.INVALID_DISPLAY) { final AdditionalDisplayInputProperties properties = @@ -1961,10 +1980,43 @@ public class InputManagerService extends IInputManager.Stub return result; } - private void setVirtualMousePointerDisplayId(int displayId) { + /** + * Update the display on which the mouse pointer is shown. + * If there is an overridden display for the mouse pointer, use that. Otherwise, query + * WindowManager for the pointer display. + * + * @return true if the pointer displayId changed, false otherwise. + */ + private boolean updatePointerDisplayId() { + synchronized (mAdditionalDisplayInputPropertiesLock) { + final int pointerDisplayId = mOverriddenPointerDisplayId != Display.INVALID_DISPLAY + ? mOverriddenPointerDisplayId : mWindowManagerCallbacks.getPointerDisplayId(); + if (mRequestedPointerDisplayId == pointerDisplayId) { + return false; + } + mRequestedPointerDisplayId = pointerDisplayId; + mNative.setPointerDisplayId(pointerDisplayId); + return true; + } + } + + private void handlePointerDisplayIdChanged(PointerDisplayIdChangedArgs args) { + synchronized (mAdditionalDisplayInputPropertiesLock) { + mAcknowledgedPointerDisplayId = args.mPointerDisplayId; + // Notify waiting threads that the display of the mouse pointer has changed. + mAdditionalDisplayInputPropertiesLock.notifyAll(); + } + mWindowManagerCallbacks.notifyPointerDisplayIdChanged( + args.mPointerDisplayId, args.mXPosition, args.mYPosition); + } + + private boolean setVirtualMousePointerDisplayIdBlocking(int displayId) { + // Indicates whether this request is for removing the override. + final boolean removingOverride = displayId == Display.INVALID_DISPLAY; + synchronized (mAdditionalDisplayInputPropertiesLock) { mOverriddenPointerDisplayId = displayId; - if (displayId != Display.INVALID_DISPLAY) { + if (!removingOverride) { final AdditionalDisplayInputProperties properties = mAdditionalDisplayInputProperties.get(displayId); if (properties != null) { @@ -1972,9 +2024,30 @@ public class InputManagerService extends IInputManager.Stub updatePointerIconVisibleLocked(properties.pointerIconVisible); } } + if (!updatePointerDisplayId() && mAcknowledgedPointerDisplayId == displayId) { + // The requested pointer display is already set. + return true; + } + if (removingOverride && mAcknowledgedPointerDisplayId == Display.INVALID_DISPLAY) { + // The pointer display override is being removed, but the current pointer display + // is already invalid. This can happen when the PointerController is destroyed as a + // result of the removal of all input devices that can control the pointer. + return true; + } + try { + // The pointer display changed, so wait until the change has propagated. + mAdditionalDisplayInputPropertiesLock.wait(5_000 /*mills*/); + } catch (InterruptedException ignored) { + } + // This request succeeds in two cases: + // - This request was to remove the override, in which case the new pointer display + // could be anything that WM has set. + // - We are setting a new override, in which case the request only succeeds if the + // reported new displayId is the one we requested. This check ensures that if two + // competing overrides are requested in succession, the caller can be notified if one + // of them fails. + return removingOverride || mAcknowledgedPointerDisplayId == displayId; } - // TODO(b/215597605): trigger MousePositionTracker update - mNative.notifyPointerDisplayIdChanged(); } private int getVirtualMousePointerDisplayId() { @@ -3154,18 +3227,6 @@ public class InputManagerService extends IInputManager.Stub return mContext.createDisplayContext(display); } - // Native callback. - @SuppressWarnings("unused") - private int getPointerDisplayId() { - synchronized (mAdditionalDisplayInputPropertiesLock) { - // Prefer the override to all other displays. - if (mOverriddenPointerDisplayId != Display.INVALID_DISPLAY) { - return mOverriddenPointerDisplayId; - } - } - return mWindowManagerCallbacks.getPointerDisplayId(); - } - // Native callback. @SuppressWarnings("unused") private String[] getKeyboardLayoutOverlay(InputDeviceIdentifier identifier) { @@ -3206,6 +3267,26 @@ public class InputManagerService extends IInputManager.Stub return null; } + private static class PointerDisplayIdChangedArgs { + final int mPointerDisplayId; + final float mXPosition; + final float mYPosition; + PointerDisplayIdChangedArgs(int pointerDisplayId, float xPosition, float yPosition) { + mPointerDisplayId = pointerDisplayId; + mXPosition = xPosition; + mYPosition = yPosition; + } + } + + // Native callback. + @SuppressWarnings("unused") + @VisibleForTesting + void onPointerDisplayIdChanged(int pointerDisplayId, float xPosition, float yPosition) { + mHandler.obtainMessage(MSG_POINTER_DISPLAY_ID_CHANGED, + new PointerDisplayIdChangedArgs(pointerDisplayId, xPosition, + yPosition)).sendToTarget(); + } + /** * Callback interface implemented by the Window Manager. */ @@ -3329,6 +3410,14 @@ public class InputManagerService extends IInputManager.Stub */ @Nullable SurfaceControl createSurfaceForGestureMonitor(String name, int displayId); + + /** + * Notify WindowManagerService when the display of the mouse pointer changes. + * @param displayId The display on which the mouse pointer is shown. + * @param x The x coordinate of the mouse pointer. + * @param y The y coordinate of the mouse pointer. + */ + void notifyPointerDisplayIdChanged(int displayId, float x, float y); } /** @@ -3381,6 +3470,9 @@ public class InputManagerService extends IInputManager.Stub boolean inTabletMode = (boolean) args.arg1; deliverTabletModeChanged(whenNanos, inTabletMode); break; + case MSG_POINTER_DISPLAY_ID_CHANGED: + handlePointerDisplayIdChanged((PointerDisplayIdChangedArgs) msg.obj); + break; } } } @@ -3631,8 +3723,9 @@ public class InputManagerService extends IInputManager.Stub } @Override - public void setVirtualMousePointerDisplayId(int pointerDisplayId) { - InputManagerService.this.setVirtualMousePointerDisplayId(pointerDisplayId); + public boolean setVirtualMousePointerDisplayId(int pointerDisplayId) { + return InputManagerService.this + .setVirtualMousePointerDisplayIdBlocking(pointerDisplayId); } @Override diff --git a/services/core/java/com/android/server/input/NativeInputManagerService.java b/services/core/java/com/android/server/input/NativeInputManagerService.java index 2169155343cd..81882d277a99 100644 --- a/services/core/java/com/android/server/input/NativeInputManagerService.java +++ b/services/core/java/com/android/server/input/NativeInputManagerService.java @@ -176,6 +176,9 @@ public interface NativeInputManagerService { void cancelCurrentTouch(); + /** Set the displayId on which the mouse cursor should be shown. */ + void setPointerDisplayId(int displayId); + /** The native implementation of InputManagerService methods. */ class NativeImpl implements NativeInputManagerService { /** Pointer to native input manager service object, used by native code. */ @@ -388,5 +391,8 @@ public interface NativeInputManagerService { @Override public native void cancelCurrentTouch(); + + @Override + public native void setPointerDisplayId(int displayId); } } diff --git a/services/core/java/com/android/server/wm/InputManagerCallback.java b/services/core/java/com/android/server/wm/InputManagerCallback.java index 67dd89ee295c..33cdd2e98113 100644 --- a/services/core/java/com/android/server/wm/InputManagerCallback.java +++ b/services/core/java/com/android/server/wm/InputManagerCallback.java @@ -270,6 +270,22 @@ final class InputManagerCallback implements InputManagerService.WindowManagerCal } } + @Override + public void notifyPointerDisplayIdChanged(int displayId, float x, float y) { + synchronized (mService.mGlobalLock) { + mService.setMousePointerDisplayId(displayId); + if (displayId == Display.INVALID_DISPLAY) return; + + final DisplayContent dc = mService.mRoot.getDisplayContent(displayId); + if (dc == null) { + Slog.wtf(TAG, "The mouse pointer was moved to display " + displayId + + " that does not have a valid DisplayContent."); + return; + } + mService.restorePointerIconLocked(dc, x, y); + } + } + /** Waits until the built-in input devices have been configured. */ public boolean waitForInputDevicesReady(long timeoutMillis) { synchronized (mInputDevicesReadyMonitor) { diff --git a/services/core/java/com/android/server/wm/WindowManagerService.java b/services/core/java/com/android/server/wm/WindowManagerService.java index 7b77fd0683cd..46a45099bf36 100644 --- a/services/core/java/com/android/server/wm/WindowManagerService.java +++ b/services/core/java/com/android/server/wm/WindowManagerService.java @@ -7198,18 +7198,42 @@ public class WindowManagerService extends IWindowManager.Stub private float mLatestMouseX; private float mLatestMouseY; - void updatePosition(float x, float y) { + /** + * The display that the pointer (mouse cursor) is currently shown on. This is updated + * directly by InputManagerService when the pointer display changes. + */ + private int mPointerDisplayId = INVALID_DISPLAY; + + /** + * Update the mouse cursor position as a result of a mouse movement. + * @return true if the position was successfully updated, false otherwise. + */ + boolean updatePosition(int displayId, float x, float y) { synchronized (this) { mLatestEventWasMouse = true; + + if (displayId != mPointerDisplayId) { + // The display of the position update does not match the display on which the + // mouse pointer is shown, so do not update the position. + return false; + } mLatestMouseX = x; mLatestMouseY = y; + return true; + } + } + + void setPointerDisplayId(int displayId) { + synchronized (this) { + mPointerDisplayId = displayId; } } @Override public void onPointerEvent(MotionEvent motionEvent) { if (motionEvent.isFromSource(InputDevice.SOURCE_MOUSE)) { - updatePosition(motionEvent.getRawX(), motionEvent.getRawY()); + updatePosition(motionEvent.getDisplayId(), motionEvent.getRawX(), + motionEvent.getRawY()); } else { synchronized (this) { mLatestEventWasMouse = false; @@ -7219,6 +7243,7 @@ public class WindowManagerService extends IWindowManager.Stub }; void updatePointerIcon(IWindow client) { + int pointerDisplayId; float mouseX, mouseY; synchronized(mMousePositionTracker) { @@ -7227,6 +7252,7 @@ public class WindowManagerService extends IWindowManager.Stub } mouseX = mMousePositionTracker.mLatestMouseX; mouseY = mMousePositionTracker.mLatestMouseY; + pointerDisplayId = mMousePositionTracker.mPointerDisplayId; } synchronized (mGlobalLock) { @@ -7243,6 +7269,10 @@ public class WindowManagerService extends IWindowManager.Stub if (displayContent == null) { return; } + if (pointerDisplayId != displayContent.getDisplayId()) { + // Do not let the pointer icon be updated by a window on a different display. + return; + } WindowState windowUnderPointer = displayContent.getTouchableWinAtPointLocked(mouseX, mouseY); if (windowUnderPointer != callingWin) { @@ -7260,7 +7290,11 @@ public class WindowManagerService extends IWindowManager.Stub void restorePointerIconLocked(DisplayContent displayContent, float latestX, float latestY) { // Mouse position tracker has not been getting updates while dragging, update it now. - mMousePositionTracker.updatePosition(latestX, latestY); + if (!mMousePositionTracker.updatePosition( + displayContent.getDisplayId(), latestX, latestY)) { + // The mouse position could not be updated, so ignore this request. + return; + } WindowState windowUnderPointer = displayContent.getTouchableWinAtPointLocked(latestX, latestY); @@ -7284,6 +7318,10 @@ public class WindowManagerService extends IWindowManager.Stub } } + void setMousePointerDisplayId(int displayId) { + mMousePositionTracker.setPointerDisplayId(displayId); + } + /** * Update a tap exclude region in the window identified by the provided id. Touches down on this * region will not: diff --git a/services/core/jni/com_android_server_input_InputManagerService.cpp b/services/core/jni/com_android_server_input_InputManagerService.cpp index 3c5ebe7c62ee..32adac7f282b 100644 --- a/services/core/jni/com_android_server_input_InputManagerService.cpp +++ b/services/core/jni/com_android_server_input_InputManagerService.cpp @@ -107,6 +107,7 @@ static struct { jmethodID interceptKeyBeforeDispatching; jmethodID dispatchUnhandledKey; jmethodID checkInjectEventsPermission; + jmethodID onPointerDisplayIdChanged; jmethodID onPointerDownOutsideFocus; jmethodID getVirtualKeyQuietTimeMillis; jmethodID getExcludedDeviceNames; @@ -120,7 +121,6 @@ static struct { jmethodID getLongPressTimeout; jmethodID getPointerLayer; jmethodID getPointerIcon; - jmethodID getPointerDisplayId; jmethodID getKeyboardLayoutOverlay; jmethodID getDeviceAlias; jmethodID getTouchCalibrationForInputDevice; @@ -277,6 +277,7 @@ public: void setFocusedDisplay(int32_t displayId); void setInputDispatchMode(bool enabled, bool frozen); void setSystemUiLightsOut(bool lightsOut); + void setPointerDisplayId(int32_t displayId); void setPointerSpeed(int32_t speed); void setPointerAcceleration(float acceleration); void setInputDeviceEnabled(uint32_t deviceId, bool enabled); @@ -288,7 +289,6 @@ public: void requestPointerCapture(const sp& windowToken, bool enabled); void setCustomPointerIcon(const SpriteIcon& icon); void setMotionClassifierEnabled(bool enabled); - void notifyPointerDisplayIdChanged(); /* --- InputReaderPolicyInterface implementation --- */ @@ -346,6 +346,7 @@ public: std::map* outAnimationResources, int32_t displayId); virtual int32_t getDefaultPointerIconId(); virtual int32_t getCustomPointerIconId(); + virtual void onPointerDisplayIdChanged(int32_t displayId, float xPos, float yPos); private: sp mInputManager; @@ -394,7 +395,6 @@ private: void updateInactivityTimeoutLocked(); void handleInterceptActions(jint wmActions, nsecs_t when, uint32_t& policyFlags); void ensureSpriteControllerLocked(); - int32_t getPointerDisplayId(); sp getParentSurfaceForPointers(int displayId); static bool checkAndClearExceptionFromCallback(JNIEnv* env, const char* methodName); @@ -498,13 +498,9 @@ void NativeInputManager::setDisplayViewports(JNIEnv* env, jobjectArray viewportO } } - // Get the preferred pointer controller displayId. - int32_t pointerDisplayId = getPointerDisplayId(); - { // acquire lock AutoMutex _l(mLock); mLocked.viewports = viewports; - mLocked.pointerDisplayId = pointerDisplayId; std::shared_ptr controller = mLocked.pointerController.lock(); if (controller != nullptr) { controller->onDisplayViewportsUpdated(mLocked.viewports); @@ -666,15 +662,12 @@ std::shared_ptr NativeInputManager::obtainPointerCon return controller; } -int32_t NativeInputManager::getPointerDisplayId() { +void NativeInputManager::onPointerDisplayIdChanged(int32_t pointerDisplayId, float xPos, + float yPos) { JNIEnv* env = jniEnv(); - jint pointerDisplayId = env->CallIntMethod(mServiceObj, - gServiceClassInfo.getPointerDisplayId); - if (checkAndClearExceptionFromCallback(env, "getPointerDisplayId")) { - pointerDisplayId = ADISPLAY_ID_DEFAULT; - } - - return pointerDisplayId; + env->CallVoidMethod(mServiceObj, gServiceClassInfo.onPointerDisplayIdChanged, pointerDisplayId, + xPos, yPos); + checkAndClearExceptionFromCallback(env, "onPointerDisplayIdChanged"); } sp NativeInputManager::getParentSurfaceForPointers(int displayId) { @@ -1032,6 +1025,22 @@ void NativeInputManager::updateInactivityTimeoutLocked() REQUIRES(mLock) { : InactivityTimeout::NORMAL); } +void NativeInputManager::setPointerDisplayId(int32_t displayId) { + { // acquire lock + AutoMutex _l(mLock); + + if (mLocked.pointerDisplayId == displayId) { + return; + } + + ALOGI("Setting pointer display id to %d.", displayId); + mLocked.pointerDisplayId = displayId; + } // release lock + + mInputManager->getReader().requestRefreshConfiguration( + InputReaderConfiguration::CHANGE_DISPLAY_INFO); +} + void NativeInputManager::setPointerSpeed(int32_t speed) { { // acquire lock AutoMutex _l(mLock); @@ -1494,18 +1503,6 @@ void NativeInputManager::setMotionClassifierEnabled(bool enabled) { mInputManager->getClassifier().setMotionClassifierEnabled(enabled); } -void NativeInputManager::notifyPointerDisplayIdChanged() { - int32_t pointerDisplayId = getPointerDisplayId(); - - { // acquire lock - AutoMutex _l(mLock); - mLocked.pointerDisplayId = pointerDisplayId; - } // release lock - - mInputManager->getReader().requestRefreshConfiguration( - InputReaderConfiguration::CHANGE_DISPLAY_INFO); -} - // ---------------------------------------------------------------------------- static NativeInputManager* getNativeInputManager(JNIEnv* env, jobject clazz) { @@ -2199,11 +2196,6 @@ static void nativeNotifyPortAssociationsChanged(JNIEnv* env, jobject nativeImplO InputReaderConfiguration::CHANGE_DISPLAY_INFO); } -static void nativeNotifyPointerDisplayIdChanged(JNIEnv* env, jobject nativeImplObj) { - NativeInputManager* im = getNativeInputManager(env, nativeImplObj); - im->notifyPointerDisplayIdChanged(); -} - static void nativeSetDisplayEligibilityForPointerCapture(JNIEnv* env, jobject nativeImplObj, jint displayId, jboolean isEligible) { NativeInputManager* im = getNativeInputManager(env, nativeImplObj); @@ -2321,6 +2313,11 @@ static void nativeCancelCurrentTouch(JNIEnv* env, jobject nativeImplObj) { im->getInputManager()->getDispatcher().cancelCurrentTouch(); } +static void nativeSetPointerDisplayId(JNIEnv* env, jobject nativeImplObj, jint displayId) { + NativeInputManager* im = getNativeInputManager(env, nativeImplObj); + im->setPointerDisplayId(displayId); +} + // ---------------------------------------------------------------------------- static const JNINativeMethod gInputManagerMethods[] = { @@ -2393,7 +2390,6 @@ static const JNINativeMethod gInputManagerMethods[] = { {"canDispatchToDisplay", "(II)Z", (void*)nativeCanDispatchToDisplay}, {"notifyPortAssociationsChanged", "()V", (void*)nativeNotifyPortAssociationsChanged}, {"changeUniqueIdAssociation", "()V", (void*)nativeChangeUniqueIdAssociation}, - {"notifyPointerDisplayIdChanged", "()V", (void*)nativeNotifyPointerDisplayIdChanged}, {"setDisplayEligibilityForPointerCapture", "(IZ)V", (void*)nativeSetDisplayEligibilityForPointerCapture}, {"setMotionClassifierEnabled", "(Z)V", (void*)nativeSetMotionClassifierEnabled}, @@ -2403,6 +2399,7 @@ static const JNINativeMethod gInputManagerMethods[] = { {"disableSensor", "(II)V", (void*)nativeDisableSensor}, {"flushSensor", "(II)Z", (void*)nativeFlushSensor}, {"cancelCurrentTouch", "()V", (void*)nativeCancelCurrentTouch}, + {"setPointerDisplayId", "(I)V", (void*)nativeSetPointerDisplayId}, }; #define FIND_CLASS(var, className) \ @@ -2498,6 +2495,9 @@ int register_android_server_InputManager(JNIEnv* env) { GET_METHOD_ID(gServiceClassInfo.checkInjectEventsPermission, clazz, "checkInjectEventsPermission", "(II)Z"); + GET_METHOD_ID(gServiceClassInfo.onPointerDisplayIdChanged, clazz, "onPointerDisplayIdChanged", + "(IFF)V"); + GET_METHOD_ID(gServiceClassInfo.onPointerDownOutsideFocus, clazz, "onPointerDownOutsideFocus", "(Landroid/os/IBinder;)V"); @@ -2537,9 +2537,6 @@ int register_android_server_InputManager(JNIEnv* env) { GET_METHOD_ID(gServiceClassInfo.getPointerIcon, clazz, "getPointerIcon", "(I)Landroid/view/PointerIcon;"); - GET_METHOD_ID(gServiceClassInfo.getPointerDisplayId, clazz, - "getPointerDisplayId", "()I"); - GET_METHOD_ID(gServiceClassInfo.getKeyboardLayoutOverlay, clazz, "getKeyboardLayoutOverlay", "(Landroid/hardware/input/InputDeviceIdentifier;)[Ljava/lang/String;"); diff --git a/services/tests/servicestests/src/com/android/server/companion/virtual/InputControllerTest.java b/services/tests/servicestests/src/com/android/server/companion/virtual/InputControllerTest.java index 92e7a86876e9..77cbb3a6398c 100644 --- a/services/tests/servicestests/src/com/android/server/companion/virtual/InputControllerTest.java +++ b/services/tests/servicestests/src/com/android/server/companion/virtual/InputControllerTest.java @@ -19,7 +19,6 @@ package com.android.server.companion.virtual; import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; -import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -67,7 +66,7 @@ public class InputControllerTest { mInputManagerMockHelper = new InputManagerMockHelper( TestableLooper.get(this), mNativeWrapperMock, mIInputManagerMock); - doNothing().when(mInputManagerInternalMock).setVirtualMousePointerDisplayId(anyInt()); + doReturn(true).when(mInputManagerInternalMock).setVirtualMousePointerDisplayId(anyInt()); LocalServices.removeServiceForTest(InputManagerInternal.class); LocalServices.addService(InputManagerInternal.class, mInputManagerInternalMock); diff --git a/services/tests/servicestests/src/com/android/server/companion/virtual/VirtualDeviceManagerServiceTest.java b/services/tests/servicestests/src/com/android/server/companion/virtual/VirtualDeviceManagerServiceTest.java index 22152a1953b9..cbb9fd7c30dd 100644 --- a/services/tests/servicestests/src/com/android/server/companion/virtual/VirtualDeviceManagerServiceTest.java +++ b/services/tests/servicestests/src/com/android/server/companion/virtual/VirtualDeviceManagerServiceTest.java @@ -180,7 +180,7 @@ public class VirtualDeviceManagerServiceTest { LocalServices.removeServiceForTest(DisplayManagerInternal.class); LocalServices.addService(DisplayManagerInternal.class, mDisplayManagerInternalMock); - doNothing().when(mInputManagerInternalMock).setVirtualMousePointerDisplayId(anyInt()); + doReturn(true).when(mInputManagerInternalMock).setVirtualMousePointerDisplayId(anyInt()); doNothing().when(mInputManagerInternalMock).setPointerAcceleration(anyFloat(), anyInt()); doNothing().when(mInputManagerInternalMock).setPointerIconVisible(anyBoolean(), anyInt()); LocalServices.removeServiceForTest(InputManagerInternal.class); diff --git a/services/tests/servicestests/src/com/android/server/input/InputManagerServiceTests.kt b/services/tests/servicestests/src/com/android/server/input/InputManagerServiceTests.kt new file mode 100644 index 000000000000..cb97c9bf91a3 --- /dev/null +++ b/services/tests/servicestests/src/com/android/server/input/InputManagerServiceTests.kt @@ -0,0 +1,222 @@ +/* + * Copyright (C) 2022 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.server.input + +import android.content.Context +import android.content.ContextWrapper +import android.hardware.display.DisplayViewport +import android.hardware.input.InputManagerInternal +import android.os.test.TestLooper +import android.platform.test.annotations.Presubmit +import android.view.Display +import androidx.test.InstrumentationRegistry +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Rule +import org.junit.Test +import org.mockito.ArgumentMatchers.any +import org.mockito.ArgumentMatchers.anyInt +import org.mockito.Mock +import org.mockito.Mockito.`when` +import org.mockito.Mockito.doAnswer +import org.mockito.Mockito.never +import org.mockito.Mockito.spy +import org.mockito.Mockito.times +import org.mockito.Mockito.verify +import org.mockito.junit.MockitoJUnit +import java.util.concurrent.CountDownLatch +import java.util.concurrent.TimeUnit + +/** + * Tests for {@link InputManagerService}. + * + * Build/Install/Run: + * atest FrameworksServicesTests:InputManagerServiceTests + */ +@Presubmit +class InputManagerServiceTests { + + @get:Rule + val rule = MockitoJUnit.rule()!! + + @Mock + private lateinit var native: NativeInputManagerService + + @Mock + private lateinit var wmCallbacks: InputManagerService.WindowManagerCallbacks + + private lateinit var service: InputManagerService + private lateinit var localService: InputManagerInternal + private lateinit var context: Context + private lateinit var testLooper: TestLooper + + @Before + fun setup() { + context = spy(ContextWrapper(InstrumentationRegistry.getContext())) + testLooper = TestLooper() + service = + InputManagerService(object : InputManagerService.Injector(context, testLooper.looper) { + override fun getNativeService( + service: InputManagerService? + ): NativeInputManagerService { + return native + } + + override fun registerLocalService(service: InputManagerInternal?) { + localService = service!! + } + }) + assertTrue("Local service must be registered", this::localService.isInitialized) + service.setWindowManagerCallbacks(wmCallbacks) + } + + @Test + fun testPointerDisplayUpdatesWhenDisplayViewportsChanged() { + val displayId = 123 + `when`(wmCallbacks.pointerDisplayId).thenReturn(displayId) + val viewports = listOf() + localService.setDisplayViewports(viewports) + verify(native).setDisplayViewports(any(Array::class.java)) + verify(native).setPointerDisplayId(displayId) + + val x = 42f + val y = 314f + service.onPointerDisplayIdChanged(displayId, x, y) + testLooper.dispatchNext() + verify(wmCallbacks).notifyPointerDisplayIdChanged(displayId, x, y) + } + + @Test + fun testSetVirtualMousePointerDisplayId() { + // Set the virtual mouse pointer displayId, and ensure that the calling thread is blocked + // until the native callback happens. + var countDownLatch = CountDownLatch(1) + val overrideDisplayId = 123 + Thread { + assertTrue("Setting virtual pointer display should succeed", + localService.setVirtualMousePointerDisplayId(overrideDisplayId)) + countDownLatch.countDown() + }.start() + assertFalse("Setting virtual pointer display should block", + countDownLatch.await(100, TimeUnit.MILLISECONDS)) + + val x = 42f + val y = 314f + service.onPointerDisplayIdChanged(overrideDisplayId, x, y) + testLooper.dispatchNext() + verify(wmCallbacks).notifyPointerDisplayIdChanged(overrideDisplayId, x, y) + assertTrue("Native callback unblocks calling thread", + countDownLatch.await(100, TimeUnit.MILLISECONDS)) + verify(native).setPointerDisplayId(overrideDisplayId) + + // Ensure that setting the same override again succeeds immediately. + assertTrue("Setting the same virtual mouse pointer displayId again should succeed", + localService.setVirtualMousePointerDisplayId(overrideDisplayId)) + + // Ensure that we did not query WM for the pointerDisplayId when setting the override + verify(wmCallbacks, never()).pointerDisplayId + + // Unset the virtual mouse pointer displayId, and ensure that we query WM for the new + // pointer displayId and the calling thread is blocked until the native callback happens. + countDownLatch = CountDownLatch(1) + val pointerDisplayId = 42 + `when`(wmCallbacks.pointerDisplayId).thenReturn(pointerDisplayId) + Thread { + assertTrue("Unsetting virtual mouse pointer displayId should succeed", + localService.setVirtualMousePointerDisplayId(Display.INVALID_DISPLAY)) + countDownLatch.countDown() + }.start() + assertFalse("Unsetting virtual mouse pointer displayId should block", + countDownLatch.await(100, TimeUnit.MILLISECONDS)) + + service.onPointerDisplayIdChanged(pointerDisplayId, x, y) + testLooper.dispatchNext() + verify(wmCallbacks).notifyPointerDisplayIdChanged(pointerDisplayId, x, y) + assertTrue("Native callback unblocks calling thread", + countDownLatch.await(100, TimeUnit.MILLISECONDS)) + verify(native).setPointerDisplayId(pointerDisplayId) + } + + @Test + fun testSetVirtualMousePointerDisplayId_unsuccessfulUpdate() { + // Set the virtual mouse pointer displayId, and ensure that the calling thread is blocked + // until the native callback happens. + val countDownLatch = CountDownLatch(1) + val overrideDisplayId = 123 + Thread { + assertFalse("Setting virtual pointer display should be unsuccessful", + localService.setVirtualMousePointerDisplayId(overrideDisplayId)) + countDownLatch.countDown() + }.start() + assertFalse("Setting virtual pointer display should block", + countDownLatch.await(100, TimeUnit.MILLISECONDS)) + + val x = 42f + val y = 314f + // Assume the native callback updates the pointerDisplayId to the incorrect value. + service.onPointerDisplayIdChanged(Display.INVALID_DISPLAY, x, y) + testLooper.dispatchNext() + verify(wmCallbacks).notifyPointerDisplayIdChanged(Display.INVALID_DISPLAY, x, y) + assertTrue("Native callback unblocks calling thread", + countDownLatch.await(100, TimeUnit.MILLISECONDS)) + verify(native).setPointerDisplayId(overrideDisplayId) + } + + @Test + fun testSetVirtualMousePointerDisplayId_competingRequests() { + val firstRequestSyncLatch = CountDownLatch(1) + doAnswer { + firstRequestSyncLatch.countDown() + }.`when`(native).setPointerDisplayId(anyInt()) + + val firstRequestLatch = CountDownLatch(1) + val firstOverride = 123 + Thread { + assertFalse("Setting virtual pointer display from thread 1 should be unsuccessful", + localService.setVirtualMousePointerDisplayId(firstOverride)) + firstRequestLatch.countDown() + }.start() + assertFalse("Setting virtual pointer display should block", + firstRequestLatch.await(100, TimeUnit.MILLISECONDS)) + + assertTrue("Wait for first thread's request should succeed", + firstRequestSyncLatch.await(100, TimeUnit.MILLISECONDS)) + + val secondRequestLatch = CountDownLatch(1) + val secondOverride = 42 + Thread { + assertTrue("Setting virtual mouse pointer from thread 2 should be successful", + localService.setVirtualMousePointerDisplayId(secondOverride)) + secondRequestLatch.countDown() + }.start() + assertFalse("Setting virtual mouse pointer should block", + secondRequestLatch.await(100, TimeUnit.MILLISECONDS)) + + val x = 42f + val y = 314f + // Assume the native callback updates directly to the second request. + service.onPointerDisplayIdChanged(secondOverride, x, y) + testLooper.dispatchNext() + verify(wmCallbacks).notifyPointerDisplayIdChanged(secondOverride, x, y) + assertTrue("Native callback unblocks first thread", + firstRequestLatch.await(100, TimeUnit.MILLISECONDS)) + assertTrue("Native callback unblocks second thread", + secondRequestLatch.await(100, TimeUnit.MILLISECONDS)) + verify(native, times(2)).setPointerDisplayId(anyInt()) + } +} \ No newline at end of file -- cgit v1.2.3-59-g8ed1b