commit 6b9ffea5e6bdc56c2a54bb6a1c8b18ee86ce8d48
author chaviw <chaviw@google.com> Mon Nov 08 09:25:48 2021 -0600
committer chaviw <chaviw@google.com> Tue Nov 09 13:33:11 2021 -0600
tree 647f02f9706a3df31fcbdffe0909ed0f723d7e5c
parent 982775d747c7767b79b955dccf7f8ec9f400035d

Invoke commit callback after latch buffers

If commit callback is invoked before the buffers can be latched, they
won't be included in the commit callback and would only be invoked in
the complete callback. This defeats the purpose of the commit callback
as an early indicator that the buffer has been committed.

Move the commit callback invocation so it's after latchBuffers

This also fixes a bug where offscreen layers could be latched in
flushAndCommitTransactions, so only offscreen layers would get the
commit callback. This would cause issues if the offscreen layer
transaction was merged with a layer that was on screen, preventing the
on screen layer from getting a commit callback.

Test: LayerCallbackTest#CommitCallbackOffscreenLayer
Test: BLASTBufferQueueTest
Test: BLASTBufferQueueTransformTest
Test: BLASTFrameEventHistoryTest
Fixes: 205563588
Change-Id: I5692211dd7717258829be8eb8a68b2dcb4a5498d

services/surfaceflinger/tests/LayerCallback_test.cpp

diff --git a/services/surfaceflinger/tests/LayerCallback_test.cpp b/services/surfaceflinger/tests/LayerCallback_test.cpp
index 91a5b52..5c16fee 100644
--- a/services/surfaceflinger/tests/LayerCallback_test.cpp
+++ b/services/surfaceflinger/tests/LayerCallback_test.cpp
@@ -26,6 +26,7 @@
 namespace android {
 
 using android::hardware::graphics::common::V1_1::BufferUsage;
+using SCHash = SurfaceComposerClient::SCHash;
 
 ::testing::Environment* const binderEnv =
         ::testing::AddGlobalTestEnvironment(new BinderEnvironment());
@@ -102,6 +103,24 @@
         }
     }
 
+    static void waitForCommitCallback(
+            CallbackHelper& helper,
+            const std::unordered_set<sp<SurfaceControl>, SCHash>& committedSc) {
+        CallbackData callbackData;
+        ASSERT_NO_FATAL_FAILURE(helper.getCallbackData(&callbackData));
+
+        const auto& surfaceControlStats = callbackData.surfaceControlStats;
+
+        ASSERT_EQ(surfaceControlStats.size(), committedSc.size()) << "wrong number of surfaces";
+
+        for (const auto& stats : surfaceControlStats) {
+            ASSERT_NE(stats.surfaceControl, nullptr) << "returned null surface control";
+
+            const auto& expectedSc = committedSc.find(stats.surfaceControl);
+            ASSERT_NE(expectedSc, committedSc.end()) << "unexpected surface control";
+        }
+    }
+
     DisplayEventReceiver mDisplayEventReceiver;
     int mEpollFd;
 
@@ -1085,4 +1104,29 @@
     EXPECT_NO_FATAL_FAILURE(waitForCallback(callback, expected, true));
 }
 
+TEST_F(LayerCallbackTest, CommitCallbackOffscreenLayer) {
+    sp<SurfaceControl> layer;
+    ASSERT_NO_FATAL_FAILURE(layer = createBufferStateLayer());
+    sp<SurfaceControl> offscreenLayer =
+            createSurface(mClient, "Offscreen Layer", 0, 0, PIXEL_FORMAT_RGBA_8888,
+                          ISurfaceComposerClient::eFXSurfaceBufferState, layer.get());
+
+    Transaction transaction;
+    CallbackHelper callback;
+    int err = fillTransaction(transaction, &callback, layer, true);
+    err |= fillTransaction(transaction, &callback, offscreenLayer, true);
+    if (err) {
+        GTEST_SUCCEED() << "test not supported";
+        return;
+    }
+
+    transaction.reparent(offscreenLayer, nullptr)
+            .addTransactionCommittedCallback(callback.function, callback.getContext());
+    transaction.apply();
+
+    std::unordered_set<sp<SurfaceControl>, SCHash> committedSc;
+    committedSc.insert(layer);
+    committedSc.insert(offscreenLayer);
+    EXPECT_NO_FATAL_FAILURE(waitForCommitCallback(callback, committedSc));
+}
 } // namespace android