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

Commit 44dc5885 authored by Gustav Sennton's avatar Gustav Sennton
Browse files

Only log smart suggestions when their notification has been expanded.

Smart suggestions in notifications are only shown when the notification
is expanded. Before this CL we logged the SMART_REPLY_VISIBLE event the
first time the notification is visible - with this CL we will log that
event the first time the notification is visible AND expanded at the
same time (i.e. when smart suggestions are visible).

Bug: 120967525
Test: atest NotificationManagerServiceTest
Test: manual - using logcat, ensure we only log smart suggestions when
they are actually shown (when the notification is expanded and visible,
but not when only expanded or only visible).

Change-Id: Ic435cf3c33db770095d903c72c0016241b1549e1
parent a62790c2
Loading
Loading
Loading
Loading
+30 −17
Original line number Diff line number Diff line
@@ -842,25 +842,13 @@ public class NotificationManagerService extends SystemService {
                        // Report to usage stats that notification was made visible
                        if (DBG) Slog.d(TAG, "Marking notification as visible " + nv.key);
                        reportSeen(r);

                        // If the newly visible notification has smart suggestions
                        // then log that the user has seen them.
                        if ((r.getNumSmartRepliesAdded() > 0 || r.getNumSmartActionsAdded() > 0)
                                && !r.hasSeenSmartReplies()) {
                            r.setSeenSmartReplies(true);
                            LogMaker logMaker = r.getLogMaker()
                                    .setCategory(MetricsEvent.SMART_REPLY_VISIBLE)
                                    .addTaggedData(MetricsEvent.NOTIFICATION_SMART_REPLY_COUNT,
                                            r.getNumSmartRepliesAdded())
                                    .addTaggedData(MetricsEvent.NOTIFICATION_SMART_ACTION_COUNT,
                                            r.getNumSmartActionsAdded())
                                    .addTaggedData(
                                            MetricsEvent.NOTIFICATION_SMART_SUGGESTION_ASSISTANT_GENERATED,
                                            r.getSuggestionsGeneratedByAssistant());
                            mMetricsLogger.write(logMaker);
                        }
                    }
                    r.setVisibility(true, nv.rank, nv.count);
                    // 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);
                    }
                    maybeRecordInterruptionLocked(r);
                    nv.recycle();
                }
@@ -884,6 +872,11 @@ public class NotificationManagerService extends SystemService {
                NotificationRecord r = mNotificationsByKey.get(key);
                if (r != null) {
                    r.stats.onExpansionChanged(userAction, expanded);
                    // 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);
                    }
                    final long now = System.currentTimeMillis();
                    if (userAction) {
                        MetricsLogger.action(r.getItemLogMaker()
@@ -961,6 +954,26 @@ public class NotificationManagerService extends SystemService {
        }
    };

    @VisibleForTesting
    void logSmartSuggestionsVisible(NotificationRecord r) {
        // If the newly visible notification has smart suggestions
        // then log that the user has seen them.
        if ((r.getNumSmartRepliesAdded() > 0 || r.getNumSmartActionsAdded() > 0)
                && !r.hasSeenSmartReplies()) {
            r.setSeenSmartReplies(true);
            LogMaker logMaker = r.getLogMaker()
                    .setCategory(MetricsEvent.SMART_REPLY_VISIBLE)
                    .addTaggedData(MetricsEvent.NOTIFICATION_SMART_REPLY_COUNT,
                            r.getNumSmartRepliesAdded())
                    .addTaggedData(MetricsEvent.NOTIFICATION_SMART_ACTION_COUNT,
                            r.getNumSmartActionsAdded())
                    .addTaggedData(
                            MetricsEvent.NOTIFICATION_SMART_SUGGESTION_ASSISTANT_GENERATED,
                            r.getSuggestionsGeneratedByAssistant());
            mMetricsLogger.write(logMaker);
        }
    }

    @GuardedBy("mNotificationLock")
    private void clearSoundLocked() {
        mSoundNotificationKey = null;
+7 −0
Original line number Diff line number Diff line
@@ -1166,6 +1166,13 @@ public final class NotificationRecord {
        mHasSeenSmartReplies = hasSeenSmartReplies;
    }

    /**
     * Returns whether this notification has been visible and expanded at the same time.
     */
    public boolean hasBeenVisiblyExpanded() {
        return stats.hasBeenVisiblyExpanded();
    }

    public void setSystemGeneratedSmartActions(
            ArrayList<Notification.Action> systemGeneratedSmartActions) {
        mSystemGeneratedSmartActions = systemGeneratedSmartActions;
+7 −0
Original line number Diff line number Diff line
@@ -916,6 +916,13 @@ public class NotificationUsageStats {
            updateVisiblyExpandedStats();
        }

        /**
         * Returns whether this notification has been visible and expanded at the same.
         */
        public boolean hasBeenVisiblyExpanded() {
            return posttimeToFirstVisibleExpansionMs >= 0;
        }

        private void updateVisiblyExpandedStats() {
            long elapsedNowMs = SystemClock.elapsedRealtime();
            if (isExpanded && isVisible) {
+48 −0
Original line number Diff line number Diff line
@@ -208,6 +208,7 @@ public class NotificationManagerServiceTest extends UiServiceTestCase {
    private static class TestableNotificationManagerService extends NotificationManagerService {
        int countSystemChecks = 0;
        boolean isSystemUid = true;
        int countLogSmartSuggestionsVisible = 0;

        public TestableNotificationManagerService(Context context) {
            super(context);
@@ -244,6 +245,14 @@ public class NotificationManagerServiceTest extends UiServiceTestCase {
        protected void handleSavePolicyFile() {
            return;
        }

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


    }

    private class TestableToastCallback extends ITransientNotification.Stub {
@@ -3777,4 +3786,43 @@ public class NotificationManagerServiceTest extends UiServiceTestCase {
        verify(mAssistants).notifyAssistantActionClicked(
                eq(r.sbn), eq(actionIndex), eq(action), eq(generatedByAssistant));
    }

    @Test
    public void testLogSmartSuggestionsVisible_triggerOnExpandAndVisible() {
        NotificationRecord r = generateNotificationRecord(mTestNotificationChannel);
        mService.addNotification(r);

        mService.mNotificationDelegate.onNotificationExpansionChanged(r.getKey(), false, true);
        NotificationVisibility[] notificationVisibility = new NotificationVisibility[] {
                NotificationVisibility.obtain(r.getKey(), 0, 0, true)
        };
        mService.mNotificationDelegate.onNotificationVisibilityChanged(notificationVisibility,
                new NotificationVisibility[0]);

        assertEquals(1, mService.countLogSmartSuggestionsVisible);
    }

    @Test
    public void testLogSmartSuggestionsVisible_noTriggerOnExpand() {
        NotificationRecord r = generateNotificationRecord(mTestNotificationChannel);
        mService.addNotification(r);

        mService.mNotificationDelegate.onNotificationExpansionChanged(r.getKey(), false, true);

        assertEquals(0, mService.countLogSmartSuggestionsVisible);
    }

    @Test
    public void testLogSmartSuggestionsVisible_noTriggerOnVisible() {
        NotificationRecord r = generateNotificationRecord(mTestNotificationChannel);
        mService.addNotification(r);

        NotificationVisibility[] notificationVisibility = new NotificationVisibility[] {
                NotificationVisibility.obtain(r.getKey(), 0, 0, true)
        };
        mService.mNotificationDelegate.onNotificationVisibilityChanged(notificationVisibility,
                new NotificationVisibility[0]);

        assertEquals(0, mService.countLogSmartSuggestionsVisible);
    }
}