Merge "More accurate notification panel logs." into rvc-dev

packages/SystemUI/src/com/android/systemui/statusbar/notification/logging/NotificationLogger.java

@@ -29,6 +29,7 @@ import android.util.Log;
 
 import androidx.annotation.Nullable;
 
+import com.android.internal.annotations.GuardedBy;
 import com.android.internal.annotations.VisibleForTesting;
 import com.android.internal.statusbar.IStatusBarService;
 import com.android.internal.statusbar.NotificationVisibility;
@@ -36,6 +37,7 @@ import com.android.systemui.dagger.qualifiers.UiBackground;
 import com.android.systemui.plugins.statusbar.StatusBarStateController;
 import com.android.systemui.plugins.statusbar.StatusBarStateController.StateListener;
 import com.android.systemui.statusbar.NotificationListener;
+import com.android.systemui.statusbar.StatusBarState;
 import com.android.systemui.statusbar.notification.NotificationEntryListener;
 import com.android.systemui.statusbar.notification.NotificationEntryManager;
 import com.android.systemui.statusbar.notification.collection.NotificationEntry;
@@ -58,6 +60,7 @@ import javax.inject.Inject;
  */
 public class NotificationLogger implements StateListener {
     private static final String TAG = "NotificationLogger";
+    private static final boolean DEBUG = false;
 
     /** The minimum delay in ms between reports of notification visibility. */
     private static final int VISIBILITY_REPORT_MIN_DELAY_MS = 500;
@@ -79,7 +82,12 @@ public class NotificationLogger implements StateListener {
     private long mLastVisibilityReportUptimeMs;
     private NotificationListContainer mListContainer;
     private final Object mDozingLock = new Object();
-    private boolean mDozing;
+    @GuardedBy("mDozingLock")
+    private Boolean mDozing = null;  // Use null to indicate state is not yet known
+    @GuardedBy("mDozingLock")
+    private Boolean mLockscreen = null;  // Use null to indicate state is not yet known
+    private Boolean mPanelExpanded = null;  // Use null to indicate state is not yet known
+    private boolean mLogging = false;
 
     protected final OnChildLocationsChangedListener mNotificationLocationsChangedListener =
             new OnChildLocationsChangedListener() {
@@ -247,33 +255,44 @@ public class NotificationLogger implements StateListener {
     }
 
     public void stopNotificationLogging() {
-        // Report all notifications as invisible and turn down the
+        if (mLogging) {
-        // reporter.
+            mLogging = false;
-        if (!mCurrentlyVisibleNotifications.isEmpty()) {
+            if (DEBUG) {
-            logNotificationVisibilityChanges(
+                Log.i(TAG, "stopNotificationLogging: log notifications invisible");
-                    Collections.emptyList(), mCurrentlyVisibleNotifications);
+            }
-            recycleAllVisibilityObjects(mCurrentlyVisibleNotifications);
+            // Report all notifications as invisible and turn down the
+            // reporter.
+            if (!mCurrentlyVisibleNotifications.isEmpty()) {
+                logNotificationVisibilityChanges(
+                        Collections.emptyList(), mCurrentlyVisibleNotifications);
+                recycleAllVisibilityObjects(mCurrentlyVisibleNotifications);
+            }
+            mHandler.removeCallbacks(mVisibilityReporter);
+            mListContainer.setChildLocationsChangedListener(null);
         }
-        mHandler.removeCallbacks(mVisibilityReporter);
-        mListContainer.setChildLocationsChangedListener(null);
     }
 
     public void startNotificationLogging() {
-        mListContainer.setChildLocationsChangedListener(mNotificationLocationsChangedListener);
+        if (!mLogging) {
-        // Some transitions like mVisibleToUser=false -> mVisibleToUser=true don't
+            mLogging = true;
-        // cause the scroller to emit child location events. Hence generate
+            if (DEBUG) {
-        // one ourselves to guarantee that we're reporting visible
+                Log.i(TAG, "startNotificationLogging");
-        // notifications.
+            }
-        // (Note that in cases where the scroller does emit events, this
+            mListContainer.setChildLocationsChangedListener(mNotificationLocationsChangedListener);
-        // additional event doesn't break anything.)
+            // Some transitions like mVisibleToUser=false -> mVisibleToUser=true don't
-        mNotificationLocationsChangedListener.onChildLocationsChanged();
+            // cause the scroller to emit child location events. Hence generate
-        mNotificationPanelLogger.logPanelShown(mListContainer.hasPulsingNotifications(),
+            // one ourselves to guarantee that we're reporting visible
-                mEntryManager.getVisibleNotifications());
+            // notifications.
+            // (Note that in cases where the scroller does emit events, this
+            // additional event doesn't break anything.)
+            mNotificationLocationsChangedListener.onChildLocationsChanged();
+        }
     }
 
     private void setDozing(boolean dozing) {
         synchronized (mDozingLock) {
             mDozing = dozing;
+            maybeUpdateLoggingStatus();
         }
     }
 
@@ -343,19 +362,12 @@ public class NotificationLogger implements StateListener {
                 for (int i = 0; i < N; i++) {
                     newlyVisibleKeyAr[i] = newlyVisibleAr[i].key;
                 }
-
+                // TODO: Call NotificationEntryManager to do this, once it exists.
-                synchronized (mDozingLock) {
+                // TODO: Consider not catching all runtime exceptions here.
-                    // setNotificationsShown should only be called if we are confident that
+                try {
-                    // the user has seen the notification, aka not when ambient display is on
+                    mNotificationListener.setNotificationsShown(newlyVisibleKeyAr);
-                    if (!mDozing) {
+                } catch (RuntimeException e) {
-                        // TODO: Call NotificationEntryManager to do this, once it exists.
+                    Log.d(TAG, "failed setNotificationsShown: ", e);
-                        // TODO: Consider not catching all runtime exceptions here.
-                        try {
-                            mNotificationListener.setNotificationsShown(newlyVisibleKeyAr);
-                        } catch (RuntimeException e) {
-                            Log.d(TAG, "failed setNotificationsShown: ", e);
-                        }
-                    }
                 }
             }
             recycleAllVisibilityObjects(newlyVisibleAr);
@@ -400,14 +412,64 @@ public class NotificationLogger implements StateListener {
 
     @Override
     public void onStateChanged(int newState) {
-        // don't care about state change
+        if (DEBUG) {
+            Log.i(TAG, "onStateChanged: new=" + newState);
+        }
+        synchronized (mDozingLock) {
+            mLockscreen = (newState == StatusBarState.KEYGUARD
+                    || newState == StatusBarState.SHADE_LOCKED);
+        }
     }
 
     @Override
     public void onDozingChanged(boolean isDozing) {
+        if (DEBUG) {
+            Log.i(TAG, "onDozingChanged: new=" + isDozing);
+        }
         setDozing(isDozing);
     }
 
+    @GuardedBy("mDozingLock")
+    private void maybeUpdateLoggingStatus() {
+        if (mPanelExpanded == null || mDozing == null) {
+            if (DEBUG) {
+                Log.i(TAG, "Panel status unclear: panelExpandedKnown="
+                        + (mPanelExpanded == null) + " dozingKnown=" + (mDozing == null));
+            }
+            return;
+        }
+        // Once we know panelExpanded and Dozing, turn logging on & off when appropriate
+        boolean lockscreen = mLockscreen == null ? false : mLockscreen;
+        if (mPanelExpanded && !mDozing) {
+            mNotificationPanelLogger.logPanelShown(lockscreen,
+                    mEntryManager.getVisibleNotifications());
+            if (DEBUG) {
+                Log.i(TAG, "Notification panel shown, lockscreen=" + lockscreen);
+            }
+            startNotificationLogging();
+        } else {
+            if (DEBUG) {
+                Log.i(TAG, "Notification panel hidden, lockscreen=" + lockscreen);
+            }
+            stopNotificationLogging();
+        }
+    }
+
+    /**
+     * Called by StatusBar to notify the logger that the panel expansion has changed.
+     * The panel may be showing any of the normal notification panel, the AOD, or the bouncer.
+     * @param isExpanded True if the panel is expanded.
+     */
+    public void onPanelExpandedChanged(boolean isExpanded) {
+        if (DEBUG) {
+            Log.i(TAG, "onPanelExpandedChanged: new=" + isExpanded);
+        }
+        mPanelExpanded = isExpanded;
+        synchronized (mDozingLock) {
+            maybeUpdateLoggingStatus();
+        }
+    }
+
     /**
      * Called when the notification is expanded / collapsed.
      */

packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBar.java

@@ -1772,6 +1772,9 @@ public class StatusBar extends SystemUI implements DemoMode,
     }
 
     public void setPanelExpanded(boolean isExpanded) {
+        if (mPanelExpanded != isExpanded) {
+            mNotificationLogger.onPanelExpandedChanged(isExpanded);
+        }
         mPanelExpanded = isExpanded;
         updateHideIconsForBouncer(false /* animate */);
         mNotificationShadeWindowController.setPanelExpanded(isExpanded);
@@ -2878,7 +2881,6 @@ public class StatusBar extends SystemUI implements DemoMode,
     }
 
     // Visibility reporting
-
     protected void handleVisibleToUserChanged(boolean visibleToUser) {
         if (visibleToUser) {
             handleVisibleToUserChangedImpl(visibleToUser);
@@ -2900,12 +2902,12 @@ public class StatusBar extends SystemUI implements DemoMode,
         }
     }
 
-    /**
+    // Visibility reporting
-     * The LEDs are turned off when the notification panel is shown, even just a little bit.
+
-     * See also StatusBar.setPanelExpanded for another place where we attempt to do this.
+    void handleVisibleToUserChangedImpl(boolean visibleToUser) {
-     */
-    private void handleVisibleToUserChangedImpl(boolean visibleToUser) {
         if (visibleToUser) {
+            /* The LEDs are turned off when the notification panel is shown, even just a little bit.
+             * See also StatusBar.setPanelExpanded for another place where we attempt to do this. */
             boolean pinnedHeadsUp = mHeadsUpManager.hasPinnedHeadsUp();
             boolean clearNotificationEffects =
                     !mPresenter.isPresenterFullyCollapsed() &&

packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/logging/NotificationLoggerTest.java

@@ -20,6 +20,8 @@ import static com.android.systemui.statusbar.notification.stack.NotificationSect
 
 import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.doAnswer;
 import static org.mockito.Mockito.mock;
@@ -42,6 +44,7 @@ import com.android.internal.statusbar.IStatusBarService;
 import com.android.internal.statusbar.NotificationVisibility;
 import com.android.systemui.SysuiTestCase;
 import com.android.systemui.statusbar.NotificationListener;
+import com.android.systemui.statusbar.StatusBarState;
 import com.android.systemui.statusbar.StatusBarStateControllerImpl;
 import com.android.systemui.statusbar.notification.NotificationEntryListener;
 import com.android.systemui.statusbar.notification.NotificationEntryManager;
@@ -163,28 +166,61 @@ public class NotificationLoggerTest extends SysuiTestCase {
         mUiBgExecutor.runAllReady();
         Mockito.reset(mBarService);
 
-        mLogger.stopNotificationLogging();
+        setStateAsleep();
+        mLogger.onDozingChanged(false);  // Wake to lockscreen
+        mLogger.onDozingChanged(true);  // And go back to sleep, turning off logging
         mUiBgExecutor.runAllReady();
         // The visibility objects are recycled by NotificationLogger, so we can't use specific
         // matchers here.
         verify(mBarService, times(1)).onNotificationVisibilityChanged(any(), any());
     }
 
+    private void setStateAsleep() {
+        mLogger.onPanelExpandedChanged(true);
+        mLogger.onDozingChanged(true);
+        mLogger.onStateChanged(StatusBarState.KEYGUARD);
+    }
+
+    private void setStateAwake() {
+        mLogger.onPanelExpandedChanged(false);
+        mLogger.onDozingChanged(false);
+        mLogger.onStateChanged(StatusBarState.SHADE);
+    }
+
     @Test
-    public void testLogPanelShownOnLoggingStart() {
+    public void testLogPanelShownOnWake() {
         when(mEntryManager.getVisibleNotifications()).thenReturn(Lists.newArrayList(mEntry));
-        mLogger.startNotificationLogging();
+        setStateAsleep();
+        mLogger.onDozingChanged(false);  // Wake to lockscreen
         assertEquals(1, mNotificationPanelLoggerFake.getCalls().size());
-        assertEquals(false, mNotificationPanelLoggerFake.get(0).isLockscreen);
+        assertTrue(mNotificationPanelLoggerFake.get(0).isLockscreen);
         assertEquals(1, mNotificationPanelLoggerFake.get(0).list.notifications.length);
         Notifications.Notification n = mNotificationPanelLoggerFake.get(0).list.notifications[0];
         assertEquals(TEST_PACKAGE_NAME, n.packageName);
         assertEquals(TEST_UID, n.uid);
         assertEquals(1, n.instanceId);
-        assertEquals(false, n.isGroupSummary);
+        assertFalse(n.isGroupSummary);
         assertEquals(1 + BUCKET_ALERTING, n.section);
     }
 
+    @Test
+    public void testLogPanelShownOnShadePull() {
+        when(mEntryManager.getVisibleNotifications()).thenReturn(Lists.newArrayList(mEntry));
+        setStateAwake();
+        // Now expand panel
+        mLogger.onPanelExpandedChanged(true);
+        assertEquals(1, mNotificationPanelLoggerFake.getCalls().size());
+        assertFalse(mNotificationPanelLoggerFake.get(0).isLockscreen);
+        assertEquals(1, mNotificationPanelLoggerFake.get(0).list.notifications.length);
+        Notifications.Notification n = mNotificationPanelLoggerFake.get(0).list.notifications[0];
+        assertEquals(TEST_PACKAGE_NAME, n.packageName);
+        assertEquals(TEST_UID, n.uid);
+        assertEquals(1, n.instanceId);
+        assertFalse(n.isGroupSummary);
+        assertEquals(1 + BUCKET_ALERTING, n.section);
+    }
+
+
     @Test
     public void testLogPanelShownHandlesNullInstanceIds() {
         // Construct a NotificationEntry like mEntry, but with a null instance id.
@@ -198,7 +234,8 @@ public class NotificationLoggerTest extends SysuiTestCase {
         entry.setRow(mRow);
 
         when(mEntryManager.getVisibleNotifications()).thenReturn(Lists.newArrayList(entry));
-        mLogger.startNotificationLogging();
+        setStateAsleep();
+        mLogger.onDozingChanged(false);  // Wake to lockscreen
         assertEquals(1, mNotificationPanelLoggerFake.getCalls().size());
         assertEquals(1, mNotificationPanelLoggerFake.get(0).list.notifications.length);
         Notifications.Notification n = mNotificationPanelLoggerFake.get(0).list.notifications[0];