Compute unique layer name in with sequence
Remove 'getUniqueLayerName' so we could remove stack lock from
'createLayer'.
Bug: 202621651
Test: atest libsurfaceflinger_unittest
Test: atest SurfaceFlinger_test
Change-Id: If5f4386b6da99bd540231b0c197c59a5823d7d4a
diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp
index 68ca2f0..3b98d50 100644
--- a/services/surfaceflinger/Layer.cpp
+++ b/services/surfaceflinger/Layer.cpp
@@ -88,7 +88,7 @@
Layer::Layer(const LayerCreationArgs& args)
: mFlinger(args.flinger),
- mName(args.name),
+ mName(base::StringPrintf("%s#%d", args.name.c_str(), sequence)),
mClientRef(args.client),
mWindowType(
static_cast<WindowInfo::Type>(args.metadata.getInt32(METADATA_WINDOW_TYPE, 0))) {
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 5f4acc9..e565bbb 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -4209,7 +4209,7 @@
sp<Layer> mirrorLayer;
sp<Layer> mirrorFrom;
- std::string uniqueName = getUniqueLayerName("MirrorRoot");
+ std::string layerName = "MirrorRoot";
{
Mutex::Autolock _l(mStateLock);
@@ -4218,7 +4218,7 @@
return NAME_NOT_FOUND;
}
- status_t result = createContainerLayer(client, std::move(uniqueName), -1, -1, 0,
+ status_t result = createContainerLayer(client, std::move(layerName), -1, -1, 0,
LayerMetadata(), outHandle, &mirrorLayer);
if (result != NO_ERROR) {
return result;
@@ -4252,12 +4252,12 @@
sp<Layer> layer;
- std::string uniqueName = getUniqueLayerName(name.string());
+ std::string layerName{name.string()};
switch (flags & ISurfaceComposerClient::eFXSurfaceMask) {
case ISurfaceComposerClient::eFXSurfaceBufferQueue:
case ISurfaceComposerClient::eFXSurfaceBufferState: {
- result = createBufferStateLayer(client, std::move(uniqueName), w, h, flags,
+ result = createBufferStateLayer(client, std::move(layerName), w, h, flags,
std::move(metadata), handle, &layer);
std::atomic<int32_t>* pendingBufferCounter = layer->getPendingBufferCounter();
if (pendingBufferCounter) {
@@ -4274,7 +4274,7 @@
return BAD_VALUE;
}
- result = createEffectLayer(client, std::move(uniqueName), w, h, flags,
+ result = createEffectLayer(client, std::move(layerName), w, h, flags,
std::move(metadata), handle, &layer);
break;
case ISurfaceComposerClient::eFXSurfaceContainer:
@@ -4284,7 +4284,7 @@
int(w), int(h));
return BAD_VALUE;
}
- result = createContainerLayer(client, std::move(uniqueName), w, h, flags,
+ result = createContainerLayer(client, std::move(layerName), w, h, flags,
std::move(metadata), handle, &layer);
break;
default:
@@ -4302,38 +4302,12 @@
if (result != NO_ERROR) {
return result;
}
- mInterceptor->saveSurfaceCreation(layer);
setTransactionFlags(eTransactionNeeded);
*outLayerId = layer->sequence;
return result;
}
-std::string SurfaceFlinger::getUniqueLayerName(const char* name) {
- unsigned dupeCounter = 0;
-
- // Tack on our counter whether there is a hit or not, so everyone gets a tag
- std::string uniqueName = base::StringPrintf("%s#%u", name, dupeCounter);
-
- // Grab the state lock since we're accessing mCurrentState
- Mutex::Autolock lock(mStateLock);
-
- // Loop over layers until we're sure there is no matching name
- bool matchFound = true;
- while (matchFound) {
- matchFound = false;
- mCurrentState.traverse([&](Layer* layer) {
- if (layer->getName() == uniqueName) {
- matchFound = true;
- uniqueName = base::StringPrintf("%s#%u", name, ++dupeCounter);
- }
- });
- }
-
- ALOGV_IF(dupeCounter > 0, "duplicate layer name: changing %s to %s", name, uniqueName.c_str());
- return uniqueName;
-}
-
status_t SurfaceFlinger::createBufferQueueLayer(const sp<Client>& client, std::string name,
uint32_t w, uint32_t h, uint32_t flags,
LayerMetadata metadata, PixelFormat& format,
@@ -6885,6 +6859,7 @@
}
}
+ mInterceptor->saveSurfaceCreation(layer);
return layer;
}
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index feee5df..276c7f6 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -786,8 +786,6 @@
status_t mirrorLayer(const sp<Client>& client, const sp<IBinder>& mirrorFromHandle,
sp<IBinder>* outHandle, int32_t* outLayerId);
- std::string getUniqueLayerName(const char* name);
-
// called when all clients have released all their references to
// this layer meaning it is entirely safe to destroy all
// resources associated to this layer.
diff --git a/services/surfaceflinger/tests/SurfaceInterceptor_test.cpp b/services/surfaceflinger/tests/SurfaceInterceptor_test.cpp
index ee16f40..2082c42 100644
--- a/services/surfaceflinger/tests/SurfaceInterceptor_test.cpp
+++ b/services/surfaceflinger/tests/SurfaceInterceptor_test.cpp
@@ -19,6 +19,7 @@
#pragma clang diagnostic ignored "-Wconversion"
#pragma clang diagnostic ignored "-Wextra"
+#include <android-base/stringprintf.h>
#include <frameworks/native/cmds/surfacereplayer/proto/src/trace.pb.h>
#include <google/protobuf/io/zero_copy_stream_impl.h>
#include <gtest/gtest.h>
@@ -56,10 +57,7 @@
const String8 DISPLAY_NAME("SurfaceInterceptor Display Test");
constexpr auto TEST_BG_SURFACE_NAME = "BG Interceptor Test Surface";
constexpr auto TEST_FG_SURFACE_NAME = "FG Interceptor Test Surface";
-constexpr auto UNIQUE_TEST_BG_SURFACE_NAME = "BG Interceptor Test Surface#0";
-constexpr auto UNIQUE_TEST_FG_SURFACE_NAME = "FG Interceptor Test Surface#0";
constexpr auto LAYER_NAME = "Layer Create and Delete Test";
-constexpr auto UNIQUE_LAYER_NAME = "Layer Create and Delete Test#0";
constexpr auto DEFAULT_FILENAME = "/data/misc/wmtrace/transaction_trace.winscope";
@@ -105,11 +103,15 @@
system("service call SurfaceFlinger 1020 i32 0 > /dev/null");
}
+std::string getUniqueName(const std::string& name, const Increment& increment) {
+ return base::StringPrintf("%s#%d", name.c_str(), increment.surface_creation().id());
+}
+
int32_t getSurfaceId(const Trace& capturedTrace, const std::string& surfaceName) {
int32_t layerId = 0;
for (const auto& increment : capturedTrace.increment()) {
if (increment.increment_case() == increment.kSurfaceCreation) {
- if (increment.surface_creation().name() == surfaceName) {
+ if (increment.surface_creation().name() == getUniqueName(surfaceName, increment)) {
layerId = increment.surface_creation().id();
}
}
@@ -293,8 +295,8 @@
}
void SurfaceInterceptorTest::preProcessTrace(const Trace& trace) {
- mBGLayerId = getSurfaceId(trace, UNIQUE_TEST_BG_SURFACE_NAME);
- mFGLayerId = getSurfaceId(trace, UNIQUE_TEST_FG_SURFACE_NAME);
+ mBGLayerId = getSurfaceId(trace, TEST_BG_SURFACE_NAME);
+ mFGLayerId = getSurfaceId(trace, TEST_FG_SURFACE_NAME);
}
void SurfaceInterceptorTest::captureTest(TestTransactionAction action,
@@ -752,9 +754,9 @@
}
bool SurfaceInterceptorTest::surfaceCreationFound(const Increment& increment, bool foundSurface) {
- bool isMatch(increment.surface_creation().name() == UNIQUE_LAYER_NAME &&
- increment.surface_creation().w() == SIZE_UPDATE &&
- increment.surface_creation().h() == SIZE_UPDATE);
+ bool isMatch(increment.surface_creation().name() == getUniqueName(LAYER_NAME, increment) &&
+ increment.surface_creation().w() == SIZE_UPDATE &&
+ increment.surface_creation().h() == SIZE_UPDATE);
if (isMatch && !foundSurface) {
foundSurface = true;
} else if (isMatch && foundSurface) {
@@ -808,7 +810,7 @@
break;
case Increment::IncrementCase::kSurfaceDeletion:
// Find the id of created surface.
- targetId = getSurfaceId(trace, UNIQUE_LAYER_NAME);
+ targetId = getSurfaceId(trace, LAYER_NAME);
foundIncrement = surfaceDeletionFound(increment, targetId, foundIncrement);
break;
case Increment::IncrementCase::kDisplayCreation: