From e718d5891e20f85790bec50a3d58707775eda7a8 Mon Sep 17 00:00:00 2001 From: Will Brockman Date: Thu, 17 Jan 2019 16:38:38 -0500 Subject: Use full Notification LogMaker in NotificationStackScrollLayout OnMenuEventListener logs, so that we get the information on which notification the user interacted with. Bug: 121380248 Test: atest SystemUITests:com.android.systemui.statusbar.notification.stack.NotificationStackScrollLayoutTest and manual testing Change-Id: I914557250bed517d2e84fdc5ea3262ab0b062d5d --- .../stack/NotificationStackScrollLayout.java | 17 +++-- .../stack/NotificationStackScrollLayoutTest.java | 80 +++++++++++++++++++++- 2 files changed, 88 insertions(+), 9 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayout.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayout.java index 2d1f989fb1c9..28aae7781412 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayout.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayout.java @@ -441,7 +441,8 @@ public class NotificationStackScrollLayout extends ViewGroup implements ScrollAd Dependency.get(NotificationEntryManager.class); private final IStatusBarService mBarService = IStatusBarService.Stub.asInterface( ServiceManager.getService(Context.STATUS_BAR_SERVICE)); - private final MetricsLogger mMetricsLogger = Dependency.get(MetricsLogger.class); + @VisibleForTesting + protected final MetricsLogger mMetricsLogger = Dependency.get(MetricsLogger.class); private final NotificationRemoteInputManager mRemoteInputManager = Dependency.get(NotificationRemoteInputManager.class); private final SysuiColorExtractor mColorExtractor = Dependency.get(SysuiColorExtractor.class); @@ -5638,8 +5639,9 @@ public class NotificationStackScrollLayout extends ViewGroup implements ScrollAd } }; + @VisibleForTesting @ShadeViewRefactor(RefactorComponent.INPUT) - private final OnMenuEventListener mMenuEventListener = new OnMenuEventListener() { + protected final OnMenuEventListener mMenuEventListener = new OnMenuEventListener() { @Override public void onMenuClicked(View view, int x, int y, MenuItem item) { if (mLongPressListener == null) { @@ -5647,8 +5649,10 @@ public class NotificationStackScrollLayout extends ViewGroup implements ScrollAd } if (view instanceof ExpandableNotificationRow) { ExpandableNotificationRow row = (ExpandableNotificationRow) view; - MetricsLogger.action(mContext, MetricsEvent.ACTION_TOUCH_GEAR, - row.getStatusBarNotification().getPackageName()); + mMetricsLogger.write(row.getStatusBarNotification().getLogMaker() + .setCategory(MetricsEvent.ACTION_TOUCH_GEAR) + .setType(MetricsEvent.TYPE_ACTION) + ); } mLongPressListener.onLongPress(view, x, y, item); } @@ -5671,8 +5675,9 @@ public class NotificationStackScrollLayout extends ViewGroup implements ScrollAd public void onMenuShown(View row) { if (row instanceof ExpandableNotificationRow) { ExpandableNotificationRow notificationRow = (ExpandableNotificationRow) row; - MetricsLogger.action(mContext, MetricsEvent.ACTION_REVEAL_GEAR, - notificationRow.getStatusBarNotification().getPackageName()); + mMetricsLogger.write(notificationRow.getStatusBarNotification().getLogMaker() + .setCategory(MetricsEvent.ACTION_REVEAL_GEAR) + .setType(MetricsEvent.TYPE_ACTION)); mHeadsUpManager.setMenuShown(notificationRow.getEntry(), true); } mSwipeHelper.onMenuShown(row); diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayoutTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayoutTest.java index c140ba25859a..37814f06b9bc 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayoutTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/stack/NotificationStackScrollLayoutTest.java @@ -24,6 +24,7 @@ import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.RETURNS_DEEP_STUBS; import static org.mockito.Mockito.atLeastOnce; @@ -35,17 +36,21 @@ import static org.mockito.Mockito.spy; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import android.metrics.LogMaker; import android.provider.Settings; import android.support.test.annotation.UiThreadTest; import android.support.test.filters.SmallTest; import android.support.test.runner.AndroidJUnit4; import android.view.View; +import com.android.internal.logging.MetricsLogger; +import com.android.internal.logging.nano.MetricsProto; import com.android.systemui.Dependency; import com.android.systemui.ExpandHelper; import com.android.systemui.InitController; import com.android.systemui.R; import com.android.systemui.SysuiTestCase; +import com.android.systemui.plugins.statusbar.NotificationMenuRowPlugin; import com.android.systemui.statusbar.EmptyShadeView; import com.android.systemui.statusbar.NotificationPresenter; import com.android.systemui.statusbar.NotificationRemoteInputManager; @@ -73,6 +78,7 @@ import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.ArgumentCaptor; +import org.mockito.ArgumentMatcher; import org.mockito.Mock; import org.mockito.junit.MockitoJUnit; import org.mockito.junit.MockitoRule; @@ -86,7 +92,8 @@ import java.util.ArrayList; @RunWith(AndroidJUnit4.class) public class NotificationStackScrollLayoutTest extends SysuiTestCase { - private NotificationStackScrollLayout mStackScroller; + private NotificationStackScrollLayout mStackScroller; // Normally test this + private NotificationStackScrollLayout mStackScrollerInternal; // See explanation below @Rule public MockitoRule mockito = MockitoJUnit.rule(); @Mock private StatusBar mBar; @@ -99,9 +106,11 @@ public class NotificationStackScrollLayoutTest extends SysuiTestCase { @Mock private NotificationData mNotificationData; @Mock private NotificationRemoteInputManager mRemoteInputManager; @Mock private RemoteInputController mRemoteInputController; + @Mock private MetricsLogger mMetricsLogger; private TestableNotificationEntryManager mEntryManager; private int mOriginalInterruptionModelSetting; + @Before @UiThreadTest public void setUp() throws Exception { @@ -110,6 +119,7 @@ public class NotificationStackScrollLayoutTest extends SysuiTestCase { NotificationBlockingHelperManager.class, mBlockingHelperManager); mDependency.injectTestDependency(StatusBarStateController.class, mBarState); + mDependency.injectTestDependency(MetricsLogger.class, mMetricsLogger); mDependency.injectTestDependency(NotificationRemoteInputManager.class, mRemoteInputManager); mDependency.injectMockDependency(ShadeController.class); @@ -123,8 +133,15 @@ public class NotificationStackScrollLayoutTest extends SysuiTestCase { NotificationShelf notificationShelf = mock(NotificationShelf.class); - mStackScroller = spy(new NotificationStackScrollLayout(getContext(), null, - true /* allowLongPress */)); + + // The actual class under test. You may need to work with this class directly when + // testing anonymous class members of mStackScroller, like mMenuEventListener, + // which refer to members of NotificationStackScrollLayout. The spy + // holds a copy of the CUT's instances of these classes, so they still refer to the CUT's + // member variables, not the spy's member variables. + mStackScrollerInternal = new NotificationStackScrollLayout(getContext(), null, + true /* allowLongPress */); + mStackScroller = spy(mStackScrollerInternal); mStackScroller.setShelf(notificationShelf); mStackScroller.setStatusBar(mBar); mStackScroller.setScrimController(mock(ScrimController.class)); @@ -422,6 +439,63 @@ public class NotificationStackScrollLayoutTest extends SysuiTestCase { assertNull(swipeActionHelper.getExposedMenuView()); } + class LogMatcher implements ArgumentMatcher { + private int mCategory, mType; + + LogMatcher(int category, int type) { + mCategory = category; + mType = type; + } + public boolean matches(LogMaker l) { + return (l.getCategory() == mCategory) + && (l.getType() == mType); + } + + public String toString() { + return String.format("LogMaker(%d, %d)", mCategory, mType); + } + } + + private LogMaker logMatcher(int category, int type) { + return argThat(new LogMatcher(category, type)); + } + + @Test + @UiThreadTest + public void testOnMenuClickedLogging() { + // Set up the object under test to have a valid mLongPressListener. We're testing an + // anonymous-class member, mMenuEventListener, so we need to modify the state of the + // class itself, not the Mockito spy copied from it. See notes in setup. + mStackScrollerInternal.setLongPressListener( + mock(ExpandableNotificationRow.LongPressListener.class)); + + ExpandableNotificationRow row = mock(ExpandableNotificationRow.class, RETURNS_DEEP_STUBS); + when(row.getStatusBarNotification().getLogMaker()).thenReturn(new LogMaker( + MetricsProto.MetricsEvent.VIEW_UNKNOWN)); + + mStackScroller.mMenuEventListener.onMenuClicked(row, 0, 0, mock( + NotificationMenuRowPlugin.MenuItem.class)); + verify(row.getStatusBarNotification()).getLogMaker(); // This writes most of the log data + verify(mMetricsLogger).write(logMatcher(MetricsProto.MetricsEvent.ACTION_TOUCH_GEAR, + MetricsProto.MetricsEvent.TYPE_ACTION)); + } + + @Test + @UiThreadTest + public void testOnMenuShownLogging() { + // Set up the object under test to have a valid mHeadsUpManager. See notes in setup. + mStackScrollerInternal.setHeadsUpManager(mHeadsUpManager); + + ExpandableNotificationRow row = mock(ExpandableNotificationRow.class, RETURNS_DEEP_STUBS); + when(row.getStatusBarNotification().getLogMaker()).thenReturn(new LogMaker( + MetricsProto.MetricsEvent.VIEW_UNKNOWN)); + + mStackScroller.mMenuEventListener.onMenuShown(row); + verify(row.getStatusBarNotification()).getLogMaker(); // This writes most of the log data + verify(mMetricsLogger).write(logMatcher(MetricsProto.MetricsEvent.ACTION_REVEAL_GEAR, + MetricsProto.MetricsEvent.TYPE_ACTION)); + } + private void setBarStateForTest(int state) { // Can't inject this through the listener or we end up on the actual implementation // rather than the mock because the spy just coppied the anonymous inner /shruggie. -- cgit v1.2.3-59-g8ed1b