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

5 files changed, 25 insertions, 14 deletions

diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h
index 286311b03f..475b584764 100644
--- a/services/surfaceflinger/Layer.h
+++ b/services/surfaceflinger/Layer.h
@@ -664,6 +664,15 @@ public:
                                   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 14a2ab1b3e..09534468d8 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -3717,9 +3717,6 @@ void SurfaceFlinger::setPowerModeInternal(const sp<DisplayDevice>& display, int
     }
 
     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::onLayerFirstRef(Layer* layer) {
 
 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 50b3ae4ecd..2459cc3923 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -313,6 +313,8 @@ public:
     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 c15355d338..8db03db7e8 100644
--- a/services/surfaceflinger/TransactionCompletedThread.cpp
+++ b/services/surfaceflinger/TransactionCompletedThread.cpp
@@ -189,15 +189,6 @@ status_t TransactionCompletedThread::finalizePendingCallbackHandles(
     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 cd95bfb040..12ea8fe7d0 100644
--- a/services/surfaceflinger/TransactionCompletedThread.h
+++ b/services/surfaceflinger/TransactionCompletedThread.h
@@ -70,8 +70,6 @@ public:
     // 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);