commit 286f4f9e6c32c0b906eba7c103964aa55ba1f74a
author Vishnu Nair <vishnun@google.com> Wed Jun 08 16:37:39 2022 -0700
committer Vishnu Nair <vishnun@google.com> Thu Jun 09 18:11:07 2022 -0700
tree 3c1f649ca0eb7f35f030ec7a813511e7ea7eec71
parent e3066de6b0baca24e0da92e12dc17463f614c432

LayerTraceGenerator: Fix duplicate layer ids

Layers can be created from API calls on the binder thread or
by surfaceflinger on the main thread for mirroring and
BSL background layers. Transaction traces keep track of
layer creation API calls to recreate layers when generating
the layer trace.

However, there was a possibility that new layers would be
recorded with a set of transactions that were applied with
an earlier vsync. This is harmless except we could end up
creating layers with duplicate layer ids since layers created
via binder would get an injected sequence id while main
thread created layers would use the global counter.

Test: atest transactiontrace_testsuite
Bug: 235376060
Change-Id: Ia11839d1880bc131ab5314779281c93393d6742f

services/surfaceflinger/SurfaceFlinger.cpp

diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 3043809..79320cd 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -2056,6 +2056,11 @@
         mGpuFrameMissedCount++;
     }
 
+    if (mTracingEnabledChanged) {
+        mLayerTracingEnabled = mLayerTracing.isEnabled();
+        mTracingEnabledChanged = false;
+    }
+
     // If we are in the middle of a mode change and the fence hasn't
     // fired yet just wait for the next commit.
     if (mSetActiveModePending) {
@@ -2104,11 +2109,6 @@
         }
     }
 
-    if (mTracingEnabledChanged) {
-        mLayerTracingEnabled = mLayerTracing.isEnabled();
-        mTracingEnabledChanged = false;
-    }
-
     if (mRefreshRateOverlaySpinner) {
         Mutex::Autolock lock(mStateLock);
         if (const auto display = getDefaultDisplayDeviceLocked()) {
@@ -2123,7 +2123,7 @@
 
         bool needsTraversal = false;
         if (clearTransactionFlags(eTransactionFlushNeeded)) {
-            needsTraversal |= commitCreatedLayers();
+            needsTraversal |= commitCreatedLayers(vsyncId);
             needsTraversal |= flushTransactionQueues(vsyncId);
         }
 
@@ -7154,7 +7154,7 @@
     return calculateMaxAcquiredBufferCount(refreshRate, presentLatency);
 }
 
-void SurfaceFlinger::handleLayerCreatedLocked(const LayerCreatedState& state) {
+void SurfaceFlinger::handleLayerCreatedLocked(const LayerCreatedState& state, int64_t vsyncId) {
     sp<Layer> layer = state.layer.promote();
     if (!layer) {
         ALOGD("Layer was destroyed soon after creation %p", state.layer.unsafe_get());
@@ -7184,7 +7184,9 @@
     }
 
     layer->updateTransformHint(mActiveDisplayTransformHint);
-
+    if (mTransactionTracing) {
+        mTransactionTracing->onLayerAddedToDrawingState(layer->getSequence(), vsyncId);
+    }
     mInterceptor->saveSurfaceCreation(layer);
 }
 
@@ -7268,7 +7270,7 @@
     return buffer;
 }
 
-bool SurfaceFlinger::commitCreatedLayers() {
+bool SurfaceFlinger::commitCreatedLayers(int64_t vsyncId) {
     std::vector<LayerCreatedState> createdLayers;
     {
         std::scoped_lock<std::mutex> lock(mCreatedLayersLock);
@@ -7281,7 +7283,7 @@
 
     Mutex::Autolock _l(mStateLock);
     for (const auto& createdLayer : createdLayers) {
-        handleLayerCreatedLocked(createdLayer);
+        handleLayerCreatedLocked(createdLayer, vsyncId);
     }
     createdLayers.clear();
     mLayersAdded = true;