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

libs/input/PointerController.cpp

@@ -251,19 +251,24 @@ void PointerController::unfade(Transition transition) {
 void PointerController::setPresentation(Presentation presentation) {
     AutoMutex _l(mLock);
 
-    if (presentation == PRESENTATION_POINTER && mLocked.additionalMouseResources.empty()) {
+    if (mLocked.presentation == presentation) {
-        mPolicy->loadAdditionalMouseResources(&mLocked.additionalMouseResources,
+        return;
-                &mLocked.animationResources, mLocked.viewport.displayId);
     }
 
-    if (mLocked.presentation != presentation) {
+    mLocked.presentation = presentation;
-        mLocked.presentation = presentation;
+    mLocked.presentationChanged = true;
-        mLocked.presentationChanged = true;
 
-        if (presentation != PRESENTATION_SPOT) {
+    if (!mLocked.viewport.isValid()) {
-            fadeOutAndReleaseAllSpotsLocked();
+        return;
+    }
+
+    if (presentation == PRESENTATION_POINTER) {
+        if (mLocked.additionalMouseResources.empty()) {
+            mPolicy->loadAdditionalMouseResources(&mLocked.additionalMouseResources,
+                                                  &mLocked.animationResources,
+                                                  mLocked.viewport.displayId);
         }
-
+        fadeOutAndReleaseAllSpotsLocked();
         updatePointerLocked();
     }
 }
@@ -285,6 +290,9 @@ void PointerController::setSpots(const PointerCoords* spotCoords,
 #endif
 
     AutoMutex _l(mLock);
+    if (!mLocked.viewport.isValid()) {
+        return;
+    }
 
     std::vector<Spot*> newSpots;
     std::map<int32_t, std::vector<Spot*>>::const_iterator iter =
@@ -331,6 +339,9 @@ void PointerController::clearSpots() {
 #endif
 
     AutoMutex _l(mLock);
+    if (!mLocked.viewport.isValid()) {
+        return;
+    }
 
     fadeOutAndReleaseAllSpotsLocked();
 }
@@ -752,6 +763,10 @@ void PointerController::fadeOutAndReleaseAllSpotsLocked() {
 }
 
 void PointerController::loadResourcesLocked() REQUIRES(mLock) {
+    if (!mLocked.viewport.isValid()) {
+        return;
+    }
+
     mPolicy->loadPointerResources(&mResources, mLocked.viewport.displayId);
     mPolicy->loadPointerIcon(&mLocked.pointerIcon, mLocked.viewport.displayId);
 

libs/input/tests/PointerController_test.cpp

@@ -39,8 +39,8 @@ enum TestCursorType {
 
 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 @@ public:
     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 @@ void MockPointerControllerPolicyInterface::loadPointerResources(PointerResources
     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 @@ void MockPointerControllerPolicyInterface::loadAdditionalMouseResources(
     anim.durationPerFrame = 10;
     (*outResources)[cursorType] = icon;
     (*outAnimationResources)[cursorType] = anim;
+
+    additionalMouseResourcesLoaded = true;
 }
 
 int32_t MockPointerControllerPolicyInterface::getDefaultPointerIconId() {
@@ -101,18 +112,27 @@ int32_t MockPointerControllerPolicyInterface::getCustomPointerIconId() {
     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 @@ PointerControllerTest::PointerControllerTest() : mPointerSprite(new NiceMock<Moc
             .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 @@ PointerControllerTest::PointerControllerTest() : mPointerSprite(new NiceMock<Moc
     viewport.deviceWidth = 400;
     viewport.deviceHeight = 300;
     mPointerController->setDisplayViewport(viewport);
-}
 
-PointerControllerTest::~PointerControllerTest() {
+    // The first call to setDisplayViewport should trigger the loading of the necessary resources.
-    mRunning.store(false, std::memory_order_relaxed);
+    EXPECT_TRUE(mPolicy->allResourcesAreLoaded());
-    mThread.join();
 }
 
 void PointerControllerTest::loopThread() {
@@ -167,6 +192,7 @@ void PointerControllerTest::loopThread() {
 }
 
 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, useDefaultCursorTypeByDefault) {
 }
 
 TEST_F(PointerControllerTest, updatePointerIcon) {
+    ensureDisplayViewportIsSet();
     mPointerController->unfade(PointerController::TRANSITION_IMMEDIATE);
 
     int32_t type = CURSOR_TYPE_ADDITIONAL;
@@ -196,6 +223,7 @@ TEST_F(PointerControllerTest, updatePointerIcon) {
 }
 
 TEST_F(PointerControllerTest, setCustomPointerIcon) {
+    ensureDisplayViewportIsSet();
     mPointerController->unfade(PointerController::TRANSITION_IMMEDIATE);
 
     int32_t style = CURSOR_TYPE_CUSTOM;
@@ -217,4 +245,18 @@ TEST_F(PointerControllerTest, setCustomPointerIcon) {
     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