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))