From 0acd9a858e5fd1abcb68edbd4ac152fcc5d62dc6 Mon Sep 17 00:00:00 2001
From: Josh Tsuji <tsuji@google.com>
Date: Fri, 26 Jun 2020 14:25:27 -0400
Subject: [PATCH] Fix some more empty bubble issues!

The main thing here is not ignoring new selected bubbles that are different instances, but otherwise equal (same key, etc.).

Also, release the animating-out surface on collapse, and don't start the switch animation if we're collapsed. I was able to cause some issues in my testing with animating-out surfaces lying around.

Bug: 159861400
Test: manual, also using modded bubble test app with instantly canceled+readded bubbles
Change-Id: Ic19ee0bd3f4700021b1c790ae267beba9a52c532
---
 .../systemui/bubbles/BubbleController.java      |  5 ++++-
 .../systemui/bubbles/BubbleStackView.java       | 17 +++++++++++++++--
 2 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/packages/SystemUI/src/com/android/systemui/bubbles/BubbleController.java b/packages/SystemUI/src/com/android/systemui/bubbles/BubbleController.java
index b739999fb6529..6ea0cde44282a 100644
--- a/packages/SystemUI/src/com/android/systemui/bubbles/BubbleController.java
+++ b/packages/SystemUI/src/com/android/systemui/bubbles/BubbleController.java
@@ -1307,7 +1307,10 @@ public class BubbleController implements ConfigurationController.ConfigurationLi
             for (Pair<Bubble, Integer> removed : removedBubbles) {
                 final Bubble bubble = removed.first;
                 @DismissReason final int reason = removed.second;
-                mStackView.removeBubble(bubble);
+
+                if (mStackView != null) {
+                    mStackView.removeBubble(bubble);
+                }
 
                 // If the bubble is removed for user switching, leave the notification in place.
                 if (reason == DISMISS_USER_CHANGED) {
diff --git a/packages/SystemUI/src/com/android/systemui/bubbles/BubbleStackView.java b/packages/SystemUI/src/com/android/systemui/bubbles/BubbleStackView.java
index 1f3d981b8c9f0..c4b993c52b69e 100644
--- a/packages/SystemUI/src/com/android/systemui/bubbles/BubbleStackView.java
+++ b/packages/SystemUI/src/com/android/systemui/bubbles/BubbleStackView.java
@@ -1581,7 +1581,13 @@ public class BubbleStackView extends FrameLayout
         if (DEBUG_BUBBLE_STACK_VIEW) {
             Log.d(TAG, "setSelectedBubble: " + bubbleToSelect);
         }
-        if (mExpandedBubble != null && mExpandedBubble.equals(bubbleToSelect)) {
+
+        // Ignore this new bubble only if it is the exact same bubble object. Otherwise, we'll want
+        // to re-render it even if it has the same key (equals() returns true). If the currently
+        // expanded bubble is removed and instantly re-added, we'll get back a new Bubble instance
+        // with the same key (with newly inflated expanded views), and we need to render those new
+        // views.
+        if (mExpandedBubble == bubbleToSelect) {
             return;
         }
         if (bubbleToSelect == null || bubbleToSelect.getKey() != BubbleOverflow.KEY) {
@@ -1661,6 +1667,13 @@ public class BubbleStackView extends FrameLayout
         if (DEBUG_BUBBLE_STACK_VIEW) {
             Log.d(TAG, "setExpanded: " + shouldExpand);
         }
+
+        if (!shouldExpand) {
+            // If we're collapsing, release the animating-out surface immediately since we have no
+            // need for it, and this ensures it cannot remain visible as we collapse.
+            releaseAnimatingOutBubbleBuffer();
+        }
+
         if (shouldExpand == mIsExpanded) {
             return;
         }
@@ -2663,7 +2676,7 @@ public class BubbleStackView extends FrameLayout
      *                   expanded bubble.
      */
     private void screenshotAnimatingOutBubbleIntoSurface(Consumer<Boolean> onComplete) {
-        if (mExpandedBubble == null || mExpandedBubble.getExpandedView() == null) {
+        if (!mIsExpanded || mExpandedBubble == null || mExpandedBubble.getExpandedView() == null) {
             // You can't animate null.
             onComplete.accept(false);
             return;