PointerController: Add guards to ensure display is valid
This change makes it so that PointerController does not ask its Policy
to load any resources for any displays until a DisplayViewport is set,
and verifies this with unit tests.
Bug: 145699789
Bug: 146385350
Test: atest libinputservice_test
Change-Id: I2e48e7ac4700e6f9fdf939a7bd0e6639b051ade6
Merged-In: I2e48e7ac4700e6f9fdf939a7bd0e6639b051ade6
diff --git a/libs/input/PointerController.cpp b/libs/input/PointerController.cpp
index abf0837..5c2ef15 100644
--- a/libs/input/PointerController.cpp
+++ b/libs/input/PointerController.cpp
@@ -257,19 +257,24 @@
void PointerController::setPresentation(Presentation presentation) {
AutoMutex _l(mLock);
- if (presentation == PRESENTATION_POINTER && mLocked.additionalMouseResources.empty()) {
- mPolicy->loadAdditionalMouseResources(&mLocked.additionalMouseResources,
- &mLocked.animationResources, mLocked.viewport.displayId);
+ if (mLocked.presentation == presentation) {
+ return;
}
- if (mLocked.presentation != presentation) {
- mLocked.presentation = presentation;
- mLocked.presentationChanged = true;
+ mLocked.presentation = presentation;
+ mLocked.presentationChanged = true;
- if (presentation != PRESENTATION_SPOT) {
- fadeOutAndReleaseAllSpotsLocked();
+ if (!mLocked.viewport.isValid()) {
+ return;
+ }
+
+ if (presentation == PRESENTATION_POINTER) {
+ if (mLocked.additionalMouseResources.empty()) {
+ mPolicy->loadAdditionalMouseResources(&mLocked.additionalMouseResources,
+ &mLocked.animationResources,
+ mLocked.viewport.displayId);
}
-
+ fadeOutAndReleaseAllSpotsLocked();
updatePointerLocked();
}
}
@@ -291,6 +296,9 @@
#endif
AutoMutex _l(mLock);
+ if (!mLocked.viewport.isValid()) {
+ return;
+ }
std::vector<Spot*> newSpots;
std::map<int32_t, std::vector<Spot*>>::const_iterator iter =
@@ -337,6 +345,9 @@
#endif
AutoMutex _l(mLock);
+ if (!mLocked.viewport.isValid()) {
+ return;
+ }
fadeOutAndReleaseAllSpotsLocked();
}
@@ -758,6 +769,10 @@
}
void PointerController::loadResourcesLocked() REQUIRES(mLock) {
+ if (!mLocked.viewport.isValid()) {
+ return;
+ }
+
mPolicy->loadPointerResources(&mResources, mLocked.viewport.displayId);
mPolicy->loadPointerIcon(&mLocked.pointerIcon, mLocked.viewport.displayId);
diff --git a/libs/input/tests/PointerController_test.cpp b/libs/input/tests/PointerController_test.cpp
index b36406d..a157426 100644
--- a/libs/input/tests/PointerController_test.cpp
+++ b/libs/input/tests/PointerController_test.cpp
@@ -39,8 +39,8 @@
using ::testing::AllOf;
using ::testing::Field;
-using ::testing::NiceMock;
using ::testing::Mock;
+using ::testing::NiceMock;
using ::testing::Return;
using ::testing::Test;
@@ -57,12 +57,20 @@
virtual int32_t getDefaultPointerIconId() override;
virtual int32_t getCustomPointerIconId() override;
+ bool allResourcesAreLoaded();
+ bool noResourcesAreLoaded();
+
private:
void loadPointerIconForType(SpriteIcon* icon, int32_t cursorType);
+
+ bool pointerIconLoaded{false};
+ bool pointerResourcesLoaded{false};
+ bool additionalMouseResourcesLoaded{false};
};
void MockPointerControllerPolicyInterface::loadPointerIcon(SpriteIcon* icon, int32_t) {
loadPointerIconForType(icon, CURSOR_TYPE_DEFAULT);
+ pointerIconLoaded = true;
}
void MockPointerControllerPolicyInterface::loadPointerResources(PointerResources* outResources,
@@ -70,6 +78,7 @@
loadPointerIconForType(&outResources->spotHover, CURSOR_TYPE_HOVER);
loadPointerIconForType(&outResources->spotTouch, CURSOR_TYPE_TOUCH);
loadPointerIconForType(&outResources->spotAnchor, CURSOR_TYPE_ANCHOR);
+ pointerResourcesLoaded = true;
}
void MockPointerControllerPolicyInterface::loadAdditionalMouseResources(
@@ -91,6 +100,8 @@
anim.durationPerFrame = 10;
(*outResources)[cursorType] = icon;
(*outAnimationResources)[cursorType] = anim;
+
+ additionalMouseResourcesLoaded = true;
}
int32_t MockPointerControllerPolicyInterface::getDefaultPointerIconId() {
@@ -101,18 +112,27 @@
return CURSOR_TYPE_CUSTOM;
}
+bool MockPointerControllerPolicyInterface::allResourcesAreLoaded() {
+ return pointerIconLoaded && pointerResourcesLoaded && additionalMouseResourcesLoaded;
+}
+
+bool MockPointerControllerPolicyInterface::noResourcesAreLoaded() {
+ return !(pointerIconLoaded || pointerResourcesLoaded || additionalMouseResourcesLoaded);
+}
+
void MockPointerControllerPolicyInterface::loadPointerIconForType(SpriteIcon* icon, int32_t type) {
icon->style = type;
std::pair<float, float> hotSpot = getHotSpotCoordinatesForType(type);
icon->hotSpotX = hotSpot.first;
icon->hotSpotY = hotSpot.second;
}
-
class PointerControllerTest : public Test {
protected:
PointerControllerTest();
~PointerControllerTest();
+ void ensureDisplayViewportIsSet();
+
sp<MockSprite> mPointerSprite;
sp<MockPointerControllerPolicyInterface> mPolicy;
sp<MockSpriteController> mSpriteController;
@@ -141,7 +161,14 @@
.WillOnce(Return(mPointerSprite));
mPointerController = new PointerController(mPolicy, mLooper, mSpriteController);
+}
+PointerControllerTest::~PointerControllerTest() {
+ mRunning.store(false, std::memory_order_relaxed);
+ mThread.join();
+}
+
+void PointerControllerTest::ensureDisplayViewportIsSet() {
DisplayViewport viewport;
viewport.displayId = ADISPLAY_ID_DEFAULT;
viewport.logicalRight = 1600;
@@ -151,11 +178,9 @@
viewport.deviceWidth = 400;
viewport.deviceHeight = 300;
mPointerController->setDisplayViewport(viewport);
-}
-PointerControllerTest::~PointerControllerTest() {
- mRunning.store(false, std::memory_order_relaxed);
- mThread.join();
+ // The first call to setDisplayViewport should trigger the loading of the necessary resources.
+ EXPECT_TRUE(mPolicy->allResourcesAreLoaded());
}
void PointerControllerTest::loopThread() {
@@ -167,6 +192,7 @@
}
TEST_F(PointerControllerTest, useDefaultCursorTypeByDefault) {
+ ensureDisplayViewportIsSet();
mPointerController->unfade(PointerController::TRANSITION_IMMEDIATE);
std::pair<float, float> hotspot = getHotSpotCoordinatesForType(CURSOR_TYPE_DEFAULT);
@@ -181,6 +207,7 @@
}
TEST_F(PointerControllerTest, updatePointerIcon) {
+ ensureDisplayViewportIsSet();
mPointerController->unfade(PointerController::TRANSITION_IMMEDIATE);
int32_t type = CURSOR_TYPE_ADDITIONAL;
@@ -196,6 +223,7 @@
}
TEST_F(PointerControllerTest, setCustomPointerIcon) {
+ ensureDisplayViewportIsSet();
mPointerController->unfade(PointerController::TRANSITION_IMMEDIATE);
int32_t style = CURSOR_TYPE_CUSTOM;
@@ -217,4 +245,18 @@
mPointerController->setCustomPointerIcon(icon);
}
+TEST_F(PointerControllerTest, doesNotGetResourcesBeforeSettingViewport) {
+ mPointerController->setPresentation(PointerController::PRESENTATION_POINTER);
+ mPointerController->setSpots(nullptr, nullptr, BitSet32(), -1);
+ mPointerController->clearSpots();
+ mPointerController->setPosition(1.0f, 1.0f);
+ mPointerController->move(1.0f, 1.0f);
+ mPointerController->unfade(PointerController::TRANSITION_IMMEDIATE);
+ mPointerController->fade(PointerController::TRANSITION_IMMEDIATE);
+
+ EXPECT_TRUE(mPolicy->noResourcesAreLoaded());
+
+ ensureDisplayViewportIsSet();
+}
+
} // namespace android