Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit be845950 authored by Prabir Pradhan's avatar Prabir Pradhan
Browse files

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
parent e0d9b2a9
Loading
Loading
Loading
Loading
+24 −9
Original line number Diff line number Diff line
@@ -251,19 +251,24 @@ void PointerController::unfade(Transition transition) {
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;

        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();
    }
}
@@ -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);

+48 −6
Original line number Diff line number Diff line
@@ -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() {
    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 @@ 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