Add logging required to diagnose the multi-entry issues

Bug: 230540148
Bug: 227254780
Test: atest StatusBarNotificationActivityStarterTest
Change-Id: I2d39596f46dce196503c51e37b7eb28f805b9ee4
This commit is contained in:
Jeff DeCew
2022-05-02 19:16:46 +00:00
parent 1b025f37ff
commit 221212c6f1
8 changed files with 114 additions and 69 deletions

View File

@@ -17,7 +17,6 @@
package com.android.systemui.statusbar.notification;
import android.content.Intent;
import android.service.notification.StatusBarNotification;
import android.view.View;
import com.android.systemui.statusbar.notification.collection.NotificationEntry;
@@ -29,7 +28,7 @@ import com.android.systemui.statusbar.notification.row.ExpandableNotificationRow
*/
public interface NotificationActivityStarter {
/** Called when the user clicks on the surface of a notification. */
void onNotificationClicked(StatusBarNotification sbn, ExpandableNotificationRow row);
void onNotificationClicked(NotificationEntry entry, ExpandableNotificationRow row);
/** Called when the user clicks on a button in the notification guts which fires an intent. */
void startNotificationGutsIntent(Intent intent, int appUid,

View File

@@ -104,7 +104,7 @@ public final class NotificationClicker implements View.OnClickListener {
mBubblesOptional.get().collapseStack();
}
mNotificationActivityStarter.onNotificationClicked(entry.getSbn(), row);
mNotificationActivityStarter.onNotificationClicked(entry, row);
}
private boolean isMenuVisible(ExpandableNotificationRow row) {

View File

@@ -27,7 +27,7 @@ class NotificationClickerLogger @Inject constructor(
) {
fun logOnClick(entry: NotificationEntry) {
buffer.log(TAG, LogLevel.DEBUG, {
str1 = entry.key
str1 = entry.logKey
str2 = entry.ranking.channel.id
}, {
"CLICK $str1 (channel=$str2)"
@@ -36,7 +36,7 @@ class NotificationClickerLogger @Inject constructor(
fun logMenuVisible(entry: NotificationEntry) {
buffer.log(TAG, LogLevel.DEBUG, {
str1 = entry.key
str1 = entry.logKey
}, {
"Ignoring click on $str1; menu is visible"
})
@@ -44,7 +44,7 @@ class NotificationClickerLogger @Inject constructor(
fun logParentMenuVisible(entry: NotificationEntry) {
buffer.log(TAG, LogLevel.DEBUG, {
str1 = entry.key
str1 = entry.logKey
}, {
"Ignoring click on $str1; parent menu is visible"
})
@@ -52,7 +52,7 @@ class NotificationClickerLogger @Inject constructor(
fun logChildrenExpanded(entry: NotificationEntry) {
buffer.log(TAG, LogLevel.DEBUG, {
str1 = entry.key
str1 = entry.logKey
}, {
"Ignoring click on $str1; children are expanded"
})
@@ -60,7 +60,7 @@ class NotificationClickerLogger @Inject constructor(
fun logGutsExposed(entry: NotificationEntry) {
buffer.log(TAG, LogLevel.DEBUG, {
str1 = entry.key
str1 = entry.logKey
}, {
"Ignoring click on $str1; guts are exposed"
})

View File

@@ -24,7 +24,8 @@ import android.widget.ImageView;
import com.android.internal.util.ContrastColorUtil;
import com.android.systemui.R;
import com.android.systemui.statusbar.notification.collection.NotificationEntry;
import com.android.systemui.statusbar.notification.collection.ListEntry;
import com.android.systemui.util.Compile;
/**
* A util class for various reusable functions
@@ -74,12 +75,18 @@ public class NotificationUtils {
return (int) (dimensionPixelSize * factor);
}
private static final boolean INCLUDE_HASH_CODE_IN_LIST_ENTRY_LOG_KEY = false;
/** Get the notification key, reformatted for logging, for the (optional) entry */
public static String logKey(NotificationEntry entry) {
public static String logKey(ListEntry entry) {
if (entry == null) {
return "null";
}
return logKey(entry.getKey());
if (Compile.IS_DEBUG && INCLUDE_HASH_CODE_IN_LIST_ENTRY_LOG_KEY) {
return logKey(entry.getKey()) + "@" + Integer.toHexString(entry.hashCode());
} else {
return logKey(entry.getKey());
}
}
/** Removes newlines from the notification key to prettify apps that have these in the tag */

View File

@@ -0,0 +1,31 @@
/*
* Copyright (C) 2022 The Android Open Source Project
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License
*/
package com.android.systemui.statusbar.notification
import android.service.notification.StatusBarNotification
import com.android.systemui.statusbar.notification.collection.ListEntry
/** Get the notification key, reformatted for logging, for the (optional) entry */
val ListEntry?.logKey: String?
get() = this?.let { NotificationUtils.logKey(it) }
/** Get the notification key, reformatted for logging, for the (optional) sbn */
val StatusBarNotification?.logKey: String?
get() = this?.key?.let { NotificationUtils.logKey(it) }
/** Removes newlines from the notification key to prettify apps that have these in the tag */
fun logKey(key: String?): String? = NotificationUtils.logKey(key)

View File

@@ -229,14 +229,13 @@ class StatusBarNotificationActivityStarter implements NotificationActivityStarte
/**
* Called when a notification is clicked.
*
* @param sbn notification that was clicked
* @param entry notification that was clicked
* @param row row for that notification
*/
@Override
public void onNotificationClicked(StatusBarNotification sbn, ExpandableNotificationRow row) {
mLogger.logStartingActivityFromClick(sbn.getKey());
public void onNotificationClicked(NotificationEntry entry, ExpandableNotificationRow row) {
mLogger.logStartingActivityFromClick(entry);
final NotificationEntry entry = row.getEntry();
if (mRemoteInputManager.isRemoteInputActive(entry)
&& !TextUtils.isEmpty(row.getActiveRemoteInputText())) {
// We have an active remote input typed and the user clicked on the notification.
@@ -244,7 +243,7 @@ class StatusBarNotificationActivityStarter implements NotificationActivityStarte
mRemoteInputManager.closeRemoteInputs();
return;
}
Notification notification = sbn.getNotification();
Notification notification = entry.getSbn().getNotification();
final PendingIntent intent = notification.contentIntent != null
? notification.contentIntent
: notification.fullScreenIntent;
@@ -254,7 +253,7 @@ class StatusBarNotificationActivityStarter implements NotificationActivityStarte
// The only valid case is Bubble notifications. Guard against other cases
// entering here.
if (intent == null && !isBubble) {
mLogger.logNonClickableNotification(sbn.getKey());
mLogger.logNonClickableNotification(entry);
return;
}
@@ -299,7 +298,7 @@ class StatusBarNotificationActivityStarter implements NotificationActivityStarte
boolean isActivityIntent,
boolean animate,
boolean showOverLockscreen) {
mLogger.logHandleClickAfterKeyguardDismissed(entry.getKey());
mLogger.logHandleClickAfterKeyguardDismissed(entry);
final Runnable runnable = () -> handleNotificationClickAfterPanelCollapsed(
entry, row, intent, isActivityIntent, animate);
@@ -326,7 +325,7 @@ class StatusBarNotificationActivityStarter implements NotificationActivityStarte
boolean isActivityIntent,
boolean animate) {
String notificationKey = entry.getKey();
mLogger.logHandleClickAfterPanelCollapsed(notificationKey);
mLogger.logHandleClickAfterPanelCollapsed(entry);
try {
// The intent we are sending is for the application, which
@@ -367,7 +366,7 @@ class StatusBarNotificationActivityStarter implements NotificationActivityStarte
}
final boolean canBubble = entry.canBubble();
if (canBubble) {
mLogger.logExpandingBubble(notificationKey);
mLogger.logExpandingBubble(entry);
removeHunAfterClick(row);
expandBubbleStackOnMainThread(entry);
mMainThreadHandler.post(
@@ -489,7 +488,7 @@ class StatusBarNotificationActivityStarter implements NotificationActivityStarte
ExpandableNotificationRow row,
boolean animate,
boolean isActivityIntent) {
mLogger.logStartNotificationIntent(entry.getKey());
mLogger.logStartNotificationIntent(entry);
try {
Runnable onFinishAnimationCallback = animate
? () -> mLaunchEventsEmitter.notifyFinishLaunchNotifActivity(entry)
@@ -515,7 +514,7 @@ class StatusBarNotificationActivityStarter implements NotificationActivityStarte
: getActivityOptions(mCentralSurfaces.getDisplayId(), adapter);
int result = intent.sendAndReturnResult(mContext, 0, fillInIntent, null,
null, null, options);
mLogger.logSendPendingIntent(entry.getKey(), intent, result);
mLogger.logSendPendingIntent(entry, intent, result);
return result;
});
} catch (PendingIntent.CanceledException e) {
@@ -622,9 +621,9 @@ class StatusBarNotificationActivityStarter implements NotificationActivityStarte
void handleFullScreenIntent(NotificationEntry entry) {
if (mNotificationInterruptStateProvider.shouldLaunchFullScreenIntentWhenAdded(entry)) {
if (shouldSuppressFullScreenIntent(entry)) {
mLogger.logFullScreenIntentSuppressedByDnD(entry.getKey());
mLogger.logFullScreenIntentSuppressedByDnD(entry);
} else if (entry.getImportance() < NotificationManager.IMPORTANCE_HIGH) {
mLogger.logFullScreenIntentNotImportantEnough(entry.getKey());
mLogger.logFullScreenIntentNotImportantEnough(entry);
} else {
// Stop screensaver if the notification has a fullscreen intent.
// (like an incoming phone call)
@@ -639,7 +638,7 @@ class StatusBarNotificationActivityStarter implements NotificationActivityStarte
// not immersive & a fullscreen alert should be shown
final PendingIntent fullscreenIntent =
entry.getSbn().getNotification().fullScreenIntent;
mLogger.logSendingFullScreenIntent(entry.getKey(), fullscreenIntent);
mLogger.logSendingFullScreenIntent(entry, fullscreenIntent);
try {
EventLog.writeEvent(EventLogTags.SYSUI_FULLSCREEN_NOTIFICATION,
entry.getKey());

View File

@@ -23,46 +23,48 @@ 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 com.android.systemui.statusbar.notification.collection.NotificationEntry
import com.android.systemui.statusbar.notification.logKey
import javax.inject.Inject
class StatusBarNotificationActivityStarterLogger @Inject constructor(
@NotifInteractionLog private val buffer: LogBuffer
) {
fun logStartingActivityFromClick(key: String) {
fun logStartingActivityFromClick(entry: NotificationEntry) {
buffer.log(TAG, DEBUG, {
str1 = key
str1 = entry.logKey
}, {
"(1/5) onNotificationClicked: $str1"
})
}
fun logHandleClickAfterKeyguardDismissed(key: String) {
fun logHandleClickAfterKeyguardDismissed(entry: NotificationEntry) {
buffer.log(TAG, DEBUG, {
str1 = key
str1 = entry.logKey
}, {
"(2/5) handleNotificationClickAfterKeyguardDismissed: $str1"
})
}
fun logHandleClickAfterPanelCollapsed(key: String) {
fun logHandleClickAfterPanelCollapsed(entry: NotificationEntry) {
buffer.log(TAG, DEBUG, {
str1 = key
str1 = entry.logKey
}, {
"(3/5) handleNotificationClickAfterPanelCollapsed: $str1"
})
}
fun logStartNotificationIntent(key: String) {
fun logStartNotificationIntent(entry: NotificationEntry) {
buffer.log(TAG, INFO, {
str1 = key
str1 = entry.logKey
}, {
"(4/5) startNotificationIntent: $str1"
})
}
fun logSendPendingIntent(key: String, pendingIntent: PendingIntent, result: Int) {
fun logSendPendingIntent(entry: NotificationEntry, pendingIntent: PendingIntent, result: Int) {
buffer.log(TAG, INFO, {
str1 = key
str1 = entry.logKey
str2 = pendingIntent.intent.toString()
int1 = result
}, {
@@ -70,9 +72,9 @@ class StatusBarNotificationActivityStarterLogger @Inject constructor(
})
}
fun logExpandingBubble(key: String) {
fun logExpandingBubble(entry: NotificationEntry) {
buffer.log(TAG, DEBUG, {
str1 = key
str1 = entry.logKey
}, {
"Expanding bubble for $str1 (rather than firing intent)"
})
@@ -86,33 +88,33 @@ class StatusBarNotificationActivityStarterLogger @Inject constructor(
})
}
fun logNonClickableNotification(key: String) {
fun logNonClickableNotification(entry: NotificationEntry) {
buffer.log(TAG, ERROR, {
str1 = key
str1 = entry.logKey
}, {
"onNotificationClicked called for non-clickable notification! $str1"
})
}
fun logFullScreenIntentSuppressedByDnD(key: String) {
fun logFullScreenIntentSuppressedByDnD(entry: NotificationEntry) {
buffer.log(TAG, DEBUG, {
str1 = key
str1 = entry.logKey
}, {
"No Fullscreen intent: suppressed by DND: $str1"
})
}
fun logFullScreenIntentNotImportantEnough(key: String) {
fun logFullScreenIntentNotImportantEnough(entry: NotificationEntry) {
buffer.log(TAG, DEBUG, {
str1 = key
str1 = entry.logKey
}, {
"No Fullscreen intent: not important enough: $str1"
})
}
fun logSendingFullScreenIntent(key: String, pendingIntent: PendingIntent) {
fun logSendingFullScreenIntent(entry: NotificationEntry, pendingIntent: PendingIntent) {
buffer.log(TAG, INFO, {
str1 = key
str1 = entry.logKey
str2 = pendingIntent.intent.toString()
}, {
"Notification $str1 has fullScreenIntent; sending fullScreenIntent $str2"

View File

@@ -265,15 +265,16 @@ public class StatusBarNotificationActivityStarterTest extends SysuiTestCase {
public void testOnNotificationClicked_keyGuardShowing()
throws PendingIntent.CanceledException, RemoteException {
// Given
StatusBarNotification sbn = mNotificationRow.getEntry().getSbn();
sbn.getNotification().contentIntent = mContentIntent;
sbn.getNotification().flags |= Notification.FLAG_AUTO_CANCEL;
NotificationEntry entry = mNotificationRow.getEntry();
Notification notification = entry.getSbn().getNotification();
notification.contentIntent = mContentIntent;
notification.flags |= Notification.FLAG_AUTO_CANCEL;
when(mKeyguardStateController.isShowing()).thenReturn(true);
when(mCentralSurfaces.isOccluded()).thenReturn(true);
// When
mNotificationActivityStarter.onNotificationClicked(sbn, mNotificationRow);
mNotificationActivityStarter.onNotificationClicked(entry, mNotificationRow);
// Then
verify(mShadeController, atLeastOnce()).collapsePanel();
@@ -285,22 +286,23 @@ public class StatusBarNotificationActivityStarterTest extends SysuiTestCase {
InOrder orderVerifier = Mockito.inOrder(mClickNotifier, mOnUserInteractionCallback);
orderVerifier.verify(mClickNotifier).onNotificationClick(
eq(sbn.getKey()), any(NotificationVisibility.class));
eq(entry.getKey()), any(NotificationVisibility.class));
// Notification calls dismiss callback to remove notification due to FLAG_AUTO_CANCEL
orderVerifier.verify(mOnUserInteractionCallback).onDismiss(mNotificationRow.getEntry(),
orderVerifier.verify(mOnUserInteractionCallback).onDismiss(entry,
REASON_CLICK, null);
}
@Test
public void testOnNotificationClicked_bubble_noContentIntent_noKeyGuard()
throws RemoteException {
StatusBarNotification sbn = mBubbleNotificationRow.getEntry().getSbn();
NotificationEntry entry = mBubbleNotificationRow.getEntry();
StatusBarNotification sbn = entry.getSbn();
// Given
sbn.getNotification().contentIntent = null;
// When
mNotificationActivityStarter.onNotificationClicked(sbn, mBubbleNotificationRow);
mNotificationActivityStarter.onNotificationClicked(entry, mBubbleNotificationRow);
// Then
verify(mBubblesManager).expandStackAndSelectBubble(eq(mBubbleNotificationRow.getEntry()));
@@ -311,7 +313,7 @@ public class StatusBarNotificationActivityStarterTest extends SysuiTestCase {
verify(mAssistManager).hideAssist();
verify(mClickNotifier).onNotificationClick(
eq(sbn.getKey()), any(NotificationVisibility.class));
eq(entry.getKey()), any(NotificationVisibility.class));
// The content intent should NOT be sent on click.
verifyZeroInteractions(mContentIntent);
@@ -324,7 +326,8 @@ public class StatusBarNotificationActivityStarterTest extends SysuiTestCase {
@Test
public void testOnNotificationClicked_bubble_noContentIntent_keyGuardShowing()
throws RemoteException {
StatusBarNotification sbn = mBubbleNotificationRow.getEntry().getSbn();
NotificationEntry entry = mBubbleNotificationRow.getEntry();
StatusBarNotification sbn = entry.getSbn();
// Given
sbn.getNotification().contentIntent = null;
@@ -332,7 +335,7 @@ public class StatusBarNotificationActivityStarterTest extends SysuiTestCase {
when(mCentralSurfaces.isOccluded()).thenReturn(true);
// When
mNotificationActivityStarter.onNotificationClicked(sbn, mBubbleNotificationRow);
mNotificationActivityStarter.onNotificationClicked(entry, mBubbleNotificationRow);
// Then
verify(mBubblesManager).expandStackAndSelectBubble(eq(mBubbleNotificationRow.getEntry()));
@@ -342,7 +345,7 @@ public class StatusBarNotificationActivityStarterTest extends SysuiTestCase {
verify(mAssistManager).hideAssist();
verify(mClickNotifier).onNotificationClick(
eq(sbn.getKey()), any(NotificationVisibility.class));
eq(entry.getKey()), any(NotificationVisibility.class));
// The content intent should NOT be sent on click.
verifyZeroInteractions(mContentIntent);
@@ -354,7 +357,8 @@ public class StatusBarNotificationActivityStarterTest extends SysuiTestCase {
@Test
public void testOnNotificationClicked_bubble_withContentIntent_keyGuardShowing()
throws RemoteException {
StatusBarNotification sbn = mBubbleNotificationRow.getEntry().getSbn();
NotificationEntry entry = mBubbleNotificationRow.getEntry();
StatusBarNotification sbn = entry.getSbn();
// Given
sbn.getNotification().contentIntent = mContentIntent;
@@ -362,7 +366,7 @@ public class StatusBarNotificationActivityStarterTest extends SysuiTestCase {
when(mCentralSurfaces.isOccluded()).thenReturn(true);
// When
mNotificationActivityStarter.onNotificationClicked(sbn, mBubbleNotificationRow);
mNotificationActivityStarter.onNotificationClicked(entry, mBubbleNotificationRow);
// Then
verify(mBubblesManager).expandStackAndSelectBubble(mBubbleNotificationRow.getEntry());
@@ -372,7 +376,7 @@ public class StatusBarNotificationActivityStarterTest extends SysuiTestCase {
verify(mAssistManager).hideAssist();
verify(mClickNotifier).onNotificationClick(
eq(sbn.getKey()), any(NotificationVisibility.class));
eq(entry.getKey()), any(NotificationVisibility.class));
// The content intent should NOT be sent on click.
verify(mContentIntent).getIntent();
@@ -408,16 +412,17 @@ public class StatusBarNotificationActivityStarterTest extends SysuiTestCase {
@Test
public void testNotifActivityStarterEventSourceStartEvent_onNotificationClicked() {
final NotificationEntry entry = mNotificationRow.getEntry();
NotifActivityLaunchEvents.Listener listener =
mock(NotifActivityLaunchEvents.Listener.class);
mLaunchEventsEmitter.registerListener(listener);
mNotificationActivityStarter
.onNotificationClicked(mNotificationRow.getEntry().getSbn(), mNotificationRow);
verify(listener).onStartLaunchNotifActivity(mNotificationRow.getEntry());
mNotificationActivityStarter.onNotificationClicked(entry, mNotificationRow);
verify(listener).onStartLaunchNotifActivity(entry);
}
@Test
public void testNotifActivityStarterEventSourceFinishEvent_dismissKeyguardCancelled() {
final NotificationEntry entry = mNotificationRow.getEntry();
NotifActivityLaunchEvents.Listener listener =
mock(NotifActivityLaunchEvents.Listener.class);
mLaunchEventsEmitter.registerListener(listener);
@@ -428,34 +433,36 @@ public class StatusBarNotificationActivityStarterTest extends SysuiTestCase {
.when(mActivityStarter)
.dismissKeyguardThenExecute(any(OnDismissAction.class), any(), anyBoolean());
mNotificationActivityStarter
.onNotificationClicked(mNotificationRow.getEntry().getSbn(), mNotificationRow);
verify(listener).onFinishLaunchNotifActivity(mNotificationRow.getEntry());
.onNotificationClicked(entry, mNotificationRow);
verify(listener).onFinishLaunchNotifActivity(entry);
}
@Test
public void testNotifActivityStarterEventSourceFinishEvent_postPanelCollapse()
throws Exception {
final NotificationEntry entry = mNotificationRow.getEntry();
NotifActivityLaunchEvents.Listener listener =
mock(NotifActivityLaunchEvents.Listener.class);
mLaunchEventsEmitter.registerListener(listener);
mNotificationActivityStarter
.onNotificationClicked(mNotificationRow.getEntry().getSbn(), mNotificationRow);
.onNotificationClicked(entry, mNotificationRow);
ArgumentCaptor<ActivityLaunchAnimator.Controller> controllerCaptor =
ArgumentCaptor.forClass(ActivityLaunchAnimator.Controller.class);
verify(mActivityLaunchAnimator).startPendingIntentWithAnimation(
controllerCaptor.capture(), anyBoolean(), any(), any());
controllerCaptor.getValue().onIntentStarted(false);
verify(listener).onFinishLaunchNotifActivity(mNotificationRow.getEntry());
verify(listener).onFinishLaunchNotifActivity(entry);
}
@Test
public void testNotifActivityStarterEventSourceFinishEvent_postPanelCollapse_noAnimate() {
final NotificationEntry entry = mNotificationRow.getEntry();
NotifActivityLaunchEvents.Listener listener =
mock(NotifActivityLaunchEvents.Listener.class);
mLaunchEventsEmitter.registerListener(listener);
when(mCentralSurfaces.shouldAnimateLaunch(anyBoolean())).thenReturn(false);
mNotificationActivityStarter
.onNotificationClicked(mNotificationRow.getEntry().getSbn(), mNotificationRow);
verify(listener).onFinishLaunchNotifActivity(mNotificationRow.getEntry());
.onNotificationClicked(entry, mNotificationRow);
verify(listener).onFinishLaunchNotifActivity(entry);
}
}