From b10856228cfcec24210be5f5de0d55c8085b9553 Mon Sep 17 00:00:00 2001 From: Jeff DeCew Date: Fri, 17 Dec 2021 21:09:51 +0000 Subject: [PATCH] Fix crash when adding a notification to a group while it is animating away. This crash happens more often with the new pipeline where rows are removed and re-added quite often (rather than being set to GONE when redacted by visibility settings), and such removals also occasionally result in moving between the NSSL (which has disappear animations) and NotificationChildrenContainer. Fixes: 210164657 Test: manual Change-Id: I669d086a957b87762bfcb6b1ec00271282bd53fb --- .../notification/row/ExpandableView.java | 32 +++++++++++++++++++ .../stack/NotificationChildrenContainer.java | 12 +++++++ .../stack/NotificationStackScrollLayout.java | 12 ++----- 3 files changed, 47 insertions(+), 9 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/ExpandableView.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/ExpandableView.java index 4b3d6f76ba9d1..ab36da5bd73d7 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/ExpandableView.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/ExpandableView.java @@ -23,8 +23,10 @@ import android.graphics.Paint; import android.graphics.Rect; import android.util.AttributeSet; import android.util.IndentingPrintWriter; +import android.util.Log; import android.view.View; import android.view.ViewGroup; +import android.view.ViewParent; import android.widget.FrameLayout; import androidx.annotation.Nullable; @@ -517,6 +519,36 @@ public abstract class ExpandableView extends FrameLayout implements Dumpable { return mChangingPosition; } + /** + * Called before adding this view to a group, which would always throw an exception if this view + * has a parent, so clean up the transient container and throw an exception if the parent isn't + * a transient container. Provide as much detail in the event of a crash as possible. + */ + public void removeFromTransientContainerForAdditionTo(ViewGroup newParent) { + final ViewParent parent = getParent(); + if (parent == null) { + // If this view has no parent, the add will succeed, so do nothing. + return; + } + ViewGroup transientContainer = getTransientContainer(); + if (transientContainer == null) { + throw new IllegalStateException( + "Can't add view " + this + " to container " + newParent + "; current parent " + + parent + " is not a transient container"); + } + if (transientContainer != parent) { + throw new IllegalStateException( + "Expandable view " + this + " has transient container " + transientContainer + + " which is not the same as its parent " + parent); + } + if (parent != newParent) { + Log.w(TAG, "Moving view " + this + " from transient container " + + transientContainer + " to parent " + newParent); + } + transientContainer.removeTransientView(this); + setTransientContainer(null); + } + public void setTransientContainer(ViewGroup transientContainer) { mTransientContainer = transientContainer; } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationChildrenContainer.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationChildrenContainer.java index 046a133741ae7..a3fe47cb1f079 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationChildrenContainer.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationChildrenContainer.java @@ -44,6 +44,7 @@ import com.android.systemui.statusbar.notification.NotificationFadeAware; import com.android.systemui.statusbar.notification.NotificationUtils; import com.android.systemui.statusbar.notification.collection.legacy.VisualStabilityManager; import com.android.systemui.statusbar.notification.row.ExpandableNotificationRow; +import com.android.systemui.statusbar.notification.row.ExpandableView; import com.android.systemui.statusbar.notification.row.HybridGroupManager; import com.android.systemui.statusbar.notification.row.HybridNotificationView; import com.android.systemui.statusbar.notification.row.wrapper.NotificationViewWrapper; @@ -272,6 +273,7 @@ public class NotificationChildrenContainer extends ViewGroup * @param childIndex the index to add it at, if -1 it will be added at the end */ public void addNotification(ExpandableNotificationRow row, int childIndex) { + ensureRemovedFromTransientContainer(row); int newIndex = childIndex < 0 ? mAttachedChildren.size() : childIndex; mAttachedChildren.add(newIndex, row); addView(row); @@ -291,6 +293,16 @@ public class NotificationChildrenContainer extends ViewGroup } } + private void ensureRemovedFromTransientContainer(View v) { + if (v.getParent() != null && v instanceof ExpandableView) { + // If the child is animating away, it will still have a parent, so detach it first + // TODO: We should really cancel the active animations here. This will + // happen automatically when the view's intro animation starts, but + // it's a fragile link. + ((ExpandableView) v).removeFromTransientContainerForAdditionTo(this); + } + } + public void removeNotification(ExpandableNotificationRow row) { int childIndex = mAttachedChildren.indexOf(row); mAttachedChildren.remove(row); diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayout.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayout.java index e1dbf4efa4dea..a11635e31d4b7 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayout.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayout.java @@ -4654,18 +4654,12 @@ public class NotificationStackScrollLayout extends ViewGroup implements Dumpable } private void ensureRemovedFromTransientContainer(View v) { - if (v.getParent() == this && v instanceof ExpandableView) { - ExpandableView expandableView = (ExpandableView) v; - ViewGroup transientContainer = expandableView.getTransientContainer(); - // If the child is animating away, it will still have a parent, so - // detach it first + if (v.getParent() != null && v instanceof ExpandableView) { + // If the child is animating away, it will still have a parent, so detach it first // TODO: We should really cancel the active animations here. This will // happen automatically when the view's intro animation starts, but // it's a fragile link. - if (transientContainer != null) { - transientContainer.removeTransientView(v); - expandableView.setTransientContainer(null); - } + ((ExpandableView) v).removeFromTransientContainerForAdditionTo(this); } }