From f274b012d3a1d1fee7cf62a89e61f6273d065b80 Mon Sep 17 00:00:00 2001 From: Mady Mellor Date: Thu, 17 Feb 2022 14:15:59 -0800 Subject: [PATCH] Only collapse bubbles when its from gesture nav Test: atest BubblesTest NewNotifPipelineBubblesTest Test: manual - force notification permission dialog to always show - open a bubble => it doesn't immediately collapse - swipe up on gesture nav => the bubble collapses Bug: 218517365 Change-Id: I4271b955cac51b85e0d92e738fa2d0e38afeb154 --- .../wm/shell/bubbles/BubbleController.java | 34 +++++++++- .../statusbar/phone/ShadeControllerImpl.java | 8 +-- .../systemui/statusbar/phone/StatusBar.java | 16 ++--- .../statusbar/phone/StatusBarTest.java | 2 +- .../android/systemui/wmshell/BubblesTest.java | 67 ++++++++++++++++++ .../wmshell/NewNotifPipelineBubblesTest.java | 68 +++++++++++++++++++ .../server/policy/PhoneWindowManager.java | 1 + 7 files changed, 175 insertions(+), 21 deletions(-) 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 241f1a7d4d1ca..d0138a4882953 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 @@ -47,7 +47,10 @@ import android.app.ActivityManager; import android.app.Notification; import android.app.NotificationChannel; import android.app.PendingIntent; +import android.content.BroadcastReceiver; import android.content.Context; +import android.content.Intent; +import android.content.IntentFilter; import android.content.pm.ActivityInfo; import android.content.pm.LauncherApps; import android.content.pm.PackageManager; @@ -131,6 +134,10 @@ public class BubbleController { public static final String RIGHT_POSITION = "Right"; public static final String BOTTOM_POSITION = "Bottom"; + // Should match with PhoneWindowManager + private static final String SYSTEM_DIALOG_REASON_KEY = "reason"; + private static final String SYSTEM_DIALOG_REASON_GESTURE_NAV = "gestureNav"; + private final Context mContext; private final BubblesImpl mImpl = new BubblesImpl(); private Bubbles.BubbleExpandListener mExpandListener; @@ -675,6 +682,7 @@ public class BubbleController { try { mAddedToWindowManager = true; + registerBroadcastReceiver(); mBubbleData.getOverflow().initialize(this); mWindowManager.addView(mStackView, mWmLayoutParams); mStackView.setOnApplyWindowInsetsListener((view, windowInsets) -> { @@ -717,6 +725,7 @@ public class BubbleController { try { mAddedToWindowManager = false; + mContext.unregisterReceiver(mBroadcastReceiver); if (mStackView != null) { mWindowManager.removeView(mStackView); mBubbleData.getOverflow().cleanUpExpandedState(); @@ -730,11 +739,34 @@ public class BubbleController { } } + private void registerBroadcastReceiver() { + IntentFilter filter = new IntentFilter(); + filter.addAction(Intent.ACTION_CLOSE_SYSTEM_DIALOGS); + filter.addAction(Intent.ACTION_SCREEN_OFF); + mContext.registerReceiver(mBroadcastReceiver, filter); + } + + private final BroadcastReceiver mBroadcastReceiver = new BroadcastReceiver() { + @Override + public void onReceive(Context context, Intent intent) { + if (!isStackExpanded()) return; // Nothing to do + + String action = intent.getAction(); + String reason = intent.getStringExtra(SYSTEM_DIALOG_REASON_KEY); + if ((Intent.ACTION_CLOSE_SYSTEM_DIALOGS.equals(action) + && SYSTEM_DIALOG_REASON_GESTURE_NAV.equals(reason)) + || Intent.ACTION_SCREEN_OFF.equals(action)) { + mMainExecutor.execute(() -> collapseStack()); + } + } + }; + /** * Called by the BubbleStackView and whenever all bubbles have animated out, and none have been * added in the meantime. */ - void onAllBubblesAnimatedOut() { + @VisibleForTesting + public void onAllBubblesAnimatedOut() { if (mStackView != null) { mStackView.setVisibility(INVISIBLE); removeFromWindowManagerMaybe(); diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/ShadeControllerImpl.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/ShadeControllerImpl.java index 72237b1ca6c6c..53ef97dc317cd 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/ShadeControllerImpl.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/ShadeControllerImpl.java @@ -27,7 +27,6 @@ import com.android.systemui.statusbar.CommandQueue; import com.android.systemui.statusbar.NotificationPresenter; import com.android.systemui.statusbar.NotificationShadeWindowController; import com.android.systemui.statusbar.StatusBarState; -import com.android.wm.shell.bubbles.Bubbles; import java.util.ArrayList; import java.util.Optional; @@ -50,7 +49,6 @@ public class ShadeControllerImpl implements ShadeController { private final int mDisplayId; protected final Lazy> mStatusBarOptionalLazy; private final Lazy mAssistManagerLazy; - private final Optional mBubblesOptional; private final ArrayList mPostCollapseRunnables = new ArrayList<>(); @@ -62,8 +60,7 @@ public class ShadeControllerImpl implements ShadeController { StatusBarKeyguardViewManager statusBarKeyguardViewManager, WindowManager windowManager, Lazy> statusBarOptionalLazy, - Lazy assistManagerLazy, - Optional bubblesOptional + Lazy assistManagerLazy ) { mCommandQueue = commandQueue; mStatusBarStateController = statusBarStateController; @@ -73,7 +70,6 @@ public class ShadeControllerImpl implements ShadeController { // TODO: Remove circular reference to StatusBar when possible. mStatusBarOptionalLazy = statusBarOptionalLazy; mAssistManagerLazy = assistManagerLazy; - mBubblesOptional = bubblesOptional; } @Override @@ -131,8 +127,6 @@ public class ShadeControllerImpl implements ShadeController { getStatusBar().getNotificationShadeWindowViewController().cancelExpandHelper(); getNotificationPanelViewController() .collapsePanel(true /* animate */, delayed, speedUpFactor); - } else if (mBubblesOptional.isPresent()) { - mBubblesOptional.get().collapseStack(); } } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBar.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBar.java index 82e0e6726437d..cffdc290c0412 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBar.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBar.java @@ -270,6 +270,7 @@ public class StatusBar extends CoreStartable implements protected static final int MSG_DISMISS_KEYBOARD_SHORTCUTS_MENU = 1027; // Should match the values in PhoneWindowManager + public static final String SYSTEM_DIALOG_REASON_KEY = "reason"; public static final String SYSTEM_DIALOG_REASON_RECENT_APPS = "recentapps"; public static final String SYSTEM_DIALOG_REASON_DREAM = "dream"; static public final String SYSTEM_DIALOG_REASON_SCREENSHOT = "screenshot"; @@ -2661,15 +2662,12 @@ public class StatusBar extends CoreStartable implements Trace.beginSection("StatusBar#onReceive"); if (DEBUG) Log.v(TAG, "onReceive: " + intent); String action = intent.getAction(); + String reason = intent.getStringExtra(SYSTEM_DIALOG_REASON_KEY); if (Intent.ACTION_CLOSE_SYSTEM_DIALOGS.equals(action)) { KeyboardShortcuts.dismiss(); mRemoteInputManager.closeRemoteInputs(); - if (mBubblesOptional.isPresent() && mBubblesOptional.get().isStackExpanded()) { - mBubblesOptional.get().collapseStack(); - } if (mLockscreenUserManager.isCurrentProfile(getSendingUserId())) { int flags = CommandQueue.FLAG_EXCLUDE_NONE; - String reason = intent.getStringExtra("reason"); if (reason != null) { if (reason.equals(SYSTEM_DIALOG_REASON_RECENT_APPS)) { flags |= CommandQueue.FLAG_EXCLUDE_RECENTS_PANEL; @@ -2684,19 +2682,13 @@ public class StatusBar extends CoreStartable implements } mShadeController.animateCollapsePanels(flags); } - } - else if (Intent.ACTION_SCREEN_OFF.equals(action)) { + } else if (Intent.ACTION_SCREEN_OFF.equals(action)) { if (mNotificationShadeWindowController != null) { mNotificationShadeWindowController.setNotTouchable(false); } - if (mBubblesOptional.isPresent() && mBubblesOptional.get().isStackExpanded()) { - // Post to main thread, since updating the UI. - mMainExecutor.execute(() -> mBubblesOptional.get().collapseStack()); - } finishBarAnimations(); resetUserExpandedStates(); - } - else if (DevicePolicyManager.ACTION_SHOW_DEVICE_MONITORING_DIALOG.equals(action)) { + } else if (DevicePolicyManager.ACTION_SHOW_DEVICE_MONITORING_DIALOG.equals(action)) { mQSPanelController.showDeviceMonitoringDialog(); } Trace.endSection(); diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/StatusBarTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/StatusBarTest.java index c7db9e43166b0..86109367d504a 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/StatusBarTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/StatusBarTest.java @@ -381,7 +381,7 @@ public class StatusBarTest extends SysuiTestCase { mShadeController = new ShadeControllerImpl(mCommandQueue, mStatusBarStateController, mNotificationShadeWindowController, mStatusBarKeyguardViewManager, mContext.getSystemService(WindowManager.class), - () -> Optional.of(mStatusBar), () -> mAssistManager, Optional.of(mBubbles)); + () -> Optional.of(mStatusBar), () -> mAssistManager); when(mOperatorNameViewControllerFactory.create(any())) .thenReturn(mOperatorNameViewController); diff --git a/packages/SystemUI/tests/src/com/android/systemui/wmshell/BubblesTest.java b/packages/SystemUI/tests/src/com/android/systemui/wmshell/BubblesTest.java index 4bc4e6ea98f9d..593b97e55739b 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/wmshell/BubblesTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/wmshell/BubblesTest.java @@ -50,8 +50,10 @@ import android.app.IActivityManager; import android.app.INotificationManager; import android.app.Notification; import android.app.PendingIntent; +import android.content.BroadcastReceiver; import android.content.Context; import android.content.Intent; +import android.content.IntentFilter; import android.content.pm.ApplicationInfo; import android.content.pm.LauncherApps; import android.content.pm.PackageManager; @@ -197,6 +199,11 @@ public class BubblesTest extends SysuiTestCase { private ArgumentCaptor mRemoveInterceptorCaptor; @Captor private ArgumentCaptor> mBubbleListCaptor; + @Captor + private ArgumentCaptor mFilterArgumentCaptor; + @Captor + private ArgumentCaptor mBroadcastReceiverArgumentCaptor; + private BubblesManager mBubblesManager; // TODO(178618782): Move tests on the controller directly to the shell @@ -1357,6 +1364,66 @@ public class BubblesTest extends SysuiTestCase { assertStackCollapsed(); } + @Test + public void testRegisterUnregisterBroadcastListener() { + spyOn(mContext); + mBubbleController.updateBubble(mBubbleEntry); + verify(mContext).registerReceiver(mBroadcastReceiverArgumentCaptor.capture(), + mFilterArgumentCaptor.capture()); + assertThat(mFilterArgumentCaptor.getValue().getAction(0)).isEqualTo( + Intent.ACTION_CLOSE_SYSTEM_DIALOGS); + assertThat(mFilterArgumentCaptor.getValue().getAction(1)).isEqualTo( + Intent.ACTION_SCREEN_OFF); + + mBubbleData.dismissBubbleWithKey(mBubbleEntry.getKey(), REASON_APP_CANCEL); + // TODO: not certain why this isn't called normally when tests are run, perhaps because + // it's after an animation in BSV. This calls BubbleController#removeFromWindowManagerMaybe + mBubbleController.onAllBubblesAnimatedOut(); + + verify(mContext).unregisterReceiver(eq(mBroadcastReceiverArgumentCaptor.getValue())); + } + + @Test + public void testBroadcastReceiverCloseDialogs_notGestureNav() { + spyOn(mContext); + mBubbleController.updateBubble(mBubbleEntry); + mBubbleData.setExpanded(true); + verify(mContext).registerReceiver(mBroadcastReceiverArgumentCaptor.capture(), + mFilterArgumentCaptor.capture()); + Intent i = new Intent(Intent.ACTION_CLOSE_SYSTEM_DIALOGS); + mBroadcastReceiverArgumentCaptor.getValue().onReceive(mContext, i); + + assertStackExpanded(); + } + + @Test + public void testBroadcastReceiverCloseDialogs_reasonGestureNav() { + spyOn(mContext); + mBubbleController.updateBubble(mBubbleEntry); + mBubbleData.setExpanded(true); + + verify(mContext).registerReceiver(mBroadcastReceiverArgumentCaptor.capture(), + mFilterArgumentCaptor.capture()); + Intent i = new Intent(Intent.ACTION_CLOSE_SYSTEM_DIALOGS); + i.putExtra("reason", "gestureNav"); + mBroadcastReceiverArgumentCaptor.getValue().onReceive(mContext, i); + assertStackCollapsed(); + } + + @Test + public void testBroadcastReceiver_screenOff() { + spyOn(mContext); + mBubbleController.updateBubble(mBubbleEntry); + mBubbleData.setExpanded(true); + + verify(mContext).registerReceiver(mBroadcastReceiverArgumentCaptor.capture(), + mFilterArgumentCaptor.capture()); + + Intent i = new Intent(Intent.ACTION_SCREEN_OFF); + mBroadcastReceiverArgumentCaptor.getValue().onReceive(mContext, i); + assertStackCollapsed(); + } + /** Creates a bubble using the userId and package. */ private Bubble createBubble(int userId, String pkg) { final UserHandle userHandle = new UserHandle(userId); diff --git a/packages/SystemUI/tests/src/com/android/systemui/wmshell/NewNotifPipelineBubblesTest.java b/packages/SystemUI/tests/src/com/android/systemui/wmshell/NewNotifPipelineBubblesTest.java index 75d8453cb0717..cc848bc34217c 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/wmshell/NewNotifPipelineBubblesTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/wmshell/NewNotifPipelineBubblesTest.java @@ -46,6 +46,9 @@ import android.app.IActivityManager; import android.app.INotificationManager; import android.app.Notification; import android.app.PendingIntent; +import android.content.BroadcastReceiver; +import android.content.Intent; +import android.content.IntentFilter; import android.content.pm.LauncherApps; import android.hardware.display.AmbientDisplayConfiguration; import android.os.Handler; @@ -179,6 +182,10 @@ public class NewNotifPipelineBubblesTest extends SysuiTestCase { private ArgumentCaptor mNotifListenerCaptor; @Captor private ArgumentCaptor> mBubbleListCaptor; + @Captor + private ArgumentCaptor mFilterArgumentCaptor; + @Captor + private ArgumentCaptor mBroadcastReceiverArgumentCaptor; private BubblesManager mBubblesManager; private TestableBubbleController mBubbleController; @@ -1176,6 +1183,67 @@ public class NewNotifPipelineBubblesTest extends SysuiTestCase { assertStackCollapsed(); } + + @Test + public void testRegisterUnregisterBroadcastListener() { + spyOn(mContext); + mBubbleController.updateBubble(mBubbleEntry); + verify(mContext).registerReceiver(mBroadcastReceiverArgumentCaptor.capture(), + mFilterArgumentCaptor.capture()); + assertThat(mFilterArgumentCaptor.getValue().getAction(0)).isEqualTo( + Intent.ACTION_CLOSE_SYSTEM_DIALOGS); + assertThat(mFilterArgumentCaptor.getValue().getAction(1)).isEqualTo( + Intent.ACTION_SCREEN_OFF); + + mBubbleData.dismissBubbleWithKey(mBubbleEntry.getKey(), REASON_APP_CANCEL); + // TODO: not certain why this isn't called normally when tests are run, perhaps because + // it's after an animation in BSV. This calls BubbleController#removeFromWindowManagerMaybe + mBubbleController.onAllBubblesAnimatedOut(); + + verify(mContext).unregisterReceiver(eq(mBroadcastReceiverArgumentCaptor.getValue())); + } + + @Test + public void testBroadcastReceiverCloseDialogs_notGestureNav() { + spyOn(mContext); + mBubbleController.updateBubble(mBubbleEntry); + mBubbleData.setExpanded(true); + verify(mContext).registerReceiver(mBroadcastReceiverArgumentCaptor.capture(), + mFilterArgumentCaptor.capture()); + Intent i = new Intent(Intent.ACTION_CLOSE_SYSTEM_DIALOGS); + mBroadcastReceiverArgumentCaptor.getValue().onReceive(mContext, i); + + assertStackExpanded(); + } + + @Test + public void testBroadcastReceiverCloseDialogs_reasonGestureNav() { + spyOn(mContext); + mBubbleController.updateBubble(mBubbleEntry); + mBubbleData.setExpanded(true); + + verify(mContext).registerReceiver(mBroadcastReceiverArgumentCaptor.capture(), + mFilterArgumentCaptor.capture()); + Intent i = new Intent(Intent.ACTION_CLOSE_SYSTEM_DIALOGS); + i.putExtra("reason", "gestureNav"); + mBroadcastReceiverArgumentCaptor.getValue().onReceive(mContext, i); + assertStackCollapsed(); + } + + @Test + public void testBroadcastReceiver_screenOff() { + spyOn(mContext); + mBubbleController.updateBubble(mBubbleEntry); + mBubbleData.setExpanded(true); + + verify(mContext).registerReceiver(mBroadcastReceiverArgumentCaptor.capture(), + mFilterArgumentCaptor.capture()); + + Intent i = new Intent(Intent.ACTION_SCREEN_OFF); + mBroadcastReceiverArgumentCaptor.getValue().onReceive(mContext, i); + assertStackCollapsed(); + } + /** * Sets the bubble metadata flags for this entry. These flags are normally set by * NotificationManagerService when the notification is sent, however, these tests do not diff --git a/services/core/java/com/android/server/policy/PhoneWindowManager.java b/services/core/java/com/android/server/policy/PhoneWindowManager.java index 4387249fa39ed..cc93d4c1c3ffc 100644 --- a/services/core/java/com/android/server/policy/PhoneWindowManager.java +++ b/services/core/java/com/android/server/policy/PhoneWindowManager.java @@ -330,6 +330,7 @@ public class PhoneWindowManager implements WindowManagerPolicy { static public final String SYSTEM_DIALOG_REASON_HOME_KEY = "homekey"; static public final String SYSTEM_DIALOG_REASON_ASSIST = "assist"; static public final String SYSTEM_DIALOG_REASON_SCREENSHOT = "screenshot"; + static public final String SYSTEM_DIALOG_REASON_GESTURE_NAV = "gestureNav"; private static final String TALKBACK_LABEL = "TalkBack";