Merge "New Pipeline: Don't skip detaching children; it causes crashes if they are attached later to a different parent."

packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/render/ShadeViewDiffer.kt

@@ -104,19 +104,14 @@ class ShadeViewDiffer(
                 views.remove(childNode.controller.view)
             }
 
-            if (childCompletelyRemoved && parentSpec == null) {
-                // If both the child and the parent are being removed at the same time, then
-                // keep the child attached to the parent for animation purposes
-                logger.logSkippingDetach(childNode.label, parentNode.label)
-            } else {
-                logger.logDetachingChild(
-                        childNode.label,
-                        !childCompletelyRemoved,
-                        parentNode.label,
-                        newParentNode?.label)
-                parentNode.removeChild(childNode, !childCompletelyRemoved)
-                childNode.parent = null
-            }
+            logger.logDetachingChild(
+                key = childNode.label,
+                isTransfer = !childCompletelyRemoved,
+                isParentRemoved = parentSpec == null,
+                oldParent = parentNode.label,
+                newParent = newParentNode?.label)
+            parentNode.removeChild(childNode, isTransfer = !childCompletelyRemoved)
+            childNode.parent = null
         }
     }
 

packages/SystemUI/src/com/android/systemui/statusbar/notification/collection/render/ShadeViewDifferLogger.kt

@@ -28,25 +28,18 @@ class ShadeViewDifferLogger @Inject constructor(
     fun logDetachingChild(
         key: String,
         isTransfer: Boolean,
+        isParentRemoved: Boolean,
         oldParent: String?,
         newParent: String?
     ) {
         buffer.log(TAG, LogLevel.DEBUG, {
             str1 = key
             bool1 = isTransfer
+            bool2 = isParentRemoved
             str2 = oldParent
             str3 = newParent
         }, {
-            "Detach $str1 isTransfer=$bool1 oldParent=$str2 newParent=$str3"
-        })
-    }
-
-    fun logSkippingDetach(key: String, parent: String?) {
-        buffer.log(TAG, LogLevel.DEBUG, {
-            str1 = key
-            str2 = parent
-        }, {
-            "Skipping detach of $str1 because its parent $str2 is also being detached"
+            "Detach $str1 isTransfer=$bool1 isParentRemoved=$bool2 oldParent=$str2 newParent=$str3"
         })
     }
 

packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/collection/render/ShadeViewDifferTest.java

@@ -17,6 +17,7 @@
 package com.android.systemui.statusbar.notification.collection.render;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
 
 import android.content.Context;
 import android.testing.AndroidTestingRunner;
@@ -138,7 +139,7 @@ public class ShadeViewDifferTest extends SysuiTestCase {
     }
 
     @Test
-    public void testRemovedGroupsAreKeptTogether() {
+    public void testRemovedGroupsAreBrokenApart() {
         // GIVEN a preexisting tree with a group
         applySpecAndCheck(
                 node(mController1),
@@ -154,10 +155,10 @@ public class ShadeViewDifferTest extends SysuiTestCase {
                 node(mController1)
         );
 
-        // THEN the group children are still attached to their parent
-        assertEquals(mController2.getView(), mController3.getView().getParent());
-        assertEquals(mController2.getView(), mController4.getView().getParent());
-        assertEquals(mController2.getView(), mController5.getView().getParent());
+        // THEN the group children are no longer attached to their parent
+        assertNull(mController3.getView().getParent());
+        assertNull(mController4.getView().getParent());
+        assertNull(mController5.getView().getParent());
     }
 
     @Test