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

Commit 161410b0 authored by chaviw's avatar chaviw
Browse files

Only detach children from a different client than parent. 

The purpose of detach is prevent another client from modifying
the layer. Therefore, layers with the same client as their parent
shouldn't be detached because the parent can and should be able
to still modify those layers.

Test: Updated Transaction_test so it has a test when the parent detaches
children with same client and detaches children with different client.

Change-Id: Ieede3c7b472e6eaa567163876b71cc9d7e889daf
parent 46f28c7b

services/surfaceflinger/Client.cpp

+2 −1
Original line number Diff line number Diff line
@@ -140,7 +140,8 @@ status_t Client::createSurface( 
{

    sp<Layer> parent = nullptr;

    if (parentHandle != nullptr) {

        parent = getLayerUser(parentHandle);

        auto layerHandle = reinterpret_cast<Layer::Handle*>(parentHandle.get());

        parent = layerHandle->owner.promote();

        if (parent == nullptr) {

            return NAME_NOT_FOUND;

        }

services/surfaceflinger/Layer.cpp

+2 −1
Original line number Diff line number Diff line
@@ -2591,8 +2591,9 @@ bool Layer::detachChildren() { 
            return;

        }



        sp<Client> parentClient = mClientRef.promote();

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

        if (client != nullptr) {

        if (client != nullptr && parentClient != client) {

            client->detachLayer(child);

        }

    });

services/surfaceflinger/tests/Transaction_test.cpp

+47 −1
Original line number Diff line number Diff line
@@ -990,7 +990,7 @@ TEST_F(ChildLayerTest, ReparentChildren) { 
    }

}



TEST_F(ChildLayerTest, DetachChildren) {

TEST_F(ChildLayerTest, DetachChildrenSameClient) {

    SurfaceComposerClient::openGlobalTransaction();

    mChild->show();

    mChild->setPosition(10, 10);
@@ -1015,6 +1015,52 @@ TEST_F(ChildLayerTest, DetachChildren) { 
    mChild->hide();

    SurfaceComposerClient::closeGlobalTransaction(true);



    // Since the child has the same client as the parent, it will not get

    // detached and will be hidden.

    {

        ScreenCapture::captureScreen(&mCapture);

        mCapture->expectFGColor(64, 64);

        mCapture->expectFGColor(74, 74);

        mCapture->expectFGColor(84, 84);

    }

}



TEST_F(ChildLayerTest, DetachChildrenDifferentClient) {

    sp<SurfaceComposerClient> mNewComposerClient = new SurfaceComposerClient;

    sp<SurfaceControl> mChildNewClient = mNewComposerClient->createSurface(

        String8("New Child Test Surface"), 10, 10, PIXEL_FORMAT_RGBA_8888,

        0, mFGSurfaceControl.get());



    ASSERT_TRUE(mChildNewClient != NULL);

    ASSERT_TRUE(mChildNewClient->isValid());



    fillSurfaceRGBA8(mChildNewClient, 200, 200, 200);



    SurfaceComposerClient::openGlobalTransaction();

    mChild->hide();

    mChildNewClient->show();

    mChildNewClient->setPosition(10, 10);

    mFGSurfaceControl->setPosition(64, 64);

    SurfaceComposerClient::closeGlobalTransaction(true);



    {

        ScreenCapture::captureScreen(&mCapture);

        // Top left of foreground must now be visible

        mCapture->expectFGColor(64, 64);

        // But 10 pixels in we should see the child surface

        mCapture->expectChildColor(74, 74);

        // And 10 more pixels we should be back to the foreground surface

        mCapture->expectFGColor(84, 84);

    }



    SurfaceComposerClient::openGlobalTransaction();

    mFGSurfaceControl->detachChildren();

    SurfaceComposerClient::closeGlobalTransaction(true);



    SurfaceComposerClient::openGlobalTransaction();

    mChildNewClient->hide();

    SurfaceComposerClient::closeGlobalTransaction(true);



    // Nothing should have changed.

    {

        ScreenCapture::captureScreen(&mCapture);