Merge "[ViewHierarchyAnimator] Remove all listeners at the end of an animation." into tm-dev

packages/SystemUI/animation/src/com/android/systemui/animation/ViewHierarchyAnimator.kt

@@ -156,17 +156,7 @@ class ViewHierarchyAnimator {
          * Any animations already in progress continue until their natural conclusion.
          */
         fun stopAnimating(rootView: View) {
-            val listener = rootView.getTag(R.id.tag_layout_listener)
+            recursivelyRemoveListener(rootView)
-            if (listener != null && listener is View.OnLayoutChangeListener) {
-                rootView.setTag(R.id.tag_layout_listener, null /* tag */)
-                rootView.removeOnLayoutChangeListener(listener)
-            }
-
-            if (rootView is ViewGroup) {
-                for (i in 0 until rootView.childCount) {
-                    stopAnimating(rootView.getChildAt(i))
-                }
-            }
         }
 
         /**
@@ -462,6 +452,20 @@ class ViewHierarchyAnimator {
             }
         }
 
+        private fun recursivelyRemoveListener(view: View) {
+            val listener = view.getTag(R.id.tag_layout_listener)
+            if (listener != null && listener is View.OnLayoutChangeListener) {
+                view.setTag(R.id.tag_layout_listener, null /* tag */)
+                view.removeOnLayoutChangeListener(listener)
+            }
+
+            if (view is ViewGroup) {
+                for (i in 0 until view.childCount) {
+                    recursivelyRemoveListener(view.getChildAt(i))
+                }
+            }
+        }
+
         private fun getBound(view: View, bound: Bound): Int? {
             return view.getTag(bound.overrideTag) as? Int
         }
@@ -513,11 +517,10 @@ class ViewHierarchyAnimator {
                     // When an animation is cancelled, a new one might be taking over. We shouldn't
                     // unregister the listener yet.
                     if (ephemeral && !cancelled) {
-                        val listener = view.getTag(R.id.tag_layout_listener)
+                        // The duration is the same for the whole hierarchy, so it's safe to remove
-                        if (listener != null && listener is View.OnLayoutChangeListener) {
+                        // the listener recursively. We do this because some descendant views might
-                            view.setTag(R.id.tag_layout_listener, null /* tag */)
+                        // not change bounds, and therefore not animate and leak the listener.
-                            view.removeOnLayoutChangeListener(listener)
+                        recursivelyRemoveListener(view)
-                        }
                     }
                 }
 

packages/SystemUI/tests/src/com/android/systemui/animation/ViewHierarchyAnimatorTest.kt

@@ -521,6 +521,38 @@ class ViewHierarchyAnimatorTest : SysuiTestCase() {
         endAnimation(rootView)
     }
 
+    @Test
+    fun cleansUpListenersCorrectly() {
+        val firstChild = View(mContext)
+        firstChild.layoutParams = LinearLayout.LayoutParams(50 /* width */, 100 /* height */)
+        rootView.addView(firstChild)
+        val secondChild = View(mContext)
+        secondChild.layoutParams = LinearLayout.LayoutParams(50 /* width */, 100 /* height */)
+        rootView.addView(secondChild)
+        rootView.measure(
+            View.MeasureSpec.makeMeasureSpec(100, View.MeasureSpec.EXACTLY),
+            View.MeasureSpec.makeMeasureSpec(100, View.MeasureSpec.EXACTLY)
+        )
+        rootView.layout(0 /* l */, 0 /* t */, 100 /* r */, 100 /* b */)
+
+        val success = ViewHierarchyAnimator.animateNextUpdate(rootView)
+        // Change all bounds.
+        rootView.measure(
+            View.MeasureSpec.makeMeasureSpec(150, View.MeasureSpec.EXACTLY),
+            View.MeasureSpec.makeMeasureSpec(100, View.MeasureSpec.EXACTLY)
+        )
+        rootView.layout(0 /* l */, 0 /* t */, 150 /* r */, 100 /* b */)
+
+        assertTrue(success)
+        assertNotNull(rootView.getTag(R.id.tag_layout_listener))
+        assertNotNull(firstChild.getTag(R.id.tag_layout_listener))
+        assertNotNull(secondChild.getTag(R.id.tag_layout_listener))
+        endAnimation(rootView)
+        assertNull(rootView.getTag(R.id.tag_layout_listener))
+        assertNull(firstChild.getTag(R.id.tag_layout_listener))
+        assertNull(secondChild.getTag(R.id.tag_layout_listener))
+    }
+
     @Test
     fun doesNotAnimateInvisibleViews() {
         rootView.layout(10 /* l */, 10 /* t */, 50 /* r */, 50 /* b */)