From 03cf35018aa023842e553f729f19931631ae4747 Mon Sep 17 00:00:00 2001 From: Evan Laird Date: Fri, 31 May 2019 16:46:48 -0400 Subject: [PATCH 1/2] Close channel dialog when the guts go away Also don't open the NotificationInfo panel from the keyguard without authenticating first. Test: long press on notification from lockscreen Test: lock the device while the channel editor dialog is open Bug: 133182818 Change-Id: I6686b77cf54ed5c1b82596217263a53d9664fc64 --- .../row/ChannelEditorDialogController.kt | 11 +++++++ .../row/NotificationGutsManager.java | 33 +++++++++++++++++++ .../notification/row/NotificationInfo.java | 18 ++++++++-- .../row/NotificationGutsManagerTest.java | 8 +++-- 4 files changed, 64 insertions(+), 6 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/ChannelEditorDialogController.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/ChannelEditorDialogController.kt index 5378f907e95a6..34b79314969b3 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/ChannelEditorDialogController.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/ChannelEditorDialogController.kt @@ -86,6 +86,10 @@ class ChannelEditorDialogController @Inject constructor( internal val groupNameLookup = hashMapOf() private val channelGroupList = mutableListOf() + /** + * Give the controller all of the information it needs to present the dialog + * for a given app. Does a bunch of querying of NoMan, but won't present anything yet + */ fun prepareDialogForApp( appName: String, packageName: String, @@ -156,6 +160,13 @@ class ChannelEditorDialogController @Inject constructor( dialog.show() } + /** + * Close the dialog without saving. For external callers + */ + fun close() { + done() + } + private fun done() { resetState() dialog.dismiss() diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationGutsManager.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationGutsManager.java index fe890fb3b471d..b5a8aadf651f5 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationGutsManager.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationGutsManager.java @@ -41,12 +41,14 @@ import com.android.internal.logging.MetricsLogger; import com.android.internal.logging.nano.MetricsProto; import com.android.systemui.Dependency; import com.android.systemui.Dumpable; +import com.android.systemui.SysUiServiceProvider; import com.android.systemui.plugins.statusbar.NotificationMenuRowPlugin; import com.android.systemui.plugins.statusbar.StatusBarStateController; import com.android.systemui.statusbar.NotificationLifetimeExtender; import com.android.systemui.statusbar.NotificationLockscreenUserManager; import com.android.systemui.statusbar.NotificationPresenter; import com.android.systemui.statusbar.StatusBarState; +import com.android.systemui.statusbar.StatusBarStateControllerImpl; import com.android.systemui.statusbar.notification.NotificationActivityStarter; import com.android.systemui.statusbar.notification.VisualStabilityManager; import com.android.systemui.statusbar.notification.collection.NotificationEntry; @@ -97,6 +99,8 @@ public class NotificationGutsManager implements Dumpable, NotificationLifetimeEx @VisibleForTesting protected String mKeyToRemoveOnGutsClosed; + private StatusBar mStatusBar; + @Inject public NotificationGutsManager( Context context, @@ -114,6 +118,7 @@ public class NotificationGutsManager implements Dumpable, NotificationLifetimeEx mListContainer = listContainer; mCheckSaveListener = checkSave; mOnSettingsClickListener = onSettingsClick; + mStatusBar = SysUiServiceProvider.getComponent(mContext, StatusBar.class); } public void setNotificationActivityStarter( @@ -376,6 +381,34 @@ public class NotificationGutsManager implements Dumpable, NotificationLifetimeEx int x, int y, NotificationMenuRowPlugin.MenuItem menuItem) { + if (menuItem.getGutsView() instanceof NotificationInfo) { + if (mStatusBarStateController instanceof StatusBarStateControllerImpl) { + ((StatusBarStateControllerImpl) mStatusBarStateController) + .setLeaveOpenOnKeyguardHide(true); + } + + Runnable r = () -> Dependency.get(Dependency.MAIN_HANDLER).post( + () -> openGutsInternal(view, x, y, menuItem)); + + mStatusBar.executeRunnableDismissingKeyguard( + r, + null /* cancelAction */, + false /* dismissShade */, + true /* afterKeyguardGone */, + true /* deferred */); + + return true; + } + return openGutsInternal(view, x, y, menuItem); + } + + @VisibleForTesting + boolean openGutsInternal( + View view, + int x, + int y, + NotificationMenuRowPlugin.MenuItem menuItem) { + if (!(view instanceof ExpandableNotificationRow)) { return false; } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationInfo.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationInfo.java index 7c6c556b5241b..148d83b5ab5c9 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationInfo.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationInfo.java @@ -118,6 +118,7 @@ public class NotificationInfo extends LinearLayout implements NotificationGuts.G private int mStartingChannelImportance; private boolean mWasShownHighPriority; private boolean mPressedApply; + private boolean mPresentingChannelEditorDialog = false; /** * The last importance level chosen by the user. Null if the user has not chosen an importance @@ -447,11 +448,15 @@ public class NotificationInfo extends LinearLayout implements NotificationGuts.G private OnClickListener getTurnOffNotificationsClickListener() { return ((View view) -> { - if (mChannelEditorDialogController != null) { + if (!mPresentingChannelEditorDialog && mChannelEditorDialogController != null) { + mPresentingChannelEditorDialog = true; + mChannelEditorDialogController.prepareDialogForApp(mAppName, mPackageName, mAppUid, mUniqueChannelsInRow, mPkgIcon, mOnSettingsClickListener); - mChannelEditorDialogController.setOnFinishListener( - () -> closeControls(this, false)); + mChannelEditorDialogController.setOnFinishListener(() -> { + mPresentingChannelEditorDialog = false; + closeControls(this, false); + }); mChannelEditorDialogController.show(); } }); @@ -772,6 +777,13 @@ public class NotificationInfo extends LinearLayout implements NotificationGuts.G @Override public boolean handleCloseControls(boolean save, boolean force) { + if (mPresentingChannelEditorDialog && mChannelEditorDialogController != null) { + mPresentingChannelEditorDialog = false; + // No need for the finish listener because we're closing + mChannelEditorDialogController.setOnFinishListener(null); + mChannelEditorDialogController.close(); + } + // Save regardless of the importance so we can lock the importance field if the user wants // to keep getting notifications if (save) { diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/NotificationGutsManagerTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/NotificationGutsManagerTest.java index ef13b61162c5f..7959484702951 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/NotificationGutsManagerTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/NotificationGutsManagerTest.java @@ -67,6 +67,7 @@ import com.android.systemui.statusbar.notification.VisualStabilityManager; import com.android.systemui.statusbar.notification.collection.NotificationEntry; import com.android.systemui.statusbar.notification.row.NotificationGutsManager.OnSettingsClickListener; import com.android.systemui.statusbar.notification.stack.NotificationStackScrollLayout; +import com.android.systemui.statusbar.phone.StatusBar; import com.android.systemui.statusbar.policy.DeviceProvisionedController; import com.android.systemui.util.Assert; @@ -105,6 +106,7 @@ public class NotificationGutsManagerTest extends SysuiTestCase { @Mock private NotificationInfo.CheckSaveListener mCheckSaveListener; @Mock private OnSettingsClickListener mOnSettingsClickListener; @Mock private DeviceProvisionedController mDeviceProvisionedController; + @Mock private StatusBar mStatusBar; @Before public void setUp() { @@ -115,7 +117,7 @@ public class NotificationGutsManagerTest extends SysuiTestCase { mDependency.injectTestDependency(MetricsLogger.class, mMetricsLogger); mDependency.injectTestDependency(VisualStabilityManager.class, mVisualStabilityManager); mHandler = Handler.createAsync(mTestableLooper.getLooper()); - + mContext.putComponent(StatusBar.class, mStatusBar); mHelper = new NotificationTestHelper(mContext); mGutsManager = new NotificationGutsManager(mContext, mVisualStabilityManager); @@ -150,7 +152,7 @@ public class NotificationGutsManagerTest extends SysuiTestCase { when(row.getWindowToken()).thenReturn(new Binder()); when(row.getGuts()).thenReturn(guts); - assertTrue(mGutsManager.openGuts(row, 0, 0, menuItem)); + assertTrue(mGutsManager.openGutsInternal(row, 0, 0, menuItem)); assertEquals(View.INVISIBLE, guts.getVisibility()); mTestableLooper.processAllMessages(); verify(guts).openControls( @@ -198,7 +200,7 @@ public class NotificationGutsManagerTest extends SysuiTestCase { when(entry.getRow()).thenReturn(row); when(entry.getGuts()).thenReturn(guts); - assertTrue(mGutsManager.openGuts(row, 0, 0, menuItem)); + assertTrue(mGutsManager.openGutsInternal(row, 0, 0, menuItem)); mTestableLooper.processAllMessages(); verify(guts).openControls( eq(true), From 0e5378e61339117b1ee23cd96c27e9235a9521d3 Mon Sep 17 00:00:00 2001 From: Evan Laird Date: Fri, 7 Jun 2019 16:12:06 -0400 Subject: [PATCH 2/2] Handle null OnSettingsClickListeners in ChannelDialogEditorController The property was already nullable, but prepareDialogForApp() didn't accept a null click listener. Test: atest ChannelDialogEditorControllerTest Fixes: 134697988 Change-Id: I08c5006ac5519f2baa0df1a5ad29d6802c63fb3c --- .../row/ChannelEditorDialogController.kt | 10 ++++++++-- .../row/ChannelEditorDialogControllerTest.kt | 12 +++++++++++- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/ChannelEditorDialogController.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/ChannelEditorDialogController.kt index 34b79314969b3..8e6822770694a 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/ChannelEditorDialogController.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/ChannelEditorDialogController.kt @@ -29,6 +29,7 @@ import android.graphics.drawable.Drawable import android.graphics.drawable.ColorDrawable import android.util.Log import android.view.Gravity +import android.view.View import android.view.ViewGroup.LayoutParams.MATCH_PARENT import android.view.ViewGroup.LayoutParams.WRAP_CONTENT import android.view.Window @@ -96,7 +97,7 @@ class ChannelEditorDialogController @Inject constructor( uid: Int, channels: Set, appIcon: Drawable, - onSettingsClickListener: NotificationInfo.OnSettingsClickListener + onSettingsClickListener: NotificationInfo.OnSettingsClickListener? ) { this.appName = appName this.packageName = packageName @@ -246,6 +247,11 @@ class ChannelEditorDialogController @Inject constructor( } } + @VisibleForTesting + fun launchSettings(sender: View) { + onSettingsClickListener?.onClick(sender, null, appUid!!) + } + private fun initDialog() { dialog = Dialog(context) @@ -268,7 +274,7 @@ class ChannelEditorDialogController @Inject constructor( } findViewById(R.id.see_more_button)?.setOnClickListener { - onSettingsClickListener?.onClick(it, null, appUid!!) + launchSettings(it) done() } diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/ChannelEditorDialogControllerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/ChannelEditorDialogControllerTest.kt index 76326303ff965..8b81a7a29f846 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/ChannelEditorDialogControllerTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/ChannelEditorDialogControllerTest.kt @@ -109,7 +109,7 @@ class ChannelEditorDialogControllerTest : SysuiTestCase() { } @Test - fun testPrepareDialogForApp_retrievesUpto4Channels() { + fun testPrepareDialogForApp_retrievesUpTo4Channels() { val channel3 = NotificationChannel("test_channel_3", "Test channel 3", IMPORTANCE_DEFAULT) val channel4 = NotificationChannel("test_channel_4", "Test channel 4", IMPORTANCE_DEFAULT) @@ -169,6 +169,16 @@ class ChannelEditorDialogControllerTest : SysuiTestCase() { eq(TEST_PACKAGE_NAME), eq(TEST_UID), eq(true)) } + @Test + fun testSettingsClickListenerNull_noCrash() { + group.channels = listOf(channel1, channel2) + controller.prepareDialogForApp(TEST_APP_NAME, TEST_PACKAGE_NAME, TEST_UID, + setOf(channel1, channel2), appIcon, null) + + // Pass in any old view, it should never actually be used + controller.launchSettings(View(context)) + } + private val clickListener = object : NotificationInfo.OnSettingsClickListener { override fun onClick(v: View, c: NotificationChannel, appUid: Int) { }