Merge "Clear remoteSyncPoints for detached layers." into qt-dev am: 9d2f210f09
am: 4380045bf8

Change-Id: I3cbfbfcb9d31d392575a44f70a6ea45d5d6fb9d6
diff --git a/services/surfaceflinger/BufferLayer.cpp b/services/surfaceflinger/BufferLayer.cpp
index a2b6ab6..9d72399 100644
--- a/services/surfaceflinger/BufferLayer.cpp
+++ b/services/surfaceflinger/BufferLayer.cpp
@@ -545,6 +545,12 @@
         if (headFrameNumber >= point->getFrameNumber() && headFenceSignaled &&
             presentTimeIsCurrent) {
             point->setFrameAvailable();
+            sp<Layer> requestedSyncLayer = point->getRequestedSyncLayer();
+            if (requestedSyncLayer) {
+                // Need to update the transaction flag to ensure the layer's pending transaction
+                // gets applied.
+                requestedSyncLayer->setTransactionFlags(eTransactionNeeded);
+            }
         }
     }
 }
diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp
index 379b004..3ca6ef5 100644
--- a/services/surfaceflinger/Layer.cpp
+++ b/services/surfaceflinger/Layer.cpp
@@ -144,6 +144,20 @@
  */
 void Layer::onLayerDisplayed(const sp<Fence>& /*releaseFence*/) {}
 
+void Layer::removeRemoteSyncPoints() {
+    for (auto& point : mRemoteSyncPoints) {
+        point->setTransactionApplied();
+    }
+    mRemoteSyncPoints.clear();
+
+    {
+        Mutex::Autolock pendingStateLock(mPendingStateMutex);
+        for (State pendingState : mPendingStates) {
+            pendingState.barrierLayer_legacy = nullptr;
+        }
+    }
+}
+
 void Layer::onRemovedFromCurrentState() {
     mRemovedFromCurrentState = true;
 
@@ -162,10 +176,7 @@
     // 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();
-    }
-    mRemoteSyncPoints.clear();
+    removeRemoteSyncPoints();
 
     {
     Mutex::Autolock syncLock(mLocalSyncPointMutex);
@@ -667,7 +678,7 @@
             // to be applied as per normal (no synchronization).
             mCurrentState.barrierLayer_legacy = nullptr;
         } else {
-            auto syncPoint = std::make_shared<SyncPoint>(mCurrentState.frameNumber_legacy);
+            auto syncPoint = std::make_shared<SyncPoint>(mCurrentState.frameNumber_legacy, this);
             if (barrierLayer->addSyncPoint(syncPoint)) {
                 mRemoteSyncPoints.push_back(std::move(syncPoint));
             } else {
@@ -1510,6 +1521,7 @@
         if (client != nullptr && parentClient != client) {
             child->mLayerDetached = true;
             child->detachChildren();
+            child->removeRemoteSyncPoints();
         }
     }
 
diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h
index 3712b2a..bbbc5b0 100644
--- a/services/surfaceflinger/Layer.h
+++ b/services/surfaceflinger/Layer.h
@@ -733,8 +733,11 @@
 
     class SyncPoint {
     public:
-        explicit SyncPoint(uint64_t frameNumber)
-              : mFrameNumber(frameNumber), mFrameIsAvailable(false), mTransactionIsApplied(false) {}
+        explicit SyncPoint(uint64_t frameNumber, wp<Layer> requestedSyncLayer)
+              : mFrameNumber(frameNumber),
+                mFrameIsAvailable(false),
+                mTransactionIsApplied(false),
+                mRequestedSyncLayer(requestedSyncLayer) {}
 
         uint64_t getFrameNumber() const { return mFrameNumber; }
 
@@ -746,10 +749,13 @@
 
         void setTransactionApplied() { mTransactionIsApplied = true; }
 
+        sp<Layer> getRequestedSyncLayer() { return mRequestedSyncLayer.promote(); }
+
     private:
         const uint64_t mFrameNumber;
         std::atomic<bool> mFrameIsAvailable;
         std::atomic<bool> mTransactionIsApplied;
+        wp<Layer> mRequestedSyncLayer;
     };
 
     // SyncPoints which will be signaled when the correct frame is at the head
@@ -928,6 +934,8 @@
     void setZOrderRelativeOf(const wp<Layer>& relativeOf);
 
     bool mGetHandleCalled = false;
+
+    void removeRemoteSyncPoints();
 };
 
 } // namespace android
diff --git a/services/surfaceflinger/tests/Transaction_test.cpp b/services/surfaceflinger/tests/Transaction_test.cpp
index ec1ac4b..62db5ec 100644
--- a/services/surfaceflinger/tests/Transaction_test.cpp
+++ b/services/surfaceflinger/tests/Transaction_test.cpp
@@ -4725,6 +4725,48 @@
         mCapture->expectColor(Rect(20, 20, 52, 52), Color::RED);
     }
 }
+TEST_F(ChildLayerTest, DetachChildrenWithDeferredTransaction) {
+    sp<SurfaceComposerClient> newComposerClient = new SurfaceComposerClient;
+    sp<SurfaceControl> childNewClient =
+            newComposerClient->createSurface(String8("New Child Test Surface"), 10, 10,
+                                             PIXEL_FORMAT_RGBA_8888, 0, mFGSurfaceControl.get());
+
+    ASSERT_TRUE(childNewClient != nullptr);
+    ASSERT_TRUE(childNewClient->isValid());
+
+    fillSurfaceRGBA8(childNewClient, 200, 200, 200);
+
+    Transaction()
+            .hide(mChild)
+            .show(childNewClient)
+            .setPosition(childNewClient, 10, 10)
+            .setPosition(mFGSurfaceControl, 64, 64)
+            .apply();
+
+    {
+        mCapture = screenshot();
+        Rect rect = Rect(74, 74, 84, 84);
+        mCapture->expectBorder(rect, Color{195, 63, 63, 255});
+        mCapture->expectColor(rect, Color{200, 200, 200, 255});
+    }
+
+    Transaction()
+            .deferTransactionUntil_legacy(childNewClient, mFGSurfaceControl->getHandle(),
+                                          mFGSurfaceControl->getSurface()->getNextFrameNumber())
+            .apply();
+    Transaction().detachChildren(mFGSurfaceControl).apply();
+    ASSERT_NO_FATAL_FAILURE(fillBufferQueueLayerColor(mFGSurfaceControl, Color::RED, 32, 32));
+
+    // BufferLayer can still dequeue buffers even though there's a detached layer with a
+    // deferred transaction.
+    {
+        SCOPED_TRACE("new buffer");
+        mCapture = screenshot();
+        Rect rect = Rect(74, 74, 84, 84);
+        mCapture->expectBorder(rect, Color::RED);
+        mCapture->expectColor(rect, Color{200, 200, 200, 255});
+    }
+}
 
 TEST_F(ChildLayerTest, ChildrenInheritNonTransformScalingFromParent) {
     asTransaction([&](Transaction& t) {