When destroying layer, add children to offscreen layers
If a layer is destroyed, make sure all children are added to the
offscreen layers. Otherwise, we may dangle a layer that is neither in
current state nor in offscreen layers.
Bug: 141111965
Test: build, boot, manual
Change-Id: Iec6788f10a24cb63faa9b40f246cbde3770d24a7
diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h
index 286311b..475b584 100644
--- a/services/surfaceflinger/Layer.h
+++ b/services/surfaceflinger/Layer.h
@@ -664,6 +664,15 @@
const LayerVector::Visitor& visitor);
size_t getChildrenCount() const;
+
+ // ONLY CALL THIS FROM THE LAYER DTOR!
+ // See b/141111965. We need to add current children to offscreen layers in
+ // the layer dtor so as not to dangle layers. Since the layer has not
+ // committed its transaction when the layer is destroyed, we must add
+ // current children. This is safe in the dtor as we will no longer update
+ // the current state, but should not be called anywhere else!
+ LayerVector& getCurrentChildren() { return mCurrentChildren; }
+
void addChild(const sp<Layer>& layer);
// Returns index if removed, or negative value otherwise
// for symmetry with Vector::remove
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 14a2ab1..0953446 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -3717,9 +3717,6 @@
}
if (currentMode == HWC_POWER_MODE_OFF) {
- // Turn on the display
- // TODO: @vhau temp fix only! See b/141111965
- mTransactionCompletedThread.clearAllPending();
getHwComposer().setPowerMode(*displayId, mode);
if (display->isPrimary() && mode != HWC_POWER_MODE_DOZE_SUSPEND) {
setVsyncEnabledInHWC(*displayId, mHWCVsyncPendingState);
@@ -5500,6 +5497,20 @@
void SurfaceFlinger::onLayerDestroyed(Layer* layer) {
mNumLayers--;
+ removeFromOffscreenLayers(layer);
+}
+
+// WARNING: ONLY CALL THIS FROM LAYER DTOR
+// Here we add children in the current state to offscreen layers and remove the
+// layer itself from the offscreen layer list. Since
+// this is the dtor, it is safe to access the current state. This keeps us
+// from dangling children layers such that they are not reachable from the
+// Drawing state nor the offscreen layer list
+// See b/141111965
+void SurfaceFlinger::removeFromOffscreenLayers(Layer* layer) {
+ for (auto& child : layer->getCurrentChildren()) {
+ mOffscreenLayers.emplace(child.get());
+ }
mOffscreenLayers.erase(layer);
}
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index 50b3ae4..2459cc3 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -313,6 +313,8 @@
void onLayerFirstRef(Layer*);
void onLayerDestroyed(Layer*);
+ void removeFromOffscreenLayers(Layer* layer);
+
TransactionCompletedThread& getTransactionCompletedThread() {
return mTransactionCompletedThread;
}
diff --git a/services/surfaceflinger/TransactionCompletedThread.cpp b/services/surfaceflinger/TransactionCompletedThread.cpp
index c15355d..8db03db 100644
--- a/services/surfaceflinger/TransactionCompletedThread.cpp
+++ b/services/surfaceflinger/TransactionCompletedThread.cpp
@@ -189,15 +189,6 @@
return NO_ERROR;
}
-void TransactionCompletedThread::clearAllPending() {
- std::lock_guard lock(mMutex);
- if (!mRunning) {
- return;
- }
- mPendingTransactions.clear();
- mConditionVariable.notify_all();
-}
-
status_t TransactionCompletedThread::registerUnpresentedCallbackHandle(
const sp<CallbackHandle>& handle) {
std::lock_guard lock(mMutex);
diff --git a/services/surfaceflinger/TransactionCompletedThread.h b/services/surfaceflinger/TransactionCompletedThread.h
index cd95bfb..12ea8fe 100644
--- a/services/surfaceflinger/TransactionCompletedThread.h
+++ b/services/surfaceflinger/TransactionCompletedThread.h
@@ -70,8 +70,6 @@
// Notifies the TransactionCompletedThread that a pending CallbackHandle has been presented.
status_t finalizePendingCallbackHandles(const std::deque<sp<CallbackHandle>>& handles);
- void clearAllPending();
-
// Adds the Transaction CallbackHandle from a layer that does not need to be relatched and
// presented this frame.
status_t registerUnpresentedCallbackHandle(const sp<CallbackHandle>& handle);