onNotificationExpansionChanged is called only when the expansion change...

is visible to users.

For example, if the notification is expanded in the background,
onNotificationExpansionChanged will be called only when the expanded
notification is visible to users.

Reason: we should only care about expansion change that is visible
to users, and at the moment that it is shown to user.
This also allows us to determine "has been visually expanded"
easily. With this change, our NotificationAssistantService can know
whether a notification has been visually expanded and calculate the CTR
of smart actions accordingly.

This should not break existing stuff in NMS.onNotificationExpansionChanged
because codes there checked isUserAction == True anyway,
which means the change must be also visible to users.

BUG: 120803809

Test: atest ExtServicesUnitTest
Test: atest ExpansionStateLoggerTest
Test: Manual test, try to expand and collapse notification.

Change-Id: Ibc6f939b2560b845de6a8a35b4557423b8a074f7

11 files changed, 319 insertions, 64 deletions

diff --git a/core/java/android/service/notification/NotificationAssistantService.java b/core/java/android/service/notification/NotificationAssistantService.java
index 63fd563dfb99..ad34ab3bddb1 100644
--- a/core/java/android/service/notification/NotificationAssistantService.java
+++ b/core/java/android/service/notification/NotificationAssistantService.java
@@ -173,7 +173,8 @@ public abstract class NotificationAssistantService extends NotificationListenerS
     }
 
     /**
-     * Implement this to know when a notification is expanded / collapsed.
+     * Implement this to know when a notification change (expanded / collapsed) is visible to user.
+     *
      * @param key the notification key
      * @param isUserAction whether the expanded change is caused by user action.
      * @param isExpanded whether the notification is expanded.
diff --git a/packages/ExtServices/src/android/ext/services/notification/Assistant.java b/packages/ExtServices/src/android/ext/services/notification/Assistant.java
index 499d493ca6a2..f346b00f85b8 100644
--- a/packages/ExtServices/src/android/ext/services/notification/Assistant.java
+++ b/packages/ExtServices/src/android/ext/services/notification/Assistant.java
@@ -344,7 +344,6 @@ public class Assistant extends NotificationAssistantService {
                 if (entry != null) {
                     entry.setSeen();
                     mAgingHelper.onNotificationSeen(entry);
-                    mSmartActionsHelper.onNotificationSeen(entry);
                 }
             }
         } catch (Throwable e) {
diff --git a/packages/ExtServices/src/android/ext/services/notification/NotificationEntry.java b/packages/ExtServices/src/android/ext/services/notification/NotificationEntry.java
index acf118066fd6..ce2c409bd379 100644
--- a/packages/ExtServices/src/android/ext/services/notification/NotificationEntry.java
+++ b/packages/ExtServices/src/android/ext/services/notification/NotificationEntry.java
@@ -236,10 +236,6 @@ public class NotificationEntry {
         return mSeen;
     }
 
-    public boolean isExpanded() {
-        return mExpanded;
-    }
-
     public boolean isShowActionEventLogged() {
         return mIsShowActionEventLogged;
     }
diff --git a/packages/ExtServices/src/android/ext/services/notification/SmartActionsHelper.java b/packages/ExtServices/src/android/ext/services/notification/SmartActionsHelper.java
index 95df5f269467..0d528e7078f8 100644
--- a/packages/ExtServices/src/android/ext/services/notification/SmartActionsHelper.java
+++ b/packages/ExtServices/src/android/ext/services/notification/SmartActionsHelper.java
@@ -152,18 +152,26 @@ public class SmartActionsHelper {
         return replies;
     }
 
-    void onNotificationSeen(@NonNull NotificationEntry entry) {
-        if (entry.isExpanded()) {
-            maybeSendActionShownEvent(entry);
-        }
-    }
-
     void onNotificationExpansionChanged(@NonNull NotificationEntry entry, boolean isUserAction,
             boolean isExpanded) {
-        // Notification can be expanded in the background, and thus the isUserAction check.
-        if (isUserAction && isExpanded) {
-            maybeSendActionShownEvent(entry);
+        if (!isExpanded) {
+            return;
+        }
+        String resultId = mNotificationKeyToResultIdCache.get(entry.getSbn().getKey());
+        if (resultId == null) {
+            return;
+        }
+        // Only report if this is the first time the user sees these suggestions.
+        if (entry.isShowActionEventLogged()) {
+            return;
         }
+        entry.setShowActionEventLogged();
+        TextClassifierEvent textClassifierEvent =
+                createTextClassifierEventBuilder(TextClassifierEvent.TYPE_ACTIONS_SHOWN,
+                        resultId)
+                        .build();
+        // TODO: If possible, report which replies / actions are actually seen by user.
+        mTextClassifier.onTextClassifierEvent(textClassifierEvent);
     }
 
     void onNotificationDirectReplied(@NonNull String key) {
@@ -234,26 +242,6 @@ public class SmartActionsHelper {
                 .setResultId(resultId);
     }
 
-    private void maybeSendActionShownEvent(@NonNull NotificationEntry entry) {
-        if (mTextClassifier == null) {
-            return;
-        }
-        String resultId = mNotificationKeyToResultIdCache.get(entry.getSbn().getKey());
-        if (resultId == null) {
-            return;
-        }
-        // Only report if this is the first time the user sees these suggestions.
-        if (entry.isShowActionEventLogged()) {
-            return;
-        }
-        entry.setShowActionEventLogged();
-        TextClassifierEvent textClassifierEvent =
-                createTextClassifierEventBuilder(TextClassifierEvent.TYPE_ACTIONS_SHOWN, resultId)
-                        .build();
-        // TODO: If possible, report which replies / actions are actually seen by user.
-        mTextClassifier.onTextClassifierEvent(textClassifierEvent);
-    }
-
     /**
      * Returns whether a notification is eligible for action adjustments.
      *
diff --git a/packages/ExtServices/tests/src/android/ext/services/notification/SmartActionHelperTest.java b/packages/ExtServices/tests/src/android/ext/services/notification/SmartActionHelperTest.java
index 7b7ce3d87f31..707349b0fd15 100644
--- a/packages/ExtServices/tests/src/android/ext/services/notification/SmartActionHelperTest.java
+++ b/packages/ExtServices/tests/src/android/ext/services/notification/SmartActionHelperTest.java
@@ -273,24 +273,22 @@ public class SmartActionHelperTest {
         final String message = "Where are you?";
         Notification notification = mNotificationBuilder.setContentText(message).build();
         when(mNotificationEntry.getNotification()).thenReturn(notification);
-        when(mNotificationEntry.isExpanded()).thenReturn(false);
 
         mSmartActionsHelper.suggestReplies(mNotificationEntry);
-        mSmartActionsHelper.onNotificationSeen(mNotificationEntry);
+        mSmartActionsHelper.onNotificationExpansionChanged(mNotificationEntry, false, false);
 
         verify(mTextClassifier, never()).onTextClassifierEvent(
                 Mockito.any(TextClassifierEvent.class));
     }
 
     @Test
-    public void testOnNotificationsSeen_expanded() {
+    public void testOnNotifications_expanded() {
         final String message = "Where are you?";
         Notification notification = mNotificationBuilder.setContentText(message).build();
         when(mNotificationEntry.getNotification()).thenReturn(notification);
-        when(mNotificationEntry.isExpanded()).thenReturn(true);
 
         mSmartActionsHelper.suggestReplies(mNotificationEntry);
-        mSmartActionsHelper.onNotificationSeen(mNotificationEntry);
+        mSmartActionsHelper.onNotificationExpansionChanged(mNotificationEntry, false, true);
 
         ArgumentCaptor<TextClassifierEvent> argumentCaptor =
                 ArgumentCaptor.forClass(TextClassifierEvent.class);
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationRowBinder.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationRowBinder.java
index cc302b16f7c1..0b8596f9cba7 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationRowBinder.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/NotificationRowBinder.java
@@ -27,13 +27,10 @@ import android.content.Context;
 import android.content.pm.ApplicationInfo;
 import android.content.pm.PackageManager;
 import android.os.Build;
-import android.os.RemoteException;
-import android.os.ServiceManager;
 import android.service.notification.StatusBarNotification;
 import android.util.Log;
 import android.view.ViewGroup;
 
-import com.android.internal.statusbar.IStatusBarService;
 import com.android.internal.util.NotificationMessagingUtil;
 import com.android.systemui.Dependency;
 import com.android.systemui.R;
@@ -43,6 +40,7 @@ import com.android.systemui.statusbar.NotificationPresenter;
 import com.android.systemui.statusbar.NotificationRemoteInputManager;
 import com.android.systemui.statusbar.NotificationUiAdjustment;
 import com.android.systemui.statusbar.notification.collection.NotificationEntry;
+import com.android.systemui.statusbar.notification.logging.NotificationLogger;
 import com.android.systemui.statusbar.notification.row.ExpandableNotificationRow;
 import com.android.systemui.statusbar.notification.row.NotificationGutsManager;
 import com.android.systemui.statusbar.notification.row.NotificationInflater;
@@ -71,7 +69,6 @@ public class NotificationRowBinder {
             Dependency.get(NotificationInterruptionStateProvider.class);
 
     private final Context mContext;
-    private final IStatusBarService mBarService;
     private final NotificationMessagingUtil mMessagingUtil;
     private final ExpandableNotificationRow.ExpansionLogger mExpansionLogger =
             this::logNotificationExpansion;
@@ -85,6 +82,7 @@ public class NotificationRowBinder {
     private ExpandableNotificationRow.OnAppOpsClickListener mOnAppOpsClickListener;
     private BindRowCallback mBindRowCallback;
     private NotificationClicker mNotificationClicker;
+    private final NotificationLogger mNotificationLogger = Dependency.get(NotificationLogger.class);
 
     @Inject
     public NotificationRowBinder(Context context,
@@ -92,8 +90,6 @@ public class NotificationRowBinder {
         mContext = context;
         mMessagingUtil = new NotificationMessagingUtil(context);
         mAllowLongPress = allowLongPress;
-        mBarService = IStatusBarService.Stub.asInterface(
-                ServiceManager.getService(Context.STATUS_BAR_SERVICE));
     }
 
     private NotificationRemoteInputManager getRemoteInputManager() {
@@ -274,13 +270,7 @@ public class NotificationRowBinder {
     }
 
     private void logNotificationExpansion(String key, boolean userAction, boolean expanded) {
-        mUiOffloadThread.submit(() -> {
-            try {
-                mBarService.onNotificationExpansionChanged(key, userAction, expanded);
-            } catch (RemoteException e) {
-                // Ignore.
-            }
-        });
+         mNotificationLogger.onExpansionChanged(key, userAction, expanded);
     }
 
     /** Callback for when a row is bound to an entry. */
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/logging/NotificationLogger.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/logging/NotificationLogger.java
index 3eec38e14083..5ba9b4b34042 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/logging/NotificationLogger.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/logging/NotificationLogger.java
@@ -23,9 +23,12 @@ import android.os.SystemClock;
 import android.service.notification.NotificationListenerService;
 import android.service.notification.NotificationStats;
 import android.service.notification.StatusBarNotification;
+import android.util.ArrayMap;
 import android.util.ArraySet;
 import android.util.Log;
 
+import androidx.annotation.Nullable;
+
 import com.android.internal.annotations.VisibleForTesting;
 import com.android.internal.statusbar.IStatusBarService;
 import com.android.internal.statusbar.NotificationVisibility;
@@ -42,6 +45,7 @@ import com.android.systemui.statusbar.policy.HeadsUpManager;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.Map;
 
 import javax.inject.Inject;
 import javax.inject.Singleton;
@@ -66,6 +70,7 @@ public class NotificationLogger implements StateListener {
     private final UiOffloadThread mUiOffloadThread;
     private final NotificationEntryManager mEntryManager;
     private HeadsUpManager mHeadsUpManager;
+    private final ExpansionStateLogger mExpansionStateLogger;
 
     protected Handler mHandler = new Handler();
     protected IStatusBarService mBarService;
@@ -144,6 +149,9 @@ public class NotificationLogger implements StateListener {
             recycleAllVisibilityObjects(mCurrentlyVisibleNotifications);
             mCurrentlyVisibleNotifications.addAll(mTmpCurrentlyVisibleNotifications);
 
+            mExpansionStateLogger.onVisibilityChanged(
+                    mTmpCurrentlyVisibleNotifications, mTmpCurrentlyVisibleNotifications);
+
             recycleAllVisibilityObjects(mTmpNoLongerVisibleNotifications);
             mTmpCurrentlyVisibleNotifications.clear();
             mTmpNewlyVisibleNotifications.clear();
@@ -155,12 +163,14 @@ public class NotificationLogger implements StateListener {
     public NotificationLogger(NotificationListener notificationListener,
             UiOffloadThread uiOffloadThread,
             NotificationEntryManager entryManager,
-            StatusBarStateController statusBarStateController) {
+            StatusBarStateController statusBarStateController,
+            ExpansionStateLogger expansionStateLogger) {
         mNotificationListener = notificationListener;
         mUiOffloadThread = uiOffloadThread;
         mEntryManager = entryManager;
         mBarService = IStatusBarService.Stub.asInterface(
                 ServiceManager.getService(Context.STATUS_BAR_SERVICE));
+        mExpansionStateLogger = expansionStateLogger;
         // Not expected to be destroyed, don't need to unsubscribe
         statusBarStateController.addCallback(this);
 
@@ -173,6 +183,7 @@ public class NotificationLogger implements StateListener {
                 if (removedByUser && visibility != null) {
                     logNotificationClear(entry.key, entry.notification, visibility);
                 }
+                mExpansionStateLogger.onEntryRemoved(entry.key);
             }
 
             @Override
@@ -319,8 +330,8 @@ public class NotificationLogger implements StateListener {
         }
     }
 
-    private NotificationVisibility[] cloneVisibilitiesAsArr(Collection<NotificationVisibility> c) {
-
+    private static NotificationVisibility[] cloneVisibilitiesAsArr(
+            Collection<NotificationVisibility> c) {
         final NotificationVisibility[] array = new NotificationVisibility[c.size()];
         int i = 0;
         for(NotificationVisibility nv: c) {
@@ -348,9 +359,133 @@ 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);
+    }
+
+    /**
      * A listener that is notified when some child locations might have changed.
      */
     public interface OnChildLocationsChangedListener {
         void onChildLocationsChanged();
     }
+
+    /**
+     * Logs the expansion state change when the notification is visible.
+     */
+    public static class ExpansionStateLogger {
+        /** Notification key -> state, should be accessed in UI offload thread only. */
+        private final Map<String, State> mExpansionStates = new ArrayMap<>();
+
+        /**
+         * Notification key -> last logged expansion state, should be accessed in UI thread only.
+         */
+        private final Map<String, Boolean> mLoggedExpansionState = new ArrayMap<>();
+        private final UiOffloadThread mUiOffloadThread;
+        @VisibleForTesting
+        IStatusBarService mBarService;
+
+        @Inject
+        public ExpansionStateLogger(UiOffloadThread uiOffloadThread) {
+            mUiOffloadThread = uiOffloadThread;
+            mBarService =
+                    IStatusBarService.Stub.asInterface(
+                            ServiceManager.getService(Context.STATUS_BAR_SERVICE));
+        }
+
+        @VisibleForTesting
+        void onExpansionChanged(String key, boolean isUserAction, boolean isExpanded) {
+            State state = getState(key);
+            state.mIsUserAction = isUserAction;
+            state.mIsExpanded = isExpanded;
+            maybeNotifyOnNotificationExpansionChanged(key, state);
+        }
+
+        @VisibleForTesting
+        void onVisibilityChanged(
+                Collection<NotificationVisibility> newlyVisible,
+                Collection<NotificationVisibility> noLongerVisible) {
+            final NotificationVisibility[] newlyVisibleAr =
+                    cloneVisibilitiesAsArr(newlyVisible);
+            final NotificationVisibility[] noLongerVisibleAr =
+                    cloneVisibilitiesAsArr(noLongerVisible);
+
+            for (NotificationVisibility nv : newlyVisibleAr) {
+                State state = getState(nv.key);
+                state.mIsVisible = true;
+                maybeNotifyOnNotificationExpansionChanged(nv.key, state);
+            }
+            for (NotificationVisibility nv : noLongerVisibleAr) {
+                State state = getState(nv.key);
+                state.mIsVisible = false;
+            }
+        }
+
+        @VisibleForTesting
+        void onEntryRemoved(String key) {
+            mExpansionStates.remove(key);
+            mLoggedExpansionState.remove(key);
+        }
+
+        private State getState(String key) {
+            State state = mExpansionStates.get(key);
+            if (state == null) {
+                state = new State();
+                mExpansionStates.put(key, state);
+            }
+            return state;
+        }
+
+        private void maybeNotifyOnNotificationExpansionChanged(final String key, State state) {
+            if (!state.isFullySet()) {
+                return;
+            }
+            if (!state.mIsVisible) {
+                return;
+            }
+            Boolean loggedExpansionState = mLoggedExpansionState.get(key);
+            // Consider notification is initially collapsed, so only expanded is logged in the
+            // first time.
+            if (loggedExpansionState == null && !state.mIsExpanded) {
+                return;
+            }
+            if (loggedExpansionState != null
+                    && state.mIsExpanded == loggedExpansionState) {
+                return;
+            }
+            mLoggedExpansionState.put(key, state.mIsExpanded);
+            final State stateToBeLogged = new State(state);
+            mUiOffloadThread.submit(() -> {
+                try {
+                    mBarService.onNotificationExpansionChanged(
+                            key, stateToBeLogged.mIsUserAction, stateToBeLogged.mIsExpanded);
+                } catch (RemoteException e) {
+                    Log.e(TAG, "Failed to call onNotificationExpansionChanged: ", e);
+                }
+            });
+        }
+
+        private static class State {
+            @Nullable
+            Boolean mIsUserAction;
+            @Nullable
+            Boolean mIsExpanded;
+            @Nullable
+            Boolean mIsVisible;
+
+            private State() {}
+
+            private State(State state) {
+                this.mIsUserAction = state.mIsUserAction;
+                this.mIsExpanded = state.mIsExpanded;
+                this.mIsVisible = state.mIsVisible;
+            }
+
+            private boolean isFullySet() {
+                return mIsUserAction != null && mIsExpanded != null && mIsVisible != null;
+            }
+        }
+    }
 }
diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/logging/ExpansionStateLoggerTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/logging/ExpansionStateLoggerTest.java
new file mode 100644
index 000000000000..4b03399a5e92
--- /dev/null
+++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/logging/ExpansionStateLoggerTest.java
@@ -0,0 +1,145 @@
+/**
+ * Copyright (C) 2018 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.logging;
+
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyBoolean;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.verify;
+
+import android.os.RemoteException;
+import android.support.test.filters.SmallTest;
+import android.testing.AndroidTestingRunner;
+import android.testing.TestableLooper;
+
+import com.android.internal.statusbar.IStatusBarService;
+import com.android.internal.statusbar.NotificationVisibility;
+import com.android.systemui.Dependency;
+import com.android.systemui.SysuiTestCase;
+import com.android.systemui.UiOffloadThread;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.Mockito;
+import org.mockito.MockitoAnnotations;
+
+import java.util.Collections;
+
+@SmallTest
+@RunWith(AndroidTestingRunner.class)
+@TestableLooper.RunWithLooper
+public class ExpansionStateLoggerTest extends SysuiTestCase {
+    private static final String NOTIFICATION_KEY = "notin_key";
+
+    private NotificationLogger.ExpansionStateLogger mLogger;
+    @Mock
+    private IStatusBarService mBarService;
+
+    @Before
+    public void setUp() {
+        MockitoAnnotations.initMocks(this);
+        mLogger = new NotificationLogger.ExpansionStateLogger(
+                Dependency.get(UiOffloadThread.class));
+        mLogger.mBarService = mBarService;
+    }
+
+    @Test
+    public void testVisible() throws RemoteException {
+        mLogger.onVisibilityChanged(
+                Collections.singletonList(createNotificationVisibility(NOTIFICATION_KEY, true)),
+                Collections.emptyList());
+        waitForUiOffloadThread();
+
+        verify(mBarService, Mockito.never()).onNotificationExpansionChanged(
+                eq(NOTIFICATION_KEY), anyBoolean(), anyBoolean());
+    }
+
+    @Test
+    public void testExpanded() throws RemoteException {
+        mLogger.onExpansionChanged(NOTIFICATION_KEY, false, true);
+        waitForUiOffloadThread();
+
+        verify(mBarService, Mockito.never()).onNotificationExpansionChanged(
+                eq(NOTIFICATION_KEY), anyBoolean(), anyBoolean());
+    }
+
+    @Test
+    public void testVisibleAndNotExpanded() throws RemoteException {
+        mLogger.onExpansionChanged(NOTIFICATION_KEY, true, false);
+        mLogger.onVisibilityChanged(
+                Collections.singletonList(createNotificationVisibility(NOTIFICATION_KEY, true)),
+                Collections.emptyList());
+        waitForUiOffloadThread();
+
+        verify(mBarService, Mockito.never()).onNotificationExpansionChanged(
+                eq(NOTIFICATION_KEY), anyBoolean(), anyBoolean());
+    }
+
+    @Test
+    public void testVisibleAndExpanded() throws RemoteException {
+        mLogger.onExpansionChanged(NOTIFICATION_KEY, true, true);
+        mLogger.onVisibilityChanged(
+                Collections.singletonList(createNotificationVisibility(NOTIFICATION_KEY, true)),
+                Collections.emptyList());
+        waitForUiOffloadThread();
+
+        verify(mBarService).onNotificationExpansionChanged(
+                NOTIFICATION_KEY, true, true);
+    }
+
+    @Test
+    public void testExpandedAndVisible_expandedBeforeVisible() throws RemoteException {
+        mLogger.onExpansionChanged(NOTIFICATION_KEY, false, true);
+        mLogger.onVisibilityChanged(
+                Collections.singletonList(createNotificationVisibility(NOTIFICATION_KEY, true)),
+                Collections.emptyList());
+        waitForUiOffloadThread();
+
+        verify(mBarService).onNotificationExpansionChanged(
+                NOTIFICATION_KEY, false, true);
+    }
+
+    @Test
+    public void testExpandedAndVisible_visibleBeforeExpanded() throws RemoteException {
+        mLogger.onVisibilityChanged(
+                Collections.singletonList(createNotificationVisibility(NOTIFICATION_KEY, true)),
+                Collections.emptyList());
+        mLogger.onExpansionChanged(NOTIFICATION_KEY, false, true);
+        waitForUiOffloadThread();
+
+        verify(mBarService).onNotificationExpansionChanged(
+                NOTIFICATION_KEY, false, true);
+    }
+
+    @Test
+    public void testExpandedAndVisible_logOnceOnly() throws RemoteException {
+        mLogger.onVisibilityChanged(
+                Collections.singletonList(createNotificationVisibility(NOTIFICATION_KEY, true)),
+                Collections.emptyList());
+        mLogger.onExpansionChanged(NOTIFICATION_KEY, false, true);
+        mLogger.onExpansionChanged(NOTIFICATION_KEY, false, true);
+        waitForUiOffloadThread();
+
+        verify(mBarService).onNotificationExpansionChanged(
+                NOTIFICATION_KEY, false, true);
+    }
+
+    private NotificationVisibility createNotificationVisibility(String key, boolean visibility) {
+        return NotificationVisibility.obtain(key, 0, 0, visibility);
+    }
+}
diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/logging/NotificationLoggerTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/logging/NotificationLoggerTest.java
index 64725896cce5..db2706bee3c9 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/logging/NotificationLoggerTest.java
+++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/logging/NotificationLoggerTest.java
@@ -72,6 +72,7 @@ public class NotificationLoggerTest extends SysuiTestCase {
     @Mock private IStatusBarService mBarService;
     @Mock private NotificationData mNotificationData;
     @Mock private ExpandableNotificationRow mRow;
+    @Mock private NotificationLogger.ExpansionStateLogger mExpansionStateLogger;
 
     // Dependency mocks:
     @Mock private NotificationEntryManager mEntryManager;
@@ -98,7 +99,8 @@ public class NotificationLoggerTest extends SysuiTestCase {
         mEntry.setRow(mRow);
 
         mLogger = new TestableNotificationLogger(mListener, Dependency.get(UiOffloadThread.class),
-                mEntryManager, mock(StatusBarStateController.class), mBarService);
+                mEntryManager, mock(StatusBarStateController.class), mBarService,
+                mExpansionStateLogger);
         mLogger.setUpWithContainer(mListContainer);
         verify(mEntryManager).addNotificationEntryListener(mEntryListenerCaptor.capture());
         mNotificationEntryListener = mEntryListenerCaptor.getValue();
@@ -166,8 +168,10 @@ public class NotificationLoggerTest extends SysuiTestCase {
                 UiOffloadThread uiOffloadThread,
                 NotificationEntryManager entryManager,
                 StatusBarStateController statusBarStateController,
-                IStatusBarService barService) {
-            super(notificationListener, uiOffloadThread, entryManager, statusBarStateController);
+                IStatusBarService barService,
+                ExpansionStateLogger expansionStateLogger) {
+            super(notificationListener, uiOffloadThread, entryManager, statusBarStateController,
+                    expansionStateLogger);
             mBarService = barService;
             // Make this on the current thread so we can wait for it during tests.
             mHandler = Handler.createAsync(Looper.myLooper());
diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/StatusBarTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/StatusBarTest.java
index 0d4046b6147b..17611ff40e40 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/StatusBarTest.java
+++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/StatusBarTest.java
@@ -159,6 +159,8 @@ public class StatusBarTest extends SysuiTestCase {
     private NotificationFilter mNotificationFilter;
     @Mock
     private NotificationAlertingManager mNotificationAlertingManager;
+    @Mock
+    private NotificationLogger.ExpansionStateLogger mExpansionStateLogger;
 
     private TestableStatusBar mStatusBar;
     private FakeMetricsLogger mMetricsLogger;
@@ -207,7 +209,8 @@ public class StatusBarTest extends SysuiTestCase {
         mDependency.injectTestDependency(MetricsLogger.class, mMetricsLogger);
         mEntryManager = new TestableNotificationEntryManager(mContext);
         mNotificationLogger = new NotificationLogger(mNotificationListener,
-                Dependency.get(UiOffloadThread.class), mEntryManager, mStatusBarStateController);
+                Dependency.get(UiOffloadThread.class), mEntryManager, mStatusBarStateController,
+                mExpansionStateLogger);
         mDependency.injectTestDependency(NotificationLogger.class, mNotificationLogger);
         DozeLog.traceDozing(mContext, false /* dozing */);
 
diff --git a/services/core/java/com/android/server/notification/NotificationManagerService.java b/services/core/java/com/android/server/notification/NotificationManagerService.java
index 1798617a9fce..20c4da416d1d 100644
--- a/services/core/java/com/android/server/notification/NotificationManagerService.java
+++ b/services/core/java/com/android/server/notification/NotificationManagerService.java
@@ -878,7 +878,6 @@ public class NotificationManagerService extends SystemService {
                     if (r.hasBeenVisiblyExpanded()) {
                         logSmartSuggestionsVisible(r);
                     }
-                    final long now = System.currentTimeMillis();
                     if (userAction) {
                         MetricsLogger.action(r.getItemLogMaker()
                                 .setType(expanded ? MetricsEvent.TYPE_DETAIL
@@ -888,9 +887,6 @@ public class NotificationManagerService extends SystemService {
                         r.recordExpanded();
                         reportUserInteraction(r);
                     }
-                    EventLogTags.writeNotificationExpansion(key,
-                            userAction ? 1 : 0, expanded ? 1 : 0,
-                            r.getLifespanMs(now), r.getFreshnessMs(now), r.getExposureMs(now));
                     mAssistants.notifyAssistantExpansionChangedLocked(r.sbn, userAction, expanded);
                 }
             }