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

Commit 96a5aaf5 authored by Vishnu Nair's avatar Vishnu Nair Committed by Android Build Coastguard Worker
Browse files

Fix region sampling for secure layers 

We were swapping a couple of boolean params when requesting
screenshots for region sampling, breaking region sampling for
secure layers in the process. Fix this by passing flags instead of
booleans in a long list of arguments and add a test to verify the
code path.

FLAG: EXEMPT bugfix
Fixes: 348944802
Test: presubmit
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:871886eebe469fc21568ff363993bde9a6593837)
Merged-In: I58f12e5cf81cb59115c1b9c500ac1e18a8ec72e5
Change-Id: I58f12e5cf81cb59115c1b9c500ac1e18a8ec72e5
parent fbdbde9a

libs/gui/tests/RegionSampling_test.cpp

+44 −14
Original line number Original line Diff line number Diff line
@@ -239,8 +239,9 @@ protected: 
    float const luma_green = 0.7152;

    float const luma_green = 0.7152;

    uint32_t const rgba_blue = 0xFFFF0000;

    uint32_t const rgba_blue = 0xFFFF0000;

    float const luma_blue = 0.0722;

    float const luma_blue = 0.0722;

    float const error_margin = 0.01;

    float const error_margin = 0.1;

    float const luma_gray = 0.50;

    float const luma_gray = 0.50;

    static constexpr std::chrono::milliseconds EVENT_WAIT_TIME_MS = 5000ms;

};

};





TEST_F(RegionSamplingTest, invalidLayerHandle_doesNotCrash) {

TEST_F(RegionSamplingTest, invalidLayerHandle_doesNotCrash) {
@@ -261,7 +262,7 @@ TEST_F(RegionSamplingTest, invalidLayerHandle_doesNotCrash) { 
    composer->removeRegionSamplingListener(listener);

    composer->removeRegionSamplingListener(listener);

}

}





TEST_F(RegionSamplingTest, DISABLED_CollectsLuma) {

TEST_F(RegionSamplingTest, CollectsLuma) {

    fill_render(rgba_green);

    fill_render(rgba_green);





    sp<gui::ISurfaceComposer> composer = ComposerServiceAIDL::getComposerService();

    sp<gui::ISurfaceComposer> composer = ComposerServiceAIDL::getComposerService();
@@ -273,7 +274,30 @@ TEST_F(RegionSamplingTest, DISABLED_CollectsLuma) { 
    sampleArea.bottom = 200;

    sampleArea.bottom = 200;

    composer->addRegionSamplingListener(sampleArea, mTopLayer->getHandle(), listener);

    composer->addRegionSamplingListener(sampleArea, mTopLayer->getHandle(), listener);





    EXPECT_TRUE(listener->wait_event(300ms)) << "timed out waiting for luma event to be received";

    EXPECT_TRUE(listener->wait_event(EVENT_WAIT_TIME_MS))

            << "timed out waiting for luma event to be received";

    EXPECT_NEAR(listener->luma(), luma_green, error_margin);



    composer->removeRegionSamplingListener(listener);

}



TEST_F(RegionSamplingTest, CollectsLumaForSecureLayer) {

    fill_render(rgba_green);

    SurfaceComposerClient::Transaction()

            .setFlags(mContentLayer, layer_state_t::eLayerSecure, layer_state_t::eLayerSecure)

            .apply(/*synchronous=*/true);



    sp<gui::ISurfaceComposer> composer = ComposerServiceAIDL::getComposerService();

    sp<Listener> listener = new Listener();

    gui::ARect sampleArea;

    sampleArea.left = 100;

    sampleArea.top = 100;

    sampleArea.right = 200;

    sampleArea.bottom = 200;

    composer->addRegionSamplingListener(sampleArea, mTopLayer->getHandle(), listener);



    EXPECT_TRUE(listener->wait_event(EVENT_WAIT_TIME_MS))

            << "timed out waiting for luma event to be received";

    EXPECT_NEAR(listener->luma(), luma_green, error_margin);

    EXPECT_NEAR(listener->luma(), luma_green, error_margin);





    composer->removeRegionSamplingListener(listener);

    composer->removeRegionSamplingListener(listener);
@@ -291,13 +315,14 @@ TEST_F(RegionSamplingTest, DISABLED_CollectsChangingLuma) { 
    sampleArea.bottom = 200;

    sampleArea.bottom = 200;

    composer->addRegionSamplingListener(sampleArea, mTopLayer->getHandle(), listener);

    composer->addRegionSamplingListener(sampleArea, mTopLayer->getHandle(), listener);





    EXPECT_TRUE(listener->wait_event(300ms)) << "timed out waiting for luma event to be received";

    EXPECT_TRUE(listener->wait_event(EVENT_WAIT_TIME_MS))

            << "timed out waiting for luma event to be received";

    EXPECT_NEAR(listener->luma(), luma_green, error_margin);

    EXPECT_NEAR(listener->luma(), luma_green, error_margin);





    listener->reset();

    listener->reset();





    fill_render(rgba_blue);

    fill_render(rgba_blue);

    EXPECT_TRUE(listener->wait_event(300ms))

    EXPECT_TRUE(listener->wait_event(EVENT_WAIT_TIME_MS))

            << "timed out waiting for 2nd luma event to be received";

            << "timed out waiting for 2nd luma event to be received";

    EXPECT_NEAR(listener->luma(), luma_blue, error_margin);

    EXPECT_NEAR(listener->luma(), luma_blue, error_margin);
@@ -323,10 +348,10 @@ TEST_F(RegionSamplingTest, DISABLED_CollectsLumaFromTwoRegions) { 
    graySampleArea.bottom = 200;

    graySampleArea.bottom = 200;

    composer->addRegionSamplingListener(graySampleArea, mTopLayer->getHandle(), grayListener);

    composer->addRegionSamplingListener(graySampleArea, mTopLayer->getHandle(), grayListener);





    EXPECT_TRUE(grayListener->wait_event(300ms))

    EXPECT_TRUE(grayListener->wait_event(EVENT_WAIT_TIME_MS))

            << "timed out waiting for luma event to be received";

            << "timed out waiting for luma event to be received";

    EXPECT_NEAR(grayListener->luma(), luma_gray, error_margin);

    EXPECT_NEAR(grayListener->luma(), luma_gray, error_margin);

    EXPECT_TRUE(greenListener->wait_event(300ms))

    EXPECT_TRUE(greenListener->wait_event(EVENT_WAIT_TIME_MS))

            << "timed out waiting for luma event to be received";

            << "timed out waiting for luma event to be received";

    EXPECT_NEAR(greenListener->luma(), luma_green, error_margin);

    EXPECT_NEAR(greenListener->luma(), luma_green, error_margin);
@@ -334,7 +359,7 @@ TEST_F(RegionSamplingTest, DISABLED_CollectsLumaFromTwoRegions) { 
    composer->removeRegionSamplingListener(grayListener);

    composer->removeRegionSamplingListener(grayListener);

}

}





TEST_F(RegionSamplingTest, DISABLED_TestIfInvalidInputParameters) {

TEST_F(RegionSamplingTest, TestIfInvalidInputParameters) {

    sp<gui::ISurfaceComposer> composer = ComposerServiceAIDL::getComposerService();

    sp<gui::ISurfaceComposer> composer = ComposerServiceAIDL::getComposerService();

    sp<Listener> listener = new Listener();

    sp<Listener> listener = new Listener();
@@ -369,7 +394,7 @@ TEST_F(RegionSamplingTest, DISABLED_TestIfInvalidInputParameters) { 
    composer->removeRegionSamplingListener(listener);

    composer->removeRegionSamplingListener(listener);

}

}





TEST_F(RegionSamplingTest, DISABLED_TestCallbackAfterRemoveListener) {

TEST_F(RegionSamplingTest, TestCallbackAfterRemoveListener) {

    fill_render(rgba_green);

    fill_render(rgba_green);

    sp<gui::ISurfaceComposer> composer = ComposerServiceAIDL::getComposerService();

    sp<gui::ISurfaceComposer> composer = ComposerServiceAIDL::getComposerService();

    sp<Listener> listener = new Listener();

    sp<Listener> listener = new Listener();
@@ -381,7 +406,8 @@ TEST_F(RegionSamplingTest, DISABLED_TestCallbackAfterRemoveListener) { 
    composer->addRegionSamplingListener(sampleArea, mTopLayer->getHandle(), listener);

    composer->addRegionSamplingListener(sampleArea, mTopLayer->getHandle(), listener);

    fill_render(rgba_green);

    fill_render(rgba_green);





    EXPECT_TRUE(listener->wait_event(300ms)) << "timed out waiting for luma event to be received";

    EXPECT_TRUE(listener->wait_event(EVENT_WAIT_TIME_MS))

            << "timed out waiting for luma event to be received";

    EXPECT_NEAR(listener->luma(), luma_green, error_margin);

    EXPECT_NEAR(listener->luma(), luma_green, error_margin);





    listener->reset();

    listener->reset();
@@ -404,11 +430,13 @@ TEST_F(RegionSamplingTest, DISABLED_CollectsLumaFromMovingLayer) { 
    // Test: listener in (100, 100). See layer before move, no layer after move.

    // Test: listener in (100, 100). See layer before move, no layer after move.

    fill_render(rgba_blue);

    fill_render(rgba_blue);

    composer->addRegionSamplingListener(sampleAreaA, mTopLayer->getHandle(), listener);

    composer->addRegionSamplingListener(sampleAreaA, mTopLayer->getHandle(), listener);

    EXPECT_TRUE(listener->wait_event(300ms)) << "timed out waiting for luma event to be received";

    EXPECT_TRUE(listener->wait_event(EVENT_WAIT_TIME_MS))

            << "timed out waiting for luma event to be received";

    EXPECT_NEAR(listener->luma(), luma_blue, error_margin);

    EXPECT_NEAR(listener->luma(), luma_blue, error_margin);

    listener->reset();

    listener->reset();

    SurfaceComposerClient::Transaction{}.setPosition(mContentLayer, 600, 600).apply();

    SurfaceComposerClient::Transaction{}.setPosition(mContentLayer, 600, 600).apply();

    EXPECT_TRUE(listener->wait_event(300ms)) << "timed out waiting for luma event to be received";

    EXPECT_TRUE(listener->wait_event(EVENT_WAIT_TIME_MS))

            << "timed out waiting for luma event to be received";

    EXPECT_NEAR(listener->luma(), luma_gray, error_margin);

    EXPECT_NEAR(listener->luma(), luma_gray, error_margin);

    composer->removeRegionSamplingListener(listener);

    composer->removeRegionSamplingListener(listener);
@@ -420,11 +448,13 @@ TEST_F(RegionSamplingTest, DISABLED_CollectsLumaFromMovingLayer) { 
    sampleAreaA.right = sampleArea.right;

    sampleAreaA.right = sampleArea.right;

    sampleAreaA.bottom = sampleArea.bottom;

    sampleAreaA.bottom = sampleArea.bottom;

    composer->addRegionSamplingListener(sampleAreaA, mTopLayer->getHandle(), listener);

    composer->addRegionSamplingListener(sampleAreaA, mTopLayer->getHandle(), listener);

    EXPECT_TRUE(listener->wait_event(300ms)) << "timed out waiting for luma event to be received";

    EXPECT_TRUE(listener->wait_event(EVENT_WAIT_TIME_MS))

            << "timed out waiting for luma event to be received";

    EXPECT_NEAR(listener->luma(), luma_gray, error_margin);

    EXPECT_NEAR(listener->luma(), luma_gray, error_margin);

    listener->reset();

    listener->reset();

    SurfaceComposerClient::Transaction{}.setPosition(mContentLayer, 600, 600).apply();

    SurfaceComposerClient::Transaction{}.setPosition(mContentLayer, 600, 600).apply();

    EXPECT_TRUE(listener->wait_event(300ms)) << "timed out waiting for luma event to be received";

    EXPECT_TRUE(listener->wait_event(EVENT_WAIT_TIME_MS))

            << "timed out waiting for luma event to be received";

    EXPECT_NEAR(listener->luma(), luma_green, error_margin);

    EXPECT_NEAR(listener->luma(), luma_green, error_margin);

    composer->removeRegionSamplingListener(listener);

    composer->removeRegionSamplingListener(listener);

}

}

services/surfaceflinger/DisplayRenderArea.cpp

+7 −9
Original line number Original line Diff line number Diff line
@@ -22,22 +22,20 @@ namespace android { 
std::unique_ptr<RenderArea> DisplayRenderArea::create(wp<const DisplayDevice> displayWeak,

std::unique_ptr<RenderArea> DisplayRenderArea::create(wp<const DisplayDevice> displayWeak,

                                                      const Rect& sourceCrop, ui::Size reqSize,

                                                      const Rect& sourceCrop, ui::Size reqSize,

                                                      ui::Dataspace reqDataSpace,

                                                      ui::Dataspace reqDataSpace,

                                                      bool hintForSeamlessTransition,

                                                      ftl::Flags<Options> options) {

                                                      bool allowSecureLayers) {

    if (auto display = displayWeak.promote()) {

    if (auto display = displayWeak.promote()) {

        // Using new to access a private constructor.

        // Using new to access a private constructor.

        return std::unique_ptr<DisplayRenderArea>(

        return std::unique_ptr<DisplayRenderArea>(new DisplayRenderArea(std::move(display),

                new DisplayRenderArea(std::move(display), sourceCrop, reqSize, reqDataSpace,

                                                                        sourceCrop, reqSize,

                                      hintForSeamlessTransition, allowSecureLayers));

                                                                        reqDataSpace, options));

    }

    }

    return nullptr;

    return nullptr;

}

}





DisplayRenderArea::DisplayRenderArea(sp<const DisplayDevice> display, const Rect& sourceCrop,

DisplayRenderArea::DisplayRenderArea(sp<const DisplayDevice> display, const Rect& sourceCrop,

                                     ui::Size reqSize, ui::Dataspace reqDataSpace,

                                     ui::Size reqSize, ui::Dataspace reqDataSpace,

                                     bool hintForSeamlessTransition, bool allowSecureLayers)

                                     ftl::Flags<Options> options)

      : RenderArea(reqSize, CaptureFill::OPAQUE, reqDataSpace, hintForSeamlessTransition,

      : RenderArea(reqSize, CaptureFill::OPAQUE, reqDataSpace, options),

                   allowSecureLayers),

        mDisplay(std::move(display)),

        mDisplay(std::move(display)),

        mSourceCrop(sourceCrop) {}

        mSourceCrop(sourceCrop) {}
@@ -46,7 +44,7 @@ const ui::Transform& DisplayRenderArea::getTransform() const { 
}

}





bool DisplayRenderArea::isSecure() const {

bool DisplayRenderArea::isSecure() const {

    return mAllowSecureLayers && mDisplay->isSecure();

    return mOptions.test(Options::CAPTURE_SECURE_LAYERS) && mDisplay->isSecure();

}

}





sp<const DisplayDevice> DisplayRenderArea::getDisplayDevice() const {

sp<const DisplayDevice> DisplayRenderArea::getDisplayDevice() const {

services/surfaceflinger/DisplayRenderArea.h

+2 −3
Original line number Original line Diff line number Diff line
@@ -29,8 +29,7 @@ class DisplayRenderArea : public RenderArea { 
public:

public:

    static std::unique_ptr<RenderArea> create(wp<const DisplayDevice>, const Rect& sourceCrop,

    static std::unique_ptr<RenderArea> create(wp<const DisplayDevice>, const Rect& sourceCrop,

                                              ui::Size reqSize, ui::Dataspace,

                                              ui::Size reqSize, ui::Dataspace,

                                              bool hintForSeamlessTransition,

                                              ftl::Flags<Options> options);

                                              bool allowSecureLayers = true);





    const ui::Transform& getTransform() const override;

    const ui::Transform& getTransform() const override;

    bool isSecure() const override;

    bool isSecure() const override;
@@ -39,7 +38,7 @@ public: 




private:

private:

    DisplayRenderArea(sp<const DisplayDevice>, const Rect& sourceCrop, ui::Size reqSize,

    DisplayRenderArea(sp<const DisplayDevice>, const Rect& sourceCrop, ui::Size reqSize,

                      ui::Dataspace, bool hintForSeamlessTransition, bool allowSecureLayers = true);

                      ui::Dataspace, ftl::Flags<Options> options);





    const sp<const DisplayDevice> mDisplay;

    const sp<const DisplayDevice> mDisplay;

    const Rect mSourceCrop;

    const Rect mSourceCrop;

services/surfaceflinger/LayerRenderArea.cpp

+4 −5
Original line number Original line Diff line number Diff line
@@ -27,10 +27,9 @@ namespace android { 




LayerRenderArea::LayerRenderArea(sp<Layer> layer, frontend::LayerSnapshot layerSnapshot,

LayerRenderArea::LayerRenderArea(sp<Layer> layer, frontend::LayerSnapshot layerSnapshot,

                                 const Rect& crop, ui::Size reqSize, ui::Dataspace reqDataSpace,

                                 const Rect& crop, ui::Size reqSize, ui::Dataspace reqDataSpace,

                                 bool allowSecureLayers, const ui::Transform& layerTransform,

                                 const ui::Transform& layerTransform, const Rect& layerBufferSize,

                                 const Rect& layerBufferSize, bool hintForSeamlessTransition)

                                 ftl::Flags<RenderArea::Options> options)

      : RenderArea(reqSize, CaptureFill::CLEAR, reqDataSpace, hintForSeamlessTransition,

      : RenderArea(reqSize, CaptureFill::CLEAR, reqDataSpace, options),

                   allowSecureLayers),

        mLayer(std::move(layer)),

        mLayer(std::move(layer)),

        mLayerSnapshot(std::move(layerSnapshot)),

        mLayerSnapshot(std::move(layerSnapshot)),

        mLayerBufferSize(layerBufferSize),

        mLayerBufferSize(layerBufferSize),
@@ -42,7 +41,7 @@ const ui::Transform& LayerRenderArea::getTransform() const { 
}

}





bool LayerRenderArea::isSecure() const {

bool LayerRenderArea::isSecure() const {

    return mAllowSecureLayers;

    return mOptions.test(Options::CAPTURE_SECURE_LAYERS);

}

}





sp<const DisplayDevice> LayerRenderArea::getDisplayDevice() const {

sp<const DisplayDevice> LayerRenderArea::getDisplayDevice() const {

services/surfaceflinger/LayerRenderArea.h

+2 −2
Original line number Original line Diff line number Diff line
@@ -33,9 +33,9 @@ class SurfaceFlinger; 
class LayerRenderArea : public RenderArea {

class LayerRenderArea : public RenderArea {

public:

public:

    LayerRenderArea(sp<Layer> layer, frontend::LayerSnapshot layerSnapshot, const Rect& crop,

    LayerRenderArea(sp<Layer> layer, frontend::LayerSnapshot layerSnapshot, const Rect& crop,

                    ui::Size reqSize, ui::Dataspace reqDataSpace, bool allowSecureLayers,

                    ui::Size reqSize, ui::Dataspace reqDataSpace,

                    const ui::Transform& layerTransform, const Rect& layerBufferSize,

                    const ui::Transform& layerTransform, const Rect& layerBufferSize,

                    bool hintForSeamlessTransition);

                    ftl::Flags<RenderArea::Options> options);





    const ui::Transform& getTransform() const override;

    const ui::Transform& getTransform() const override;

    bool isSecure() const override;

    bool isSecure() const override;