From d5861c1b2ee28ad192502e82a52a427cb413f321 Mon Sep 17 00:00:00 2001 From: Mady Mellor Date: Thu, 3 Feb 2022 14:11:12 -0800 Subject: [PATCH] Fix crash when overflow is selected and last bubble is removed When the last bubble is removed we clean up the views associated with the bubbles, including the overflow view. BubbleOverflow#getExpandedView would create the view if it didn't exist but it wouldn't be initialized which means it wouldn't have the Bubble Positioner set on it. When a bubble is removed some code runs that would do stuff to the last expanded view, if it's non-null. If the overflow was the last bubble expanded, rather than skipping that code because the expandedView is null (because all the bubbles are gone), it was actually recreating the overflow expandedView but it wouldn't be initialized at that time resulting in the NPE. This fixes the issue by modifying BubbleOverflow's getExpandedView to only return the view, not do any view creation. Additionally this change removes some redundant code in BubbleStackView. Bug: 214399371 Test: atest BubbleOverflowTest Change-Id: Ib8d94fc7ea1163e79146e1439ec3f404d3caa303 --- .../wm/shell/bubbles/BubbleController.java | 3 +- .../wm/shell/bubbles/BubbleExpandedView.java | 6 +- .../wm/shell/bubbles/BubbleOverflow.kt | 15 ++-- .../wm/shell/bubbles/BubbleStackView.java | 3 +- .../android/wm/shell/BubbleOverflowTest.java | 85 +++++++++++++++++++ 5 files changed, 101 insertions(+), 11 deletions(-) create mode 100644 libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/BubbleOverflowTest.java diff --git a/libs/WindowManager/Shell/src/com/android/wm/shell/bubbles/BubbleController.java b/libs/WindowManager/Shell/src/com/android/wm/shell/bubbles/BubbleController.java index 6ed1ba9f561fe..6ffcf101693eb 100644 --- a/libs/WindowManager/Shell/src/com/android/wm/shell/bubbles/BubbleController.java +++ b/libs/WindowManager/Shell/src/com/android/wm/shell/bubbles/BubbleController.java @@ -618,7 +618,8 @@ public class BubbleController { } /** Contains information to help position things on the screen. */ - BubblePositioner getPositioner() { + @VisibleForTesting + public BubblePositioner getPositioner() { return mBubblePositioner; } diff --git a/libs/WindowManager/Shell/src/com/android/wm/shell/bubbles/BubbleExpandedView.java b/libs/WindowManager/Shell/src/com/android/wm/shell/bubbles/BubbleExpandedView.java index 9ae67a9bf2278..da8308ee7bab3 100644 --- a/libs/WindowManager/Shell/src/com/android/wm/shell/bubbles/BubbleExpandedView.java +++ b/libs/WindowManager/Shell/src/com/android/wm/shell/bubbles/BubbleExpandedView.java @@ -60,6 +60,7 @@ import android.widget.LinearLayout; import androidx.annotation.Nullable; +import com.android.internal.annotations.VisibleForTesting; import com.android.internal.policy.ScreenDecorationsUtils; import com.android.wm.shell.R; import com.android.wm.shell.TaskView; @@ -418,8 +419,9 @@ public class BubbleExpandedView extends LinearLayout { mPointerView.setBackground(mCurrentPointer); } - private String getBubbleKey() { - return mBubble != null ? mBubble.getKey() : "null"; + @VisibleForTesting + public String getBubbleKey() { + return mBubble != null ? mBubble.getKey() : mIsOverflow ? BubbleOverflow.KEY : null; } /** diff --git a/libs/WindowManager/Shell/src/com/android/wm/shell/bubbles/BubbleOverflow.kt b/libs/WindowManager/Shell/src/com/android/wm/shell/bubbles/BubbleOverflow.kt index dd751d24e7702..eb7929b8ca54e 100644 --- a/libs/WindowManager/Shell/src/com/android/wm/shell/bubbles/BubbleOverflow.kt +++ b/libs/WindowManager/Shell/src/com/android/wm/shell/bubbles/BubbleOverflow.kt @@ -54,6 +54,7 @@ class BubbleOverflow( /** Call before use and again if cleanUpExpandedState was called. */ fun initialize(controller: BubbleController) { + createExpandedView() getExpandedView()?.initialize(controller, controller.stackView, true /* isOverflow */) } @@ -123,13 +124,15 @@ class BubbleOverflow( overflowBtn?.updateDotVisibility(true /* animate */) } + fun createExpandedView(): BubbleExpandedView? { + expandedView = inflater.inflate(R.layout.bubble_expanded_view, + null /* root */, false /* attachToRoot */) as BubbleExpandedView + expandedView?.applyThemeAttrs() + updateResources() + return expandedView + } + override fun getExpandedView(): BubbleExpandedView? { - if (expandedView == null) { - expandedView = inflater.inflate(R.layout.bubble_expanded_view, - null /* root */, false /* attachToRoot */) as BubbleExpandedView - expandedView?.applyThemeAttrs() - updateResources() - } return expandedView } diff --git a/libs/WindowManager/Shell/src/com/android/wm/shell/bubbles/BubbleStackView.java b/libs/WindowManager/Shell/src/com/android/wm/shell/bubbles/BubbleStackView.java index c2eb08ca09ddf..9219baa061571 100644 --- a/libs/WindowManager/Shell/src/com/android/wm/shell/bubbles/BubbleStackView.java +++ b/libs/WindowManager/Shell/src/com/android/wm/shell/bubbles/BubbleStackView.java @@ -787,7 +787,7 @@ public class BubbleStackView extends FrameLayout ta.recycle(); final Runnable onBubbleAnimatedOut = () -> { - if (getBubbleCount() == 0 && !mBubbleData.isShowingOverflow()) { + if (getBubbleCount() == 0) { mBubbleController.onAllBubblesAnimatedOut(); } }; @@ -1651,7 +1651,6 @@ public class BubbleStackView extends FrameLayout } else { bubble.cleanupViews(); } - updatePointerPosition(false /* forIme */); updateExpandedView(); logBubbleEvent(bubble, FrameworkStatsLog.BUBBLE_UICHANGED__ACTION__DISMISSED); return; diff --git a/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/BubbleOverflowTest.java b/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/BubbleOverflowTest.java new file mode 100644 index 0000000000000..8278c67a9b4f4 --- /dev/null +++ b/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/BubbleOverflowTest.java @@ -0,0 +1,85 @@ +/* + * Copyright (C) 2022 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.wm.shell; + +import static com.google.common.truth.Truth.assertThat; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import android.testing.AndroidTestingRunner; +import android.testing.TestableLooper; +import android.view.WindowManager; + +import androidx.test.filters.SmallTest; + +import com.android.wm.shell.bubbles.BubbleController; +import com.android.wm.shell.bubbles.BubbleOverflow; +import com.android.wm.shell.bubbles.BubbleStackView; +import com.android.wm.shell.bubbles.TestableBubblePositioner; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; + +/** + * Unit tests for {@link com.android.wm.shell.bubbles.BubbleOverflow}. + */ +@SmallTest +@RunWith(AndroidTestingRunner.class) +@TestableLooper.RunWithLooper(setAsMainLooper = true) +public class BubbleOverflowTest extends ShellTestCase { + + private TestableBubblePositioner mPositioner; + private BubbleOverflow mOverflow; + + @Mock + private BubbleController mBubbleController; + + @Before + public void setUp() throws Exception { + MockitoAnnotations.initMocks(this); + + mPositioner = new TestableBubblePositioner(mContext, mock(WindowManager.class)); + when(mBubbleController.getPositioner()).thenReturn(mPositioner); + when(mBubbleController.getStackView()).thenReturn(mock(BubbleStackView.class)); + + mOverflow = new BubbleOverflow(mContext, mPositioner); + } + + @Test + public void test_initialize() { + assertThat(mOverflow.getExpandedView()).isNull(); + + mOverflow.initialize(mBubbleController); + + assertThat(mOverflow.getExpandedView()).isNotNull(); + assertThat(mOverflow.getExpandedView().getBubbleKey()).isEqualTo(BubbleOverflow.KEY); + } + + @Test + public void test_cleanUpExpandedState() { + mOverflow.createExpandedView(); + assertThat(mOverflow.getExpandedView()).isNotNull(); + + mOverflow.cleanUpExpandedState(); + assertThat(mOverflow.getExpandedView()).isNull(); + } + +}