From 412f356ec0d7aa00f38bbc13dd80ae5acca4f928 Mon Sep 17 00:00:00 2001
From: Robert Carr <racarr@google.com>
Date: Thu, 11 Apr 2019 13:01:31 -0700
Subject: [PATCH] More SurfaceAnimator lifetime fixes.

1. The last fix to Dimmer was insufficient, the mDimLayer in the context
   of DimState() is not the mDimLayer that we return from DimAnimatable#getSurfaceControl.
   Here we null the correct layer.
2. This bug is hard to fix without repro so we add some isValid checks preventing further crashes
   at the cost of a few cycles and a code wart.

Test: Existing tests pass
Bug: 130315011
Bug: 130364451
Change-Id: I3d4d3e0e7110f943e5a2614e07a1f4ae85c2b3da
---
 services/core/java/com/android/server/wm/Dimmer.java   | 10 +++++++---
 .../java/com/android/server/wm/SurfaceAnimator.java    |  8 ++++++--
 2 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/services/core/java/com/android/server/wm/Dimmer.java b/services/core/java/com/android/server/wm/Dimmer.java
index 4bd8cab057000..a7a793fa8d340 100644
--- a/services/core/java/com/android/server/wm/Dimmer.java
+++ b/services/core/java/com/android/server/wm/Dimmer.java
@@ -41,7 +41,7 @@ class Dimmer {
     private static final int DEFAULT_DIM_ANIM_DURATION = 200;
 
     private class DimAnimatable implements SurfaceAnimator.Animatable {
-        private final SurfaceControl mDimLayer;
+        private SurfaceControl mDimLayer;
 
         private DimAnimatable(SurfaceControl dimLayer) {
             mDimLayer = dimLayer;
@@ -100,6 +100,11 @@ class Dimmer {
             // See getSurfaceWidth() above for explanation.
             return mHost.getSurfaceHeight();
         }
+
+        void removeSurface() {
+            getPendingTransaction().remove(mDimLayer);
+            mDimLayer = null;
+        }
     }
 
     @VisibleForTesting
@@ -129,8 +134,7 @@ class Dimmer {
             final DimAnimatable dimAnimatable = new DimAnimatable(dimLayer);
             mSurfaceAnimator = new SurfaceAnimator(dimAnimatable, () -> {
                 if (!mDimming) {
-                    dimAnimatable.getPendingTransaction().remove(mDimLayer);
-                    mDimLayer = null;
+                    dimAnimatable.removeSurface();
                 }
             }, mHost.mWmService);
         }
diff --git a/services/core/java/com/android/server/wm/SurfaceAnimator.java b/services/core/java/com/android/server/wm/SurfaceAnimator.java
index 33d952e80a6b8..b3b41b733cdb2 100644
--- a/services/core/java/com/android/server/wm/SurfaceAnimator.java
+++ b/services/core/java/com/android/server/wm/SurfaceAnimator.java
@@ -286,8 +286,12 @@ class SurfaceAnimator {
         final boolean destroy = mLeash != null && surface != null && parent != null;
         if (destroy) {
             if (DEBUG_ANIM) Slog.i(TAG, "Reparenting to original parent");
-            t.reparent(surface, parent);
-            scheduleAnim = true;
+            // We shouldn't really need these isValid checks but we do
+            // b/130364451
+            if (surface.isValid() && parent.isValid()) {
+                t.reparent(surface, parent);
+                scheduleAnim = true;
+            }
         }
         mService.mAnimationTransferMap.remove(mAnimation);
         if (mLeash != null && destroyLeash) {