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

Commit e0eafa8e authored by Ady Abraham's avatar Ady Abraham
Browse files

CompositionEngine: fix memory leak in unit tests 

LayerFE implements RefBase, and instantiating it on stack
causes a memory leak of the shared state.

Bug: 198190384
Test: libcompositionengine_test
Change-Id: If0f847bdf93359b206f7ce77ded9a8d5feb4e31c
parent 51ae9d7e

services/surfaceflinger/CompositionEngine/include/compositionengine/mock/LayerFE.h

+11 −1
Original line number Diff line number Diff line
@@ -26,8 +26,18 @@ namespace android::compositionengine::mock { 
// Defines the interface used by the CompositionEngine to make requests

// of the front-end layer.

class LayerFE : public compositionengine::LayerFE {

public:

private:

    // Making the constructor private as this class implements RefBase,

    // and constructing it with a different way than sp<LayerFE>::make() causes

    // a memory leak of the shared state.

    LayerFE();



    // friends class to allow instantiation via sp<LayerFE>::make() and

    // sp<StrictMock<LayerFE>>::make()

    friend class sp<LayerFE>;

    friend class testing::StrictMock<LayerFE>;



public:

    virtual ~LayerFE();



    MOCK_CONST_METHOD0(getCompositionState, const LayerFECompositionState*());

services/surfaceflinger/CompositionEngine/tests/CompositionEngineTest.cpp

+3 −3
Original line number Diff line number Diff line
@@ -203,9 +203,9 @@ TEST_F(CompositionEngineUpdateCursorAsyncTest, handlesMultipleLayersBeingCursorL 
 */



struct CompositionTestPreComposition : public CompositionEngineTest {

    sp<StrictMock<mock::LayerFE>> mLayer1FE{new StrictMock<mock::LayerFE>()};

    sp<StrictMock<mock::LayerFE>> mLayer2FE{new StrictMock<mock::LayerFE>()};

    sp<StrictMock<mock::LayerFE>> mLayer3FE{new StrictMock<mock::LayerFE>()};

    sp<StrictMock<mock::LayerFE>> mLayer1FE = sp<StrictMock<mock::LayerFE>>::make();

    sp<StrictMock<mock::LayerFE>> mLayer2FE = sp<StrictMock<mock::LayerFE>>::make();

    sp<StrictMock<mock::LayerFE>> mLayer3FE = sp<StrictMock<mock::LayerFE>>::make();

};



TEST_F(CompositionTestPreComposition, preCompositionSetsFrameTimestamp) {

services/surfaceflinger/CompositionEngine/tests/DisplayTest.cpp

+3 −3
Original line number Diff line number Diff line
@@ -74,7 +74,7 @@ struct Layer { 
        EXPECT_CALL(*outputLayer, getHwcLayer()).WillRepeatedly(Return(&hwc2Layer));

    }



    sp<mock::LayerFE> layerFE = new StrictMock<mock::LayerFE>();

    sp<StrictMock<mock::LayerFE>> layerFE = sp<StrictMock<mock::LayerFE>>::make();

    StrictMock<mock::OutputLayer>* outputLayer = new StrictMock<mock::OutputLayer>();

    StrictMock<HWC2::mock::Layer> hwc2Layer;

};
@@ -85,7 +85,7 @@ struct LayerNoHWC2Layer { 
        EXPECT_CALL(*outputLayer, getHwcLayer()).WillRepeatedly(Return(nullptr));

    }



    sp<mock::LayerFE> layerFE = new StrictMock<mock::LayerFE>();

    sp<StrictMock<mock::LayerFE>> layerFE = sp<StrictMock<mock::LayerFE>>::make();

    StrictMock<mock::OutputLayer>* outputLayer = new StrictMock<mock::OutputLayer>();

};
@@ -468,7 +468,7 @@ TEST_F(DisplayCreateRenderSurfaceTest, setsRenderSurface) { 
using DisplayCreateOutputLayerTest = FullDisplayImplTestCommon;



TEST_F(DisplayCreateOutputLayerTest, setsHwcLayer) {

    sp<mock::LayerFE> layerFE = new StrictMock<mock::LayerFE>();

    sp<StrictMock<mock::LayerFE>> layerFE = sp<StrictMock<mock::LayerFE>>::make();

    auto hwcLayer = std::make_shared<StrictMock<HWC2::mock::Layer>>();



    EXPECT_CALL(mHwComposer, createLayer(HalDisplayId(DEFAULT_DISPLAY_ID)))

services/surfaceflinger/CompositionEngine/tests/OutputLayerTest.cpp

+2 −2
Original line number Diff line number Diff line
@@ -109,8 +109,8 @@ struct OutputLayerTest : public testing::Test { 
    }



    compositionengine::mock::Output mOutput;

    sp<compositionengine::mock::LayerFE> mLayerFE{

            new StrictMock<compositionengine::mock::LayerFE>()};

    sp<StrictMock<compositionengine::mock::LayerFE>> mLayerFE =

            sp<StrictMock<compositionengine::mock::LayerFE>>::make();

    OutputLayer mOutputLayer{mOutput, mLayerFE};



    LayerFECompositionState mLayerFEState;

services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp

+9 −9
Original line number Diff line number Diff line
@@ -106,7 +106,7 @@ struct InjectedLayer { 
    }



    mock::OutputLayer* outputLayer = {new StrictMock<mock::OutputLayer>};

    sp<StrictMock<mock::LayerFE>> layerFE = new StrictMock<mock::LayerFE>();

    sp<StrictMock<mock::LayerFE>> layerFE = sp<StrictMock<mock::LayerFE>>::make();

    LayerFECompositionState layerFEState;

    impl::OutputLayerCompositionState outputLayerState;

};
@@ -123,7 +123,7 @@ struct NonInjectedLayer { 
    }



    mock::OutputLayer outputLayer;

    sp<StrictMock<mock::LayerFE>> layerFE = new StrictMock<mock::LayerFE>();

    sp<StrictMock<mock::LayerFE>> layerFE = sp<StrictMock<mock::LayerFE>>::make();

    LayerFECompositionState layerFEState;

    impl::OutputLayerCompositionState outputLayerState;

};
@@ -722,9 +722,9 @@ TEST_F(OutputTest, getOutputLayerForLayerWorks) { 
using OutputSetReleasedLayersTest = OutputTest;



TEST_F(OutputSetReleasedLayersTest, setReleasedLayersTakesGivenLayers) {

    sp<StrictMock<mock::LayerFE>> layer1FE{new StrictMock<mock::LayerFE>()};

    sp<StrictMock<mock::LayerFE>> layer2FE{new StrictMock<mock::LayerFE>()};

    sp<StrictMock<mock::LayerFE>> layer3FE{new StrictMock<mock::LayerFE>()};

    sp<StrictMock<mock::LayerFE>> layer1FE = sp<StrictMock<mock::LayerFE>>::make();

    sp<StrictMock<mock::LayerFE>> layer2FE = sp<StrictMock<mock::LayerFE>>::make();

    sp<StrictMock<mock::LayerFE>> layer3FE = sp<StrictMock<mock::LayerFE>>::make();



    Output::ReleasedLayers layers;

    layers.push_back(layer1FE);
@@ -1209,7 +1209,7 @@ struct OutputCollectVisibleLayersTest : public testing::Test { 


        StrictMock<mock::OutputLayer> outputLayer;

        impl::OutputLayerCompositionState outputLayerState;

        sp<StrictMock<mock::LayerFE>> layerFE{new StrictMock<mock::LayerFE>()};

        sp<StrictMock<mock::LayerFE>> layerFE = sp<StrictMock<mock::LayerFE>>::make();

    };



    OutputCollectVisibleLayersTest() {
@@ -2957,9 +2957,9 @@ TEST_F(OutputPostFramebufferTest, releasedLayersSentPresentFence) { 
    EXPECT_CALL(mOutput, getOutputLayerCount()).WillOnce(Return(0u));



    // Load up the released layers with some mock instances

    sp<StrictMock<mock::LayerFE>> releasedLayer1{new StrictMock<mock::LayerFE>()};

    sp<StrictMock<mock::LayerFE>> releasedLayer2{new StrictMock<mock::LayerFE>()};

    sp<StrictMock<mock::LayerFE>> releasedLayer3{new StrictMock<mock::LayerFE>()};

    sp<StrictMock<mock::LayerFE>> releasedLayer1 = sp<StrictMock<mock::LayerFE>>::make();

    sp<StrictMock<mock::LayerFE>> releasedLayer2 = sp<StrictMock<mock::LayerFE>>::make();

    sp<StrictMock<mock::LayerFE>> releasedLayer3 = sp<StrictMock<mock::LayerFE>>::make();

    Output::ReleasedLayers layers;

    layers.push_back(releasedLayer1);

    layers.push_back(releasedLayer2);