commit 161410b01a37bcc5522d8e91fe0a743989c86e70
author chaviw <chaviw@google.com> Thu Jul 27 10:46:08 2017 -0700
committer chaviw <chaviw@google.com> Thu Jul 27 13:48:34 2017 -0700
tree c653aeab5fb51dc4e21eaaecf79679c00a0e1cbd
parent 46f28c7b3180e2944ea5553d85c2d534c8fdba00

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

services/surfaceflinger/tests/Transaction_test.cpp

diff --git a/services/surfaceflinger/tests/Transaction_test.cpp b/services/surfaceflinger/tests/Transaction_test.cpp
index 4ce14f8..5d9e8b3 100644
--- a/services/surfaceflinger/tests/Transaction_test.cpp
+++ b/services/surfaceflinger/tests/Transaction_test.cpp
@@ -990,7 +990,7 @@
     }
 }
 
-TEST_F(ChildLayerTest, DetachChildren) {
+TEST_F(ChildLayerTest, DetachChildrenSameClient) {
     SurfaceComposerClient::openGlobalTransaction();
     mChild->show();
     mChild->setPosition(10, 10);
@@ -1015,6 +1015,52 @@
     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);