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

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

Don't invoke transactions on layers that will be removed. 

A layer can be marked as removed, but still be present in memory.
This check ensures that transactions aren't invoked on layers that
will be removed on the next commitTransaction.

Normally, this would be harmless since the layer will get removed
as soon as a commitTransaction is called. However, for cases like
re-parenting, a removed child layer can be re-parented to a non-removed
layer, which prevents the child from getting removed.

Test: Added code that would destroy a layer before the re-parent was
called. Ensure that the re-parent was ignored. There doesn't
seem to be an easy way to write a test case right now.

Change-Id: I17614447fc4253bdbbb0c06469bb09117b55c1ab
parent 817112a0

services/surfaceflinger/Layer.cpp

+2 −0
Original line number Diff line number Diff line
@@ -175,6 +175,8 @@ void Layer::onLayerDisplayed(const sp<const DisplayDevice>& /* hw */, 
void Layer::onRemovedFromCurrentState() {

    // the layer is removed from SF mCurrentState to mLayersPendingRemoval



    mPendingRemoval = true;



    if (mCurrentState.zOrderRelativeOf != nullptr) {

        sp<Layer> strongRelative = mCurrentState.zOrderRelativeOf.promote();

        if (strongRelative != nullptr) {

services/surfaceflinger/Layer.h

+4 −0
Original line number Diff line number Diff line
@@ -283,6 +283,8 @@ public: 
     */

    virtual bool isFixedSize() const = 0;



    bool isPendingRemoval() const { return mPendingRemoval; }



    void writeToProto(LayerProto* layerInfo,

                      LayerVector::StateSet stateSet = LayerVector::StateSet::Drawing);
@@ -666,6 +668,8 @@ protected: 
    // The mesh used to draw the layer in GLES composition mode

    mutable Mesh mMesh;



    bool mPendingRemoval = false;



#ifdef USE_HWC2

    // HWC items, accessed from the main thread

    struct HWCInfo {

services/surfaceflinger/SurfaceFlinger.cpp

+120 −108
Original line number Diff line number Diff line
@@ -2857,6 +2857,10 @@ status_t SurfaceFlinger::addClientLayer(const sp<Client>& client, 
status_t SurfaceFlinger::removeLayer(const sp<Layer>& layer, bool topLevelOnly) {

    Mutex::Autolock _l(mStateLock);



    if (layer->isPendingRemoval()) {

        return NO_ERROR;

    }



    const auto& p = layer->getParent();

    ssize_t index;

    if (p != nullptr) {
@@ -3060,9 +3064,18 @@ uint32_t SurfaceFlinger::setClientStateLocked( 
        const sp<Client>& client,

        const layer_state_t& s)

{

    uint32_t flags = 0;

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

    if (layer != 0) {

    if (layer == nullptr) {

        return 0;

    }



    if (layer->isPendingRemoval()) {

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

        return 0;

    }



    uint32_t flags = 0;



    const uint32_t what = s.what;

    bool geometryAppliesWithResize =

            what & layer_state_t::eGeometryAppliesWithResize;
@@ -3184,7 +3197,6 @@ uint32_t SurfaceFlinger::setClientStateLocked( 
        // We don't trigger a traversal here because if no other state is

        // changed, we don't want this to cause any more work

    }

    }

    return flags;

}