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

Commit d7de52cf authored by Rob Carr's avatar Rob Carr Committed by Android (Google) Code Review
Browse files

Merge "SurfaceFlinger: Allows Surfaces to outlive their parents."

parents b3ee0688 4340607f

services/surfaceflinger/Layer.cpp

+8 −11
Original line number Diff line number Diff line
@@ -118,12 +118,6 @@ Layer::~Layer() { 
        c->detachLayer(this);

    }



    for (auto& point : mRemoteSyncPoints) {

        point->setTransactionApplied();

    }

    for (auto& point : mLocalSyncPoints) {

        point->setFrameAvailable();

    }

    mFrameTracker.logAndResetStats(mName);

}
@@ -141,8 +135,6 @@ void Layer::onLayerDisplayed(const sp<Fence>& /*releaseFence*/) {} 
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) {
@@ -163,8 +155,13 @@ void Layer::onRemoved() { 


    destroyAllHwcLayers();



    for (const auto& child : mCurrentChildren) {

        child->onRemoved();

    mRemoved = true;



    for (auto& point : mRemoteSyncPoints) {

        point->setTransactionApplied();

    }

    for (auto& point : mLocalSyncPoints) {

        point->setFrameAvailable();

    }

}

services/surfaceflinger/Layer.h

+4 −4
Original line number Diff line number Diff line
@@ -345,7 +345,7 @@ public: 
    virtual bool isCreatedFromMainThread() const { return false; }





    bool isPendingRemoval() const { return mPendingRemoval; }

    bool isRemoved() const { return mRemoved; }



    void writeToProto(LayerProto* layerInfo,

                      LayerVector::StateSet stateSet = LayerVector::StateSet::Drawing);
@@ -594,12 +594,12 @@ protected: 
     */

    class LayerCleaner {

        sp<SurfaceFlinger> mFlinger;

        wp<Layer> mLayer;

        sp<Layer> mLayer;



    protected:

        ~LayerCleaner() {

            // destroy client resources

            mFlinger->onLayerDestroyed(mLayer);

            mFlinger->removeLayer(mLayer, true);

        }



    public:
@@ -744,7 +744,7 @@ protected: 
    // Whether filtering is needed b/c of the drawingstate

    bool mNeedsFiltering{false};



    bool mPendingRemoval{false};

    bool mRemoved{false};



    // page-flip thread (currently main thread)

    bool mProtectedByApp{false}; // application requires protected path to external sink

services/surfaceflinger/SurfaceFlinger.cpp

+6 −43
Original line number Diff line number Diff line
@@ -2852,6 +2852,7 @@ void SurfaceFlinger::commitTransaction() 
            recordBufferingStats(l->getName().string(),

                    l->getOccupancyHistory(true));

            l->onRemoved();

            mNumLayers -= 1;

        }

        mLayersPendingRemoval.clear();

    }
@@ -3283,7 +3284,7 @@ status_t SurfaceFlinger::addClientLayer(const sp<Client>& client, 
        if (parent == nullptr) {

            mCurrentState.layersSortedByZ.add(lbc);

        } else {

            if (parent->isPendingRemoval()) {

            if (parent->isRemoved()) {

                ALOGE("addClientLayer called with a removed parent");

                return NAME_NOT_FOUND;

            }
@@ -3315,7 +3316,7 @@ status_t SurfaceFlinger::removeLayer(const sp<Layer>& layer, bool topLevelOnly) 


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

                                           bool topLevelOnly) {

    if (layer->isPendingRemoval()) {

    if (layer->isRemoved()) {

        return NO_ERROR;

    }
@@ -3325,39 +3326,14 @@ status_t SurfaceFlinger::removeLayerLocked(const Mutex&, const sp<Layer>& layer, 
        if (topLevelOnly) {

            return NO_ERROR;

        }



        sp<Layer> ancestor = p;

        while (ancestor->getParent() != nullptr) {

            ancestor = ancestor->getParent();

        }

        if (mCurrentState.layersSortedByZ.indexOf(ancestor) < 0) {

            ALOGE("removeLayer called with a layer whose parent has been removed");

            return NAME_NOT_FOUND;

        }



        index = p->removeChild(layer);

    } else {

        index = mCurrentState.layersSortedByZ.remove(layer);

    }



    // As a matter of normal operation, the LayerCleaner will produce a second

    // attempt to remove the surface. The Layer will be kept alive in mDrawingState

    // so we will succeed in promoting it, but it's already been removed

    // from mCurrentState. As long as we can find it in mDrawingState we have no problem

    // otherwise something has gone wrong and we are leaking the layer.

    if (index < 0 && mDrawingState.layersSortedByZ.indexOf(layer) < 0) {

        ALOGE("Failed to find layer (%s) in layer parent (%s).",

                layer->getName().string(),

                (p != nullptr) ? p->getName().string() : "no-parent");

        return BAD_VALUE;

    } else if (index < 0) {

        return NO_ERROR;

    }



    layer->onRemovedFromCurrentState();

    mLayersPendingRemoval.add(layer);

    mLayersRemoved = true;

    mNumLayers -= 1 + layer->getChildrenCount();

    setTransactionFlags(eTransactionNeeded);

    return NO_ERROR;

}
@@ -3560,7 +3536,7 @@ uint32_t SurfaceFlinger::setClientStateLocked(const ComposerState& composerState 
        return 0;

    }



    if (layer->isPendingRemoval()) {

    if (layer->isRemoved()) {

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

        return 0;

    }
@@ -3768,7 +3744,7 @@ void SurfaceFlinger::setDestroyStateLocked(const ComposerState& composerState) { 
        return;

    }



    if (layer->isPendingRemoval()) {

    if (layer->isRemoved()) {

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

        return;

    }
@@ -3945,19 +3921,6 @@ status_t SurfaceFlinger::onLayerRemoved(const sp<Client>& client, const sp<IBind 
    return err;

}



status_t SurfaceFlinger::onLayerDestroyed(const wp<Layer>& layer)

{

    // called by ~LayerCleaner() when all references to the IBinder (handle)

    // are gone

    sp<Layer> l = layer.promote();

    if (l == nullptr) {

        // The layer has already been removed, carry on

        return NO_ERROR;

    }

    // If we have a parent, then we can continue to live as long as it does.

    return removeLayer(l, true);

}



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



void SurfaceFlinger::onInitializeDisplays() {
@@ -5331,7 +5294,7 @@ status_t SurfaceFlinger::captureLayers(const sp<IBinder>& layerHandleBinder, 
    auto layerHandle = reinterpret_cast<Layer::Handle*>(layerHandleBinder.get());

    auto parent = layerHandle->owner.promote();



    if (parent == nullptr || parent->isPendingRemoval()) {

    if (parent == nullptr || parent->isRemoved()) {

        ALOGE("captureLayers called with a removed parent");

        return NAME_NOT_FOUND;

    }

services/surfaceflinger/SurfaceFlinger.h

+0 −5
Original line number Diff line number Diff line
@@ -561,11 +561,6 @@ private: 
    // ISurfaceComposerClient::destroySurface()

    status_t onLayerRemoved(const sp<Client>& client, const sp<IBinder>& handle);



    // called when all clients have released all their references to

    // this layer meaning it is entirely safe to destroy all

    // resources associated to this layer.

    status_t onLayerDestroyed(const wp<Layer>& layer);



    // remove a layer from SurfaceFlinger immediately

    status_t removeLayer(const sp<Layer>& layer, bool topLevelOnly = false);

    status_t removeLayerLocked(const Mutex&, const sp<Layer>& layer, bool topLevelOnly = false);

services/surfaceflinger/tests/Transaction_test.cpp

+31 −0
Original line number Diff line number Diff line
@@ -2582,6 +2582,37 @@ TEST_F(ChildLayerTest, DetachChildrenSameClient) { 
    }

}



TEST_F(ChildLayerTest, ChildrenSurviveParentDestruction) {

    sp<SurfaceControl> mGrandChild =

        mClient->createSurface(String8("Grand Child"), 10, 10,

                PIXEL_FORMAT_RGBA_8888, 0, mChild.get());

    fillSurfaceRGBA8(mGrandChild, 111, 111, 111);



    {

        SCOPED_TRACE("Grandchild visible");

        ScreenCapture::captureScreen(&mCapture);

        mCapture->checkPixel(64, 64, 111, 111, 111);

    }



    mChild->clear();



    {

        SCOPED_TRACE("After destroying child");

        ScreenCapture::captureScreen(&mCapture);

        mCapture->expectFGColor(64, 64);

    }



    asTransaction([&](Transaction& t) {

         t.reparent(mGrandChild, mFGSurfaceControl->getHandle());

    });



    {

        SCOPED_TRACE("After reparenting grandchild");

        ScreenCapture::captureScreen(&mCapture);

        mCapture->checkPixel(64, 64, 111, 111, 111);

    }

}



TEST_F(ChildLayerTest, DetachChildrenDifferentClient) {

    sp<SurfaceComposerClient> mNewComposerClient = new SurfaceComposerClient;

    sp<SurfaceControl> mChildNewClient =