Merge "Re-parent invoked on child instead of on parent."

11 files changed, 94 insertions, 54 deletions

diff --git a/libs/gui/LayerState.cpp b/libs/gui/LayerState.cpp
index 573f6856d6..3418a4983e 100644
--- a/libs/gui/LayerState.cpp
+++ b/libs/gui/LayerState.cpp
@@ -46,7 +46,6 @@ status_t layer_state_t::write(Parcel& output) const
     output.writeStrongBinder(IInterface::asBinder(barrierGbp));
     output.writeStrongBinder(relativeLayerHandle);
     output.writeStrongBinder(parentHandleForChild);
-    output.writeStrongBinder(childHandle);
     output.write(transparentRegion);
     return NO_ERROR;
 }
@@ -80,7 +79,6 @@ status_t layer_state_t::read(const Parcel& input)
         interface_cast<IGraphicBufferProducer>(input.readStrongBinder());
     relativeLayerHandle = input.readStrongBinder();
     parentHandleForChild = input.readStrongBinder();
-    childHandle = input.readStrongBinder();
     input.read(transparentRegion);
     return NO_ERROR;
 }
diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp
index b0ae7e0bdf..be7b1d2f6a 100644
--- a/libs/gui/SurfaceComposerClient.cpp
+++ b/libs/gui/SurfaceComposerClient.cpp
@@ -176,9 +176,8 @@ public:
     status_t reparentChildren(const sp<SurfaceComposerClient>& client,
             const sp<IBinder>& id,
             const sp<IBinder>& newParentHandle);
-    status_t reparentChild(const sp<SurfaceComposerClient>& client,
-            const sp<IBinder>& id, const sp<IBinder>& newParentHandle,
-            const sp<IBinder>& childHandle);
+    status_t reparent(const sp<SurfaceComposerClient>& client,
+            const sp<IBinder>& id, const sp<IBinder>& newParentHandle);
     status_t detachChildren(const sp<SurfaceComposerClient>& client,
             const sp<IBinder>& id);
     status_t setOverrideScalingMode(const sp<SurfaceComposerClient>& client,
@@ -496,18 +495,16 @@ status_t Composer::reparentChildren(
     return NO_ERROR;
 }
 
-status_t Composer::reparentChild(const sp<SurfaceComposerClient>& client,
+status_t Composer::reparent(const sp<SurfaceComposerClient>& client,
         const sp<IBinder>& id,
-        const sp<IBinder>& newParentHandle,
-        const sp<IBinder>& childHandle) {
+        const sp<IBinder>& newParentHandle) {
     Mutex::Autolock lock(mLock);
     layer_state_t* s = getLayerStateLocked(client, id);
     if (!s) {
         return BAD_INDEX;
     }
-    s->what |= layer_state_t::eReparentChild;
+    s->what |= layer_state_t::eReparent;
     s->parentHandleForChild = newParentHandle;
-    s->childHandle = childHandle;
     return NO_ERROR;
 }
 
@@ -849,9 +846,9 @@ status_t SurfaceComposerClient::reparentChildren(const sp<IBinder>& id,
     return getComposer().reparentChildren(this, id, newParentHandle);
 }
 
-status_t SurfaceComposerClient::reparentChild(const sp<IBinder>& id,
-        const sp<IBinder>& newParentHandle, const sp<IBinder>& childHandle) {
-    return getComposer().reparentChild(this, id, newParentHandle, childHandle);
+status_t SurfaceComposerClient::reparent(const sp<IBinder>& id,
+        const sp<IBinder>& newParentHandle) {
+    return getComposer().reparent(this, id, newParentHandle);
 }
 
 status_t SurfaceComposerClient::detachChildren(const sp<IBinder>& id) {
diff --git a/libs/gui/SurfaceControl.cpp b/libs/gui/SurfaceControl.cpp
index b9c5ef9580..d801d12738 100644
--- a/libs/gui/SurfaceControl.cpp
+++ b/libs/gui/SurfaceControl.cpp
@@ -191,11 +191,10 @@ status_t SurfaceControl::reparentChildren(const sp<IBinder>& newParentHandle) {
     return mClient->reparentChildren(mHandle, newParentHandle);
 }
 
-status_t SurfaceControl::reparentChild(const sp<IBinder>& newParentHandle,
-        const sp<IBinder>& childHandle) {
+status_t SurfaceControl::reparent(const sp<IBinder>& newParentHandle) {
     status_t err = validate();
     if (err < 0) return err;
-    return mClient->reparentChild(mHandle, newParentHandle, childHandle);
+    return mClient->reparent(mHandle, newParentHandle);
 }
 
 status_t SurfaceControl::detachChildren() {
diff --git a/libs/gui/include/gui/SurfaceComposerClient.h b/libs/gui/include/gui/SurfaceComposerClient.h
index 6e2cb83544..cf2ff5b96e 100644
--- a/libs/gui/include/gui/SurfaceComposerClient.h
+++ b/libs/gui/include/gui/SurfaceComposerClient.h
@@ -161,8 +161,7 @@ public:
             const sp<Surface>& handle, uint64_t frameNumber);
     status_t    reparentChildren(const sp<IBinder>& id,
             const sp<IBinder>& newParentHandle);
-    status_t    reparentChild(const sp<IBinder>& id, const sp<IBinder>& newParentHandle,
-            const sp<IBinder>& childHandle);
+    status_t    reparent(const sp<IBinder>& id, const sp<IBinder>& newParentHandle);
     status_t    detachChildren(const sp<IBinder>& id);
     status_t    setOverrideScalingMode(const sp<IBinder>& id,
             int32_t overrideScalingMode);
diff --git a/libs/gui/include/gui/SurfaceControl.h b/libs/gui/include/gui/SurfaceControl.h
index d8b67ef96a..b506e00672 100644
--- a/libs/gui/include/gui/SurfaceControl.h
+++ b/libs/gui/include/gui/SurfaceControl.h
@@ -124,11 +124,10 @@ public:
     // Reparents all children of this layer to the new parent handle.
     status_t reparentChildren(const sp<IBinder>& newParentHandle);
 
-    // Reparents a specified child from this layer to the new parent handle.
-    // The child, parent, and new parent must all have the same client.
+    // Reparents the current layer to the new parent handle. The new parent must not be null.
     // This can be used instead of reparentChildren if the caller wants to
-    // only re-parent specific children.
-    status_t reparentChild(const sp<IBinder>& newParentHandle, const sp<IBinder>& childHandle);
+    // only re-parent a specific child.
+    status_t reparent(const sp<IBinder>& newParentHandle);
 
     // Detaches all child surfaces (and their children recursively)
     // from their SurfaceControl.
diff --git a/libs/gui/include/private/gui/LayerState.h b/libs/gui/include/private/gui/LayerState.h
index 4f73e04e22..4ff2e5e0d6 100644
--- a/libs/gui/include/private/gui/LayerState.h
+++ b/libs/gui/include/private/gui/LayerState.h
@@ -60,7 +60,7 @@ struct layer_state_t {
         eReparentChildren           = 0x00002000,
         eDetachChildren             = 0x00004000,
         eRelativeLayerChanged       = 0x00008000,
-        eReparentChild              = 0x00010000
+        eReparent                   = 0x00010000
     };
 
     layer_state_t()
@@ -109,7 +109,6 @@ struct layer_state_t {
             sp<IBinder>     relativeLayerHandle;
 
             sp<IBinder>     parentHandleForChild;
-            sp<IBinder>     childHandle;
 
             // non POD must be last. see write/read
             Region          transparentRegion;
diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp
index 9435a187d9..fd30e1614b 100755
--- a/services/surfaceflinger/Layer.cpp
+++ b/services/surfaceflinger/Layer.cpp
@@ -2625,8 +2625,8 @@ bool Layer::reparentChildren(const sp<IBinder>& newParentHandle) {
     return true;
 }
 
-bool Layer::reparentChild(const sp<IBinder>& newParentHandle, const sp<IBinder>& childHandle) {
-    if (newParentHandle == nullptr || childHandle == nullptr) {
+bool Layer::reparent(const sp<IBinder>& newParentHandle) {
+    if (newParentHandle == nullptr) {
         return false;
     }
 
@@ -2637,29 +2637,19 @@ bool Layer::reparentChild(const sp<IBinder>& newParentHandle, const sp<IBinder>&
         return false;
     }
 
-    handle = static_cast<Handle*>(childHandle.get());
-    sp<Layer> child = handle->owner.promote();
-    if (child == nullptr) {
-        ALOGE("Unable to promote child Layer handle");
-        return false;
-    }
-
-    if (mCurrentChildren.indexOf(child) < 0) {
-        ALOGE("Child layer is not child of current layer");
-        return false;
+    sp<Layer> parent = getParent();
+    if (parent != nullptr) {
+        parent->removeChild(this);
     }
+    newParent->addChild(this);
 
-    sp<Client> parentClient(mClientRef.promote());
-    sp<Client> childClient(child->mClientRef.promote());
+    sp<Client> client(mClientRef.promote());
     sp<Client> newParentClient(newParent->mClientRef.promote());
 
-    if (parentClient != childClient || childClient != newParentClient) {
-        ALOGE("Current layer, child layer, and new parent layer must have the same client");
-        return false;
+    if (client != newParentClient) {
+        client->setParentLayer(newParent);
     }
 
-    newParent->addChild(child);
-    mCurrentChildren.remove(child);
     return true;
 }
 
diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h
index f94833b32f..e7ece4579e 100644
--- a/services/surfaceflinger/Layer.h
+++ b/services/surfaceflinger/Layer.h
@@ -241,7 +241,7 @@ public:
     bool setOverrideScalingMode(int32_t overrideScalingMode);
     void setInfo(uint32_t type, uint32_t appId);
     bool reparentChildren(const sp<IBinder>& layer);
-    bool reparentChild(const sp<IBinder>& newParentHandle, const sp<IBinder>& childHandle);
+    bool reparent(const sp<IBinder>& newParentHandle);
     bool detachChildren();
 
     // If we have received a new buffer this frame, we will pass its surface
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index cae4deacb0..6ee14fe72a 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -3121,10 +3121,8 @@ uint32_t SurfaceFlinger::setClientStateLocked(
             // We don't trigger a traversal here because if no other state is
             // changed, we don't want this to cause any more work
         }
-        // Always re-parent the children that explicitly requested to get
-        // re-parented before the general re-parent of all children.
-        if (what & layer_state_t::eReparentChild) {
-            if (layer->reparentChild(s.parentHandleForChild, s.childHandle)) {
+        if (what & layer_state_t::eReparent) {
+            if (layer->reparent(s.parentHandleForChild)) {
                 flags |= eTransactionNeeded|eTraversalNeeded;
             }
         }
diff --git a/services/surfaceflinger/SurfaceFlinger_hwc1.cpp b/services/surfaceflinger/SurfaceFlinger_hwc1.cpp
index 3b4db0574f..b1c8c0ab69 100644
--- a/services/surfaceflinger/SurfaceFlinger_hwc1.cpp
+++ b/services/surfaceflinger/SurfaceFlinger_hwc1.cpp
@@ -2681,10 +2681,8 @@ uint32_t SurfaceFlinger::setClientStateLocked(
             // We don't trigger a traversal here because if no other state is
             // changed, we don't want this to cause any more work
         }
-        // Always re-parent the children that explicitly requested to get
-        // re-parented before the general re-parent of all children.
-        if (what & layer_state_t::eReparentChild) {
-            if (layer->reparentChild(s.parentHandleForChild, s.childHandle)) {
+        if (what & layer_state_t::eReparent) {
+            if (layer->reparent(s.parentHandleForChild)) {
                 flags |= eTransactionNeeded|eTraversalNeeded;
             }
         }
diff --git a/services/surfaceflinger/tests/Transaction_test.cpp b/services/surfaceflinger/tests/Transaction_test.cpp
index dea6503e4e..21194926da 100644
--- a/services/surfaceflinger/tests/Transaction_test.cpp
+++ b/services/surfaceflinger/tests/Transaction_test.cpp
@@ -1169,7 +1169,7 @@ TEST_F(ChildLayerTest, Bug36858924) {
     fillSurfaceRGBA8(mFGSurfaceControl, 0, 255, 0);
 }
 
-TEST_F(ChildLayerTest, ReparentChild) {
+TEST_F(ChildLayerTest, Reparent) {
     SurfaceComposerClient::openGlobalTransaction();
     mChild->show();
     mChild->setPosition(10, 10);
@@ -1185,7 +1185,7 @@ TEST_F(ChildLayerTest, ReparentChild) {
         // And 10 more pixels we should be back to the foreground surface
         mCapture->expectFGColor(84, 84);
     }
-    mFGSurfaceControl->reparentChild(mBGSurfaceControl->getHandle(), mChild->getHandle());
+    mChild->reparent(mBGSurfaceControl->getHandle());
     {
         ScreenCapture::captureScreen(&mCapture);
         mCapture->expectFGColor(64, 64);
@@ -1198,6 +1198,69 @@ TEST_F(ChildLayerTest, ReparentChild) {
     }
 }
 
+TEST_F(ChildLayerTest, ReparentToNoParent) {
+    SurfaceComposerClient::openGlobalTransaction();
+    mChild->show();
+    mChild->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);
+    }
+    mChild->reparent(nullptr);
+    {
+        ScreenCapture::captureScreen(&mCapture);
+        // Nothing should have changed.
+        mCapture->expectFGColor(64, 64);
+        mCapture->expectChildColor(74, 74);
+        mCapture->expectFGColor(84, 84);
+    }
+}
+
+TEST_F(ChildLayerTest, ReparentFromNoParent) {
+    sp<SurfaceControl> newSurface = mComposerClient->createSurface(
+        String8("New Surface"), 10, 10, PIXEL_FORMAT_RGBA_8888, 0);
+    ASSERT_TRUE(newSurface != NULL);
+    ASSERT_TRUE(newSurface->isValid());
+
+    fillSurfaceRGBA8(newSurface, 63, 195, 63);
+    SurfaceComposerClient::openGlobalTransaction();
+    mChild->hide();
+    newSurface->show();
+    newSurface->setPosition(10, 10);
+    newSurface->setLayer(INT32_MAX-2);
+    mFGSurfaceControl->setPosition(64, 64);
+    SurfaceComposerClient::closeGlobalTransaction(true);
+
+    {
+        ScreenCapture::captureScreen(&mCapture);
+        // Top left of foreground must now be visible
+        mCapture->expectFGColor(64, 64);
+        // At 10, 10 we should see the new surface
+        mCapture->checkPixel(10, 10, 63, 195, 63);
+    }
+
+    SurfaceComposerClient::openGlobalTransaction();
+    newSurface->reparent(mFGSurfaceControl->getHandle());
+    SurfaceComposerClient::closeGlobalTransaction(true);
+
+    {
+        ScreenCapture::captureScreen(&mCapture);
+        // newSurface will now be a child of mFGSurface so it will be 10, 10 offset from
+        // mFGSurface, putting it at 74, 74.
+        mCapture->expectFGColor(64, 64);
+        mCapture->checkPixel(74, 74, 63, 195, 63);
+        mCapture->expectFGColor(84, 84);
+    }
+}
+
 TEST_F(ChildLayerTest, NestedChildren) {
     sp<SurfaceControl> grandchild = mComposerClient->createSurface(
         String8("Grandchild surface"),