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

Commit 013f5c0d authored by TreeHugger Robot's avatar TreeHugger Robot Committed by Android (Google) Code Review
Browse files

Merge "Revert "SurfaceFlinger: Share ownership of layers between State and Handle.""

parents 069c68b7 10c985e0

services/surfaceflinger/Layer.cpp

+18 −29
Original line number Diff line number Diff line
@@ -119,20 +119,13 @@ Layer::~Layer() { 
        c->detachLayer(this);

    }



    mFrameTracker.logAndResetStats(mName);



    // The remote sync points are cleared out when we are

    // removed from current state.

    Mutex::Autolock lock(mLocalSyncPointMutex);

    for (auto& point : mRemoteSyncPoints) {

        point->setTransactionApplied();

    }

    for (auto& point : mLocalSyncPoints) {

        point->setFrameAvailable();

    }



    abandon();



    destroyAllHwcLayers();



    mFlinger->onLayerDestroyed();

    mFrameTracker.logAndResetStats(mName);

}



// ---------------------------------------------------------------------------
@@ -147,9 +140,10 @@ Layer::~Layer() { 
void Layer::onLayerDisplayed(const sp<Fence>& /*releaseFence*/) {}



void Layer::onRemovedFromCurrentState() {

    mRemovedFromCurrentState = true;



    // the layer is removed from SF mCurrentState to mLayersPendingRemoval



    mPendingRemoval = true;



    if (mCurrentState.zOrderRelativeOf != nullptr) {

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

        if (strongRelative != nullptr) {
@@ -159,18 +153,19 @@ void Layer::onRemovedFromCurrentState() { 
        mCurrentState.zOrderRelativeOf = nullptr;

    }



    // Since we are no longer reachable from CurrentState SurfaceFlinger

    // will no longer invoke doTransaction for us, and so we will

    // never finish applying transactions. We signal the sync point

    // now so that another layer will not become indefinitely

    // blocked.

    for (auto& point: mRemoteSyncPoints) {

        point->setTransactionApplied();

    for (const auto& child : mCurrentChildren) {

        child->onRemovedFromCurrentState();

    }

}

    mRemoteSyncPoints.clear();



void Layer::onRemoved() {

    // the layer is removed from SF mLayersPendingRemoval

    abandon();



    destroyAllHwcLayers();



    for (const auto& child : mCurrentChildren) {

        child->onRemovedFromCurrentState();

        child->onRemoved();

    }

}
@@ -233,10 +228,6 @@ void Layer::destroyAllHwcLayers() { 
    }

    LOG_ALWAYS_FATAL_IF(!getBE().mHwcLayers.empty(),

                        "All hardware composer layers should have been destroyed");



    for (const sp<Layer>& child : mDrawingChildren) {

        child->destroyAllHwcLayers();

    }

}



Rect Layer::getContentCrop() const {
@@ -834,9 +825,7 @@ void Layer::pushPendingState() { 


    // If this transaction is waiting on the receipt of a frame, generate a sync

    // point and send it to the remote layer.

    // We don't allow installing sync points after we are removed from the current state

    // as we won't be able to signal our end.

    if (mCurrentState.barrierLayer_legacy != nullptr && !mRemovedFromCurrentState) {

    if (mCurrentState.barrierLayer_legacy != nullptr) {

        sp<Layer> barrierLayer = mCurrentState.barrierLayer_legacy.promote();

        if (barrierLayer == nullptr) {

            ALOGE("[%s] Unable to promote barrier Layer.", mName.string());

services/surfaceflinger/Layer.h

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





    bool isRemovedFromCurrentState() const { return mRemovedFromCurrentState; }

    bool isPendingRemoval() const { return mPendingRemoval; }



    void writeToProto(LayerProto* layerInfo,

                      LayerVector::StateSet stateSet = LayerVector::StateSet::Drawing);
@@ -475,6 +475,12 @@ public: 
     */

    void onRemovedFromCurrentState();



    /*

     * called with the state lock from the main thread when the layer is

     * removed from the pending removal list

     */

    void onRemoved();



    // Updates the transform hint in our SurfaceFlingerConsumer to match

    // the current orientation of the display device.

    void updateTransformHint(const sp<const DisplayDevice>& display) const;
@@ -589,12 +595,12 @@ protected: 
     */

    class LayerCleaner {

        sp<SurfaceFlinger> mFlinger;

        sp<Layer> mLayer;

        wp<Layer> mLayer;



    protected:

        ~LayerCleaner() {

            // destroy client resources

            mFlinger->onHandleDestroyed(mLayer);

            mFlinger->onLayerDestroyed(mLayer);

        }



    public:
@@ -696,8 +702,6 @@ public: 
    virtual PixelFormat getPixelFormat() const { return PIXEL_FORMAT_NONE; }

    bool getPremultipledAlpha() const;



    bool mPendingHWCDestroy{false};



protected:

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

    bool usingRelativeZ(LayerVector::StateSet stateSet);
@@ -741,7 +745,7 @@ protected: 
    // Whether filtering is needed b/c of the drawingstate

    bool mNeedsFiltering{false};



    bool mRemovedFromCurrentState{false};

    bool mPendingRemoval{false};



    // page-flip thread (currently main thread)

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

services/surfaceflinger/SurfaceFlinger.cpp

+59 −21
Original line number Diff line number Diff line
@@ -2723,14 +2723,7 @@ void SurfaceFlinger::commitTransaction() 
        for (const auto& l : mLayersPendingRemoval) {

            recordBufferingStats(l->getName().string(),

                    l->getOccupancyHistory(true));



            // We need to release the HWC layers when the Layer is removed

            // from the current state otherwise the HWC layer just continues

            // showing at it's last configured state until we eventually

            // abandon the buffer queue.

            if (l->isRemovedFromCurrentState()) {

                l->destroyAllHwcLayers();

            }

            l->onRemoved();

        }

        mLayersPendingRemoval.clear();

    }
@@ -3163,6 +3156,10 @@ status_t SurfaceFlinger::addClientLayer(const sp<Client>& client, 
        if (parent == nullptr) {

            mCurrentState.layersSortedByZ.add(lbc);

        } else {

            if (parent->isPendingRemoval()) {

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

                return NAME_NOT_FOUND;

            }

            parent->addChild(lbc);

        }
@@ -3189,22 +3186,52 @@ status_t SurfaceFlinger::removeLayer(const sp<Layer>& layer, bool topLevelOnly) 
    return removeLayerLocked(mStateLock, layer, topLevelOnly);

}



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

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

                                           bool topLevelOnly) {

    if (layer->isPendingRemoval()) {

        return NO_ERROR;

    }



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

    ssize_t index;

    if (p != nullptr) {

        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);

    }



    layer->onRemovedFromCurrentState();

    // 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;

    }



    markLayerPendingRemovalLocked(lock, layer);

    layer->onRemovedFromCurrentState();

    mLayersPendingRemoval.add(layer);

    mLayersRemoved = true;

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

    setTransactionFlags(eTransactionNeeded);

    return NO_ERROR;

}
@@ -3405,6 +3432,11 @@ uint32_t SurfaceFlinger::setClientStateLocked(const ComposerState& composerState 
        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;
@@ -3608,6 +3640,11 @@ void SurfaceFlinger::setDestroyStateLocked(const ComposerState& composerState) { 
        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);

    }
@@ -3780,16 +3817,17 @@ status_t SurfaceFlinger::onLayerRemoved(const sp<Client>& client, const sp<IBind 
    return err;

}



void SurfaceFlinger::markLayerPendingRemovalLocked(const Mutex&, const sp<Layer>& layer) {

    mLayersPendingRemoval.add(layer);

    mLayersRemoved = true;

    setTransactionFlags(eTransactionNeeded);

}



void SurfaceFlinger::onHandleDestroyed(const sp<Layer>& layer)

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

{

    Mutex::Autolock lock(mStateLock);

    markLayerPendingRemovalLocked(mStateLock, 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);

}



// ---------------------------------------------------------------------------
@@ -5182,7 +5220,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->isRemovedFromCurrentState()) {

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

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

        return NAME_NOT_FOUND;

    }

services/surfaceflinger/SurfaceFlinger.h

+1 −5
Original line number Diff line number Diff line
@@ -356,8 +356,6 @@ public: 
    bool authenticateSurfaceTextureLocked(

        const sp<IGraphicBufferProducer>& bufferProducer) const;



    inline void onLayerDestroyed() { mNumLayers--; }



private:

    friend class Client;

    friend class DisplayEventConnection;
@@ -563,12 +561,10 @@ private: 
    // ISurfaceComposerClient::destroySurface()

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



    void markLayerPendingRemovalLocked(const Mutex& /* mStateLock */, const sp<Layer>& layer);



    // 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.

    void onHandleDestroyed(const sp<Layer>& layer);

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



    // remove a layer from SurfaceFlinger immediately

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

services/surfaceflinger/tests/Transaction_test.cpp

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

}



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, DetachChildrenSameClient) {

    asTransaction([&](Transaction& t) {

        t.show(mChild);