Skip to content
Snippets Groups Projects
Commit 09655e94 authored by Prabir Pradhan's avatar Prabir Pradhan Committed by Android (Google) Code Review
Browse files

Merge changes I0bb8ac5d,I358a7259 

* changes:
  libinputservice: Enable thread safety annotations
  Allow its WindowInfosListener to outlive PointerController
parents a26e957b 32189e82
No related branches found
No related tags found
No related merge requests found
Hide whitespace changes
Inline Side-by-side
libs/input/Android.bp
+ 1
− 0
View file @ 09655e94
......@@ -55,6 +55,7 @@ cc_library_shared {
"-Wall",
"-Wextra",
"-Werror",
"-Wthread-safety",
],
}
libs/input/PointerController.cpp
+ 55
− 23
View file @ 09655e94
......@@ -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;
}
......
libs/input/PointerController.h
+ 31
− 9
View file @ 09655e94
......@@ -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
......
libs/input/tests/PointerController_test.cpp
+ 32
− 0
View file @ 09655e94
......@@ -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
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment