Add transient notification views of group children to the child container, not directly to the row.

Ever since the introduction of the new pipeline, we have been removing adding the transient child to the ExpandableNotificationRow instead of the NotificationChildrenContainer due to a trivial oversight.  This works nearly all of the time because ExpandableViews track their transient container from which they can be removed at the end of their animation.  However, there are some situations where animations end abruptly, and transient views are cleaned up through traversal.  In this case we were only checking the NotificationChildrenContainer which would allow the transient children in the ExpandableNotificationRow to stay around forever.

We had a single "Transient Views" output at the end of the NSSL's dump() but that didn't include views which were child groups, so this change also adds a dump of transient views added to NotificationChildrenContainer.  There is no dump for ENR transient views, because there should not be any.

Fixes: 260191323
Test: atest ExpandableNotificationRowControllerTest
Change-Id: Ib5722ee5e6f5ae46902f276bd2bca7d6544e32a2
This commit is contained in:
Jeff DeCew
2023-02-22 03:31:42 +00:00
parent 993662b323
commit 506a24fa71
5 changed files with 78 additions and 5 deletions

View File

@@ -3734,7 +3734,9 @@ public class ExpandableNotificationRow extends ActivatableNotificationView
}
pw.println("Roundness: " + getRoundableState().debugString());
if (mIsSummaryWithChildren) {
int transientViewCount = mChildrenContainer == null
? 0 : mChildrenContainer.getTransientViewCount();
if (mIsSummaryWithChildren || transientViewCount > 0) {
pw.println();
pw.print("ChildrenContainer");
pw.print(" visibility: " + mChildrenContainer.getVisibility());
@@ -3742,8 +3744,7 @@ public class ExpandableNotificationRow extends ActivatableNotificationView
pw.print(", translationY: " + mChildrenContainer.getTranslationY());
pw.println();
List<ExpandableNotificationRow> notificationChildren = getAttachedChildren();
pw.println("Children: " + notificationChildren.size());
pw.print("{");
pw.print("Children: " + notificationChildren.size() + " {");
pw.increaseIndent();
for (ExpandableNotificationRow child : notificationChildren) {
pw.println();
@@ -3751,6 +3752,15 @@ public class ExpandableNotificationRow extends ActivatableNotificationView
}
pw.decreaseIndent();
pw.println("}");
pw.print("Transient Views: " + transientViewCount + " {");
pw.increaseIndent();
for (int i = 0; i < transientViewCount; i++) {
pw.println();
ExpandableView child = (ExpandableView) mChildrenContainer.getTransientView(i);
child.dump(pw, args);
}
pw.decreaseIndent();
pw.println("}");
} else if (mPrivateLayout != null) {
mPrivateLayout.dumpSmartReplies(pw);
}

View File

@@ -317,7 +317,7 @@ public class ExpandableNotificationRowController implements NotifViewController
}
mView.removeChildNotification(childView);
if (!isTransfer) {
mListContainer.notifyGroupChildRemoved(childView, mView);
mListContainer.notifyGroupChildRemoved(childView, mView.getChildrenContainer());
}
}

View File

@@ -2792,7 +2792,7 @@ public class NotificationStackScrollLayout extends ViewGroup implements Dumpable
}
child.setOnHeightChangedListener(null);
updateScrollStateForRemovedChild(child);
boolean animationGenerated = generateRemoveAnimation(child);
boolean animationGenerated = container != null && generateRemoveAnimation(child);
if (animationGenerated) {
if (!mSwipedOutViews.contains(child) || !isFullySwipedOut(child)) {
container.addTransientView(child, 0);

View File

@@ -0,0 +1,29 @@
/*
* Copyright (c) 2023 The Android Open Source Project
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
*/
package com.android.systemui.statusbar.notification.collection.render
import android.view.View
class FakeNodeController(
override val view: View,
override val nodeLabel: String = "fakeNodeController"
) : NodeController {
override fun offerToKeepInParentForAnimation(): Boolean = false
override fun removeFromParentIfKeptForAnimation(): Boolean = false
override fun resetKeepInParentForAnimation() = Unit
}

View File

@@ -31,15 +31,19 @@ import com.android.systemui.plugins.statusbar.StatusBarStateController
import com.android.systemui.statusbar.NotificationMediaManager
import com.android.systemui.statusbar.SmartReplyController
import com.android.systemui.statusbar.notification.collection.provider.NotificationDismissibilityProvider
import com.android.systemui.statusbar.notification.collection.render.FakeNodeController
import com.android.systemui.statusbar.notification.collection.render.GroupExpansionManager
import com.android.systemui.statusbar.notification.collection.render.GroupMembershipManager
import com.android.systemui.statusbar.notification.logging.NotificationLogger
import com.android.systemui.statusbar.notification.people.PeopleNotificationIdentifier
import com.android.systemui.statusbar.notification.stack.NotificationChildrenContainer
import com.android.systemui.statusbar.notification.stack.NotificationListContainer
import com.android.systemui.statusbar.phone.KeyguardBypassController
import com.android.systemui.statusbar.policy.HeadsUpManager
import com.android.systemui.statusbar.policy.SmartReplyConstants
import com.android.systemui.statusbar.policy.dagger.RemoteInputViewSubcomponent
import com.android.systemui.util.mockito.any
import com.android.systemui.util.mockito.eq
import com.android.systemui.util.mockito.mock
import com.android.systemui.util.time.SystemClock
import com.android.systemui.wmshell.BubblesManager
@@ -68,6 +72,7 @@ class ExpandableNotificationRowControllerTest : SysuiTestCase() {
private val metricsLogger: MetricsLogger = mock()
private val logBufferLogger: NotificationRowLogger = mock()
private val listContainer: NotificationListContainer = mock()
private val childrenContainer: NotificationChildrenContainer = mock()
private val mediaManager: NotificationMediaManager = mock()
private val smartReplyConstants: SmartReplyConstants = mock()
private val smartReplyController: SmartReplyController = mock()
@@ -129,6 +134,7 @@ class ExpandableNotificationRowControllerTest : SysuiTestCase() {
dragController,
dismissibilityProvider
)
whenever(view.childrenContainer).thenReturn(childrenContainer)
}
@After
@@ -173,4 +179,32 @@ class ExpandableNotificationRowControllerTest : SysuiTestCase() {
Assert.assertFalse(controller.removeFromParentIfKeptForAnimation())
Mockito.verifyNoMoreInteractions(parentView)
}
@Test
fun removeChild_whenTransfer() {
val childView: ExpandableNotificationRow = mock()
val childNodeController = FakeNodeController(childView)
// GIVEN a child is removed for transfer
controller.removeChild(childNodeController, /* isTransfer= */ true)
// VERIFY the listContainer is not notified
Mockito.verify(childView).isChangingPosition = eq(true)
Mockito.verify(view).removeChildNotification(eq(childView))
Mockito.verify(listContainer, never()).notifyGroupChildRemoved(any(), any())
}
@Test
fun removeChild_whenNotTransfer() {
val childView: ExpandableNotificationRow = mock()
val childNodeController = FakeNodeController(childView)
// GIVEN a child is removed for real
controller.removeChild(childNodeController, /* isTransfer= */ false)
// VERIFY the listContainer is passed the childrenContainer for transient animations
Mockito.verify(childView, never()).isChangingPosition = any()
Mockito.verify(view).removeChildNotification(eq(childView))
Mockito.verify(listContainer).notifyGroupChildRemoved(eq(childView), eq(childrenContainer))
}
}