SF: Carve out LayerHandle
Move LayerHandle outside of layer so it can
be used with the new LayerLifecycleManager.
Make Handle fields like layer private and provide
access functions so callers have to safely cast
from a binder instead of directly casting it.
Add layerid to LayerHandle since we want to
expose the layer class in fewer places.
Finally fold LayerCleaner class into LayerHandler.
Bug: 238781169
Test: presubmit
Change-Id: I86e08050cfcc89d68e6ed8fa0e8ff30063cf3603
diff --git a/services/surfaceflinger/Android.bp b/services/surfaceflinger/Android.bp
index b65f1b4..e76b191 100644
--- a/services/surfaceflinger/Android.bp
+++ b/services/surfaceflinger/Android.bp
@@ -156,6 +156,7 @@
"Effects/Daltonizer.cpp",
"EventLog/EventLog.cpp",
"FrontEnd/LayerCreationArgs.cpp",
+ "FrontEnd/LayerHandle.cpp",
"FrontEnd/TransactionHandler.cpp",
"FlagManager.cpp",
"FpsReporter.cpp",
diff --git a/services/surfaceflinger/FrontEnd/LayerHandle.cpp b/services/surfaceflinger/FrontEnd/LayerHandle.cpp
new file mode 100644
index 0000000..75e4e3a
--- /dev/null
+++ b/services/surfaceflinger/FrontEnd/LayerHandle.cpp
@@ -0,0 +1,62 @@
+/*
+ * Copyright 2022 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.
+ */
+
+#include "LayerHandle.h"
+#include <cstdint>
+#include "Layer.h"
+#include "LayerCreationArgs.h"
+#include "SurfaceFlinger.h"
+
+namespace android::surfaceflinger {
+
+LayerHandle::LayerHandle(const sp<android::SurfaceFlinger>& flinger,
+ const sp<android::Layer>& layer)
+ : mFlinger(flinger), mLayer(layer), mLayerId(static_cast<uint32_t>(layer->getSequence())) {}
+
+LayerHandle::~LayerHandle() {
+ if (mFlinger) {
+ mFlinger->onHandleDestroyed(this, mLayer, mLayerId);
+ }
+}
+
+const String16 LayerHandle::kDescriptor = String16("android.Layer.LayerHandle");
+
+sp<LayerHandle> LayerHandle::fromIBinder(const sp<IBinder>& binder) {
+ if (binder == nullptr) {
+ return nullptr;
+ }
+
+ BBinder* b = binder->localBinder();
+ if (b == nullptr || b->getInterfaceDescriptor() != LayerHandle::kDescriptor) {
+ ALOGD("handle does not have a valid descriptor");
+ return nullptr;
+ }
+
+ // We can safely cast this binder since its local and we verified its interface descriptor.
+ return sp<LayerHandle>::cast(binder);
+}
+
+sp<android::Layer> LayerHandle::getLayer(const sp<IBinder>& binder) {
+ sp<LayerHandle> handle = LayerHandle::fromIBinder(binder);
+ return handle ? handle->mLayer : nullptr;
+}
+
+uint32_t LayerHandle::getLayerId(const sp<IBinder>& binder) {
+ sp<LayerHandle> handle = LayerHandle::fromIBinder(binder);
+ return handle ? handle->mLayerId : UNASSIGNED_LAYER_ID;
+}
+
+} // namespace android::surfaceflinger
diff --git a/services/surfaceflinger/FrontEnd/LayerHandle.h b/services/surfaceflinger/FrontEnd/LayerHandle.h
new file mode 100644
index 0000000..5d0f783
--- /dev/null
+++ b/services/surfaceflinger/FrontEnd/LayerHandle.h
@@ -0,0 +1,58 @@
+/*
+ * Copyright 2022 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 once
+
+#include <binder/Binder.h>
+#include <utils/StrongPointer.h>
+
+namespace android {
+class SurfaceFlinger;
+class Layer;
+} // namespace android
+
+namespace android::surfaceflinger {
+
+/*
+ * The layer handle is just a BBinder object passed to the client
+ * (remote process) -- we don't keep any reference on our side such that
+ * the dtor is called when the remote side let go of its reference.
+ *
+ * ~LayerHandle ensures that mFlinger->onLayerDestroyed() is called for
+ * this layer when the handle is destroyed.
+ */
+class LayerHandle : public BBinder {
+public:
+ LayerHandle(const sp<android::SurfaceFlinger>& flinger, const sp<android::Layer>& layer);
+ // for testing
+ LayerHandle(uint32_t layerId) : mFlinger(nullptr), mLayer(nullptr), mLayerId(layerId) {}
+ ~LayerHandle();
+
+ // Static functions to access the layer and layer id safely from an incoming binder.
+ static sp<LayerHandle> fromIBinder(const sp<IBinder>& handle);
+ static sp<android::Layer> getLayer(const sp<IBinder>& handle);
+ static uint32_t getLayerId(const sp<IBinder>& handle);
+ static const String16 kDescriptor;
+
+ const String16& getInterfaceDescriptor() const override { return kDescriptor; }
+
+private:
+ sp<android::SurfaceFlinger> mFlinger;
+ sp<android::Layer> mLayer;
+ const uint32_t mLayerId;
+};
+
+} // namespace android::surfaceflinger
diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp
index def0dfa..631cf65 100644
--- a/services/surfaceflinger/Layer.cpp
+++ b/services/surfaceflinger/Layer.cpp
@@ -70,6 +70,7 @@
#include "FrameTimeline.h"
#include "FrameTracer/FrameTracer.h"
#include "FrontEnd/LayerCreationArgs.h"
+#include "FrontEnd/LayerHandle.h"
#include "LayerProtoHelper.h"
#include "SurfaceFlinger.h"
#include "TimeStats/TimeStats.h"
@@ -332,7 +333,7 @@
return nullptr;
}
mGetHandleCalled = true;
- return sp<Handle>::make(mFlinger, sp<Layer>::fromExisting(this));
+ return sp<LayerHandle>::make(mFlinger, sp<Layer>::fromExisting(this));
}
// ---------------------------------------------------------------------------
@@ -774,7 +775,7 @@
}
bool Layer::setRelativeLayer(const sp<IBinder>& relativeToHandle, int32_t relativeZ) {
- sp<Layer> relative = fromHandle(relativeToHandle).promote();
+ sp<Layer> relative = LayerHandle::getLayer(relativeToHandle);
if (relative == nullptr) {
return false;
}
@@ -1543,7 +1544,7 @@
bool Layer::reparent(const sp<IBinder>& newParentHandle) {
sp<Layer> newParent;
if (newParentHandle != nullptr) {
- newParent = fromHandle(newParentHandle).promote();
+ newParent = LayerHandle::getLayer(newParentHandle);
if (newParent == nullptr) {
ALOGE("Unable to promote Layer handle");
return false;
@@ -1936,7 +1937,8 @@
void Layer::setInputInfo(const WindowInfo& info) {
mDrawingState.inputInfo = info;
- mDrawingState.touchableRegionCrop = fromHandle(info.touchableRegionCropHandle.promote());
+ mDrawingState.touchableRegionCrop =
+ LayerHandle::getLayer(info.touchableRegionCropHandle.promote());
mDrawingState.modified = true;
mFlinger->mUpdateInputInfo = true;
setTransactionFlags(eTransactionNeeded);
@@ -2583,23 +2585,6 @@
mFlinger->mNumClones++;
}
-const String16 Layer::Handle::kDescriptor = String16("android.Layer.Handle");
-
-wp<Layer> Layer::fromHandle(const sp<IBinder>& handleBinder) {
- if (handleBinder == nullptr) {
- return nullptr;
- }
-
- BBinder* b = handleBinder->localBinder();
- if (b == nullptr || b->getInterfaceDescriptor() != Handle::kDescriptor) {
- return nullptr;
- }
-
- // We can safely cast this binder since its local and we verified its interface descriptor.
- sp<Handle> handle = sp<Handle>::cast(handleBinder);
- return handle->owner;
-}
-
bool Layer::setDropInputMode(gui::DropInputMode mode) {
if (mDrawingState.dropInputMode == mode) {
return false;
diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h
index 1773c03..c2b8169 100644
--- a/services/surfaceflinger/Layer.h
+++ b/services/surfaceflinger/Layer.h
@@ -226,46 +226,6 @@
bool dimmingEnabled = true;
};
- /*
- * Trivial class, used to ensure that mFlinger->onLayerDestroyed(mLayer)
- * is called.
- */
- class LayerCleaner {
- sp<SurfaceFlinger> mFlinger;
- sp<Layer> mLayer;
- BBinder* mHandle;
-
- protected:
- ~LayerCleaner() {
- // destroy client resources
- mFlinger->onHandleDestroyed(mHandle, mLayer);
- }
-
- public:
- LayerCleaner(const sp<SurfaceFlinger>& flinger, const sp<Layer>& layer, BBinder* handle)
- : mFlinger(flinger), mLayer(layer), mHandle(handle) {}
- };
-
- /*
- * The layer handle is just a BBinder object passed to the client
- * (remote process) -- we don't keep any reference on our side such that
- * the dtor is called when the remote side let go of its reference.
- *
- * LayerCleaner ensures that mFlinger->onLayerDestroyed() is called for
- * this layer when the handle is destroyed.
- */
- class Handle : public BBinder, public LayerCleaner {
- public:
- Handle(const sp<SurfaceFlinger>& flinger, const sp<Layer>& layer)
- : LayerCleaner(flinger, layer, this), owner(layer) {}
- const String16& getInterfaceDescriptor() const override { return kDescriptor; }
-
- static const String16 kDescriptor;
- wp<Layer> owner;
- };
-
- static wp<Layer> fromHandle(const sp<IBinder>& handle);
-
explicit Layer(const LayerCreationArgs& args);
virtual ~Layer();
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 029dd9f..f0271c6 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -124,6 +124,7 @@
#include "FrameTimeline/FrameTimeline.h"
#include "FrameTracer/FrameTracer.h"
#include "FrontEnd/LayerCreationArgs.h"
+#include "FrontEnd/LayerHandle.h"
#include "HdrLayerInfoReporter.h"
#include "Layer.h"
#include "LayerProtoHelper.h"
@@ -1579,7 +1580,10 @@
return BAD_VALUE;
}
- const wp<Layer> stopLayer = fromHandle(stopLayerHandle);
+ // LayerHandle::getLayer promotes the layer object in a binder thread but we will not destroy
+ // the layer here since the caller has a strong ref to the layer's handle.
+ // TODO (b/238781169): replace layer with layer id
+ const wp<Layer> stopLayer = LayerHandle::getLayer(stopLayerHandle);
mRegionSamplingThread->addListener(samplingArea, stopLayer, listener);
return NO_ERROR;
}
@@ -3701,7 +3705,7 @@
using TransactionReadiness = TransactionHandler::TransactionReadiness;
auto ready = TransactionReadiness::Ready;
flushState.transaction->traverseStatesWithBuffersWhileTrue([&](const layer_state_t& s) -> bool {
- sp<Layer> layer = Layer::fromHandle(s.surface).promote();
+ sp<Layer> layer = LayerHandle::getLayer(s.surface);
const auto& transaction = *flushState.transaction;
// check for barrier frames
if (s.bufferData->hasBarrier &&
@@ -3969,7 +3973,7 @@
setClientStateLocked(frameTimelineInfo, state, desiredPresentTime, isAutoTimestamp,
postTime, permissions, transactionId);
if ((flags & eAnimation) && state.state.surface) {
- if (const auto layer = fromHandle(state.state.surface).promote()) {
+ if (const auto layer = LayerHandle::getLayer(state.state.surface)) {
using LayerUpdateType = scheduler::LayerHistory::LayerUpdateType;
mScheduler->recordLayerHistory(layer.get(),
isAutoTimestamp ? 0 : desiredPresentTime,
@@ -4110,7 +4114,7 @@
uint32_t flags = 0;
sp<Layer> layer = nullptr;
if (s.surface) {
- layer = fromHandle(s.surface).promote();
+ layer = LayerHandle::getLayer(s.surface);
} else {
// The client may provide us a null handle. Treat it as if the layer was removed.
ALOGW("Attempt to set client state with a null layer handle");
@@ -4415,7 +4419,7 @@
sp<Layer> mirrorFrom;
{
Mutex::Autolock _l(mStateLock);
- mirrorFrom = fromHandle(mirrorFromHandle).promote();
+ mirrorFrom = LayerHandle::getLayer(mirrorFromHandle);
if (!mirrorFrom) {
return NAME_NOT_FOUND;
}
@@ -4522,19 +4526,15 @@
}
args.addToRoot = args.addToRoot && callingThreadHasUnscopedSurfaceFlingerAccess();
- wp<Layer> parent = fromHandle(args.parentHandle.promote());
+ // We can safely promote the parent layer in binder thread because we have a strong reference
+ // to the layer's handle inside this scope.
+ sp<Layer> parent = LayerHandle::getLayer(args.parentHandle.promote());
if (args.parentHandle != nullptr && parent == nullptr) {
- ALOGE("Invalid parent handle %p.", args.parentHandle.promote().get());
+ ALOGE("Invalid parent handle %p", args.parentHandle.promote().get());
args.addToRoot = false;
}
- int parentId = -1;
- // We can safely promote the layer in binder thread because we have a strong reference
- // to the layer's handle inside this scope or we were passed in a sp reference to the layer.
- sp<Layer> parentSp = parent.promote();
- if (parentSp != nullptr) {
- parentId = parentSp->getSequence();
- }
+ const int parentId = parent ? parent->getSequence() : -1;
if (mTransactionTracing) {
mTransactionTracing->onLayerAdded(outResult.handle->localBinder(), layer->sequence,
args.name, args.flags, parentId);
@@ -4573,7 +4573,7 @@
setTransactionFlags(eTransactionNeeded);
}
-void SurfaceFlinger::onHandleDestroyed(BBinder* handle, sp<Layer>& layer) {
+void SurfaceFlinger::onHandleDestroyed(BBinder* handle, sp<Layer>& layer, uint32_t /* layerId */) {
Mutex::Autolock lock(mStateLock);
markLayerPendingRemovalLocked(layer);
mBufferCountTracker.remove(handle);
@@ -6178,7 +6178,7 @@
{
Mutex::Autolock lock(mStateLock);
- parent = fromHandle(args.layerHandle).promote();
+ parent = LayerHandle::getLayer(args.layerHandle);
if (parent == nullptr) {
ALOGE("captureLayers called with an invalid or removed parent");
return NAME_NOT_FOUND;
@@ -6209,7 +6209,7 @@
reqSize = ui::Size(crop.width() * args.frameScaleX, crop.height() * args.frameScaleY);
for (const auto& handle : args.excludeHandles) {
- sp<Layer> excludeLayer = fromHandle(handle).promote();
+ sp<Layer> excludeLayer = LayerHandle::getLayer(handle);
if (excludeLayer != nullptr) {
excludeLayers.emplace(excludeLayer);
} else {
@@ -6724,10 +6724,6 @@
return NO_ERROR;
}
-wp<Layer> SurfaceFlinger::fromHandle(const sp<IBinder>& handle) const {
- return Layer::fromHandle(handle);
-}
-
void SurfaceFlinger::onLayerFirstRef(Layer* layer) {
mNumLayers++;
if (!layer->isRemovedFromCurrentState()) {
@@ -7016,7 +7012,7 @@
for (const auto& mirrorDisplay : mirrorDisplays) {
// Set mirror layer's default layer stack to -1 so it doesn't end up rendered on a display
// accidentally.
- sp<Layer> rootMirrorLayer = Layer::fromHandle(mirrorDisplay.rootHandle).promote();
+ sp<Layer> rootMirrorLayer = LayerHandle::getLayer(mirrorDisplay.rootHandle);
rootMirrorLayer->setLayerStack(ui::LayerStack::fromValue(-1));
for (const auto& layer : mDrawingState.layersSortedByZ) {
if (layer->getLayerStack() != mirrorDisplay.layerStack ||
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index b65dec4..9a2f186 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -272,6 +272,11 @@
void removeHierarchyFromOffscreenLayers(Layer* layer);
void removeFromOffscreenLayers(Layer* layer);
+ // Called when all clients have released all their references to
+ // this layer. The layer may still be kept alive by its parents but
+ // the client can no longer modify this layer directly.
+ void onHandleDestroyed(BBinder* handle, sp<Layer>& layer, uint32_t layerId);
+
// TODO: Remove atomic if move dtor to main thread CL lands
std::atomic<uint32_t> mNumClones;
@@ -279,12 +284,6 @@
return mTransactionCallbackInvoker;
}
- // Converts from a binder handle to a Layer
- // Returns nullptr if the handle does not point to an existing layer.
- // Otherwise, returns a weak reference so that callers off the main-thread
- // won't accidentally hold onto the last strong reference.
- wp<Layer> fromHandle(const sp<IBinder>& handle) const;
-
// If set, disables reusing client composition buffers. This can be set by
// debug.sf.disable_client_composition_cache
bool mDisableClientCompositionCache = false;
@@ -771,10 +770,6 @@
status_t mirrorDisplay(DisplayId displayId, const LayerCreationArgs& args,
gui::CreateSurfaceResult& outResult);
- // 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.
- void onHandleDestroyed(BBinder* handle, sp<Layer>& layer);
void markLayerPendingRemovalLocked(const sp<Layer>& layer);
// add a layer to SurfaceFlinger
diff --git a/services/surfaceflinger/fuzzer/surfaceflinger_fuzzer.cpp b/services/surfaceflinger/fuzzer/surfaceflinger_fuzzer.cpp
index 22d80ca..14384a7 100644
--- a/services/surfaceflinger/fuzzer/surfaceflinger_fuzzer.cpp
+++ b/services/surfaceflinger/fuzzer/surfaceflinger_fuzzer.cpp
@@ -150,7 +150,7 @@
sp<IBinder> handle = defaultServiceManager()->checkService(
String16(mFdp.ConsumeRandomLengthString().c_str()));
- mFlinger->fromHandle(handle);
+ LayerHandle::getLayer(handle);
mFlinger->disableExpensiveRendering();
}
diff --git a/services/surfaceflinger/fuzzer/surfaceflinger_fuzzers_utils.h b/services/surfaceflinger/fuzzer/surfaceflinger_fuzzers_utils.h
index e2ae4f4..a350020 100644
--- a/services/surfaceflinger/fuzzer/surfaceflinger_fuzzers_utils.h
+++ b/services/surfaceflinger/fuzzer/surfaceflinger_fuzzers_utils.h
@@ -34,6 +34,7 @@
#include "DisplayHardware/ComposerHal.h"
#include "FrameTimeline/FrameTimeline.h"
#include "FrameTracer/FrameTracer.h"
+#include "FrontEnd/LayerHandle.h"
#include "Layer.h"
#include "NativeWindowSurface.h"
#include "Scheduler/EventThread.h"
@@ -766,7 +767,7 @@
auto& mutableDisplays() { return mFlinger->mDisplays; }
auto& mutableDrawingState() { return mFlinger->mDrawingState; }
- auto fromHandle(const sp<IBinder> &handle) { return mFlinger->fromHandle(handle); }
+ auto fromHandle(const sp<IBinder> &handle) { return LayerHandle::getLayer(handle); }
~TestableSurfaceFlinger() {
mutableDisplays().clear();
diff --git a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
index 89812aa..7f471bc 100644
--- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
+++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
@@ -34,6 +34,7 @@
#include "FakeVsyncConfiguration.h"
#include "FrameTracer/FrameTracer.h"
#include "FrontEnd/LayerCreationArgs.h"
+#include "FrontEnd/LayerHandle.h"
#include "Layer.h"
#include "NativeWindowSurface.h"
#include "Scheduler/MessageQueue.h"
@@ -530,9 +531,7 @@
auto& mutablePrimaryHwcDisplayId() { return getHwComposer().mPrimaryHwcDisplayId; }
auto& mutableActiveDisplayId() { return mFlinger->mActiveDisplayId; }
- auto fromHandle(const sp<IBinder>& handle) {
- return mFlinger->fromHandle(handle);
- }
+ auto fromHandle(const sp<IBinder>& handle) { return LayerHandle::getLayer(handle); }
~TestableSurfaceFlinger() {
// All these pointer and container clears help ensure that GMock does
diff --git a/services/surfaceflinger/tests/unittests/TransactionApplicationTest.cpp b/services/surfaceflinger/tests/unittests/TransactionApplicationTest.cpp
index de84faa..9888f00 100644
--- a/services/surfaceflinger/tests/unittests/TransactionApplicationTest.cpp
+++ b/services/surfaceflinger/tests/unittests/TransactionApplicationTest.cpp
@@ -294,7 +294,7 @@
TEST_F(TransactionApplicationTest, FromHandle) {
sp<IBinder> badHandle;
auto ret = mFlinger.fromHandle(badHandle);
- EXPECT_EQ(nullptr, ret.promote().get());
+ EXPECT_EQ(nullptr, ret.get());
}
class LatchUnsignaledTest : public TransactionApplicationTest {