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

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

Merge changes from topic 'layer-wp-race' into oc-dev 

* changes:
  surfaceflinger: Layer::getParent requires state lock held
  surfaceflinger: distinguish mCurrentParent/mDrawingParent
  surfaceflinger: protect Client::mParentLayer with a lock
parents 4e623e25 515dc9c5

services/surfaceflinger/Client.cpp

+15 −4
Original line number Diff line number Diff line
@@ -57,9 +57,19 @@ Client::~Client() 
}



void Client::setParentLayer(const sp<Layer>& parentLayer) {

    Mutex::Autolock _l(mLock);

    mParentLayer = parentLayer;

}



sp<Layer> Client::getParentLayer(bool* outParentDied) const {

    Mutex::Autolock _l(mLock);

    sp<Layer> parent = mParentLayer.promote();

    if (outParentDied != nullptr) {

        *outParentDied = (mParentLayer != nullptr && parent == nullptr);

    }

    return parent;

}



status_t Client::initCheck() const {

    return NO_ERROR;

}
@@ -108,7 +118,7 @@ status_t Client::onTransact( 
     // We grant an exception in the case that the Client has a "parent layer", as its

     // effects will be scoped to that layer.

     if (CC_UNLIKELY(pid != self_pid && uid != AID_GRAPHICS && uid != AID_SYSTEM && uid != 0)

             && (mParentLayer.promote() == nullptr)) {

             && (getParentLayer() == nullptr)) {

         // we're called from a different process, do the real check

         if (!PermissionCache::checkCallingPermission(sAccessSurfaceFlinger))

         {
@@ -135,11 +145,12 @@ status_t Client::createSurface( 
            return NAME_NOT_FOUND;

        }

    }

    if (parent == nullptr && mParentLayer != nullptr) {

        parent = mParentLayer.promote();

    if (parent == nullptr) {

        bool parentDied;

        parent = getParentLayer(&parentDied);

        // If we had a parent, but it died, we've lost all

        // our capabilities.

        if (parent == nullptr) {

        if (parentDied) {

            return NAME_NOT_FOUND;

        }

    }

services/surfaceflinger/Client.h

+2 −1
Original line number Diff line number Diff line
@@ -71,12 +71,13 @@ private: 
    virtual status_t onTransact(

        uint32_t code, const Parcel& data, Parcel* reply, uint32_t flags);



    sp<Layer> getParentLayer(bool* outParentDied = nullptr) const;



    // constant

    sp<SurfaceFlinger> mFlinger;



    // protected by mLock

    DefaultKeyedVector< wp<IBinder>, wp<Layer> > mLayers;



    wp<Layer> mParentLayer;



    // thread-safe

services/surfaceflinger/Layer.cpp

+15 −33
Original line number Diff line number Diff line
@@ -403,7 +403,7 @@ Rect Layer::computeScreenBounds(bool reduceTransparentRegion) const { 
        win.intersect(s.finalCrop, &win);

    }



    const sp<Layer>& p = getParent();

    const sp<Layer>& p = mDrawingParent.promote();

    // Now we need to calculate the parent bounds, so we can clip ourselves to those.

    // When calculating the parent bounds for purposes of clipping,

    // we don't need to constrain the parent to its transparent region.
@@ -440,7 +440,7 @@ Rect Layer::computeBounds(const Region& activeTransparentRegion) const { 
    }



    Rect bounds = win;

    const auto& p = getParent();

    const auto& p = mDrawingParent.promote();

    if (p != nullptr) {

        // Look in computeScreenBounds recursive call for explanation of

        // why we pass false here.
@@ -498,7 +498,7 @@ FloatRect Layer::computeCrop(const sp<const DisplayDevice>& hw) const { 


    // Screen space to make reduction to parent crop clearer.

    Rect activeCrop = computeInitialCrop(hw);

    const auto& p = getParent();

    const auto& p = mDrawingParent.promote();

    if (p != nullptr) {

        auto parentCrop = p->computeInitialCrop(hw);

        activeCrop.intersect(parentCrop, &activeCrop);
@@ -710,7 +710,7 @@ void Layer::setGeometry( 


    int type = s.type;

    int appId = s.appId;

    sp<Layer> parent = mParent.promote();

    sp<Layer> parent = mDrawingParent.promote();

    if (parent.get()) {

        auto& parentState = parent->getDrawingState();

        type = parentState.type;
@@ -1109,8 +1109,9 @@ void Layer::onDraw(const sp<const DisplayDevice>& hw, const Region& clip, 
             * of a camera where the buffer remains in native orientation,

             * we want the pixels to always be upright.

             */

            if (getParent() != nullptr) {

                const auto parentTransform = getParent()->getTransform();

            sp<Layer> p = mDrawingParent.promote();

            if (p != nullptr) {

                const auto parentTransform = p->getTransform();

                tr = tr * inverseOrientation(parentTransform.getOrientation());

            }
@@ -1928,7 +1929,7 @@ bool Layer::setDataSpace(android_dataspace dataSpace) { 
}



uint32_t Layer::getLayerStack() const {

    auto p = getParent();

    auto p = mDrawingParent.promote();

    if (p == nullptr) {

        return getDrawingState().layerStack;

    }
@@ -2071,7 +2072,7 @@ void Layer::releasePendingBuffer(nsecs_t dequeueReadyTime) { 


bool Layer::isHiddenByPolicy() const {

    const Layer::State& s(mDrawingState);

    const auto& parent = getParent();

    const auto& parent = mDrawingParent.promote();

    if (parent != nullptr && parent->isHiddenByPolicy()) {

        return true;

    }
@@ -2555,25 +2556,7 @@ bool Layer::reparentChildren(const sp<IBinder>& newParentHandle) { 
    }



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

        // We don't call addChild as we need to delay updating the child's parent pointer until

        // a transaction occurs. Remember a refresh could occur in between now and the next

        // transaction, in which case the Layer's parent pointer would be updated, but changes

        // made to the parent in the same transaction would not have applied.

        // This means that the following kind of scenario wont work:

        //

        // 1. Existing and visible child and parent surface exist

        // 2. Create new surface hidden

        // 3. Open transaction

        // 4. Show the new surface, and reparent the old surface's children to it.

        // 5. Close transaction.

        //

        // If we were to update the parent pointer immediately, then the child surface

        // could disappear for one frame as it pointed at the new parent which

        // hasn't yet become visible as the transaction hasn't yet occurred.

        //

        // Instead we defer the reparenting to commitChildList which happens as part

        // of the global transaction.

        newParent->mCurrentChildren.add(child);

        newParent->addChild(child);



        sp<Client> client(child->mClientRef.promote());

        if (client != nullptr) {
@@ -2601,7 +2584,7 @@ bool Layer::detachChildren() { 
}



void Layer::setParent(const sp<Layer>& layer) {

    mParent = layer;

    mCurrentParent = layer;

}



void Layer::clearSyncPoints() {
@@ -2691,7 +2674,7 @@ void Layer::traverseInReverseZOrder(LayerVector::StateSet stateSet, 


Transform Layer::getTransform() const {

    Transform t;

    const auto& p = getParent();

    const auto& p = mDrawingParent.promote();

    if (p != nullptr) {

        t = p->getTransform();
@@ -2724,14 +2707,14 @@ Transform Layer::getTransform() const { 


#ifdef USE_HWC2

float Layer::getAlpha() const {

    const auto& p = getParent();

    const auto& p = mDrawingParent.promote();



    float parentAlpha = (p != nullptr) ? p->getAlpha() : 1.0;

    return parentAlpha * getDrawingState().alpha;

}

#else

uint8_t Layer::getAlpha() const {

    const auto& p = getParent();

    const auto& p = mDrawingParent.promote();



    float parentAlpha = (p != nullptr) ? (p->getAlpha() / 255.0f) : 1.0;

    float drawingAlpha = getDrawingState().alpha / 255.0f;
@@ -2743,11 +2726,10 @@ uint8_t Layer::getAlpha() const { 
void Layer::commitChildList() {

    for (size_t i = 0; i < mCurrentChildren.size(); i++) {

        const auto& child = mCurrentChildren[i];

        child->setParent(this);



        child->commitChildList();

    }

    mDrawingChildren = mCurrentChildren;

    mDrawingParent = mCurrentParent;

}



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

services/surfaceflinger/Layer.h

+3 −2
Original line number Diff line number Diff line
@@ -524,7 +524,7 @@ public: 
    // Returns index if removed, or negative value otherwise

    // for symmetry with Vector::remove

    ssize_t removeChild(const sp<Layer>& layer);

    sp<Layer> getParent() const { return mParent.promote(); }

    sp<Layer> getParent() const { return mCurrentParent.promote(); }

    bool hasParent() const { return getParent() != nullptr; }



    Rect computeScreenBounds(bool reduceTransparentRegion = true) const;
@@ -801,7 +801,8 @@ private: 
    // Child list used for rendering.

    LayerVector mDrawingChildren;



    wp<Layer> mParent;

    wp<Layer> mCurrentParent;

    wp<Layer> mDrawingParent;

};



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

services/surfaceflinger/SurfaceFlinger.cpp

+12 −8
Original line number Diff line number Diff line
@@ -2679,14 +2679,18 @@ status_t SurfaceFlinger::addClientLayer(const sp<Client>& client, 
    return NO_ERROR;

}



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

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

    Mutex::Autolock _l(mStateLock);



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

    const ssize_t index = (p != nullptr) ? p->removeChild(layer) :

        mCurrentState.layersSortedByZ.remove(layer);



    ssize_t index;

    if (p != nullptr) {

        if (topLevelOnly) {

            return NO_ERROR;

        }



        index = p->removeChild(layer);



        sp<Layer> ancestor = p;

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

            ancestor = ancestor->getParent();
@@ -2695,6 +2699,8 @@ status_t SurfaceFlinger::removeLayer(const sp<Layer>& layer) { 
            ALOGE("removeLayer called with a layer whose parent has been removed");

            return NAME_NOT_FOUND;

        }

    } else {

        index = mCurrentState.layersSortedByZ.remove(layer);

    }



    // As a matter of normal operation, the LayerCleaner will produce a second
@@ -3125,11 +3131,9 @@ status_t SurfaceFlinger::onLayerDestroyed(const wp<Layer>& layer) 
    if (l == nullptr) {

        // The layer has already been removed, carry on

        return NO_ERROR;

    } if (l->getParent() != nullptr) {

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

        return NO_ERROR;

    }

    return removeLayer(l);

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

    return removeLayer(l, true);

}



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