summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--libs/input/PointerController.cpp78
-rw-r--r--libs/input/PointerController.h40
-rw-r--r--libs/input/tests/PointerController_test.cpp32
3 files changed, 118 insertions, 32 deletions
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 <SkBlendMode.h>
#include <SkCanvas.h>
#include <SkColor.h>
+#include <android-base/thread_annotations.h>
+
+#include "PointerControllerContext.h"
namespace android {
@@ -36,8 +38,18 @@ const ui::Transform kIdentityTransform;
void PointerController::DisplayInfoListener::onWindowInfosChanged(
const std::vector<android::gui::WindowInfo>&,
- const std::vector<android::gui::DisplayInfo>& displayInfo) {
- mPointerController.onDisplayInfosChanged(displayInfo);
+ const std::vector<android::gui::DisplayInfo>& 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> PointerController::create(
PointerController::PointerController(const sp<PointerControllerPolicyInterface>& policy,
const sp<Looper>& looper,
const sp<SpriteController>& spriteController)
+ : PointerController(
+ policy, looper, spriteController,
+ [](const sp<android::gui::WindowInfosListener>& listener) {
+ SurfaceComposerClient::getDefault()->addWindowInfosListener(listener);
+ },
+ [](const sp<android::gui::WindowInfosListener>& listener) {
+ SurfaceComposerClient::getDefault()->removeWindowInfosListener(listener);
+ }) {}
+
+PointerController::PointerController(const sp<PointerControllerPolicyInterface>& policy,
+ const sp<Looper>& looper,
+ const sp<SpriteController>& 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<PointerCoords, MAX_POINTERS> 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<DisplayViewport>&
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<DisplayViewport>&
}
}
-void PointerController::onDisplayInfosChanged(const std::vector<gui::DisplayInfo>& displayInfo) {
- std::scoped_lock lock(mLock);
+void PointerController::onDisplayInfosChangedLocked(
+ const std::vector<gui::DisplayInfo>& 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<DisplayViewport>& viewports);
- void onDisplayInfosChanged(const std::vector<gui::DisplayInfo>& displayInfos);
+ void onDisplayInfosChangedLocked(const std::vector<gui::DisplayInfo>& displayInfos)
+ REQUIRES(getLock());
+
+protected:
+ using WindowListenerConsumer =
+ std::function<void(const sp<android::gui::WindowInfosListener>&)>;
+
+ // Constructor used to test WindowInfosListener registration.
+ PointerController(const sp<PointerControllerPolicyInterface>& policy, const sp<Looper>& looper,
+ const sp<SpriteController>& spriteController,
+ WindowListenerConsumer registerListener,
+ WindowListenerConsumer unregisterListener);
private:
+ PointerController(const sp<PointerControllerPolicyInterface>& policy, const sp<Looper>& looper,
+ const sp<SpriteController>& 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<gui::DisplayInfo> mDisplayInfos;
std::unordered_map<int32_t /* displayId */, TouchSpotController> 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<android::gui::WindowInfo>&,
const std::vector<android::gui::DisplayInfo>&) 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<DisplayInfoListener> 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<PointerControllerPolicyInterface>& policy, const sp<Looper>& looper,
- const sp<SpriteController>& 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<android::gui::WindowInfosListener>& registeredListener,
+ const sp<Looper>& looper)
+ : PointerController(
+ new MockPointerControllerPolicyInterface(), looper,
+ new NiceMock<MockSpriteController>(looper),
+ [&registeredListener](const sp<android::gui::WindowInfosListener>& listener) {
+ // Register listener
+ registeredListener = listener;
+ },
+ [&registeredListener](const sp<android::gui::WindowInfosListener>& listener) {
+ // Unregister listener
+ if (registeredListener == listener) registeredListener = nullptr;
+ }) {}
+};
+
+TEST_F(PointerControllerWindowInfoListenerTest,
+ doesNotCrashIfListenerCalledAfterPointerControllerDestroyed) {
+ sp<android::gui::WindowInfosListener> registeredListener;
+ sp<android::gui::WindowInfosListener> 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
generated by cgit v1.2.3-59-g8ed1b (git 2.39.0) at 2025-10-20 03:14:14 +0000