Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit a399f023 authored by Will Brockman's avatar Will Brockman
Browse files

Reconcile logging of blocking helper and manual notification management.

Unify on ACTION_NOTE_CONTROLS for opening of a NotificationInfo, with
subtype to distinguish whether it's for BlockingHelper or not.

In the non-blocking-helper case, don't log user clicks, just the
action. We can come back to this, but the logs are currently
inaccurate and they aren't essential.

Bug: 123519980
Change-Id: I9189d52f9bd20126b0df5568c8be264107d569b3
Test: atest SystemUITests and manual testing.
parent 7447f3bd
Loading
Loading
Loading
Loading
+0 −7
Original line number Diff line number Diff line
@@ -25,7 +25,6 @@ import android.app.NotificationChannel;
import android.content.Context;
import android.content.Intent;
import android.content.pm.PackageManager;
import android.metrics.LogMaker;
import android.net.Uri;
import android.os.ServiceManager;
import android.os.UserHandle;
@@ -392,12 +391,6 @@ public class NotificationGutsManager implements Dumpable, NotificationLifetimeEx
            return false;
        }

        LogMaker logMaker = (row.getStatusBarNotification() == null)
                ? new LogMaker(MetricsProto.MetricsEvent.ACTION_NOTE_CONTROLS)
                : row.getStatusBarNotification().getLogMaker();
        mMetricsLogger.write(logMaker.setCategory(MetricsProto.MetricsEvent.ACTION_NOTE_CONTROLS)
                    .setType(MetricsProto.MetricsEvent.TYPE_ACTION));

        // ensure that it's laid but not visible until actually laid out
        guts.setVisibility(View.INVISIBLE);
        // Post to ensure the the guts are properly laid out.
+65 −35
Original line number Diff line number Diff line
@@ -126,18 +126,23 @@ public class NotificationInfo extends LinearLayout implements NotificationGuts.G
    private OnClickListener mOnKeepShowing = v -> {
        mExitReason = NotificationCounters.BLOCKING_HELPER_KEEP_SHOWING;
        closeControls(v);
        mMetricsLogger.write(getLogMaker().setCategory(MetricsEvent.NOTIFICATION_BLOCKING_HELPER)
        if (mIsForBlockingHelper) {
            mMetricsLogger.write(getLogMaker().setCategory(
                    MetricsEvent.NOTIFICATION_BLOCKING_HELPER)
                    .setType(MetricsEvent.TYPE_ACTION)
                    .setSubtype(MetricsEvent.BLOCKING_HELPER_CLICK_STAY_SILENT));
        }
    };

    private OnClickListener mOnToggleSilent = v -> {
        Runnable saveImportance = () -> {
            swapContent(ACTION_TOGGLE_SILENT, true /* animate */);
            if (mIsForBlockingHelper) {
                mMetricsLogger.write(getLogMaker()
                        .setCategory(MetricsEvent.NOTIFICATION_BLOCKING_HELPER)
                        .setType(MetricsEvent.TYPE_ACTION)
                        .setSubtype(MetricsEvent.BLOCKING_HELPER_CLICK_ALERT_ME));
            }
        };
        if (mCheckSaveListener != null) {
            mCheckSaveListener.checkSave(saveImportance, mSbn);
@@ -149,10 +154,12 @@ public class NotificationInfo extends LinearLayout implements NotificationGuts.G
    private OnClickListener mOnStopOrMinimizeNotifications = v -> {
        Runnable saveImportance = () -> {
            swapContent(ACTION_BLOCK, true /* animate */);
            if (mIsForBlockingHelper) {
                mMetricsLogger.write(getLogMaker()
                        .setCategory(MetricsEvent.NOTIFICATION_BLOCKING_HELPER)
                        .setType(MetricsEvent.TYPE_ACTION)
                        .setSubtype(MetricsEvent.BLOCKING_HELPER_CLICK_BLOCKED));
            }
        };
        if (mCheckSaveListener != null) {
            mCheckSaveListener.checkSave(saveImportance, mSbn);
@@ -164,12 +171,16 @@ public class NotificationInfo extends LinearLayout implements NotificationGuts.G
    private OnClickListener mOnUndo = v -> {
        // Reset exit counter that we'll log and record an undo event separately (not an exit event)
        mExitReason = NotificationCounters.BLOCKING_HELPER_DISMISSED;
        if (mIsForBlockingHelper) {
            logBlockingHelperCounter(NotificationCounters.BLOCKING_HELPER_UNDO);
        mMetricsLogger.write(importanceChangeLogMaker().setType(MetricsEvent.TYPE_DISMISS));
        swapContent(ACTION_UNDO, true /* animate */);
        mMetricsLogger.write(getLogMaker().setCategory(MetricsEvent.NOTIFICATION_BLOCKING_HELPER)
            mMetricsLogger.write(getLogMaker().setCategory(
                    MetricsEvent.NOTIFICATION_BLOCKING_HELPER)
                    .setType(MetricsEvent.TYPE_DISMISS)
                    .setSubtype(MetricsEvent.BLOCKING_HELPER_CLICK_UNDO));
        } else {
            mMetricsLogger.write(importanceChangeLogMaker().setType(MetricsEvent.TYPE_DISMISS));
        }
        swapContent(ACTION_UNDO, true /* animate */);
    };

    public NotificationInfo(Context context, AttributeSet attrs) {
@@ -269,11 +280,11 @@ public class NotificationInfo extends LinearLayout implements NotificationGuts.G
        bindPrompt();
        bindButtons();

        mMetricsLogger.write(getLogMaker().setCategory(MetricsEvent.NOTIFICATION_BLOCKING_HELPER)
                .setType(MetricsEvent.TYPE_OPEN)
                .setSubtype(MetricsEvent.BLOCKING_HELPER_DISPLAY));
        mMetricsLogger.write(notificationControlsLogMaker());
    }



    private void bindHeader() throws RemoteException {
        // Package name
        Drawable pkgicon = null;
@@ -404,19 +415,6 @@ public class NotificationInfo extends LinearLayout implements NotificationGuts.G
        }
    }

    /**
     * Returns an initialized LogMaker for logging importance changes.
     * The caller may override the type (to DISMISS) before passing it to mMetricsLogger.
     * @return new LogMaker
     */
    private LogMaker importanceChangeLogMaker() {
        Integer chosenImportance =
                mChosenImportance != null ? mChosenImportance : mStartingChannelImportance;
        return new LogMaker(MetricsEvent.ACTION_SAVE_IMPORTANCE)
                .setType(MetricsEvent.TYPE_ACTION)
                .setSubtype(chosenImportance - mStartingChannelImportance);
    }

    private boolean hasImportanceChanged() {
        return mSingleNotificationChannel != null
                && mChosenImportance != null
@@ -616,8 +614,8 @@ public class NotificationInfo extends LinearLayout implements NotificationGuts.G
        confirmation.setAlpha(1f);
        header.setVisibility(VISIBLE);
        header.setAlpha(1f);
        mMetricsLogger.write(getLogMaker().setCategory(MetricsEvent.NOTIFICATION_BLOCKING_HELPER)
                .setType(MetricsEvent.TYPE_CLOSE));

        mMetricsLogger.write(notificationControlsLogMaker().setType(MetricsEvent.TYPE_CLOSE));
    }

    @Override
@@ -764,7 +762,39 @@ public class NotificationInfo extends LinearLayout implements NotificationGuts.G
        }
    }

    /**
     * Returns a LogMaker with all available notification information.
     * Caller should set category, type, and maybe subtype, before passing it to mMetricsLogger.
     * @return LogMaker
     */
    private LogMaker getLogMaker() {
        return mSbn.getLogMaker();
        // The constructor requires a category, so also do it in the other branch for consistency.
        return mSbn == null ? new LogMaker(MetricsEvent.NOTIFICATION_BLOCKING_HELPER)
                : mSbn.getLogMaker().setCategory(MetricsEvent.NOTIFICATION_BLOCKING_HELPER);
    }

    /**
     * Returns an initialized LogMaker for logging importance changes.
     * The caller may override the type before passing it to mMetricsLogger.
     * @return LogMaker
     */
    private LogMaker importanceChangeLogMaker() {
        Integer chosenImportance =
                mChosenImportance != null ? mChosenImportance : mStartingChannelImportance;
        return getLogMaker().setCategory(MetricsEvent.ACTION_SAVE_IMPORTANCE)
                .setType(MetricsEvent.TYPE_ACTION)
                .setSubtype(chosenImportance - mStartingChannelImportance);
    }

    /**
     * Returns an initialized LogMaker for logging open/close of the info display.
     * The caller may override the type before passing it to mMetricsLogger.
     * @return LogMaker
     */
    private LogMaker notificationControlsLogMaker() {
        return getLogMaker().setCategory(MetricsEvent.ACTION_NOTE_CONTROLS)
                .setType(MetricsEvent.TYPE_OPEN)
                .setSubtype(mIsForBlockingHelper ? MetricsEvent.BLOCKING_HELPER_DISPLAY
                        : MetricsEvent.BLOCKING_HELPER_UNKNOWN);
    }
}
+0 −30
Original line number Diff line number Diff line
@@ -45,7 +45,6 @@ import android.app.Notification;
import android.app.NotificationChannel;
import android.content.Intent;
import android.content.pm.PackageManager;
import android.metrics.LogMaker;
import android.os.Binder;
import android.os.Handler;
import android.provider.Settings;
@@ -57,7 +56,6 @@ import android.util.ArraySet;
import android.view.View;

import com.android.internal.logging.MetricsLogger;
import com.android.internal.logging.nano.MetricsProto;
import com.android.systemui.SysuiTestCase;
import com.android.systemui.plugins.statusbar.NotificationMenuRowPlugin;
import com.android.systemui.statusbar.NotificationPresenter;
@@ -219,34 +217,6 @@ public class NotificationGutsManagerTest extends SysuiTestCase {
        verify(row, times(2)).setGutsView(any());
    }

    @Test
    public void testOpenGutsLogging() {
        NotificationGutsManager gutsManager = spy(mGutsManager);
        doReturn(true).when(gutsManager).bindGuts(any(), any());

        NotificationGuts guts = spy(new NotificationGuts(mContext));
        doReturn(true).when(guts).post(any());

        ExpandableNotificationRow realRow = createTestNotificationRow();
        NotificationMenuRowPlugin.MenuItem menuItem = createTestMenuItem(realRow);

        ExpandableNotificationRow row = spy(realRow);
        when(row.getWindowToken()).thenReturn(new Binder());
        when(row.getGuts()).thenReturn(guts);
        StatusBarNotification notification = spy(realRow.getStatusBarNotification());
        when(row.getStatusBarNotification()).thenReturn(notification);

        assertTrue(gutsManager.openGuts(row, 0, 0, menuItem));

        ArgumentCaptor<LogMaker> logMakerCaptor = ArgumentCaptor.forClass(LogMaker.class);
        verify(notification).getLogMaker();
        verify(mMetricsLogger).write(logMakerCaptor.capture());
        assertEquals(MetricsProto.MetricsEvent.ACTION_NOTE_CONTROLS,
                logMakerCaptor.getValue().getCategory());
        assertEquals(MetricsProto.MetricsEvent.TYPE_ACTION,
                logMakerCaptor.getValue().getType());
    }

    @Test
    public void testAppOpsSettingsIntent_camera() {
        ArraySet<Integer> ops = new ArraySet<>();
+85 −60
Original line number Diff line number Diff line
@@ -83,7 +83,6 @@ 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;
@@ -107,11 +106,16 @@ public class NotificationInfoTest extends SysuiTestCase {
    private NotificationChannel mDefaultNotificationChannel;
    private StatusBarNotification mSbn;

    @Rule public MockitoRule mockito = MockitoJUnit.rule();
    @Mock private MetricsLogger mMetricsLogger;
    @Mock private INotificationManager mMockINotificationManager;
    @Mock private PackageManager mMockPackageManager;
    @Mock private NotificationBlockingHelperManager mBlockingHelperManager;
    @Rule
    public MockitoRule mockito = MockitoJUnit.rule();
    @Mock
    private MetricsLogger mMetricsLogger;
    @Mock
    private INotificationManager mMockINotificationManager;
    @Mock
    private PackageManager mMockPackageManager;
    @Mock
    private NotificationBlockingHelperManager mBlockingHelperManager;

    @Before
    public void setUp() throws Exception {
@@ -172,44 +176,25 @@ public class NotificationInfoTest extends SysuiTestCase {
        PollingCheck.waitFor(1000,
                () -> VISIBLE == mNotificationInfo.findViewById(R.id.confirmation).getVisibility());
    }

    private void ensureNoUndoButton() {
        PollingCheck.waitFor(1000,
                () -> GONE == mNotificationInfo.findViewById(R.id.confirmation).getVisibility()
                        && !mNotificationInfo.isAnimating());
    }

    private void waitForStopButton() {
        PollingCheck.waitFor(1000,
                () -> VISIBLE == mNotificationInfo.findViewById(R.id.prompt).getVisibility());
    }

    class ImportanceChangeLogMaker implements ArgumentMatcher<LogMaker> {
        private static final int CATEGORY = MetricsProto.MetricsEvent.ACTION_SAVE_IMPORTANCE;
        private int mType, mSubtype;

        ImportanceChangeLogMaker(int type, int subtype) {
            mType = type;
            mSubtype = subtype;
        }
        public boolean matches(LogMaker l) {
            return (l.getCategory() == CATEGORY)
                    && (l.getType() == mType)
                    && (l.getSubtype() == mSubtype);
        }

        public String toString() {
            return String.format("LogMaker(%d, %d, %d)", CATEGORY, mType, mSubtype);
        }
    }

    private LogMaker importanceChangeLog(int type, int subtype) {
        return argThat(new ImportanceChangeLogMaker(type, subtype));
    }

    @Test
    public void testBindNotification_SetsTextApplicationName() throws Exception {
        when(mMockPackageManager.getApplicationLabel(any())).thenReturn("App Name");
        mNotificationInfo.bindNotification(mMockPackageManager, mMockINotificationManager,
                TEST_PACKAGE_NAME, mNotificationChannel, 1, mSbn, null, null, null, true, false,
                TEST_PACKAGE_NAME, mNotificationChannel, 1, mSbn,
                null, null, null,
                true, false,
                IMPORTANCE_DEFAULT, true);
        final TextView textView = mNotificationInfo.findViewById(R.id.pkgname);
        assertTrue(textView.getText().toString().contains("App Name"));
@@ -340,7 +325,7 @@ public class NotificationInfoTest extends SysuiTestCase {
    }

    @Test
    public void testBindNotification_BlockButton_BlockHelper() throws Exception {
    public void testBindNotification_BlockButton_BlockingHelper() throws Exception {
        mNotificationInfo.bindNotification(mMockPackageManager, mMockINotificationManager,
                TEST_PACKAGE_NAME, mNotificationChannel, 1, mSbn, null, null, null, true, false,
                true /* isBlockingHelper */, false, IMPORTANCE_DEFAULT, true);
@@ -498,13 +483,29 @@ public class NotificationInfoTest extends SysuiTestCase {
    }

    @Test
    public void testLogBlockingHelperCounter_logGutsViewDisplayed() throws Exception {
    public void testBindNotificationLogging_notBlockingHelper() throws Exception {
        mNotificationInfo.bindNotification(mMockPackageManager, mMockINotificationManager,
                TEST_PACKAGE_NAME, mNotificationChannel, 1, mSbn, null, null, null, true, false,
                TEST_PACKAGE_NAME, mNotificationChannel, 1, mSbn,
                null, null, null,
                true, false,
                IMPORTANCE_DEFAULT, true);
        mNotificationInfo.logBlockingHelperCounter("HowCanNotifsBeRealIfAppsArent");
        verify(mMetricsLogger).write(argThat(logMaker ->
                logMaker.getCategory() == MetricsEvent.NOTIFICATION_BLOCKING_HELPER
                logMaker.getCategory() == MetricsEvent.ACTION_NOTE_CONTROLS
                        && logMaker.getType() == MetricsEvent.TYPE_OPEN
                        && logMaker.getSubtype() == MetricsEvent.BLOCKING_HELPER_UNKNOWN
        ));
    }

    @Test
    public void testBindNotificationLogging_BlockingHelper() throws Exception {
        mNotificationInfo.bindNotification(mMockPackageManager, mMockINotificationManager,
                TEST_PACKAGE_NAME, mNotificationChannel, 1, mSbn,
                null, null, null,
                false, true,
                true, true,
                IMPORTANCE_DEFAULT, true);
        verify(mMetricsLogger).write(argThat(logMaker ->
                logMaker.getCategory() == MetricsEvent.ACTION_NOTE_CONTROLS
                        && logMaker.getType() == MetricsEvent.TYPE_OPEN
                        && logMaker.getSubtype() == MetricsEvent.BLOCKING_HELPER_DISPLAY
        ));
@@ -513,8 +514,11 @@ public class NotificationInfoTest extends SysuiTestCase {
    @Test
    public void testLogBlockingHelperCounter_logsForBlockingHelper() throws Exception {
        mNotificationInfo.bindNotification(mMockPackageManager, mMockINotificationManager,
                TEST_PACKAGE_NAME, mNotificationChannel, 1, mSbn, null, null, null, false, true,
                true, true, IMPORTANCE_DEFAULT, true);
                TEST_PACKAGE_NAME, mNotificationChannel, 1, mSbn,
                null, null, null,
                false, true,
                true, true,
                IMPORTANCE_DEFAULT, true);
        mNotificationInfo.logBlockingHelperCounter("HowCanNotifsBeRealIfAppsArent");
        verify(mMetricsLogger).count(eq("HowCanNotifsBeRealIfAppsArent"), eq(1));
    }
@@ -744,7 +748,7 @@ public class NotificationInfoTest extends SysuiTestCase {
    }

    @Test
    public void testCloseControls_nonNullCheckSaveListenerDoesntDelayKeepShowing()
    public void testCloseControls_nonNullCheckSaveListenerDoesntDelayKeepShowing_BlockingHelper()
            throws Exception {
        NotificationInfo.CheckSaveListener listener =
                mock(NotificationInfo.CheckSaveListener.class);
@@ -772,7 +776,7 @@ public class NotificationInfoTest extends SysuiTestCase {
    }

    @Test
    public void testCloseControls_nonNullCheckSaveListenerDoesntDelayDismiss()
    public void testCloseControls_nonNullCheckSaveListenerDoesntDelayDismiss_BlockingHelper()
            throws Exception {
        NotificationInfo.CheckSaveListener listener =
                mock(NotificationInfo.CheckSaveListener.class);
@@ -791,7 +795,7 @@ public class NotificationInfoTest extends SysuiTestCase {
    }

    @Test
    public void testCloseControls_checkSaveListenerDelaysStopNotifications()
    public void testCloseControls_checkSaveListenerDelaysStopNotifications_BlockingHelper()
            throws Exception {
        NotificationInfo.CheckSaveListener listener =
                mock(NotificationInfo.CheckSaveListener.class);
@@ -849,18 +853,25 @@ public class NotificationInfoTest extends SysuiTestCase {
    }

    @Test
    public void testBlockChangedCallsUpdateNotificationChannel() throws Exception {
    public void testBlockChangedCallsUpdateNotificationChannel_notBlockingHelper()
            throws Exception {
        mNotificationChannel.setImportance(IMPORTANCE_LOW);
        mNotificationInfo.bindNotification(mMockPackageManager, mMockINotificationManager,
                TEST_PACKAGE_NAME, mNotificationChannel, 1, mSbn, null, null, null, true, false,
                TEST_PACKAGE_NAME, mNotificationChannel, 1, mSbn,
                null, null, null,
                true, false,
                IMPORTANCE_DEFAULT, false);

        mNotificationInfo.findViewById(R.id.int_block).performClick();
        waitForUndoButton();
        mNotificationInfo.handleCloseControls(true, false);

        verify(mMetricsLogger).write(importanceChangeLog(
                MetricsProto.MetricsEvent.TYPE_ACTION, IMPORTANCE_NONE - IMPORTANCE_LOW));
        ArgumentCaptor<LogMaker> logMakerCaptor = ArgumentCaptor.forClass(LogMaker.class);
        verify(mMetricsLogger, times(2)).write(logMakerCaptor.capture());
        assertEquals(MetricsProto.MetricsEvent.TYPE_ACTION,
                logMakerCaptor.getValue().getType());
        assertEquals(IMPORTANCE_NONE - IMPORTANCE_LOW,
                logMakerCaptor.getValue().getSubtype());

        mTestableLooper.processAllMessages();
        ArgumentCaptor<NotificationChannel> updated =
@@ -896,8 +907,12 @@ public class NotificationInfoTest extends SysuiTestCase {
        waitForUndoButton();
        mNotificationInfo.handleCloseControls(true, false);

        verify(mMetricsLogger).write(importanceChangeLog(
                MetricsProto.MetricsEvent.TYPE_ACTION, IMPORTANCE_NONE - IMPORTANCE_LOW));
        ArgumentCaptor<LogMaker> logMakerCaptor = ArgumentCaptor.forClass(LogMaker.class);
        verify(mMetricsLogger, times(3)).write(logMakerCaptor.capture());
        assertEquals(MetricsProto.MetricsEvent.TYPE_ACTION,
                logMakerCaptor.getValue().getType());
        assertEquals(IMPORTANCE_NONE - IMPORTANCE_LOW,
                logMakerCaptor.getValue().getSubtype());

        mTestableLooper.processAllMessages();
        ArgumentCaptor<NotificationChannel> updated =
@@ -965,10 +980,12 @@ public class NotificationInfoTest extends SysuiTestCase {
    }

    @Test
    public void testBlockUndoDoesNotBlockNotificationChannel() throws Exception {
    public void testBlockUndoDoesNotBlockNotificationChannel_notBlockingHelper() throws Exception {
        mNotificationChannel.setImportance(IMPORTANCE_LOW);
        mNotificationInfo.bindNotification(mMockPackageManager, mMockINotificationManager,
                TEST_PACKAGE_NAME, mNotificationChannel, 1, mSbn, null, null, null, true, false,
                TEST_PACKAGE_NAME, mNotificationChannel, 1, mSbn,
                null, null, null,
                true, false,
                IMPORTANCE_DEFAULT, false);

        mNotificationInfo.findViewById(R.id.int_block).performClick();
@@ -977,8 +994,15 @@ public class NotificationInfoTest extends SysuiTestCase {
        waitForStopButton();
        // mNotificationInfo.handleCloseControls doesn't get called by this interaction.

        verify(mMetricsLogger).write(importanceChangeLog(
                MetricsProto.MetricsEvent.TYPE_DISMISS, IMPORTANCE_NONE - IMPORTANCE_LOW));
        ArgumentCaptor<LogMaker> logMakerCaptor = ArgumentCaptor.forClass(LogMaker.class);
        verify(mMetricsLogger, times(2)).write(logMakerCaptor.capture());
        assertEquals(MetricsEvent.ACTION_SAVE_IMPORTANCE,
                logMakerCaptor.getValue().getCategory());
        assertEquals(MetricsEvent.TYPE_DISMISS,
                logMakerCaptor.getValue().getType());
        assertEquals(IMPORTANCE_NONE - IMPORTANCE_LOW,
                logMakerCaptor.getValue().getSubtype());


        mTestableLooper.processAllMessages();
        verify(mMockINotificationManager, never()).updateNotificationChannelForPackage(
@@ -986,11 +1010,12 @@ public class NotificationInfoTest extends SysuiTestCase {
    }

    @Test
    public void testMinUndoDoesNotMinNotificationChannel() throws Exception {
    public void testMinUndoDoesNotMinNotificationChannel_notBlockingHelper() throws Exception {
        mNotificationChannel.setImportance(IMPORTANCE_LOW);
        mNotificationInfo.bindNotification(mMockPackageManager, mMockINotificationManager,
                TEST_PACKAGE_NAME, mNotificationChannel, 1, mSbn, null, null, null, true, true,
                IMPORTANCE_DEFAULT, false);
                TEST_PACKAGE_NAME, mNotificationChannel, 1, mSbn,
                null, null, null, true,
                true, IMPORTANCE_DEFAULT, false);

        mNotificationInfo.findViewById(R.id.minimize).performClick();
        waitForUndoButton();