[sf] Use layer id as snapshot ids
Flicker tests rely on layer ids for some assertions. So when possible
use the layer id for the snapshot used to generate layer traces.
Also keep track of the order in which layers are added so its easier to
write tests.
Test: atest FlickerTests w/new fe
Bug: 238781169
Change-Id: Ie0f93ca956e6d043c9d95d00bc205d242e47c4cc
diff --git a/services/surfaceflinger/FrontEnd/LayerCreationArgs.cpp b/services/surfaceflinger/FrontEnd/LayerCreationArgs.cpp
index 6af352c..cfa2b03 100644
--- a/services/surfaceflinger/FrontEnd/LayerCreationArgs.cpp
+++ b/services/surfaceflinger/FrontEnd/LayerCreationArgs.cpp
@@ -23,6 +23,7 @@
namespace android::surfaceflinger {
std::atomic<uint32_t> LayerCreationArgs::sSequence{1};
+std::atomic<uint32_t> LayerCreationArgs::sInternalSequence{1};
uint32_t LayerCreationArgs::getInternalLayerId(uint32_t id) {
return id | INTERNAL_LAYER_PREFIX;
@@ -48,13 +49,11 @@
metadata.getInt32(gui::METADATA_OWNER_UID, static_cast<int32_t>(ownerUid)));
}
- if (id) {
+ if (internalLayer) {
+ sequence = getInternalLayerId(sInternalSequence++);
+ } else if (id) {
sequence = *id;
- if (internalLayer) {
- sequence = getInternalLayerId(*id);
- } else {
- sSequence = *id + 1;
- }
+ sSequence = *id + 1;
} else {
sequence = sSequence++;
if (sequence >= INTERNAL_LAYER_PREFIX) {
diff --git a/services/surfaceflinger/FrontEnd/LayerCreationArgs.h b/services/surfaceflinger/FrontEnd/LayerCreationArgs.h
index 3a0fc6d..c26edb5 100644
--- a/services/surfaceflinger/FrontEnd/LayerCreationArgs.h
+++ b/services/surfaceflinger/FrontEnd/LayerCreationArgs.h
@@ -36,6 +36,7 @@
struct LayerCreationArgs {
static std::atomic<uint32_t> sSequence;
+ static std::atomic<uint32_t> sInternalSequence;
static uint32_t getInternalLayerId(uint32_t id);
static LayerCreationArgs fromOtherArgs(const LayerCreationArgs& other);
diff --git a/services/surfaceflinger/FrontEnd/LayerLifecycleManager.cpp b/services/surfaceflinger/FrontEnd/LayerLifecycleManager.cpp
index 33d9dbe..6cacfb5 100644
--- a/services/surfaceflinger/FrontEnd/LayerLifecycleManager.cpp
+++ b/services/surfaceflinger/FrontEnd/LayerLifecycleManager.cpp
@@ -41,7 +41,7 @@
LOG_ALWAYS_FATAL("Duplicate layer id %d found. Existing layer: %s", layer.id,
it->second.owner.getDebugString().c_str());
}
-
+ mAddedLayers.push_back(newLayer.get());
layer.parentId = linkLayer(layer.parentId, layer.id);
layer.relativeParentId = linkLayer(layer.relativeParentId, layer.id);
if (layer.layerStackToMirror != ui::INVALID_LAYER_STACK) {
@@ -258,23 +258,19 @@
}
void LayerLifecycleManager::commitChanges() {
- for (auto& layer : mLayers) {
- if (layer->changes.test(RequestedLayerState::Changes::Created)) {
- for (auto listener : mListeners) {
- listener->onLayerAdded(*layer);
- }
+ for (auto layer : mAddedLayers) {
+ for (auto& listener : mListeners) {
+ listener->onLayerAdded(*layer);
}
+ }
+ mAddedLayers.clear();
+
+ for (auto& layer : mLayers) {
layer->clearChanges();
}
for (auto& destroyedLayer : mDestroyedLayers) {
- if (destroyedLayer->changes.test(RequestedLayerState::Changes::Created)) {
- for (auto listener : mListeners) {
- listener->onLayerAdded(*destroyedLayer);
- }
- }
-
- for (auto listener : mListeners) {
+ for (auto& listener : mListeners) {
listener->onLayerDestroyed(*destroyedLayer);
}
}
diff --git a/services/surfaceflinger/FrontEnd/LayerLifecycleManager.h b/services/surfaceflinger/FrontEnd/LayerLifecycleManager.h
index f258678..f0d2c22 100644
--- a/services/surfaceflinger/FrontEnd/LayerLifecycleManager.h
+++ b/services/surfaceflinger/FrontEnd/LayerLifecycleManager.h
@@ -108,6 +108,9 @@
std::vector<std::unique_ptr<RequestedLayerState>> mLayers;
// Layers pending destruction. Layers will be destroyed once changes are committed.
std::vector<std::unique_ptr<RequestedLayerState>> mDestroyedLayers;
+ // Keeps track of all the layers that were added in order. Changes will be cleared once
+ // committed.
+ std::vector<RequestedLayerState*> mAddedLayers;
};
} // namespace android::surfaceflinger::frontend
diff --git a/services/surfaceflinger/FrontEnd/LayerSnapshot.cpp b/services/surfaceflinger/FrontEnd/LayerSnapshot.cpp
index 1e931a7..a992584 100644
--- a/services/surfaceflinger/FrontEnd/LayerSnapshot.cpp
+++ b/services/surfaceflinger/FrontEnd/LayerSnapshot.cpp
@@ -27,7 +27,6 @@
LayerSnapshot::LayerSnapshot(const RequestedLayerState& state,
const LayerHierarchy::TraversalPath& path)
: path(path) {
- static uint32_t sUniqueSequenceId = 0;
// Provide a unique id for all snapshots.
// A front end layer can generate multiple snapshots if its mirrored.
// Additionally, if the layer is not reachable, we may choose to destroy
@@ -35,7 +34,12 @@
// change. The consumer shouldn't tie any lifetimes to this unique id but
// register a LayerLifecycleManager::ILifecycleListener or get a list of
// destroyed layers from LayerLifecycleManager.
- uniqueSequence = sUniqueSequenceId++;
+ if (path.isClone()) {
+ uniqueSequence =
+ LayerCreationArgs::getInternalLayerId(LayerCreationArgs::sInternalSequence++);
+ } else {
+ uniqueSequence = state.id;
+ }
sequence = static_cast<int32_t>(state.id);
name = state.name;
textureName = state.textureName;
diff --git a/services/surfaceflinger/LayerProtoHelper.cpp b/services/surfaceflinger/LayerProtoHelper.cpp
index 5d92485..3472d20 100644
--- a/services/surfaceflinger/LayerProtoHelper.cpp
+++ b/services/surfaceflinger/LayerProtoHelper.cpp
@@ -392,7 +392,11 @@
layerInfo->set_id(snapshot.uniqueSequence);
layerInfo->set_original_id(snapshot.sequence);
- layerInfo->set_name(requestedState.name);
+ if (!snapshot.path.isClone()) {
+ layerInfo->set_name(requestedState.name);
+ } else {
+ layerInfo->set_name(requestedState.name + "(Mirror)");
+ }
layerInfo->set_type("Layer");
LayerProtoHelper::writeToProto(requestedState.transparentRegion,
diff --git a/services/surfaceflinger/tests/unittests/LayerHierarchyTest.cpp b/services/surfaceflinger/tests/unittests/LayerHierarchyTest.cpp
index ddf3363..0d6a486 100644
--- a/services/surfaceflinger/tests/unittests/LayerHierarchyTest.cpp
+++ b/services/surfaceflinger/tests/unittests/LayerHierarchyTest.cpp
@@ -466,7 +466,8 @@
updateBackgroundColor(1, 0.5);
UPDATE_AND_VERIFY(hierarchyBuilder);
- auto bgLayerId = LayerCreationArgs::getInternalLayerId(1);
+ auto hierarchy = hierarchyBuilder.getPartialHierarchy(1, /*childrenOnly=*/true);
+ auto bgLayerId = hierarchy.mChildren.front().first->getLayer()->id;
std::vector<uint32_t> expectedTraversalPath = {1, bgLayerId, 11, 111, 12,
121, 122, 1221, 13, 2};
EXPECT_EQ(getTraversalPath(hierarchyBuilder.getHierarchy()), expectedTraversalPath);
diff --git a/services/surfaceflinger/tests/unittests/LayerLifecycleManagerTest.cpp b/services/surfaceflinger/tests/unittests/LayerLifecycleManagerTest.cpp
index 14b8e4b..97ef5a2 100644
--- a/services/surfaceflinger/tests/unittests/LayerLifecycleManagerTest.cpp
+++ b/services/surfaceflinger/tests/unittests/LayerLifecycleManagerTest.cpp
@@ -36,13 +36,13 @@
class ExpectLayerLifecycleListener : public LayerLifecycleManager::ILifecycleListener {
public:
void onLayerAdded(const RequestedLayerState& layer) override {
- mActualLayersAdded.emplace(layer.id);
+ mActualLayersAdded.push_back(layer.id);
};
void onLayerDestroyed(const RequestedLayerState& layer) override {
mActualLayersDestroyed.emplace(layer.id);
};
- void expectLayersAdded(const std::unordered_set<uint32_t>& expectedLayersAdded) {
+ void expectLayersAdded(const std::vector<uint32_t>& expectedLayersAdded) {
EXPECT_EQ(expectedLayersAdded, mActualLayersAdded);
mActualLayersAdded.clear();
}
@@ -51,7 +51,7 @@
mActualLayersDestroyed.clear();
}
- std::unordered_set<uint32_t> mActualLayersAdded;
+ std::vector<uint32_t> mActualLayersAdded;
std::unordered_set<uint32_t> mActualLayersDestroyed;
};
@@ -318,7 +318,8 @@
EXPECT_TRUE(lifecycleManager.getGlobalChanges().test(RequestedLayerState::Changes::Hierarchy));
lifecycleManager.commitChanges();
- auto bgLayerId = LayerCreationArgs::getInternalLayerId(1);
+ ASSERT_EQ(listener->mActualLayersAdded.size(), 2u);
+ auto bgLayerId = listener->mActualLayersAdded[1];
listener->expectLayersAdded({1, bgLayerId});
listener->expectLayersDestroyed({});
EXPECT_EQ(getRequestedLayerState(lifecycleManager, bgLayerId)->color.a, 0.5_hf);
@@ -352,7 +353,8 @@
EXPECT_TRUE(lifecycleManager.getGlobalChanges().test(RequestedLayerState::Changes::Hierarchy));
lifecycleManager.commitChanges();
- auto bgLayerId = LayerCreationArgs::getInternalLayerId(1);
+ ASSERT_EQ(listener->mActualLayersAdded.size(), 2u);
+ auto bgLayerId = listener->mActualLayersAdded[1];
listener->expectLayersAdded({1, bgLayerId});
listener->expectLayersDestroyed({bgLayerId});
}
@@ -381,7 +383,8 @@
EXPECT_TRUE(lifecycleManager.getGlobalChanges().test(RequestedLayerState::Changes::Hierarchy));
lifecycleManager.commitChanges();
- auto bgLayerId = LayerCreationArgs::getInternalLayerId(1);
+ ASSERT_EQ(listener->mActualLayersAdded.size(), 2u);
+ auto bgLayerId = listener->mActualLayersAdded[1];
listener->expectLayersAdded({1, bgLayerId});
listener->expectLayersDestroyed({1, bgLayerId});
}