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

Commit c7d0d32b authored by Gustav Sennton's avatar Gustav Sennton
Browse files

Log smart suggestions becoming visible in HUNs.

To measure the CTR of smart suggestions in notifications we need to log
when smart suggestions become visible. With this CL we add a
notification location log tag to know where a notification is displayed.
In this way we know which smart suggestions are shown in a heads-up
notification vs. the notification shade.

Bug: 120767764
Test: atest SystemUITests
Test: manually ensure notification surface is logged when a heads-up or
a notification in the shade is shown (and expanded).
Change-Id: Ia738f17ee6c47147b44639657dd0c1c352b4f314
parent f892fe95
Loading
Loading
Loading
Loading
+14 −7
Original line number Diff line number Diff line
@@ -399,7 +399,9 @@ public class NotificationLogger implements StateListener {
     * Called when the notification is expanded / collapsed.
     */
    public void onExpansionChanged(String key, boolean isUserAction, boolean isExpanded) {
        mExpansionStateLogger.onExpansionChanged(key, isUserAction, isExpanded);
        NotificationVisibility.NotificationLocation location =
                getNotificationLocation(mEntryManager.getNotificationData().get(key));
        mExpansionStateLogger.onExpansionChanged(key, isUserAction, isExpanded, location);
    }

    /**
@@ -433,10 +435,12 @@ public class NotificationLogger implements StateListener {
        }

        @VisibleForTesting
        void onExpansionChanged(String key, boolean isUserAction, boolean isExpanded) {
        void onExpansionChanged(String key, boolean isUserAction, boolean isExpanded,
                NotificationVisibility.NotificationLocation location) {
            State state = getState(key);
            state.mIsUserAction = isUserAction;
            state.mIsExpanded = isExpanded;
            state.mLocation = location;
            maybeNotifyOnNotificationExpansionChanged(key, state);
        }

@@ -452,6 +456,7 @@ public class NotificationLogger implements StateListener {
            for (NotificationVisibility nv : newlyVisibleAr) {
                State state = getState(nv.key);
                state.mIsVisible = true;
                state.mLocation = nv.location;
                maybeNotifyOnNotificationExpansionChanged(nv.key, state);
            }
            for (NotificationVisibility nv : noLongerVisibleAr) {
@@ -496,10 +501,8 @@ public class NotificationLogger implements StateListener {
            final State stateToBeLogged = new State(state);
            mUiOffloadThread.submit(() -> {
                try {
                    mBarService.onNotificationExpansionChanged(
                            key, stateToBeLogged.mIsUserAction, stateToBeLogged.mIsExpanded,
                            // TODO (b/120767764): fill in location
                            ExpandableViewState.LOCATION_UNKNOWN /* notificationLocation */);
                    mBarService.onNotificationExpansionChanged(key, stateToBeLogged.mIsUserAction,
                            stateToBeLogged.mIsExpanded, stateToBeLogged.mLocation.ordinal());
                } catch (RemoteException e) {
                    Log.e(TAG, "Failed to call onNotificationExpansionChanged: ", e);
                }
@@ -513,6 +516,8 @@ public class NotificationLogger implements StateListener {
            Boolean mIsExpanded;
            @Nullable
            Boolean mIsVisible;
            @Nullable
            NotificationVisibility.NotificationLocation mLocation;

            private State() {}

@@ -520,10 +525,12 @@ public class NotificationLogger implements StateListener {
                this.mIsUserAction = state.mIsUserAction;
                this.mIsExpanded = state.mIsExpanded;
                this.mIsVisible = state.mIsVisible;
                this.mLocation = state.mLocation;
            }

            private boolean isFullySet() {
                return mIsUserAction != null && mIsExpanded != null && mIsVisible != null;
                return mIsUserAction != null && mIsExpanded != null && mIsVisible != null
                        && mLocation != null;
            }
        }
    }
+34 −14
Original line number Diff line number Diff line
@@ -28,7 +28,6 @@ import android.testing.TestableLooper;

import com.android.internal.statusbar.IStatusBarService;
import com.android.internal.statusbar.NotificationVisibility;
import com.android.systemui.statusbar.notification.stack.ExpandableViewState;
import com.android.systemui.Dependency;
import com.android.systemui.SysuiTestCase;
import com.android.systemui.UiOffloadThread;
@@ -73,7 +72,8 @@ public class ExpansionStateLoggerTest extends SysuiTestCase {

    @Test
    public void testExpanded() throws RemoteException {
        mLogger.onExpansionChanged(NOTIFICATION_KEY, false, true);
        mLogger.onExpansionChanged(NOTIFICATION_KEY, false, true,
                NotificationVisibility.NotificationLocation.LOCATION_UNKNOWN);
        waitForUiOffloadThread();

        verify(mBarService, Mockito.never()).onNotificationExpansionChanged(
@@ -82,7 +82,8 @@ public class ExpansionStateLoggerTest extends SysuiTestCase {

    @Test
    public void testVisibleAndNotExpanded() throws RemoteException {
        mLogger.onExpansionChanged(NOTIFICATION_KEY, true, false);
        mLogger.onExpansionChanged(NOTIFICATION_KEY, true, false,
                NotificationVisibility.NotificationLocation.LOCATION_UNKNOWN);
        mLogger.onVisibilityChanged(
                Collections.singletonList(createNotificationVisibility(NOTIFICATION_KEY, true)),
                Collections.emptyList());
@@ -94,26 +95,33 @@ public class ExpansionStateLoggerTest extends SysuiTestCase {

    @Test
    public void testVisibleAndExpanded() throws RemoteException {
        mLogger.onExpansionChanged(NOTIFICATION_KEY, true, true);
        mLogger.onExpansionChanged(NOTIFICATION_KEY, true, true,
                NotificationVisibility.NotificationLocation.LOCATION_UNKNOWN);
        mLogger.onVisibilityChanged(
                Collections.singletonList(createNotificationVisibility(NOTIFICATION_KEY, true)),
                Collections.emptyList());
        waitForUiOffloadThread();

        verify(mBarService).onNotificationExpansionChanged(
                NOTIFICATION_KEY, true, true, ExpandableViewState.LOCATION_UNKNOWN);
                NOTIFICATION_KEY, true, true,
                NotificationVisibility.NotificationLocation.LOCATION_UNKNOWN.toMetricsEventEnum());
    }

    @Test
    public void testExpandedAndVisible_expandedBeforeVisible() throws RemoteException {
        mLogger.onExpansionChanged(NOTIFICATION_KEY, false, true);
        mLogger.onExpansionChanged(NOTIFICATION_KEY, false, true,
                NotificationVisibility.NotificationLocation.LOCATION_UNKNOWN);
        mLogger.onVisibilityChanged(
                Collections.singletonList(createNotificationVisibility(NOTIFICATION_KEY, true)),
                Collections.singletonList(createNotificationVisibility(NOTIFICATION_KEY, true,
                    NotificationVisibility.NotificationLocation.LOCATION_MAIN_AREA)),
                Collections.emptyList());
        waitForUiOffloadThread();

        verify(mBarService).onNotificationExpansionChanged(
                NOTIFICATION_KEY, false, true, ExpandableViewState.LOCATION_UNKNOWN);
                NOTIFICATION_KEY, false, true,
                // The last location seen should be logged (the one passed to onVisibilityChanged).
                NotificationVisibility.NotificationLocation.LOCATION_MAIN_AREA.toMetricsEventEnum()
        );
    }

    @Test
@@ -121,11 +129,14 @@ public class ExpansionStateLoggerTest extends SysuiTestCase {
        mLogger.onVisibilityChanged(
                Collections.singletonList(createNotificationVisibility(NOTIFICATION_KEY, true)),
                Collections.emptyList());
        mLogger.onExpansionChanged(NOTIFICATION_KEY, false, true);
        mLogger.onExpansionChanged(NOTIFICATION_KEY, false, true,
                NotificationVisibility.NotificationLocation.LOCATION_FIRST_HEADS_UP);
        waitForUiOffloadThread();

        verify(mBarService).onNotificationExpansionChanged(
                NOTIFICATION_KEY, false, true, ExpandableViewState.LOCATION_UNKNOWN);
                NOTIFICATION_KEY, false, true,
                // The last location seen should be logged (the one passed to onExpansionChanged).
                NotificationVisibility.NotificationLocation.LOCATION_FIRST_HEADS_UP.toMetricsEventEnum());
    }

    @Test
@@ -133,15 +144,24 @@ public class ExpansionStateLoggerTest extends SysuiTestCase {
        mLogger.onVisibilityChanged(
                Collections.singletonList(createNotificationVisibility(NOTIFICATION_KEY, true)),
                Collections.emptyList());
        mLogger.onExpansionChanged(NOTIFICATION_KEY, false, true);
        mLogger.onExpansionChanged(NOTIFICATION_KEY, false, true);
        mLogger.onExpansionChanged(NOTIFICATION_KEY, false, true,
                NotificationVisibility.NotificationLocation.LOCATION_UNKNOWN);
        mLogger.onExpansionChanged(NOTIFICATION_KEY, false, true,
                NotificationVisibility.NotificationLocation.LOCATION_UNKNOWN);
        waitForUiOffloadThread();

        verify(mBarService).onNotificationExpansionChanged(
                NOTIFICATION_KEY, false, true, ExpandableViewState.LOCATION_UNKNOWN);
                NOTIFICATION_KEY, false, true,
                NotificationVisibility.NotificationLocation.LOCATION_UNKNOWN.toMetricsEventEnum());
    }

    private NotificationVisibility createNotificationVisibility(String key, boolean visibility) {
        return NotificationVisibility.obtain(key, 0, 0, visibility);
        return createNotificationVisibility(key, visibility,
                NotificationVisibility.NotificationLocation.LOCATION_UNKNOWN);
    }

    private NotificationVisibility createNotificationVisibility(String key, boolean visibility,
            NotificationVisibility.NotificationLocation location) {
        return NotificationVisibility.obtain(key, 0, 0, visibility, location);
    }
}
+10 −5
Original line number Diff line number Diff line
@@ -845,10 +845,12 @@ public class NotificationManagerService extends SystemService {
                        reportSeen(r);
                    }
                    r.setVisibility(true, nv.rank, nv.count);
                    boolean isHun = (nv.location
                            == NotificationVisibility.NotificationLocation.LOCATION_FIRST_HEADS_UP);
                    // hasBeenVisiblyExpanded must be called after updating the expansion state of
                    // the NotificationRecord to ensure the expansion state is up-to-date.
                    if (r.hasBeenVisiblyExpanded()) {
                        logSmartSuggestionsVisible(r);
                    if (isHun || r.hasBeenVisiblyExpanded()) {
                        logSmartSuggestionsVisible(r, nv.location.toMetricsEventEnum());
                    }
                    maybeRecordInterruptionLocked(r);
                    nv.recycle();
@@ -876,7 +878,7 @@ public class NotificationManagerService extends SystemService {
                    // hasBeenVisiblyExpanded must be called after updating the expansion state of
                    // the NotificationRecord to ensure the expansion state is up-to-date.
                    if (r.hasBeenVisiblyExpanded()) {
                        logSmartSuggestionsVisible(r);
                        logSmartSuggestionsVisible(r, notificationLocation);
                    }
                    if (userAction) {
                        MetricsLogger.action(r.getItemLogMaker()
@@ -952,7 +954,7 @@ public class NotificationManagerService extends SystemService {
    };

    @VisibleForTesting
    void logSmartSuggestionsVisible(NotificationRecord r) {
    void logSmartSuggestionsVisible(NotificationRecord r, int notificationLocation) {
        // If the newly visible notification has smart suggestions
        // then log that the user has seen them.
        if ((r.getNumSmartRepliesAdded() > 0 || r.getNumSmartActionsAdded() > 0)
@@ -966,7 +968,10 @@ public class NotificationManagerService extends SystemService {
                            r.getNumSmartActionsAdded())
                    .addTaggedData(
                            MetricsEvent.NOTIFICATION_SMART_SUGGESTION_ASSISTANT_GENERATED,
                            r.getSuggestionsGeneratedByAssistant() ? 1 : 0);
                            r.getSuggestionsGeneratedByAssistant() ? 1 : 0)
                    // The fields in the NotificationVisibility.NotificationLocation enum map
                    // directly to the fields in the MetricsEvent.NotificationLocation enum.
                    .addTaggedData(MetricsEvent.NOTIFICATION_LOCATION, notificationLocation);
            mMetricsLogger.write(logMaker);
        }
    }
+2 −2
Original line number Diff line number Diff line
@@ -245,8 +245,8 @@ public class NotificationManagerServiceTest extends UiServiceTestCase {
        }

        @Override
        void logSmartSuggestionsVisible(NotificationRecord r) {
            super.logSmartSuggestionsVisible(r);
        void logSmartSuggestionsVisible(NotificationRecord r, int notificationLocation) {
            super.logSmartSuggestionsVisible(r, notificationLocation);
            countLogSmartSuggestionsVisible++;
        }