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

Commit ca27f250 authored by chaviw's avatar chaviw
Browse files

Allow destroySurface to get called in transaction. 

Previously, destroy was always initiated immediatley and could not be
synchronized with a client transaction. This change allows
destroySurface to be called in the same transaction as other client
state updates.

Test: Unit tests pass
Test: Call from Java fixes bugs.
Change-Id: I841359530538961a0187216cc455cc388c0ede77
Fixes: 72953020
Fixes: 71499373
parent e9e63913

libs/gui/LayerState.cpp

+3 −0
Original line number Diff line number Diff line
@@ -231,6 +231,9 @@ void layer_state_t::merge(const layer_state_t& other) { 
        what |= eReparent;

        parentHandleForChild = other.parentHandleForChild;

    }

    if (other.what & eDestroySurface) {

        what |= eDestroySurface;

    }

}



}; // namespace android

libs/gui/SurfaceComposerClient.cpp

+11 −0
Original line number Diff line number Diff line
@@ -472,6 +472,17 @@ SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::setGeome 
    return *this;

}



SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::destroySurface(

        const sp<SurfaceControl>& sc) {

    layer_state_t* s = getLayerStateLocked(sc);

    if (!s) {

        mStatus = BAD_INDEX;

        return *this;

    }

    s->what |= layer_state_t::eDestroySurface;

    return *this;

}



// ---------------------------------------------------------------------------



DisplayState& SurfaceComposerClient::Transaction::getDisplayStateLocked(const sp<IBinder>& token) {

libs/gui/include/gui/LayerState.h

+2 −1
Original line number Diff line number Diff line
@@ -62,7 +62,8 @@ struct layer_state_t { 
        eDetachChildren             = 0x00004000,

        eRelativeLayerChanged       = 0x00008000,

        eReparent                   = 0x00010000,

        eColorChanged               = 0x00020000

        eColorChanged               = 0x00020000,

        eDestroySurface             = 0x00040000

    };



    layer_state_t()

libs/gui/include/gui/SurfaceComposerClient.h

+2 −0
Original line number Diff line number Diff line
@@ -229,6 +229,8 @@ public: 
        // freezing the total geometry of a surface until a resize is completed.

        Transaction& setGeometryAppliesWithResize(const sp<SurfaceControl>& sc);



        Transaction& destroySurface(const sp<SurfaceControl>& sc);



        status_t setDisplaySurface(const sp<IBinder>& token,

                const sp<IGraphicBufferProducer>& bufferProducer);

services/surfaceflinger/SurfaceFlinger.cpp

+67 −29
Original line number Diff line number Diff line
@@ -2901,7 +2901,11 @@ status_t SurfaceFlinger::addClientLayer(const sp<Client>& client, 


status_t SurfaceFlinger::removeLayer(const sp<Layer>& layer, bool topLevelOnly) {

    Mutex::Autolock _l(mStateLock);

    return removeLayerLocked(mStateLock, layer, topLevelOnly);

}



status_t SurfaceFlinger::removeLayerLocked(const Mutex&, const sp<Layer>& layer,

                                           bool topLevelOnly) {

    if (layer->isPendingRemoval()) {

        return NO_ERROR;

    }
@@ -2965,8 +2969,30 @@ uint32_t SurfaceFlinger::setTransactionFlags(uint32_t flags) { 
    return old;

}



bool SurfaceFlinger::containsAnyInvalidClientState(const Vector<ComposerState>& states) {

    for (const ComposerState& state : states) {

        // Here we need to check that the interface we're given is indeed

        // one of our own. A malicious client could give us a nullptr

        // IInterface, or one of its own or even one of our own but a

        // different type. All these situations would cause us to crash.

        if (state.client == nullptr) {

            return true;

        }



        sp<IBinder> binder = IInterface::asBinder(state.client);

        if (binder == nullptr) {

            return true;

        }



        if (binder->queryLocalInterface(ISurfaceComposerClient::descriptor) == nullptr) {

            return true;

        }

    }

    return false;

}



void SurfaceFlinger::setTransactionState(

        const Vector<ComposerState>& state,

        const Vector<ComposerState>& states,

        const Vector<DisplayState>& displays,

        uint32_t flags)

{
@@ -2974,6 +3000,10 @@ void SurfaceFlinger::setTransactionState( 
    Mutex::Autolock _l(mStateLock);

    uint32_t transactionFlags = 0;



    if (containsAnyInvalidClientState(states)) {

        return;

    }



    if (flags & eAnimation) {

        // For window updates that are part of an animation we must wait for

        // previous animation "frames" to be handled.
@@ -2990,31 +3020,20 @@ void SurfaceFlinger::setTransactionState( 
        }

    }



    size_t count = displays.size();

    for (size_t i=0 ; i<count ; i++) {

        const DisplayState& s(displays[i]);

        transactionFlags |= setDisplayStateLocked(s);

    for (const DisplayState& display : displays) {

        transactionFlags |= setDisplayStateLocked(display);

    }



    count = state.size();

    for (size_t i=0 ; i<count ; i++) {

        const ComposerState& s(state[i]);

        // Here we need to check that the interface we're given is indeed

        // one of our own. A malicious client could give us a nullptr

        // IInterface, or one of its own or even one of our own but a

        // different type. All these situations would cause us to crash.

        //

        // NOTE: it would be better to use RTTI as we could directly check

        // that we have a Client*. however, RTTI is disabled in Android.

        if (s.client != nullptr) {

            sp<IBinder> binder = IInterface::asBinder(s.client);

            if (binder != nullptr) {

                if (binder->queryLocalInterface(ISurfaceComposerClient::descriptor) != nullptr) {

                    sp<Client> client( static_cast<Client *>(s.client.get()) );

                    transactionFlags |= setClientStateLocked(client, s.state);

                }

            }

    for (const ComposerState& state : states) {

        transactionFlags |= setClientStateLocked(state);

    }



    // Iterate through all layers again to determine if any need to be destroyed. Marking layers

    // as destroyed should only occur after setting all other states. This is to allow for a

    // child re-parent to happen before marking its original parent as destroyed (which would

    // then mark the child as destroyed).

    for (const ComposerState& state : states) {

        setDestroyStateLocked(state);

    }



    // If a synchronous transaction is explicitly requested without any changes, force a transaction
@@ -3028,7 +3047,7 @@ void SurfaceFlinger::setTransactionState( 


    if (transactionFlags) {

        if (mInterceptor.isEnabled()) {

            mInterceptor.saveTransaction(state, mCurrentState.displays, displays, flags);

            mInterceptor.saveTransaction(states, mCurrentState.displays, displays, flags);

        }



        // this triggers the transaction
@@ -3105,10 +3124,10 @@ uint32_t SurfaceFlinger::setDisplayStateLocked(const DisplayState& s) 
    return flags;

}



uint32_t SurfaceFlinger::setClientStateLocked(

        const sp<Client>& client,

        const layer_state_t& s)

{

uint32_t SurfaceFlinger::setClientStateLocked(const ComposerState& composerState) {

    const layer_state_t& s = composerState.state;

    sp<Client> client(static_cast<Client*>(composerState.client.get()));



    sp<Layer> layer(client->getLayerUser(s.surface));

    if (layer == nullptr) {

        return 0;
@@ -3259,6 +3278,25 @@ uint32_t SurfaceFlinger::setClientStateLocked( 
    return flags;

}



void SurfaceFlinger::setDestroyStateLocked(const ComposerState& composerState) {

    const layer_state_t& state = composerState.state;

    sp<Client> client(static_cast<Client*>(composerState.client.get()));



    sp<Layer> layer(client->getLayerUser(state.surface));

    if (layer == nullptr) {

        return;

    }



    if (layer->isPendingRemoval()) {

        ALOGW("Attempting to destroy on removed layer: %s", layer->getName().string());

        return;

    }



    if (state.what & layer_state_t::eDestroySurface) {

        removeLayerLocked(mStateLock, layer);

    }

}



status_t SurfaceFlinger::createLayer(

        const String8& name,

        const sp<Client>& client,