From c1b97d73b48ac036ea233418f263fe4f39adb6fa Mon Sep 17 00:00:00 2001 From: Ned Burns Date: Fri, 10 Apr 2020 17:40:29 -0400 Subject: [PATCH 1/2] Apply consistent order to SBNActivityStarter It has a lot of fields and constructor parameters, as does its builder. None of them match each other's order. Rearranged everything so they're all in the same order (and somewhat make sense within ordering). Bug: 112656837 Test: manual Change-Id: I37b36e1b7bfc83c7f7328267f4e70d6b6ca5db4c --- .../StatusBarNotificationActivityStarter.java | 254 ++++++++++-------- ...tusBarNotificationActivityStarterTest.java | 51 ++-- 2 files changed, 177 insertions(+), 128 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarNotificationActivityStarter.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarNotificationActivityStarter.java index 53fa2630a9c3d..c258fee7ad3a2 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarNotificationActivityStarter.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarNotificationActivityStarter.java @@ -94,89 +94,116 @@ public class StatusBarNotificationActivityStarter implements NotificationActivit private static final String TAG = "NotifActivityStarter"; protected static final boolean DEBUG = Log.isLoggable(TAG, Log.DEBUG); - private final Lazy mAssistManagerLazy; - private final NotificationGroupManager mGroupManager; - private final StatusBarRemoteInputCallback mStatusBarRemoteInputCallback; - private final NotificationRemoteInputManager mRemoteInputManager; - private final NotificationLockscreenUserManager mLockscreenUserManager; - private final ShadeController mShadeController; - private final StatusBar mStatusBar; - private final KeyguardStateController mKeyguardStateController; - private final ActivityStarter mActivityStarter; + private final Context mContext; + + private final CommandQueue mCommandQueue; + private final Handler mMainThreadHandler; + private final Handler mBackgroundHandler; + private final Executor mUiBgExecutor; + private final NotificationEntryManager mEntryManager; private final NotifPipeline mNotifPipeline; private final NotifCollection mNotifCollection; - private final FeatureFlags mFeatureFlags; - private final StatusBarStateController mStatusBarStateController; - private final NotificationInterruptStateProvider mNotificationInterruptStateProvider; - private final MetricsLogger mMetricsLogger; - private final Context mContext; - private final NotificationPanelViewController mNotificationPanel; - private final NotificationPresenter mPresenter; - private final LockPatternUtils mLockPatternUtils; private final HeadsUpManagerPhone mHeadsUpManager; + private final ActivityStarter mActivityStarter; + private final IStatusBarService mBarService; + private final StatusBarStateController mStatusBarStateController; private final StatusBarKeyguardViewManager mStatusBarKeyguardViewManager; private final KeyguardManager mKeyguardManager; - private final ActivityLaunchAnimator mActivityLaunchAnimator; - private final IStatusBarService mBarService; - private final CommandQueue mCommandQueue; private final IDreamManager mDreamManager; - private final Handler mMainThreadHandler; - private final Handler mBackgroundHandler; - private final ActivityIntentHelper mActivityIntentHelper; private final BubbleController mBubbleController; - private final Executor mUiBgExecutor; + private final Lazy mAssistManagerLazy; + private final NotificationRemoteInputManager mRemoteInputManager; + private final NotificationGroupManager mGroupManager; + private final NotificationLockscreenUserManager mLockscreenUserManager; + private final ShadeController mShadeController; + private final KeyguardStateController mKeyguardStateController; + private final NotificationInterruptStateProvider mNotificationInterruptStateProvider; + private final LockPatternUtils mLockPatternUtils; + private final StatusBarRemoteInputCallback mStatusBarRemoteInputCallback; + private final ActivityIntentHelper mActivityIntentHelper; + + private final FeatureFlags mFeatureFlags; + private final MetricsLogger mMetricsLogger; + + private final StatusBar mStatusBar; + private final NotificationPresenter mPresenter; + private final NotificationPanelViewController mNotificationPanel; + private final ActivityLaunchAnimator mActivityLaunchAnimator; private boolean mIsCollapsingToShowActivityOverLockscreen; - private StatusBarNotificationActivityStarter(Context context, CommandQueue commandQueue, - Lazy assistManagerLazy, NotificationPanelViewController panel, - NotificationPresenter presenter, NotificationEntryManager entryManager, - HeadsUpManagerPhone headsUpManager, ActivityStarter activityStarter, - ActivityLaunchAnimator activityLaunchAnimator, IStatusBarService statusBarService, + private StatusBarNotificationActivityStarter( + Context context, + CommandQueue commandQueue, + Handler mainThreadHandler, + Handler backgroundHandler, + Executor uiBgExecutor, + NotificationEntryManager entryManager, + NotifPipeline notifPipeline, + NotifCollection notifCollection, + HeadsUpManagerPhone headsUpManager, + ActivityStarter activityStarter, + IStatusBarService statusBarService, StatusBarStateController statusBarStateController, StatusBarKeyguardViewManager statusBarKeyguardViewManager, KeyguardManager keyguardManager, - IDreamManager dreamManager, NotificationRemoteInputManager remoteInputManager, - StatusBarRemoteInputCallback remoteInputCallback, NotificationGroupManager groupManager, + IDreamManager dreamManager, + BubbleController bubbleController, + Lazy assistManagerLazy, + NotificationRemoteInputManager remoteInputManager, + NotificationGroupManager groupManager, NotificationLockscreenUserManager lockscreenUserManager, - ShadeController shadeController, StatusBar statusBar, + ShadeController shadeController, KeyguardStateController keyguardStateController, NotificationInterruptStateProvider notificationInterruptStateProvider, - MetricsLogger metricsLogger, LockPatternUtils lockPatternUtils, - Handler mainThreadHandler, Handler backgroundHandler, Executor uiBgExecutor, - ActivityIntentHelper activityIntentHelper, BubbleController bubbleController, - FeatureFlags featureFlags, NotifPipeline notifPipeline, - NotifCollection notifCollection) { + LockPatternUtils lockPatternUtils, + StatusBarRemoteInputCallback remoteInputCallback, + ActivityIntentHelper activityIntentHelper, + + FeatureFlags featureFlags, + MetricsLogger metricsLogger, + + StatusBar statusBar, + NotificationPresenter presenter, + NotificationPanelViewController panel, + ActivityLaunchAnimator activityLaunchAnimator) { mContext = context; - mNotificationPanel = panel; - mPresenter = presenter; - mHeadsUpManager = headsUpManager; - mActivityLaunchAnimator = activityLaunchAnimator; - mBarService = statusBarService; mCommandQueue = commandQueue; + mMainThreadHandler = mainThreadHandler; + mBackgroundHandler = backgroundHandler; + mUiBgExecutor = uiBgExecutor; + mEntryManager = entryManager; + mNotifPipeline = notifPipeline; + mNotifCollection = notifCollection; + mHeadsUpManager = headsUpManager; + mActivityStarter = activityStarter; + mBarService = statusBarService; + mStatusBarStateController = statusBarStateController; mStatusBarKeyguardViewManager = statusBarKeyguardViewManager; mKeyguardManager = keyguardManager; mDreamManager = dreamManager; + mBubbleController = bubbleController; + mAssistManagerLazy = assistManagerLazy; mRemoteInputManager = remoteInputManager; + mGroupManager = groupManager; mLockscreenUserManager = lockscreenUserManager; mShadeController = shadeController; + mKeyguardStateController = keyguardStateController; + mNotificationInterruptStateProvider = notificationInterruptStateProvider; + mLockPatternUtils = lockPatternUtils; + mStatusBarRemoteInputCallback = remoteInputCallback; + mActivityIntentHelper = activityIntentHelper; + + mFeatureFlags = featureFlags; + mMetricsLogger = metricsLogger; + // TODO: use KeyguardStateController#isOccluded to remove this dependency mStatusBar = statusBar; - mKeyguardStateController = keyguardStateController; - mActivityStarter = activityStarter; - mEntryManager = entryManager; - mStatusBarStateController = statusBarStateController; - mNotificationInterruptStateProvider = notificationInterruptStateProvider; - mMetricsLogger = metricsLogger; - mAssistManagerLazy = assistManagerLazy; - mGroupManager = groupManager; - mLockPatternUtils = lockPatternUtils; - mBackgroundHandler = backgroundHandler; - mUiBgExecutor = uiBgExecutor; - mFeatureFlags = featureFlags; - mNotifPipeline = notifPipeline; - mNotifCollection = notifCollection; + mPresenter = presenter; + mNotificationPanel = panel; + mActivityLaunchAnimator = activityLaunchAnimator; + if (!mFeatureFlags.isNewNotifPipelineRenderingEnabled()) { mEntryManager.addNotificationEntryListener(new NotificationEntryListener() { @Override @@ -192,11 +219,6 @@ public class StatusBarNotificationActivityStarter implements NotificationActivit } }); } - - mStatusBarRemoteInputCallback = remoteInputCallback; - mMainThreadHandler = mainThreadHandler; - mActivityIntentHelper = activityIntentHelper; - mBubbleController = bubbleController; } /** @@ -578,9 +600,10 @@ public class StatusBarNotificationActivityStarter implements NotificationActivit public static class Builder { private final Context mContext; private final CommandQueue mCommandQueue; - private final Lazy mAssistManagerLazy; + private final Handler mMainThreadHandler; + private final Handler mBackgroundHandler; + private final Executor mUiBgExecutor; private final NotificationEntryManager mEntryManager; - private final FeatureFlags mFeatureFlags; private final NotifPipeline mNotifPipeline; private final NotifCollection mNotifCollection; private final HeadsUpManagerPhone mHeadsUpManager; @@ -590,30 +613,36 @@ public class StatusBarNotificationActivityStarter implements NotificationActivit private final StatusBarKeyguardViewManager mStatusBarKeyguardViewManager; private final KeyguardManager mKeyguardManager; private final IDreamManager mDreamManager; + private final BubbleController mBubbleController; + private final Lazy mAssistManagerLazy; private final NotificationRemoteInputManager mRemoteInputManager; - private final StatusBarRemoteInputCallback mRemoteInputCallback; private final NotificationGroupManager mGroupManager; private final NotificationLockscreenUserManager mLockscreenUserManager; - private final KeyguardStateController mKeyguardStateController; - private final MetricsLogger mMetricsLogger; - private final LockPatternUtils mLockPatternUtils; - private final Handler mMainThreadHandler; - private final Handler mBackgroundHandler; - private final Executor mUiBgExecutor; - private final ActivityIntentHelper mActivityIntentHelper; - private final BubbleController mBubbleController; - private NotificationPanelViewController mNotificationPanelViewController; - private NotificationInterruptStateProvider mNotificationInterruptStateProvider; private final ShadeController mShadeController; - private NotificationPresenter mNotificationPresenter; - private ActivityLaunchAnimator mActivityLaunchAnimator; + private final KeyguardStateController mKeyguardStateController; + private final NotificationInterruptStateProvider mNotificationInterruptStateProvider; + private final LockPatternUtils mLockPatternUtils; + private final StatusBarRemoteInputCallback mRemoteInputCallback; + private final ActivityIntentHelper mActivityIntentHelper; + + private final FeatureFlags mFeatureFlags; + private final MetricsLogger mMetricsLogger; + private StatusBar mStatusBar; + private NotificationPresenter mNotificationPresenter; + private NotificationPanelViewController mNotificationPanelViewController; + private ActivityLaunchAnimator mActivityLaunchAnimator; @Inject - public Builder(Context context, + public Builder( + Context context, CommandQueue commandQueue, - Lazy assistManagerLazy, + @Main Handler mainThreadHandler, + @Background Handler backgroundHandler, + @UiBackground Executor uiBgExecutor, NotificationEntryManager entryManager, + NotifPipeline notifPipeline, + NotifCollection notifCollection, HeadsUpManagerPhone headsUpManager, ActivityStarter activityStarter, IStatusBarService statusBarService, @@ -621,27 +650,29 @@ public class StatusBarNotificationActivityStarter implements NotificationActivit StatusBarKeyguardViewManager statusBarKeyguardViewManager, KeyguardManager keyguardManager, IDreamManager dreamManager, + BubbleController bubbleController, + Lazy assistManagerLazy, NotificationRemoteInputManager remoteInputManager, - StatusBarRemoteInputCallback remoteInputCallback, NotificationGroupManager groupManager, NotificationLockscreenUserManager lockscreenUserManager, + ShadeController shadeController, KeyguardStateController keyguardStateController, NotificationInterruptStateProvider notificationInterruptStateProvider, - MetricsLogger metricsLogger, LockPatternUtils lockPatternUtils, - @Main Handler mainThreadHandler, - @Background Handler backgroundHandler, - @UiBackground Executor uiBgExecutor, + StatusBarRemoteInputCallback remoteInputCallback, ActivityIntentHelper activityIntentHelper, - BubbleController bubbleController, - ShadeController shadeController, + FeatureFlags featureFlags, - NotifPipeline notifPipeline, - NotifCollection notifCollection) { + MetricsLogger metricsLogger) { + mContext = context; mCommandQueue = commandQueue; - mAssistManagerLazy = assistManagerLazy; + mMainThreadHandler = mainThreadHandler; + mBackgroundHandler = backgroundHandler; + mUiBgExecutor = uiBgExecutor; mEntryManager = entryManager; + mNotifPipeline = notifPipeline; + mNotifCollection = notifCollection; mHeadsUpManager = headsUpManager; mActivityStarter = activityStarter; mStatusBarService = statusBarService; @@ -649,23 +680,20 @@ public class StatusBarNotificationActivityStarter implements NotificationActivit mStatusBarKeyguardViewManager = statusBarKeyguardViewManager; mKeyguardManager = keyguardManager; mDreamManager = dreamManager; + mBubbleController = bubbleController; + mAssistManagerLazy = assistManagerLazy; mRemoteInputManager = remoteInputManager; - mRemoteInputCallback = remoteInputCallback; mGroupManager = groupManager; mLockscreenUserManager = lockscreenUserManager; + mShadeController = shadeController; mKeyguardStateController = keyguardStateController; mNotificationInterruptStateProvider = notificationInterruptStateProvider; - mMetricsLogger = metricsLogger; mLockPatternUtils = lockPatternUtils; - mMainThreadHandler = mainThreadHandler; - mBackgroundHandler = backgroundHandler; - mUiBgExecutor = uiBgExecutor; + mRemoteInputCallback = remoteInputCallback; mActivityIntentHelper = activityIntentHelper; - mBubbleController = bubbleController; - mShadeController = shadeController; + mFeatureFlags = featureFlags; - mNotifPipeline = notifPipeline; - mNotifCollection = notifCollection; + mMetricsLogger = metricsLogger; } /** Sets the status bar to use as {@link StatusBar}. */ @@ -692,37 +720,41 @@ public class StatusBarNotificationActivityStarter implements NotificationActivit } public StatusBarNotificationActivityStarter build() { - return new StatusBarNotificationActivityStarter(mContext, - mCommandQueue, mAssistManagerLazy, - mNotificationPanelViewController, - mNotificationPresenter, + return new StatusBarNotificationActivityStarter( + mContext, + mCommandQueue, + mMainThreadHandler, + mBackgroundHandler, + mUiBgExecutor, mEntryManager, + mNotifPipeline, + mNotifCollection, mHeadsUpManager, mActivityStarter, - mActivityLaunchAnimator, mStatusBarService, mStatusBarStateController, mStatusBarKeyguardViewManager, mKeyguardManager, mDreamManager, + mBubbleController, + mAssistManagerLazy, mRemoteInputManager, - mRemoteInputCallback, mGroupManager, mLockscreenUserManager, mShadeController, - mStatusBar, mKeyguardStateController, mNotificationInterruptStateProvider, - mMetricsLogger, mLockPatternUtils, - mMainThreadHandler, - mBackgroundHandler, - mUiBgExecutor, + mRemoteInputCallback, mActivityIntentHelper, - mBubbleController, + mFeatureFlags, - mNotifPipeline, - mNotifCollection); + mMetricsLogger, + + mStatusBar, + mNotificationPresenter, + mNotificationPanelViewController, + mActivityLaunchAnimator); } } } diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/StatusBarNotificationActivityStarterTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/StatusBarNotificationActivityStarterTest.java index dd28687e749c1..4c7620fe21649 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/StatusBarNotificationActivityStarterTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/StatusBarNotificationActivityStarterTest.java @@ -176,25 +176,42 @@ public class StatusBarNotificationActivityStarterTest extends SysuiTestCase { when(mStatusBarStateController.getState()).thenReturn(StatusBarState.SHADE); when(mFeatureFlags.isNewNotifPipelineRenderingEnabled()).thenReturn(false); - mNotificationActivityStarter = (new StatusBarNotificationActivityStarter.Builder( - getContext(), mock(CommandQueue.class), () -> mAssistManager, - mEntryManager, mock(HeadsUpManagerPhone.class), - mActivityStarter, mStatusBarService, - mock(StatusBarStateController.class), mStatusBarKeyguardViewManager, - mock(KeyguardManager.class), - mock(IDreamManager.class), mRemoteInputManager, - mock(StatusBarRemoteInputCallback.class), mock(NotificationGroupManager.class), - mock(NotificationLockscreenUserManager.class), - mKeyguardStateController, - mock(NotificationInterruptStateProvider.class), mock(MetricsLogger.class), - mock(LockPatternUtils.class), mHandler, mHandler, mUiBgExecutor, - mActivityIntentHelper, mBubbleController, mShadeController, mFeatureFlags, - mNotifPipeline, mNotifCollection) + mNotificationActivityStarter = + new StatusBarNotificationActivityStarter.Builder( + getContext(), + mock(CommandQueue.class), + mHandler, + mHandler, + mUiBgExecutor, + mEntryManager, + mNotifPipeline, + mNotifCollection, + mock(HeadsUpManagerPhone.class), + mActivityStarter, + mStatusBarService, + mock(StatusBarStateController.class), + mStatusBarKeyguardViewManager, + mock(KeyguardManager.class), + mock(IDreamManager.class), + mBubbleController, + () -> mAssistManager, + mRemoteInputManager, + mock(NotificationGroupManager.class), + mock(NotificationLockscreenUserManager.class), + mShadeController, + mKeyguardStateController, + mock(NotificationInterruptStateProvider.class), + mock(LockPatternUtils.class), + mock(StatusBarRemoteInputCallback.class), + mActivityIntentHelper, + + mFeatureFlags, + mock(MetricsLogger.class)) .setStatusBar(mStatusBar) - .setNotificationPanelViewController(mock(NotificationPanelViewController.class)) .setNotificationPresenter(mock(NotificationPresenter.class)) - .setActivityLaunchAnimator(mock(ActivityLaunchAnimator.class))) - .build(); + .setNotificationPanelViewController(mock(NotificationPanelViewController.class)) + .setActivityLaunchAnimator(mock(ActivityLaunchAnimator.class)) + .build(); // set up dismissKeyguardThenExecute to synchronously invoke the OnDismissAction arg doAnswer(mCallOnDismiss).when(mActivityStarter).dismissKeyguardThenExecute( From 7b81353e965a357054b61c9eb49aae321bcec0ea Mon Sep 17 00:00:00 2001 From: Ned Burns Date: Fri, 10 Apr 2020 19:08:06 -0400 Subject: [PATCH 2/2] Log when user taps on notifs Add buffer logging to NotificationClicker and SBNActivityStarter to (a) log when the user clicks on a notification and (b) the intent that we fire in response, if any. Bug: 112656837 Test: manual, atest Change-Id: Ib9bdfbc184b58a7260e44e6cda48ba8016358e44 --- .../systemui/log/dagger/LogModule.java | 12 ++ .../log/dagger/NotifInteractionLog.java | 36 ++++++ .../notification/NotificationClicker.java | 52 ++++++-- .../notification/NotificationClickerLogger.kt | 70 +++++++++++ .../init/NotificationsControllerImpl.kt | 10 +- .../StatusBarNotificationActivityStarter.java | 53 +++++--- ...tusBarNotificationActivityStarterLogger.kt | 114 ++++++++++++++++++ ...tusBarNotificationActivityStarterTest.java | 3 +- 8 files changed, 313 insertions(+), 37 deletions(-) create mode 100644 packages/SystemUI/src/com/android/systemui/log/dagger/NotifInteractionLog.java create mode 100644 packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationClickerLogger.kt create mode 100644 packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarNotificationActivityStarterLogger.kt diff --git a/packages/SystemUI/src/com/android/systemui/log/dagger/LogModule.java b/packages/SystemUI/src/com/android/systemui/log/dagger/LogModule.java index 14e3e93908252..123cf78d74f8b 100644 --- a/packages/SystemUI/src/com/android/systemui/log/dagger/LogModule.java +++ b/packages/SystemUI/src/com/android/systemui/log/dagger/LogModule.java @@ -61,6 +61,18 @@ public class LogModule { return buffer; } + /** Provides a logging buffer for all logs related to the data layer of notifications. */ + @Provides + @Singleton + @NotifInteractionLog + public static LogBuffer provideNotifInteractionLogBuffer( + LogcatEchoTracker echoTracker, + DumpManager dumpManager) { + LogBuffer buffer = new LogBuffer("NotifInteractionLog", 50, 10, echoTracker); + buffer.attach(dumpManager); + return buffer; + } + /** Provides a logging buffer for all logs related to Quick Settings. */ @Provides @Singleton diff --git a/packages/SystemUI/src/com/android/systemui/log/dagger/NotifInteractionLog.java b/packages/SystemUI/src/com/android/systemui/log/dagger/NotifInteractionLog.java new file mode 100644 index 0000000000000..20fc6ff445a6b --- /dev/null +++ b/packages/SystemUI/src/com/android/systemui/log/dagger/NotifInteractionLog.java @@ -0,0 +1,36 @@ +/* + * Copyright (C) 2020 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.systemui.log.dagger; + +import static java.lang.annotation.RetentionPolicy.RUNTIME; + +import com.android.systemui.log.LogBuffer; + +import java.lang.annotation.Documented; +import java.lang.annotation.Retention; + +import javax.inject.Qualifier; + +/** + * A {@link LogBuffer} for messages related to the user interacting with notifications (e.g. + * clicking on them). + */ +@Qualifier +@Documented +@Retention(RUNTIME) +public @interface NotifInteractionLog { +} diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationClicker.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationClicker.java index 4e6df0ad1ba43..d364689a65d43 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationClicker.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationClicker.java @@ -23,11 +23,14 @@ import android.view.View; import com.android.systemui.DejankUtils; import com.android.systemui.bubbles.BubbleController; +import com.android.systemui.statusbar.notification.collection.NotificationEntry; import com.android.systemui.statusbar.notification.row.ExpandableNotificationRow; import com.android.systemui.statusbar.phone.StatusBar; import java.util.Optional; +import javax.inject.Inject; + /** * Click handler for generic clicks on notifications. Clicks on specific areas (expansion caret, * app ops icon, etc) are handled elsewhere. @@ -35,15 +38,19 @@ import java.util.Optional; public final class NotificationClicker implements View.OnClickListener { private static final String TAG = "NotificationClicker"; - private final Optional mStatusBar; private final BubbleController mBubbleController; + private final NotificationClickerLogger mLogger; + private final Optional mStatusBar; private final NotificationActivityStarter mNotificationActivityStarter; - public NotificationClicker(Optional statusBar, + private NotificationClicker( BubbleController bubbleController, + NotificationClickerLogger logger, + Optional statusBar, NotificationActivityStarter notificationActivityStarter) { - mStatusBar = statusBar; mBubbleController = bubbleController; + mLogger = logger; + mStatusBar = statusBar; mNotificationActivityStarter = notificationActivityStarter; } @@ -58,25 +65,26 @@ public final class NotificationClicker implements View.OnClickListener { SystemClock.uptimeMillis(), v, "NOTIFICATION_CLICK")); final ExpandableNotificationRow row = (ExpandableNotificationRow) v; - final StatusBarNotification sbn = row.getEntry().getSbn(); - if (sbn == null) { - Log.e(TAG, "NotificationClicker called on an unclickable notification,"); - return; - } + final NotificationEntry entry = row.getEntry(); + mLogger.logOnClick(entry); // Check if the notification is displaying the menu, if so slide notification back if (isMenuVisible(row)) { + mLogger.logMenuVisible(entry); row.animateTranslateNotification(0); return; } else if (row.isChildInGroup() && isMenuVisible(row.getNotificationParent())) { + mLogger.logParentMenuVisible(entry); row.getNotificationParent().animateTranslateNotification(0); return; } else if (row.isSummaryWithChildren() && row.areChildrenExpanded()) { // We never want to open the app directly if the user clicks in between // the notifications. + mLogger.logChildrenExpanded(entry); return; } else if (row.areGutsExposed()) { // ignore click if guts are exposed + mLogger.logGutsExposed(entry); return; } @@ -88,7 +96,7 @@ public final class NotificationClicker implements View.OnClickListener { mBubbleController.collapseStack(); } - mNotificationActivityStarter.onNotificationClicked(sbn, row); + mNotificationActivityStarter.onNotificationClicked(entry.getSbn(), row); } private boolean isMenuVisible(ExpandableNotificationRow row) { @@ -107,4 +115,30 @@ public final class NotificationClicker implements View.OnClickListener { row.setOnClickListener(null); } } + + /** Daggerized builder for NotificationClicker. */ + public static class Builder { + private final BubbleController mBubbleController; + private final NotificationClickerLogger mLogger; + + @Inject + public Builder( + BubbleController bubbleController, + NotificationClickerLogger logger) { + mBubbleController = bubbleController; + mLogger = logger; + } + + /** Builds an instance. */ + public NotificationClicker build( + Optional statusBar, + NotificationActivityStarter notificationActivityStarter + ) { + return new NotificationClicker( + mBubbleController, + mLogger, + statusBar, + notificationActivityStarter); + } + } } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationClickerLogger.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationClickerLogger.kt new file mode 100644 index 0000000000000..fbf033bd22912 --- /dev/null +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationClickerLogger.kt @@ -0,0 +1,70 @@ +/* + * Copyright (C) 2020 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.systemui.statusbar.notification + +import com.android.systemui.log.LogBuffer +import com.android.systemui.log.LogLevel +import com.android.systemui.log.dagger.NotifInteractionLog +import com.android.systemui.statusbar.notification.collection.NotificationEntry +import javax.inject.Inject + +class NotificationClickerLogger @Inject constructor( + @NotifInteractionLog private val buffer: LogBuffer +) { + fun logOnClick(entry: NotificationEntry) { + buffer.log(TAG, LogLevel.DEBUG, { + str1 = entry.key + str2 = entry.ranking.channel.id + }, { + "CLICK $str1 (channel=$str2)" + }) + } + + fun logMenuVisible(entry: NotificationEntry) { + buffer.log(TAG, LogLevel.DEBUG, { + str1 = entry.key + }, { + "Ignoring click on $str1; menu is visible" + }) + } + + fun logParentMenuVisible(entry: NotificationEntry) { + buffer.log(TAG, LogLevel.DEBUG, { + str1 = entry.key + }, { + "Ignoring click on $str1; parent menu is visible" + }) + } + + fun logChildrenExpanded(entry: NotificationEntry) { + buffer.log(TAG, LogLevel.DEBUG, { + str1 = entry.key + }, { + "Ignoring click on $str1; children are expanded" + }) + } + + fun logGutsExposed(entry: NotificationEntry) { + buffer.log(TAG, LogLevel.DEBUG, { + str1 = entry.key + }, { + "Ignoring click on $str1; guts are exposed" + }) + } +} + +private const val TAG = "NotificationClicker" diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/init/NotificationsControllerImpl.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/init/NotificationsControllerImpl.kt index 7f2f898565d80..c9754048e1d14 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/init/NotificationsControllerImpl.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/init/NotificationsControllerImpl.kt @@ -17,7 +17,6 @@ package com.android.systemui.statusbar.notification.init import android.service.notification.StatusBarNotification -import com.android.systemui.bubbles.BubbleController import com.android.systemui.plugins.statusbar.NotificationSwipeActionHelper.SnoozeOption import com.android.systemui.statusbar.FeatureFlags import com.android.systemui.statusbar.NotificationListener @@ -62,12 +61,12 @@ class NotificationsControllerImpl @Inject constructor( private val deviceProvisionedController: DeviceProvisionedController, private val notificationRowBinder: NotificationRowBinderImpl, private val remoteInputUriController: RemoteInputUriController, - private val bubbleController: BubbleController, private val groupManager: NotificationGroupManager, private val groupAlertTransferHelper: NotificationGroupAlertTransferHelper, private val headsUpManager: HeadsUpManager, private val headsUpController: HeadsUpController, - private val headsUpViewBinder: HeadsUpViewBinder + private val headsUpViewBinder: HeadsUpViewBinder, + private val clickerBuilder: NotificationClicker.Builder ) : NotificationsController { override fun initialize( @@ -87,10 +86,7 @@ class NotificationsControllerImpl @Inject constructor( listController.bind() notificationRowBinder.setNotificationClicker( - NotificationClicker( - Optional.of(statusBar), - bubbleController, - notificationActivityStarter)) + clickerBuilder.build(Optional.of(statusBar), notificationActivityStarter)) notificationRowBinder.setUpWithPresenter( presenter, listContainer, diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarNotificationActivityStarter.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarNotificationActivityStarter.java index c258fee7ad3a2..fbe3e9b19248d 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarNotificationActivityStarter.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarNotificationActivityStarter.java @@ -40,7 +40,6 @@ import android.service.notification.NotificationStats; import android.service.notification.StatusBarNotification; import android.text.TextUtils; import android.util.EventLog; -import android.util.Log; import android.view.RemoteAnimationAdapter; import android.view.View; @@ -91,9 +90,6 @@ import dagger.Lazy; */ public class StatusBarNotificationActivityStarter implements NotificationActivityStarter { - private static final String TAG = "NotifActivityStarter"; - protected static final boolean DEBUG = Log.isLoggable(TAG, Log.DEBUG); - private final Context mContext; private final CommandQueue mCommandQueue; @@ -125,6 +121,7 @@ public class StatusBarNotificationActivityStarter implements NotificationActivit private final FeatureFlags mFeatureFlags; private final MetricsLogger mMetricsLogger; + private final StatusBarNotificationActivityStarterLogger mLogger; private final StatusBar mStatusBar; private final NotificationPresenter mPresenter; @@ -163,6 +160,7 @@ public class StatusBarNotificationActivityStarter implements NotificationActivit FeatureFlags featureFlags, MetricsLogger metricsLogger, + StatusBarNotificationActivityStarterLogger logger, StatusBar statusBar, NotificationPresenter presenter, @@ -197,6 +195,7 @@ public class StatusBarNotificationActivityStarter implements NotificationActivit mFeatureFlags = featureFlags; mMetricsLogger = metricsLogger; + mLogger = logger; // TODO: use KeyguardStateController#isOccluded to remove this dependency mStatusBar = statusBar; @@ -229,6 +228,8 @@ public class StatusBarNotificationActivityStarter implements NotificationActivit */ @Override public void onNotificationClicked(StatusBarNotification sbn, ExpandableNotificationRow row) { + mLogger.logStartingActivityFromClick(sbn.getKey()); + RemoteInputController controller = mRemoteInputManager.getController(); if (controller.isRemoteInputActive(row.getEntry()) && !TextUtils.isEmpty(row.getActiveRemoteInputText())) { @@ -247,7 +248,7 @@ public class StatusBarNotificationActivityStarter implements NotificationActivit // The only valid case is Bubble notifications. Guard against other cases // entering here. if (intent == null && !isBubble) { - Log.e(TAG, "onNotificationClicked called for non-clickable notification!"); + mLogger.logNonClickableNotification(sbn.getKey()); return; } @@ -280,6 +281,8 @@ public class StatusBarNotificationActivityStarter implements NotificationActivit boolean isActivityIntent, boolean wasOccluded, boolean showOverLockscreen) { + mLogger.logHandleClickAfterKeyguardDismissed(sbn.getKey()); + // TODO: Some of this code may be able to move to NotificationEntryManager. if (mHeadsUpManager != null && mHeadsUpManager.isAlerting(sbn.getKey())) { // Release the HUN notification to the shade. @@ -326,6 +329,8 @@ public class StatusBarNotificationActivityStarter implements NotificationActivit boolean isActivityIntent, boolean wasOccluded, NotificationEntry parentToCancelFinal) { + mLogger.logHandleClickAfterPanelCollapsed(sbn.getKey()); + String notificationKey = sbn.getKey(); try { // The intent we are sending is for the application, which @@ -365,9 +370,11 @@ public class StatusBarNotificationActivityStarter implements NotificationActivit remoteInputText.toString()); } if (isBubble) { + mLogger.logExpandingBubble(notificationKey); expandBubbleStackOnMainThread(notificationKey); } else { - startNotificationIntent(intent, fillInIntent, row, wasOccluded, isActivityIntent); + startNotificationIntent( + intent, fillInIntent, entry, row, wasOccluded, isActivityIntent); } if (isActivityIntent || isBubble) { mAssistManagerLazy.get().hideAssist(); @@ -414,10 +421,16 @@ public class StatusBarNotificationActivityStarter implements NotificationActivit } } - private void startNotificationIntent(PendingIntent intent, Intent fillInIntent, - View row, boolean wasOccluded, boolean isActivityIntent) { + private void startNotificationIntent( + PendingIntent intent, + Intent fillInIntent, + NotificationEntry entry, + View row, + boolean wasOccluded, + boolean isActivityIntent) { RemoteAnimationAdapter adapter = mActivityLaunchAnimator.getLaunchAnimation(row, wasOccluded); + mLogger.logStartNotificationIntent(entry.getKey(), intent); try { if (adapter != null) { ActivityTaskManager.getService() @@ -430,7 +443,7 @@ public class StatusBarNotificationActivityStarter implements NotificationActivit } catch (RemoteException | PendingIntent.CanceledException e) { // the stack trace isn't very helpful here. // Just log the exception message. - Log.w(TAG, "Sending contentIntent failed: " + e); + mLogger.logSendingIntentFailed(e); // TODO: Dismiss Keyguard. } } @@ -460,13 +473,9 @@ public class StatusBarNotificationActivityStarter implements NotificationActivit private void handleFullScreenIntent(NotificationEntry entry) { if (mNotificationInterruptStateProvider.shouldLaunchFullScreenIntentWhenAdded(entry)) { if (shouldSuppressFullScreenIntent(entry)) { - if (DEBUG) { - Log.d(TAG, "No Fullscreen intent: suppressed by DND: " + entry.getKey()); - } + mLogger.logFullScreenIntentSuppressedByDnD(entry.getKey()); } else if (entry.getImportance() < NotificationManager.IMPORTANCE_HIGH) { - if (DEBUG) { - Log.d(TAG, "No Fullscreen intent: not important enough: " + entry.getKey()); - } + mLogger.logFullScreenIntentNotImportantEnough(entry.getKey()); } else { // Stop screensaver if the notification has a fullscreen intent. // (like an incoming phone call) @@ -479,13 +488,13 @@ public class StatusBarNotificationActivityStarter implements NotificationActivit }); // not immersive & a fullscreen alert should be shown - if (DEBUG) { - Log.d(TAG, "Notification has fullScreenIntent; sending fullScreenIntent"); - } + final PendingIntent fullscreenIntent = + entry.getSbn().getNotification().fullScreenIntent; + mLogger.logSendingFullScreenIntent(entry.getKey(), fullscreenIntent); try { EventLog.writeEvent(EventLogTags.SYSUI_FULLSCREEN_NOTIFICATION, entry.getKey()); - entry.getSbn().getNotification().fullScreenIntent.send(); + fullscreenIntent.send(); entry.notifyFullScreenIntentLaunched(); mMetricsLogger.count("note_fullscreen", 1); } catch (PendingIntent.CanceledException e) { @@ -627,6 +636,7 @@ public class StatusBarNotificationActivityStarter implements NotificationActivit private final FeatureFlags mFeatureFlags; private final MetricsLogger mMetricsLogger; + private final StatusBarNotificationActivityStarterLogger mLogger; private StatusBar mStatusBar; private NotificationPresenter mNotificationPresenter; @@ -663,7 +673,8 @@ public class StatusBarNotificationActivityStarter implements NotificationActivit ActivityIntentHelper activityIntentHelper, FeatureFlags featureFlags, - MetricsLogger metricsLogger) { + MetricsLogger metricsLogger, + StatusBarNotificationActivityStarterLogger logger) { mContext = context; mCommandQueue = commandQueue; @@ -694,6 +705,7 @@ public class StatusBarNotificationActivityStarter implements NotificationActivit mFeatureFlags = featureFlags; mMetricsLogger = metricsLogger; + mLogger = logger; } /** Sets the status bar to use as {@link StatusBar}. */ @@ -750,6 +762,7 @@ public class StatusBarNotificationActivityStarter implements NotificationActivit mFeatureFlags, mMetricsLogger, + mLogger, mStatusBar, mNotificationPresenter, diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarNotificationActivityStarterLogger.kt b/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarNotificationActivityStarterLogger.kt new file mode 100644 index 0000000000000..d118747a03656 --- /dev/null +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarNotificationActivityStarterLogger.kt @@ -0,0 +1,114 @@ +/* + * Copyright (C) 2020 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.systemui.statusbar.phone + +import android.app.PendingIntent +import com.android.systemui.log.LogBuffer +import com.android.systemui.log.LogLevel.DEBUG +import com.android.systemui.log.LogLevel.ERROR +import com.android.systemui.log.LogLevel.INFO +import com.android.systemui.log.LogLevel.WARNING +import com.android.systemui.log.dagger.NotifInteractionLog +import javax.inject.Inject + +class StatusBarNotificationActivityStarterLogger @Inject constructor( + @NotifInteractionLog private val buffer: LogBuffer +) { + fun logStartingActivityFromClick(key: String) { + buffer.log(TAG, DEBUG, { + str1 = key + }, { + "(1/4) onNotificationClicked: $str1" + }) + } + + fun logHandleClickAfterKeyguardDismissed(key: String) { + buffer.log(TAG, DEBUG, { + str1 = key + }, { + "(2/4) handleNotificationClickAfterKeyguardDismissed: $str1" + }) + } + + fun logHandleClickAfterPanelCollapsed(key: String) { + buffer.log(TAG, DEBUG, { + str1 = key + }, { + "(3/4) handleNotificationClickAfterPanelCollapsed: $str1" + }) + } + + fun logStartNotificationIntent(key: String, pendingIntent: PendingIntent) { + buffer.log(TAG, INFO, { + str1 = key + str2 = pendingIntent.intent.toString() + }, { + "(4/4) Starting $str2 for notification $str1" + }) + } + + fun logExpandingBubble(key: String) { + buffer.log(TAG, DEBUG, { + str1 = key + }, { + "Expanding bubble for $str1 (rather than firing intent)" + }) + } + + fun logSendingIntentFailed(e: Exception) { + buffer.log(TAG, WARNING, { + str1 = e.toString() + }, { + "Sending contentIntentFailed: $str1" + }) + } + + fun logNonClickableNotification(key: String) { + buffer.log(TAG, ERROR, { + str1 = key + }, { + "onNotificationClicked called for non-clickable notification! $str1" + }) + } + + fun logFullScreenIntentSuppressedByDnD(key: String) { + buffer.log(TAG, DEBUG, { + str1 = key + }, { + "No Fullscreen intent: suppressed by DND: $str1" + }) + } + + fun logFullScreenIntentNotImportantEnough(key: String) { + buffer.log(TAG, DEBUG, { + str1 = key + }, { + "No Fullscreen intent: not important enough: $str1" + }) + } + + fun logSendingFullScreenIntent(key: String, pendingIntent: PendingIntent) { + buffer.log(TAG, INFO, { + str1 = key + str2 = pendingIntent.intent.toString() + }, { + "Notification $str1 has fullScreenIntent; sending fullScreenIntent $str2" + }) + } +} + +private const val TAG = "NotifActivityStarter" diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/StatusBarNotificationActivityStarterTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/StatusBarNotificationActivityStarterTest.java index 4c7620fe21649..1afe132f00a97 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/StatusBarNotificationActivityStarterTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/StatusBarNotificationActivityStarterTest.java @@ -206,7 +206,8 @@ public class StatusBarNotificationActivityStarterTest extends SysuiTestCase { mActivityIntentHelper, mFeatureFlags, - mock(MetricsLogger.class)) + mock(MetricsLogger.class), + mock(StatusBarNotificationActivityStarterLogger.class)) .setStatusBar(mStatusBar) .setNotificationPresenter(mock(NotificationPresenter.class)) .setNotificationPanelViewController(mock(NotificationPanelViewController.class))