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

Commit 8e3fe5d6 authored by chaviw's avatar chaviw
Browse files

Store SurfaceControl reference when creating transactions 

The SurfaceControl needs to be stored when adding a new transaction.
This ensures the ref count is incremented so the SC isn't removed before
the transaction is applied.

Change-Id: I27a060e4c221c5dfa565ceb3a916574105fd1175
Fixes: 73448047
Test: DereferenceSurfaceControlTest
parent 1eb1ef28

libs/gui/SurfaceComposerClient.cpp

+14 −15
Original line number Diff line number Diff line
@@ -105,12 +105,11 @@ SurfaceComposerClient::Transaction::Transaction(const Transaction& other) : 
}



SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::merge(Transaction&& other) {

    for (auto const& state : other.mComposerStates) {

        ssize_t index = mComposerStates.indexOf(state);

        if (index < 0) {

            mComposerStates.add(state);

    for (auto const& kv : other.mComposerStates) {

        if (mComposerStates.count(kv.first) == 0) {

            mComposerStates[kv.first] = kv.second;

        } else {

            mComposerStates.editItemAt(static_cast<size_t>(index)).state.merge(state.state);

            mComposerStates[kv.first].state.merge(kv.second.state);

        }

    }

    other.mComposerStates.clear();
@@ -141,7 +140,10 @@ status_t SurfaceComposerClient::Transaction::apply(bool synchronous) { 


    mForceSynchronous |= synchronous;



    composerStates = mComposerStates;

    for (auto const& kv : mComposerStates){

        composerStates.add(kv.second);

    }



    mComposerStates.clear();



    displayStates = mDisplayStates;
@@ -182,18 +184,15 @@ void SurfaceComposerClient::Transaction::setAnimationTransaction() { 
}



layer_state_t* SurfaceComposerClient::Transaction::getLayerState(const sp<SurfaceControl>& sc) {

    if (mComposerStates.count(sc) == 0) {

        // we don't have it, add an initialized layer_state to our list

        ComposerState s;

        s.client = sc->getClient()->mClient;

        s.state.surface = sc->getHandle();



    ssize_t index = mComposerStates.indexOf(s);

    if (index < 0) {

        // we don't have it, add an initialized layer_state to our list

        index = mComposerStates.add(s);

        mComposerStates[sc] = s;

    }



    ComposerState* const out = mComposerStates.editArray();

    return &(out[index].state);

    return &(mComposerStates[sc].state);

}



SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::setPosition(

libs/gui/include/gui/SurfaceComposerClient.h

+8 −1
Original line number Diff line number Diff line
@@ -19,6 +19,7 @@ 


#include <stdint.h>

#include <sys/types.h>

#include <unordered_map>



#include <binder/IBinder.h>
@@ -127,8 +128,14 @@ public: 


    static status_t injectVSync(nsecs_t when);



    struct SCHash {

        std::size_t operator()(const sp<SurfaceControl>& sc) const {

            return std::hash<SurfaceControl *>{}(sc.get());

        }

    };



    class Transaction {

        SortedVector<ComposerState> mComposerStates;

        std::unordered_map<sp<SurfaceControl>, ComposerState, SCHash> mComposerStates;

        SortedVector<DisplayState > mDisplayStates;

        uint32_t                    mForceSynchronous = 0;

        uint32_t                    mTransactionNestCount = 0;

services/surfaceflinger/tests/SurfaceFlinger_test.filter

+1 −1
Original line number Diff line number Diff line 
{

        "presubmit": {

            "filter": "LayerTransactionTest.*:LayerUpdateTest.*:ChildLayerTest.*:SurfaceFlingerStress.*:CropLatchingTest.*:GeometryLatchingTest.*"

            "filter": "LayerTransactionTest.*:LayerUpdateTest.*:ChildLayerTest.*:SurfaceFlingerStress.*:CropLatchingTest.*:GeometryLatchingTest.*:ScreenCaptureTest.*:DereferenceSurfaceControlTest.*"

        }

}

services/surfaceflinger/tests/Transaction_test.cpp

+45 −0
Original line number Diff line number Diff line
@@ -2460,4 +2460,49 @@ TEST_F(ScreenCaptureTest, CaptureInvalidLayer) { 
    ASSERT_EQ(NAME_NOT_FOUND, sf->captureLayers(redLayerHandle, &outBuffer, Rect::EMPTY_RECT, 1.0));

}





class DereferenceSurfaceControlTest : public LayerTransactionTest {

protected:

    void SetUp() override {

        LayerTransactionTest::SetUp();

        bgLayer = createLayer("BG layer", 20, 20);

        fillLayerColor(bgLayer, Color::RED);

        fgLayer = createLayer("FG layer", 20, 20);

        fillLayerColor(fgLayer, Color::BLUE);

        Transaction().setLayer(fgLayer, mLayerZBase + 1).apply();

        {

            SCOPED_TRACE("before anything");

            auto shot = screenshot();

            shot->expectColor(Rect(0, 0, 20, 20), Color::BLUE);

        }

    }

    void TearDown() override {

        LayerTransactionTest::TearDown();

        bgLayer = 0;

        fgLayer = 0;

    }



    sp<SurfaceControl> bgLayer;

    sp<SurfaceControl> fgLayer;

};



TEST_F(DereferenceSurfaceControlTest, LayerNotInTransaction) {

    fgLayer = nullptr;

    {

        SCOPED_TRACE("after setting null");

        auto shot = screenshot();

        shot->expectColor(Rect(0, 0, 20, 20), Color::RED);

    }

}



TEST_F(DereferenceSurfaceControlTest, LayerInTransaction) {

    auto transaction = Transaction().show(fgLayer);

    fgLayer = nullptr;

    {

        SCOPED_TRACE("after setting null");

        auto shot = screenshot();

        shot->expectColor(Rect(0, 0, 20, 20), Color::BLUE);

    }

}



} // namespace android