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

Commit f9a2a2ce authored by Alec Mouri's avatar Alec Mouri
Browse files

[SurfaceFlinger] Don't persist buffers to HWC when powered off. 

This fixes an extremely rare crash, where stale buffer handles were
parceled over to HWC.

The cause was that HWC's command queue is not flushed while the display is
powered off, so buffers handles may become stale while they are sitting in the
command queue. If a layer's buffer goes out of scope in SurfaceFlinger, e.g.
an app continues renderng while the display is powered down, SurfaceFlinger
latches the new buffers, and consequently releases old buffers, then those
buffers will be deallocated while still sending the handles over to HWC
the next time a frame needs to be presented.

The fix prevents buffers from being queued while the display power mode
is OFF, so that buffer handles should never become stale while in the
command queue.

Bug: 141290044
Test: Enabling HWSAN: covering the phone during Hangouts video calling with
speaker-phone disabled to trigger display power down.
Test: libcompositionengine_test

Change-Id: I2592fecbbc17cf1ed70c348df8e53e9c59afb073
parent 0be01b4e

services/surfaceflinger/CompositionEngine/src/Output.cpp

+4 −0
Original line number Diff line number Diff line
@@ -551,6 +551,10 @@ void Output::updateAndWriteCompositionState( 
    ATRACE_CALL();

    ALOGV(__FUNCTION__);



    if (!getState().isEnabled) {

        return;

    }



    for (auto* layer : getOutputLayersOrderedByZ()) {

        layer->updateCompositionState(refreshArgs.updatingGeometryThisFrame,

                                      refreshArgs.devOptForceClientComposition);

services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp

+25 −0
Original line number Diff line number Diff line
@@ -482,6 +482,31 @@ TEST_F(OutputTest, getOutputLayerForLayerWorks) { 
    EXPECT_EQ(nullptr, mOutput->getOutputLayerForLayer(&layer));

}



/*

 * Output::updateAndWriteCompositionState()

 */



TEST_F(OutputTest, updateAndWriteCompositionState_takesEarlyOutIfNotEnabled) {

    mOutput->editState().isEnabled = false;



    CompositionRefreshArgs args;

    mOutput->updateAndWriteCompositionState(args);

}



TEST_F(OutputTest, updateAndWriteCompositionState_updatesLayers) {

    mOutput->editState().isEnabled = true;

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

    mOutput->injectOutputLayerForTest(std::unique_ptr<OutputLayer>(outputLayer));



    EXPECT_CALL(*outputLayer, updateCompositionState(true, true)).Times(1);

    EXPECT_CALL(*outputLayer, writeStateToHWC(true)).Times(1);



    CompositionRefreshArgs args;

    args.updatingGeometryThisFrame = true;

    args.devOptForceClientComposition = true;

    mOutput->updateAndWriteCompositionState(args);

}



/*

 * Output::prepareFrame()

 */

services/surfaceflinger/tests/unittests/CompositionTest.cpp

+78 −54
Original line number Diff line number Diff line
@@ -168,6 +168,7 @@ public: 


    std::unordered_set<HWC2::Capability> mDefaultCapabilities = {HWC2::Capability::SidebandStream};



    bool mDisplayOff = false;

    TestableSurfaceFlinger mFlinger;

    sp<DisplayDevice> mDisplay;

    sp<DisplayDevice> mExternalDisplay;
@@ -534,6 +535,7 @@ struct BaseLayerProperties { 
    }



    static void setupHwcSetGeometryCallExpectations(CompositionTest* test) {

        if (!test->mDisplayOff) {

            // TODO: Coverage of other values

            EXPECT_CALL(*test->mComposer,

                        setLayerBlendMode(HWC_DISPLAY, HWC_LAYER, LayerProperties::BLENDMODE))
@@ -555,37 +557,48 @@ struct BaseLayerProperties { 
            // These expectations retire on saturation as the code path these

            // expectations are for appears to make an extra call to them.

            // TODO: Investigate this extra call

        EXPECT_CALL(*test->mComposer, setLayerTransform(HWC_DISPLAY, HWC_LAYER, DEFAULT_TRANSFORM))

            EXPECT_CALL(*test->mComposer,

                        setLayerTransform(HWC_DISPLAY, HWC_LAYER, DEFAULT_TRANSFORM))

                    .Times(AtLeast(1))

                    .RetiresOnSaturation();

        }

    }



    static void setupHwcSetSourceCropBufferCallExpectations(CompositionTest* test) {

        if (!test->mDisplayOff) {

            EXPECT_CALL(*test->mComposer,

                        setLayerSourceCrop(HWC_DISPLAY, HWC_LAYER,

                                           IComposerClient::FRect({0.f, 0.f, LayerProperties::WIDTH,

                                                                   LayerProperties::HEIGHT})))

                    .Times(1);

        }

    }



    static void setupHwcSetSourceCropColorCallExpectations(CompositionTest* test) {

        if (!test->mDisplayOff) {

            EXPECT_CALL(*test->mComposer,

                        setLayerSourceCrop(HWC_DISPLAY, HWC_LAYER,

                                           IComposerClient::FRect({0.f, 0.f, 0.f, 0.f})))

                    .Times(1);

        }

    }



    static void setupHwcSetPerFrameCallExpectations(CompositionTest* test) {

        if (!test->mDisplayOff) {

            EXPECT_CALL(*test->mComposer,

                        setLayerVisibleRegion(HWC_DISPLAY, HWC_LAYER,

                                          std::vector<IComposerClient::Rect>({IComposerClient::Rect(

                                              std::vector<IComposerClient::Rect>(

                                                      {IComposerClient::Rect(

                                                              {0, 0, LayerProperties::WIDTH,

                                                               LayerProperties::HEIGHT})})))

                    .Times(1);

        }

    }



    static void setupHwcSetPerFrameColorCallExpectations(CompositionTest* test) {

        EXPECT_CALL(*test->mComposer, setLayerSurfaceDamage(HWC_DISPLAY, HWC_LAYER, _)).Times(1);

        if (!test->mDisplayOff) {

            EXPECT_CALL(*test->mComposer, setLayerSurfaceDamage(HWC_DISPLAY, HWC_LAYER, _))

                    .Times(1);



            // TODO: use COLOR

            EXPECT_CALL(*test->mComposer,
@@ -593,10 +606,14 @@ struct BaseLayerProperties { 
                                      IComposerClient::Color({0xff, 0xff, 0xff, 0xff})))

                    .Times(1);

        }

    }



    static void setupHwcSetPerFrameBufferCallExpectations(CompositionTest* test) {

        EXPECT_CALL(*test->mComposer, setLayerSurfaceDamage(HWC_DISPLAY, HWC_LAYER, _)).Times(1);

        if (!test->mDisplayOff) {

            EXPECT_CALL(*test->mComposer, setLayerSurfaceDamage(HWC_DISPLAY, HWC_LAYER, _))

                    .Times(1);

            EXPECT_CALL(*test->mComposer, setLayerBuffer(HWC_DISPLAY, HWC_LAYER, _, _, _)).Times(1);

        }



        setupBufferLayerPostFrameCallExpectations(test);

    }
@@ -940,10 +957,12 @@ struct KeepCompositionTypeVariant { 
    static constexpr HWC2::Composition TYPE = static_cast<HWC2::Composition>(CompositionType);



    static void setupHwcSetCallExpectations(CompositionTest* test) {

        if (!test->mDisplayOff) {

            EXPECT_CALL(*test->mComposer,

                        setLayerCompositionType(HWC_DISPLAY, HWC_LAYER, CompositionType))

                    .Times(1);

        }

    }



    static void setupHwcGetCallExpectations(CompositionTest* test) {

        EXPECT_CALL(*test->mComposer, getChangedCompositionTypes(HWC_DISPLAY, _, _)).Times(1);
@@ -956,10 +975,12 @@ struct ChangeCompositionTypeVariant { 
    static constexpr HWC2::Composition TYPE = static_cast<HWC2::Composition>(FinalCompositionType);



    static void setupHwcSetCallExpectations(CompositionTest* test) {

        if (!test->mDisplayOff) {

            EXPECT_CALL(*test->mComposer,

                        setLayerCompositionType(HWC_DISPLAY, HWC_LAYER, InitialCompositionType))

                    .Times(1);

        }

    }



    static void setupHwcGetCallExpectations(CompositionTest* test) {

        EXPECT_CALL(*test->mComposer, getChangedCompositionTypes(HWC_DISPLAY, _, _))
@@ -1341,6 +1362,7 @@ TEST_F(CompositionTest, captureScreenCursorLayer) { 
 */



TEST_F(CompositionTest, displayOffHWCComposedNormalBufferLayerWithDirtyGeometry) {

    mDisplayOff = true;

    displayRefreshCompositionDirtyGeometry<CompositionCase<

            PoweredOffDisplaySetupVariant, BufferLayerVariant<DefaultLayerProperties>,

            KeepCompositionTypeVariant<IComposerClient::Composition::DEVICE>,
@@ -1348,6 +1370,7 @@ TEST_F(CompositionTest, displayOffHWCComposedNormalBufferLayerWithDirtyGeometry) 
}



TEST_F(CompositionTest, displayOffHWCComposedNormalBufferLayerWithDirtyFrame) {

    mDisplayOff = true;

    displayRefreshCompositionDirtyFrame<CompositionCase<

            PoweredOffDisplaySetupVariant, BufferLayerVariant<DefaultLayerProperties>,

            KeepCompositionTypeVariant<IComposerClient::Composition::DEVICE>,
@@ -1355,6 +1378,7 @@ TEST_F(CompositionTest, displayOffHWCComposedNormalBufferLayerWithDirtyFrame) { 
}



TEST_F(CompositionTest, displayOffREComposedNormalBufferLayer) {

    mDisplayOff = true;

    displayRefreshCompositionDirtyFrame<CompositionCase<

            PoweredOffDisplaySetupVariant, BufferLayerVariant<DefaultLayerProperties>,

            ChangeCompositionTypeVariant<IComposerClient::Composition::DEVICE,