Remove detachChildren
Since there are no longer callers of detachChildren, remove code and
test that involve detachChildren
Test: Builds and remaining tests pass
Bug: 177557720
Change-Id: Id5b7120b7f2c289d4e7ffb5565b29e2be18d6587
diff --git a/cmds/surfacereplayer/proto/src/trace.proto b/cmds/surfacereplayer/proto/src/trace.proto
index c6f656b..79aab82 100644
--- a/cmds/surfacereplayer/proto/src/trace.proto
+++ b/cmds/surfacereplayer/proto/src/trace.proto
@@ -50,11 +50,10 @@
CornerRadiusChange corner_radius = 16;
ReparentChange reparent = 17;
RelativeParentChange relative_parent = 18;
- DetachChildrenChange detach_children = 19;
- ReparentChildrenChange reparent_children = 20;
- BackgroundBlurRadiusChange background_blur_radius = 21;
- ShadowRadiusChange shadow_radius = 22;
- BlurRegionsChange blur_regions = 23;
+ ReparentChildrenChange reparent_children = 19;
+ BackgroundBlurRadiusChange background_blur_radius = 20;
+ ShadowRadiusChange shadow_radius = 21;
+ BlurRegionsChange blur_regions = 22;
}
}
@@ -200,10 +199,6 @@
required int32 z = 2;
}
-message DetachChildrenChange {
- required bool detach_children = 1;
-}
-
message ShadowRadiusChange {
required float radius = 1;
}
diff --git a/cmds/surfacereplayer/replayer/Replayer.cpp b/cmds/surfacereplayer/replayer/Replayer.cpp
index 5849212..58d6582 100644
--- a/cmds/surfacereplayer/replayer/Replayer.cpp
+++ b/cmds/surfacereplayer/replayer/Replayer.cpp
@@ -417,9 +417,6 @@
case SurfaceChange::SurfaceChangeCase::kRelativeParent:
setRelativeParentChange(transaction, change.id(), change.relative_parent());
break;
- case SurfaceChange::SurfaceChangeCase::kDetachChildren:
- setDetachChildrenChange(transaction, change.id(), change.detach_children());
- break;
case SurfaceChange::SurfaceChangeCase::kShadowRadius:
setShadowRadiusChange(transaction, change.id(), change.shadow_radius());
break;
@@ -713,11 +710,6 @@
t.setRelativeLayer(mLayers[id], mLayers[c.relative_parent_id()], c.z());
}
-void Replayer::setDetachChildrenChange(SurfaceComposerClient::Transaction& t,
- layer_id id, const DetachChildrenChange& c) {
- t.detachChildren(mLayers[id]);
-}
-
void Replayer::setReparentChildrenChange(SurfaceComposerClient::Transaction& t,
layer_id id, const ReparentChildrenChange& c) {
if (mLayers.count(c.parent_id()) == 0 || mLayers[c.parent_id()] == nullptr) {
diff --git a/cmds/surfacereplayer/replayer/Replayer.h b/cmds/surfacereplayer/replayer/Replayer.h
index a22262a..324d591 100644
--- a/cmds/surfacereplayer/replayer/Replayer.h
+++ b/cmds/surfacereplayer/replayer/Replayer.h
@@ -116,8 +116,6 @@
layer_id id, const ReparentChange& c);
void setRelativeParentChange(SurfaceComposerClient::Transaction& t,
layer_id id, const RelativeParentChange& c);
- void setDetachChildrenChange(SurfaceComposerClient::Transaction& t,
- layer_id id, const DetachChildrenChange& c);
void setReparentChildrenChange(SurfaceComposerClient::Transaction& t,
layer_id id, const ReparentChildrenChange& c);
void setShadowRadiusChange(SurfaceComposerClient::Transaction& t,
diff --git a/libs/gui/LayerState.cpp b/libs/gui/LayerState.cpp
index 63be3ed..e1aba42 100644
--- a/libs/gui/LayerState.cpp
+++ b/libs/gui/LayerState.cpp
@@ -427,9 +427,6 @@
what |= eReparentChildren;
reparentSurfaceControl = other.reparentSurfaceControl;
}
- if (other.what & eDetachChildren) {
- what |= eDetachChildren;
- }
if (other.what & eRelativeLayerChanged) {
what |= eRelativeLayerChanged;
what &= ~eLayerChanged;
diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp
index 4a372bb..d203b3d 100644
--- a/libs/gui/SurfaceComposerClient.cpp
+++ b/libs/gui/SurfaceComposerClient.cpp
@@ -1390,19 +1390,6 @@
return *this;
}
-SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::detachChildren(
- const sp<SurfaceControl>& sc) {
- layer_state_t* s = getLayerState(sc);
- if (!s) {
- mStatus = BAD_INDEX;
- return *this;
- }
- s->what |= layer_state_t::eDetachChildren;
-
- registerSurfaceControlForCallback(sc);
- return *this;
-}
-
#ifndef NO_INPUT
SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::setInputWindowInfo(
const sp<SurfaceControl>& sc,
diff --git a/libs/gui/include/gui/LayerState.h b/libs/gui/include/gui/LayerState.h
index 4a291ae..042f901 100644
--- a/libs/gui/include/gui/LayerState.h
+++ b/libs/gui/include/gui/LayerState.h
@@ -98,7 +98,7 @@
/* was ScalingModeChanged, now available 0x00000400, */
eShadowRadiusChanged = 0x00000800,
eReparentChildren = 0x00001000,
- eDetachChildren = 0x00002000,
+ /* was eDetachChildren, now available 0x00002000, */
eRelativeLayerChanged = 0x00004000,
eReparent = 0x00008000,
eColorChanged = 0x00010000,
diff --git a/libs/gui/include/gui/SurfaceComposerClient.h b/libs/gui/include/gui/SurfaceComposerClient.h
index 11db658..884f054 100644
--- a/libs/gui/include/gui/SurfaceComposerClient.h
+++ b/libs/gui/include/gui/SurfaceComposerClient.h
@@ -508,18 +508,6 @@
// Set the framenumber generated by the graphics producer to mimic BufferQueue behaviour.
Transaction& setFrameNumber(const sp<SurfaceControl>& sc, uint64_t frameNumber);
- // Detaches all child surfaces (and their children recursively)
- // from their SurfaceControl.
- // The child SurfaceControls will not throw exceptions or return errors,
- // but transactions will have no effect.
- // The child surfaces will continue to follow their parent surfaces,
- // and remain eligible for rendering, but their relative state will be
- // frozen. We use this in the WindowManager, in app shutdown/relaunch
- // scenarios, where the app would otherwise clean up its child Surfaces.
- // Sometimes the WindowManager needs to extend their lifetime slightly
- // in order to perform an exit animation or prevent flicker.
- Transaction& detachChildren(const sp<SurfaceControl>& sc);
-
#ifndef NO_INPUT
Transaction& setInputWindowInfo(const sp<SurfaceControl>& sc, const InputWindowInfo& info);
Transaction& setFocusedWindow(const FocusRequest& request);
diff --git a/services/surfaceflinger/BufferStateLayer.cpp b/services/surfaceflinger/BufferStateLayer.cpp
index 0cc15c2..3dc62e3 100644
--- a/services/surfaceflinger/BufferStateLayer.cpp
+++ b/services/surfaceflinger/BufferStateLayer.cpp
@@ -216,8 +216,7 @@
// Returns true if the most recent Transaction applied to CurrentState will be presented.
return (getSidebandStreamChanged() || getAutoRefresh() ||
(mCurrentState.modified &&
- (mCurrentState.buffer != nullptr || mCurrentState.bgColorLayer != nullptr))) &&
- !mLayerDetached;
+ (mCurrentState.buffer != nullptr || mCurrentState.bgColorLayer != nullptr)));
}
/* TODO: vhau uncomment once deferred transaction migration complete in
@@ -461,11 +460,6 @@
return willPresent;
}
-void BufferStateLayer::forceSendCallbacks() {
- mFlinger->getTransactionCompletedThread().finalizePendingCallbackHandles(
- mCurrentState.callbackHandles, std::vector<JankData>());
-}
-
bool BufferStateLayer::setTransparentRegionHint(const Region& transparent) {
mCurrentState.transparentRegionHint = transparent;
mCurrentState.modified = true;
diff --git a/services/surfaceflinger/BufferStateLayer.h b/services/surfaceflinger/BufferStateLayer.h
index f638caa..b93d567 100644
--- a/services/surfaceflinger/BufferStateLayer.h
+++ b/services/surfaceflinger/BufferStateLayer.h
@@ -80,7 +80,6 @@
bool setApi(int32_t api) override;
bool setSidebandStream(const sp<NativeHandle>& sidebandStream) override;
bool setTransactionCompletedListeners(const std::vector<sp<CallbackHandle>>& handles) override;
- void forceSendCallbacks() override;
bool addFrameEvent(const sp<Fence>& acquireFence, nsecs_t postedTime,
nsecs_t requestedPresentTime) override;
diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp
index a545d18..da888f6 100644
--- a/services/surfaceflinger/Layer.cpp
+++ b/services/surfaceflinger/Layer.cpp
@@ -1004,19 +1004,6 @@
uint32_t Layer::doTransaction(uint32_t flags) {
ATRACE_CALL();
- if (mLayerDetached) {
- // Ensure BLAST buffer callbacks are processed.
- // detachChildren and mLayerDetached were implemented to avoid geometry updates
- // to layers in the cases of animation. For BufferQueue layers buffers are still
- // consumed as normal. This is useful as otherwise the client could get hung
- // inevitably waiting on a buffer to return. We recreate this semantic for BufferQueue
- // even though it is a little consistent. detachChildren is shortly slated for removal
- // by the hierarchy mirroring work so we don't need to worry about it too much.
- forceSendCallbacks();
- mCurrentState.callbackHandles = {};
- return flags;
- }
-
if (mChildrenChanged) {
flags |= eVisibleRegion;
mChildrenChanged = false;
@@ -1512,12 +1499,6 @@
void Layer::deferTransactionUntil_legacy(const sp<Layer>& barrierLayer, uint64_t frameNumber) {
ATRACE_CALL();
- if (mLayerDetached) {
- // If the layer is detached, then we don't defer this transaction since we will not
- // commit the pending state while the layer is detached. Adding sync points may cause
- // the barrier layer to wait for the states to be committed before dequeuing a buffer.
- return;
- }
mCurrentState.barrierLayer_legacy = barrierLayer;
mCurrentState.barrierFrameNumber = frameNumber;
@@ -1801,10 +1782,6 @@
}
void Layer::reparentChildren(const sp<Layer>& newParent) {
- if (attachChildren()) {
- setTransactionFlags(eTransactionNeeded);
- }
-
for (const sp<Layer>& child : mCurrentChildren) {
newParent->addChild(child);
}
@@ -1840,17 +1817,6 @@
}
bool Layer::reparent(const sp<IBinder>& newParentHandle) {
- bool callSetTransactionFlags = false;
-
- // While layers are detached, we allow most operations
- // and simply halt performing the actual transaction. However
- // for reparent != null we would enter the mRemovedFromCurrentState
- // state, regardless of whether doTransaction was called, and
- // so we need to prevent the update here.
- if (mLayerDetached && newParentHandle == nullptr) {
- return false;
- }
-
sp<Layer> newParent;
if (newParentHandle != nullptr) {
auto handle = static_cast<Handle*>(newParentHandle.get());
@@ -1877,52 +1843,13 @@
} else {
onRemovedFromCurrentState();
}
-
- if (mLayerDetached) {
- mLayerDetached = false;
- callSetTransactionFlags = true;
- }
} else {
onRemovedFromCurrentState();
}
- if (attachChildren() || callSetTransactionFlags) {
- setTransactionFlags(eTransactionNeeded);
- }
return true;
}
-bool Layer::detachChildren() {
- for (const sp<Layer>& child : mCurrentChildren) {
- sp<Client> parentClient = mClientRef.promote();
- sp<Client> client(child->mClientRef.promote());
- if (client != nullptr && parentClient != client) {
- child->mLayerDetached = true;
- child->detachChildren();
- child->removeRemoteSyncPoints();
- }
- }
-
- return true;
-}
-
-bool Layer::attachChildren() {
- bool changed = false;
- for (const sp<Layer>& child : mCurrentChildren) {
- sp<Client> parentClient = mClientRef.promote();
- sp<Client> client(child->mClientRef.promote());
- if (client != nullptr && parentClient != client) {
- if (child->mLayerDetached) {
- child->mLayerDetached = false;
- child->attachChildren();
- changed = true;
- }
- }
- }
-
- return changed;
-}
-
bool Layer::setColorTransform(const mat4& matrix) {
static const mat4 identityMatrix = mat4();
diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h
index f78b5f3..50c0597 100644
--- a/services/surfaceflinger/Layer.h
+++ b/services/surfaceflinger/Layer.h
@@ -432,7 +432,6 @@
virtual bool setMetadata(const LayerMetadata& data);
virtual void setChildrenDrawingParent(const sp<Layer>&);
virtual bool reparent(const sp<IBinder>& newParentHandle);
- virtual bool detachChildren();
virtual bool setColorTransform(const mat4& matrix);
virtual mat4 getColorTransform() const;
virtual bool hasColorTransform() const;
@@ -459,7 +458,6 @@
const std::vector<sp<CallbackHandle>>& /*handles*/) {
return false;
};
- virtual void forceSendCallbacks() {}
virtual bool addFrameEvent(const sp<Fence>& /*acquireFence*/, nsecs_t /*postedTime*/,
nsecs_t /*requestedPresentTime*/) {
return false;
@@ -664,8 +662,6 @@
bool reparentChildren(const sp<IBinder>& newParentHandle);
void reparentChildren(const sp<Layer>& newParent);
- bool attachChildren();
- bool isLayerDetached() const { return mLayerDetached; }
bool setShadowRadius(float shadowRadius);
// Before color management is introduced, contents on Android have to be
@@ -1098,8 +1094,6 @@
wp<Layer> mDrawingParent;
// Can only be accessed with the SF state lock held.
- bool mLayerDetached{false};
- // Can only be accessed with the SF state lock held.
bool mChildrenChanged{false};
// Window types from WindowManager.LayoutParams
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 0a51659..83a83e0 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -3811,9 +3811,6 @@
flags |= eTransactionNeeded|eTraversalNeeded;
}
}
- if (what & layer_state_t::eDetachChildren) {
- layer->detachChildren();
- }
if (what & layer_state_t::eTransformChanged) {
if (layer->setTransform(s.transform)) flags |= eTraversalNeeded;
}
diff --git a/services/surfaceflinger/SurfaceInterceptor.cpp b/services/surfaceflinger/SurfaceInterceptor.cpp
index 3548923..61005c9 100644
--- a/services/surfaceflinger/SurfaceInterceptor.cpp
+++ b/services/surfaceflinger/SurfaceInterceptor.cpp
@@ -150,7 +150,6 @@
layer_state_t::eLayerHidden | layer_state_t::eLayerOpaque |
layer_state_t::eLayerSecure);
addReparentLocked(transaction, layerId, getLayerIdFromWeakRef(layer->mCurrentParent));
- addDetachChildrenLocked(transaction, layerId, layer->isLayerDetached());
addRelativeParentLocked(transaction, layerId,
getLayerIdFromWeakRef(layer->mCurrentState.zOrderRelativeOf),
layer->mCurrentState.z);
@@ -409,13 +408,6 @@
overrideChange->set_parent_id(parentId);
}
-void SurfaceInterceptor::addDetachChildrenLocked(Transaction* transaction, int32_t layerId,
- bool detached) {
- SurfaceChange* change(createSurfaceChangeLocked(transaction, layerId));
- DetachChildrenChange* overrideChange(change->mutable_detach_children());
- overrideChange->set_detach_children(detached);
-}
-
void SurfaceInterceptor::addRelativeParentLocked(Transaction* transaction, int32_t layerId,
int32_t parentId, int z) {
SurfaceChange* change(createSurfaceChangeLocked(transaction, layerId));
@@ -498,9 +490,6 @@
addReparentChildrenLocked(transaction, layerId,
getLayerIdFromHandle(state.reparentSurfaceControl->getHandle()));
}
- if (state.what & layer_state_t::eDetachChildren) {
- addDetachChildrenLocked(transaction, layerId, true);
- }
if (state.what & layer_state_t::eRelativeLayerChanged) {
addRelativeParentLocked(transaction, layerId,
getLayerIdFromHandle(
diff --git a/services/surfaceflinger/SurfaceInterceptor.h b/services/surfaceflinger/SurfaceInterceptor.h
index 3df79c6..3e27e83 100644
--- a/services/surfaceflinger/SurfaceInterceptor.h
+++ b/services/surfaceflinger/SurfaceInterceptor.h
@@ -177,7 +177,6 @@
uint64_t transactionId);
void addReparentLocked(Transaction* transaction, int32_t layerId, int32_t parentId);
void addReparentChildrenLocked(Transaction* transaction, int32_t layerId, int32_t parentId);
- void addDetachChildrenLocked(Transaction* transaction, int32_t layerId, bool detached);
void addRelativeParentLocked(Transaction* transaction, int32_t layerId, int32_t parentId,
int z);
void addShadowRadiusLocked(Transaction* transaction, int32_t layerId, float shadowRadius);
diff --git a/services/surfaceflinger/tests/Android.bp b/services/surfaceflinger/tests/Android.bp
index e8b24b4..cfaf229 100644
--- a/services/surfaceflinger/tests/Android.bp
+++ b/services/surfaceflinger/tests/Android.bp
@@ -21,7 +21,6 @@
"CommonTypes_test.cpp",
"Credentials_test.cpp",
"DereferenceSurfaceControl_test.cpp",
- "DetachChildren_test.cpp",
"DisplayConfigs_test.cpp",
"EffectLayer_test.cpp",
"InvalidHandles_test.cpp",
diff --git a/services/surfaceflinger/tests/DetachChildren_test.cpp b/services/surfaceflinger/tests/DetachChildren_test.cpp
deleted file mode 100644
index abf8b1a..0000000
--- a/services/surfaceflinger/tests/DetachChildren_test.cpp
+++ /dev/null
@@ -1,377 +0,0 @@
-/*
- * Copyright (C) 2020 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-#pragma clang diagnostic push
-#pragma clang diagnostic ignored "-Wconversion"
-
-#include "LayerTransactionTest.h"
-
-namespace android {
-
-class DetachChildren : public LayerTransactionTest {
-protected:
- virtual void SetUp() {
- LayerTransactionTest::SetUp();
-
- mMainSurface = createLayer(String8("Main Test Surface"), mMainSurfaceBounds.width(),
- mMainSurfaceBounds.height(), 0, mBlackBgSurface.get());
-
- ASSERT_TRUE(mMainSurface != nullptr);
- ASSERT_TRUE(mMainSurface->isValid());
-
- TransactionUtils::fillSurfaceRGBA8(mMainSurface, mMainSurfaceColor);
-
- asTransaction([&](Transaction& t) {
- t.setLayer(mMainSurface, INT32_MAX - 1)
- .setPosition(mMainSurface, mMainSurfaceBounds.left, mMainSurfaceBounds.top)
- .show(mMainSurface);
- });
- }
-
- virtual void TearDown() {
- LayerTransactionTest::TearDown();
- mMainSurface = 0;
- }
-
- sp<SurfaceControl> mMainSurface;
- Color mMainSurfaceColor = {195, 63, 63, 255};
- Rect mMainSurfaceBounds = Rect(64, 64, 128, 128);
- std::unique_ptr<ScreenCapture> mCapture;
-};
-
-TEST_F(DetachChildren, RelativesAreNotDetached) {
- Color relativeColor = {10, 10, 10, 255};
- Rect relBounds = Rect(64, 64, 74, 74);
-
- sp<SurfaceControl> relative =
- createLayer(String8("relativeTestSurface"), relBounds.width(), relBounds.height(), 0);
- TransactionUtils::fillSurfaceRGBA8(relative, relativeColor);
-
- Transaction{}
- .setRelativeLayer(relative, mMainSurface, 1)
- .setPosition(relative, relBounds.left, relBounds.top)
- .apply();
-
- {
- // The relative should be on top of the FG control.
- mCapture = screenshot();
- mCapture->expectColor(relBounds, relativeColor);
- }
- Transaction{}.detachChildren(mMainSurface).apply();
-
- {
- // Nothing should change at this point.
- mCapture = screenshot();
- mCapture->expectColor(relBounds, relativeColor);
- }
-
- Transaction{}.hide(relative).apply();
-
- {
- // Ensure that the relative was actually hidden, rather than
- // being left in the detached but visible state.
- mCapture = screenshot();
- mCapture->expectColor(mMainSurfaceBounds, mMainSurfaceColor);
- }
-}
-
-TEST_F(DetachChildren, DetachChildrenSameClient) {
- Color childColor = {200, 200, 200, 255};
- Rect childBounds = Rect(74, 74, 84, 84);
- sp<SurfaceControl> child = createLayer(String8("Child surface"), childBounds.width(),
- childBounds.height(), 0, mMainSurface.get());
- ASSERT_TRUE(child->isValid());
-
- TransactionUtils::fillSurfaceRGBA8(child, childColor);
-
- asTransaction([&](Transaction& t) {
- t.show(child);
- t.setPosition(child, childBounds.left - mMainSurfaceBounds.left,
- childBounds.top - mMainSurfaceBounds.top);
- });
-
- {
- mCapture = screenshot();
- // Expect main color around the child surface
- mCapture->expectBorder(childBounds, mMainSurfaceColor);
- mCapture->expectColor(childBounds, childColor);
- }
-
- asTransaction([&](Transaction& t) { t.detachChildren(mMainSurface); });
-
- asTransaction([&](Transaction& t) { t.hide(child); });
-
- // Since the child has the same client as the parent, it will not get
- // detached and will be hidden.
- {
- mCapture = screenshot();
- mCapture->expectColor(mMainSurfaceBounds, mMainSurfaceColor);
- }
-}
-
-TEST_F(DetachChildren, DetachChildrenDifferentClient) {
- Color childColor = {200, 200, 200, 255};
- Rect childBounds = Rect(74, 74, 84, 84);
-
- sp<SurfaceComposerClient> newComposerClient = new SurfaceComposerClient;
- sp<SurfaceControl> childNewClient =
- createSurface(newComposerClient, "New Child Test Surface", childBounds.width(),
- childBounds.height(), PIXEL_FORMAT_RGBA_8888, 0, mMainSurface.get());
- ASSERT_TRUE(childNewClient->isValid());
-
- TransactionUtils::fillSurfaceRGBA8(childNewClient, childColor);
-
- asTransaction([&](Transaction& t) {
- t.show(childNewClient);
- t.setPosition(childNewClient, childBounds.left - mMainSurfaceBounds.left,
- childBounds.top - mMainSurfaceBounds.top);
- });
-
- {
- mCapture = screenshot();
- // Expect main color around the child surface
- mCapture->expectBorder(childBounds, mMainSurfaceColor);
- mCapture->expectColor(childBounds, childColor);
- }
-
- asTransaction([&](Transaction& t) { t.detachChildren(mMainSurface); });
-
- asTransaction([&](Transaction& t) { t.hide(childNewClient); });
-
- // Nothing should have changed.
- {
- mCapture = screenshot();
- mCapture->expectBorder(childBounds, mMainSurfaceColor);
- mCapture->expectColor(childBounds, childColor);
- }
-}
-
-TEST_F(DetachChildren, DetachChildrenThenAttach) {
- Color childColor = {200, 200, 200, 255};
- Rect childBounds = Rect(74, 74, 84, 84);
-
- sp<SurfaceComposerClient> newComposerClient = new SurfaceComposerClient;
- sp<SurfaceControl> childNewClient =
- createSurface(newComposerClient, "New Child Test Surface", childBounds.width(),
- childBounds.height(), PIXEL_FORMAT_RGBA_8888, 0, mMainSurface.get());
- ASSERT_TRUE(childNewClient->isValid());
-
- TransactionUtils::fillSurfaceRGBA8(childNewClient, childColor);
-
- Transaction()
- .show(childNewClient)
- .setPosition(childNewClient, childBounds.left - mMainSurfaceBounds.left,
- childBounds.top - mMainSurfaceBounds.top)
- .apply();
-
- {
- mCapture = screenshot();
- // Expect main color around the child surface
- mCapture->expectBorder(childBounds, mMainSurfaceColor);
- mCapture->expectColor(childBounds, childColor);
- }
-
- Transaction().detachChildren(mMainSurface).apply();
- Transaction().hide(childNewClient).apply();
-
- // Nothing should have changed.
- {
- mCapture = screenshot();
- mCapture->expectBorder(childBounds, mMainSurfaceColor);
- mCapture->expectColor(childBounds, childColor);
- }
-
- Color newParentColor = Color::RED;
- Rect newParentBounds = Rect(20, 20, 52, 52);
- sp<SurfaceControl> newParentSurface =
- createLayer(String8("New Parent Surface"), newParentBounds.width(),
- newParentBounds.height(), 0);
- TransactionUtils::fillSurfaceRGBA8(newParentSurface, newParentColor);
- Transaction()
- .setLayer(newParentSurface, INT32_MAX - 1)
- .show(newParentSurface)
- .setPosition(newParentSurface, newParentBounds.left, newParentBounds.top)
- .reparent(childNewClient, newParentSurface)
- .apply();
- {
- mCapture = screenshot();
- // Child is now hidden.
- mCapture->expectColor(newParentBounds, newParentColor);
- }
-}
-
-TEST_F(DetachChildren, DetachChildrenWithDeferredTransaction) {
- Color childColor = {200, 200, 200, 255};
- Rect childBounds = Rect(74, 74, 84, 84);
-
- sp<SurfaceComposerClient> newComposerClient = new SurfaceComposerClient;
- sp<SurfaceControl> childNewClient =
- createSurface(newComposerClient, "New Child Test Surface", childBounds.width(),
- childBounds.height(), PIXEL_FORMAT_RGBA_8888, 0, mMainSurface.get());
- ASSERT_TRUE(childNewClient->isValid());
-
- TransactionUtils::fillSurfaceRGBA8(childNewClient, childColor);
-
- Transaction()
- .show(childNewClient)
- .setPosition(childNewClient, childBounds.left - mMainSurfaceBounds.left,
- childBounds.top - mMainSurfaceBounds.top)
- .apply();
-
- {
- mCapture = screenshot();
- mCapture->expectBorder(childBounds, mMainSurfaceColor);
- mCapture->expectColor(childBounds, childColor);
- }
-
- Transaction()
- .deferTransactionUntil_legacy(childNewClient, mMainSurface,
- mMainSurface->getSurface()->getNextFrameNumber())
- .apply();
- Transaction().detachChildren(mMainSurface).apply();
- ASSERT_NO_FATAL_FAILURE(fillBufferQueueLayerColor(mMainSurface, Color::RED,
- mMainSurfaceBounds.width(),
- mMainSurfaceBounds.height()));
-
- // BufferLayer can still dequeue buffers even though there's a detached layer with a
- // deferred transaction.
- {
- SCOPED_TRACE("new buffer");
- mCapture = screenshot();
- mCapture->expectBorder(childBounds, Color::RED);
- mCapture->expectColor(childBounds, childColor);
- }
-}
-
-/**
- * Tests that a deferring transaction on an already detached layer will be dropped gracefully and
- * allow the barrier layer to dequeue buffers.
- *
- * Fixes b/150924737 - buffer cannot be latched because it waits for a detached layer
- * to commit its pending states.
- */
-TEST_F(DetachChildren, DeferredTransactionOnDetachedChildren) {
- Color childColor = {200, 200, 200, 255};
- Rect childBounds = Rect(74, 74, 84, 84);
-
- sp<SurfaceComposerClient> newComposerClient = new SurfaceComposerClient;
- sp<SurfaceControl> childNewClient =
- createSurface(newComposerClient, "New Child Test Surface", childBounds.width(),
- childBounds.height(), PIXEL_FORMAT_RGBA_8888, 0, mMainSurface.get());
- ASSERT_TRUE(childNewClient->isValid());
-
- TransactionUtils::fillSurfaceRGBA8(childNewClient, childColor);
-
- Transaction()
- .show(childNewClient)
- .setPosition(childNewClient, childBounds.left - mMainSurfaceBounds.left,
- childBounds.top - mMainSurfaceBounds.top)
- .apply();
-
- {
- mCapture = screenshot();
- mCapture->expectBorder(childBounds, mMainSurfaceColor);
- mCapture->expectColor(childBounds, childColor);
- }
-
- Transaction().detachChildren(mMainSurface).apply();
- Transaction()
- .setCrop_legacy(childNewClient, {0, 0, childBounds.width(), childBounds.height()})
- .deferTransactionUntil_legacy(childNewClient, mMainSurface,
- mMainSurface->getSurface()->getNextFrameNumber())
- .apply();
-
- ASSERT_NO_FATAL_FAILURE(fillBufferQueueLayerColor(mMainSurface, Color::RED,
- mMainSurfaceBounds.width(),
- mMainSurfaceBounds.height()));
-
- // BufferLayer can still dequeue buffers even though there's a detached layer with a
- // deferred transaction.
- {
- SCOPED_TRACE("new buffer");
- mCapture = screenshot();
- mCapture->expectBorder(childBounds, Color::RED);
- mCapture->expectColor(childBounds, childColor);
- }
-}
-
-TEST_F(DetachChildren, ReparentParentLayerOfDetachedChildren) {
- Color childColor = {200, 200, 200, 255};
- Rect childBounds = Rect(74, 74, 94, 94);
- Color grandchildColor = Color::RED;
- Rect grandchildBounds = Rect(80, 80, 90, 90);
-
- sp<SurfaceComposerClient> newClient1 = new SurfaceComposerClient;
- sp<SurfaceComposerClient> newClient2 = new SurfaceComposerClient;
-
- sp<SurfaceControl> childSurface =
- createSurface(newClient1, "Child surface", childBounds.width(), childBounds.height(),
- PIXEL_FORMAT_RGBA_8888, 0, mMainSurface.get());
- sp<SurfaceControl> grandchildSurface =
- createSurface(newClient2, "Grandchild Surface", grandchildBounds.width(),
- grandchildBounds.height(), PIXEL_FORMAT_RGBA_8888, 0, childSurface.get());
-
- TransactionUtils::fillSurfaceRGBA8(childSurface, childColor);
- TransactionUtils::fillSurfaceRGBA8(grandchildSurface, grandchildColor);
-
- Transaction()
- .show(childSurface)
- .show(grandchildSurface)
- .setPosition(childSurface, childBounds.left - mMainSurfaceBounds.left,
- childBounds.top - mMainSurfaceBounds.top)
- .setPosition(grandchildSurface, grandchildBounds.left - childBounds.left,
- grandchildBounds.top - childBounds.top)
- .apply();
-
- {
- mCapture = screenshot();
- mCapture->expectBorder(childBounds, mMainSurfaceColor);
- mCapture->expectBorder(grandchildBounds, childColor);
- mCapture->expectColor(grandchildBounds, grandchildColor);
- }
-
- Transaction().detachChildren(childSurface).apply();
-
- // Remove main surface offscreen
- Transaction().reparent(mMainSurface, nullptr).apply();
- {
- mCapture = screenshot();
- mCapture->expectColor(mMainSurfaceBounds, Color::BLACK);
- }
-
- Transaction().reparent(mMainSurface, mBlackBgSurface).apply();
- {
- mCapture = screenshot();
- mCapture->expectBorder(childBounds, mMainSurfaceColor);
- mCapture->expectBorder(grandchildBounds, childColor);
- mCapture->expectColor(grandchildBounds, grandchildColor);
- }
-
- Transaction().hide(grandchildSurface).apply();
-
- // grandchild is still detached so it will not hide
- {
- mCapture = screenshot();
- mCapture->expectBorder(childBounds, mMainSurfaceColor);
- mCapture->expectBorder(grandchildBounds, childColor);
- mCapture->expectColor(grandchildBounds, grandchildColor);
- }
-}
-
-} // namespace android
-
-// TODO(b/129481165): remove the #pragma below and fix conversion issues
-#pragma clang diagnostic pop // ignored "-Wconversion"
\ No newline at end of file
diff --git a/services/surfaceflinger/tests/SurfaceInterceptor_test.cpp b/services/surfaceflinger/tests/SurfaceInterceptor_test.cpp
index 8dc9a12..fa88ca5 100644
--- a/services/surfaceflinger/tests/SurfaceInterceptor_test.cpp
+++ b/services/surfaceflinger/tests/SurfaceInterceptor_test.cpp
@@ -194,7 +194,6 @@
bool deferredTransactionUpdateFound(const SurfaceChange& change, bool foundDeferred);
bool reparentUpdateFound(const SurfaceChange& change, bool found);
bool relativeParentUpdateFound(const SurfaceChange& change, bool found);
- bool detachChildrenUpdateFound(const SurfaceChange& change, bool found);
bool reparentChildrenUpdateFound(const SurfaceChange& change, bool found);
bool shadowRadiusUpdateFound(const SurfaceChange& change, bool found);
bool surfaceUpdateFound(const Trace& trace, SurfaceChange::SurfaceChangeCase changeCase);
@@ -232,7 +231,6 @@
void deferredTransactionUpdate(Transaction&);
void reparentUpdate(Transaction&);
void relativeParentUpdate(Transaction&);
- void detachChildrenUpdate(Transaction&);
void reparentChildrenUpdate(Transaction&);
void shadowRadiusUpdate(Transaction&);
void surfaceCreation(Transaction&);
@@ -412,10 +410,6 @@
t.setRelativeLayer(mBGSurfaceControl, mFGSurfaceControl, RELATIVE_Z);
}
-void SurfaceInterceptorTest::detachChildrenUpdate(Transaction& t) {
- t.detachChildren(mBGSurfaceControl);
-}
-
void SurfaceInterceptorTest::reparentChildrenUpdate(Transaction& t) {
t.reparentChildren(mBGSurfaceControl, mFGSurfaceControl);
}
@@ -452,7 +446,6 @@
runInTransaction(&SurfaceInterceptorTest::deferredTransactionUpdate);
runInTransaction(&SurfaceInterceptorTest::reparentUpdate);
runInTransaction(&SurfaceInterceptorTest::reparentChildrenUpdate);
- runInTransaction(&SurfaceInterceptorTest::detachChildrenUpdate);
runInTransaction(&SurfaceInterceptorTest::relativeParentUpdate);
runInTransaction(&SurfaceInterceptorTest::shadowRadiusUpdate);
}
@@ -667,16 +660,6 @@
return found;
}
-bool SurfaceInterceptorTest::detachChildrenUpdateFound(const SurfaceChange& change, bool found) {
- bool detachChildren(change.detach_children().detach_children());
- if (detachChildren && !found) {
- found = true;
- } else if (detachChildren && found) {
- []() { FAIL(); }();
- }
- return found;
-}
-
bool SurfaceInterceptorTest::reparentChildrenUpdateFound(const SurfaceChange& change, bool found) {
bool hasId(change.reparent_children().parent_id() == mFGLayerId);
if (hasId && !found) {
@@ -761,9 +744,6 @@
case SurfaceChange::SurfaceChangeCase::kRelativeParent:
foundUpdate = relativeParentUpdateFound(change, foundUpdate);
break;
- case SurfaceChange::SurfaceChangeCase::kDetachChildren:
- foundUpdate = detachChildrenUpdateFound(change, foundUpdate);
- break;
case SurfaceChange::SurfaceChangeCase::kShadowRadius:
foundUpdate = shadowRadiusUpdateFound(change, foundUpdate);
break;
@@ -793,7 +773,6 @@
ASSERT_TRUE(surfaceUpdateFound(trace, SurfaceChange::SurfaceChangeCase::kReparent));
ASSERT_TRUE(surfaceUpdateFound(trace, SurfaceChange::SurfaceChangeCase::kReparentChildren));
ASSERT_TRUE(surfaceUpdateFound(trace, SurfaceChange::SurfaceChangeCase::kRelativeParent));
- ASSERT_TRUE(surfaceUpdateFound(trace, SurfaceChange::SurfaceChangeCase::kDetachChildren));
}
bool SurfaceInterceptorTest::surfaceCreationFound(const Increment& increment, bool foundSurface) {
@@ -968,11 +947,6 @@
SurfaceChange::SurfaceChangeCase::kRelativeParent);
}
-TEST_F(SurfaceInterceptorTest, InterceptDetachChildrenUpdateWorks) {
- captureTest(&SurfaceInterceptorTest::detachChildrenUpdate,
- SurfaceChange::SurfaceChangeCase::kDetachChildren);
-}
-
TEST_F(SurfaceInterceptorTest, InterceptShadowRadiusUpdateWorks) {
captureTest(&SurfaceInterceptorTest::shadowRadiusUpdate,
SurfaceChange::SurfaceChangeCase::kShadowRadius);
diff --git a/services/surfaceflinger/tests/fakehwc/SFFakeHwc_test.cpp b/services/surfaceflinger/tests/fakehwc/SFFakeHwc_test.cpp
index bd49728..4d7b396 100644
--- a/services/surfaceflinger/tests/fakehwc/SFFakeHwc_test.cpp
+++ b/services/surfaceflinger/tests/fakehwc/SFFakeHwc_test.cpp
@@ -1639,82 +1639,6 @@
EXPECT_TRUE(framesAreSame(referenceFrame2, Base::sFakeComposer->getLatestFrame()));
}
- void Test_DetachChildrenSameClient() {
- {
- TransactionScope ts(*Base::sFakeComposer);
- ts.show(mChild);
- ts.setPosition(mChild, 10, 10);
- ts.setPosition(Base::mFGSurfaceControl, 64, 64);
- }
-
- auto referenceFrame = Base::mBaseFrame;
- referenceFrame[Base::FG_LAYER].mDisplayFrame = hwc_rect_t{64, 64, 64 + 64, 64 + 64};
- referenceFrame[CHILD_LAYER].mDisplayFrame =
- hwc_rect_t{64 + 10, 64 + 10, 64 + 10 + 10, 64 + 10 + 10};
- EXPECT_TRUE(framesAreSame(referenceFrame, Base::sFakeComposer->getLatestFrame()));
-
- {
- TransactionScope ts(*Base::sFakeComposer);
- ts.setPosition(Base::mFGSurfaceControl, 0, 0);
- ts.detachChildren(Base::mFGSurfaceControl);
- }
-
- {
- TransactionScope ts(*Base::sFakeComposer);
- ts.setPosition(Base::mFGSurfaceControl, 64, 64);
- ts.hide(mChild);
- }
-
- std::vector<RenderState> refFrame(2);
- refFrame[Base::BG_LAYER] = Base::mBaseFrame[Base::BG_LAYER];
- refFrame[Base::FG_LAYER] = Base::mBaseFrame[Base::FG_LAYER];
-
- EXPECT_TRUE(framesAreSame(refFrame, Base::sFakeComposer->getLatestFrame()));
- }
-
- void Test_DetachChildrenDifferentClient() {
- sp<SurfaceComposerClient> newComposerClient = new SurfaceComposerClient;
- sp<SurfaceControl> childNewClient =
- newComposerClient->createSurface(String8("New Child Test Surface"), 10, 10,
- PIXEL_FORMAT_RGBA_8888, 0,
- Base::mFGSurfaceControl->getHandle());
- ASSERT_TRUE(childNewClient != nullptr);
- ASSERT_TRUE(childNewClient->isValid());
- fillSurfaceRGBA8(childNewClient, LIGHT_GRAY);
-
- {
- TransactionScope ts(*Base::sFakeComposer);
- ts.hide(mChild);
- ts.show(childNewClient);
- ts.setPosition(childNewClient, 10, 10);
- ts.setPosition(Base::mFGSurfaceControl, 64, 64);
- }
-
- auto referenceFrame = Base::mBaseFrame;
- referenceFrame[Base::FG_LAYER].mDisplayFrame = hwc_rect_t{64, 64, 64 + 64, 64 + 64};
- referenceFrame[CHILD_LAYER].mDisplayFrame =
- hwc_rect_t{64 + 10, 64 + 10, 64 + 10 + 10, 64 + 10 + 10};
- EXPECT_TRUE(framesAreSame(referenceFrame, Base::sFakeComposer->getLatestFrame()));
-
- {
- TransactionScope ts(*Base::sFakeComposer);
- ts.detachChildren(Base::mFGSurfaceControl);
- ts.setPosition(Base::mFGSurfaceControl, 0, 0);
- }
-
- {
- TransactionScope ts(*Base::sFakeComposer);
- ts.setPosition(Base::mFGSurfaceControl, 64, 64);
- ts.setPosition(childNewClient, 0, 0);
- ts.hide(childNewClient);
- }
-
- // Nothing should have changed. The child control becomes a no-op
- // zombie on detach. See comments for detachChildren in the
- // SurfaceControl.h file.
- EXPECT_TRUE(framesAreSame(referenceFrame, Base::sFakeComposer->getLatestFrame()));
- }
-
// Regression test for b/37673612
void Test_ChildrenWithParentBufferTransform() {
{
@@ -1815,14 +1739,6 @@
Test_ReparentChildren();
}
-TEST_F(ChildLayerTest_2_1, DISABLED_DetachChildrenSameClient) {
- Test_DetachChildrenSameClient();
-}
-
-TEST_F(ChildLayerTest_2_1, DISABLED_DetachChildrenDifferentClient) {
- Test_DetachChildrenDifferentClient();
-}
-
// Regression test for b/37673612
TEST_F(ChildLayerTest_2_1, DISABLED_ChildrenWithParentBufferTransform) {
Test_ChildrenWithParentBufferTransform();