From 64db51ae8c270d53b3dcf031fbecdc1583d465d0 Mon Sep 17 00:00:00 2001 From: Mady Mellor Date: Wed, 27 May 2020 19:00:14 -0700 Subject: [PATCH] Fix crash when opening overflow after dismissing stack It changed recently such that we remove the bubble stack view when its dismissed (previously we just made it invisible). This revealed that we weren't cleaning up the bubble overflow activity view when the stack was dismissed, bc the parent of that view would be different resulting in a crash. This also renamed / simplified a method. Bug: 157567251 Bug: 155417004 Test: manual - have bubbles, visit the overflow - dismiss the stack - get new bubble, open it, visit the overflow => no crash Change-Id: Ia2823e5c2cb9b2a53b2379945287588edc30c072 --- .../systemui/bubbles/BubbleController.java | 8 ++++++- .../systemui/bubbles/BubbleExpandedView.java | 4 +++- .../systemui/bubbles/BubbleOverflow.java | 2 +- .../systemui/bubbles/BubbleStackView.java | 24 +++++++------------ 4 files changed, 20 insertions(+), 18 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/bubbles/BubbleController.java b/packages/SystemUI/src/com/android/systemui/bubbles/BubbleController.java index 5f157c1042003..31b0e4b71ec3a 100644 --- a/packages/SystemUI/src/com/android/systemui/bubbles/BubbleController.java +++ b/packages/SystemUI/src/com/android/systemui/bubbles/BubbleController.java @@ -655,7 +655,13 @@ public class BubbleController implements ConfigurationController.ConfigurationLi try { mAddedToWindowManager = false; - mWindowManager.removeView(mStackView); + if (mStackView != null) { + mWindowManager.removeView(mStackView); + mStackView.removeView(mBubbleScrim); + mStackView = null; + } else { + Log.w(TAG, "StackView added to WindowManager, but was null when removing!"); + } } catch (IllegalArgumentException e) { // This means the stack has already been removed - it shouldn't happen, but ignore if it // does, since we wanted it removed anyway. diff --git a/packages/SystemUI/src/com/android/systemui/bubbles/BubbleExpandedView.java b/packages/SystemUI/src/com/android/systemui/bubbles/BubbleExpandedView.java index c4b4f4316d935..503047b837b55 100644 --- a/packages/SystemUI/src/com/android/systemui/bubbles/BubbleExpandedView.java +++ b/packages/SystemUI/src/com/android/systemui/bubbles/BubbleExpandedView.java @@ -311,7 +311,9 @@ public class BubbleExpandedView extends LinearLayout { * if a view has been added or removed from on top of the ActivityView, such as the manage menu. */ void updateObscuredTouchableRegion() { - mActivityView.onLocationChanged(); + if (mActivityView != null) { + mActivityView.onLocationChanged(); + } } void applyThemeAttrs() { diff --git a/packages/SystemUI/src/com/android/systemui/bubbles/BubbleOverflow.java b/packages/SystemUI/src/com/android/systemui/bubbles/BubbleOverflow.java index af6e66aeb9893..b77e2261e39ba 100644 --- a/packages/SystemUI/src/com/android/systemui/bubbles/BubbleOverflow.java +++ b/packages/SystemUI/src/com/android/systemui/bubbles/BubbleOverflow.java @@ -129,7 +129,7 @@ public class BubbleOverflow implements BubbleViewProvider { return mOverflowBtn; } - void setBtnVisible(int visible) { + void setVisible(int visible) { mOverflowBtn.setVisibility(visible); } diff --git a/packages/SystemUI/src/com/android/systemui/bubbles/BubbleStackView.java b/packages/SystemUI/src/com/android/systemui/bubbles/BubbleStackView.java index 418cc505daa85..7d64111b56b2d 100644 --- a/packages/SystemUI/src/com/android/systemui/bubbles/BubbleStackView.java +++ b/packages/SystemUI/src/com/android/systemui/bubbles/BubbleStackView.java @@ -1119,6 +1119,9 @@ public class BubbleStackView extends FrameLayout super.onDetachedFromWindow(); getViewTreeObserver().removeOnPreDrawListener(mViewUpdater); getViewTreeObserver().removeOnComputeInternalInsetsListener(this); + if (mBubbleOverflow != null && mBubbleOverflow.getExpandedView() != null) { + mBubbleOverflow.getExpandedView().cleanUpExpandedState(); + } } @Override @@ -1339,21 +1342,12 @@ public class BubbleStackView extends FrameLayout Log.d(TAG, "was asked to remove Bubble, but didn't find the view! " + bubble); } - private void updateOverflowBtnVisibility() { - if (!BubbleExperimentConfig.allowBubbleOverflow(mContext)) { + private void updateOverflowVisibility() { + if (!BubbleExperimentConfig.allowBubbleOverflow(mContext) + || mBubbleOverflow == null) { return; } - if (mIsExpanded) { - if (DEBUG_BUBBLE_STACK_VIEW) { - Log.d(TAG, "Show overflow button."); - } - mBubbleOverflow.setBtnVisible(VISIBLE); - } else { - if (DEBUG_BUBBLE_STACK_VIEW) { - Log.d(TAG, "Collapsed. Hide overflow button."); - } - mBubbleOverflow.setBtnVisible(GONE); - } + mBubbleOverflow.setVisible(mIsExpanded ? VISIBLE : GONE); } // via BubbleData.Listener @@ -1607,7 +1601,7 @@ public class BubbleStackView extends FrameLayout Log.d(TAG, BubbleDebugConfig.formatBubblesString(getBubblesOnScreen(), mExpandedBubble)); } - updateOverflowBtnVisibility(); + updateOverflowVisibility(); mBubbleContainer.cancelAllAnimations(); mExpandedAnimationController.collapseBackToStack( mStackAnimationController.getStackPositionAlongNearestHorizontalEdge() @@ -1631,7 +1625,7 @@ public class BubbleStackView extends FrameLayout beforeExpandedViewAnimation(); mBubbleContainer.setActiveController(mExpandedAnimationController); - updateOverflowBtnVisibility(); + updateOverflowVisibility(); mExpandedAnimationController.expandFromStack(() -> { updatePointerPosition(); afterExpandedViewAnimation();