Merge "HeadsUpViewBinder: Don't WTF if notif stops alerting after cleanup." into tm-qpr-dev

5 files changed, 129 insertions, 2 deletions

diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/interruption/HeadsUpViewBinder.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/interruption/HeadsUpViewBinder.java
index 6f41425b506d..9a7610ddd354 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/interruption/HeadsUpViewBinder.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/interruption/HeadsUpViewBinder.java
@@ -114,7 +114,18 @@ public class HeadsUpViewBinder {
      */
     public void unbindHeadsUpView(NotificationEntry entry) {
         abortBindCallback(entry);
-        mStage.getStageParams(entry).markContentViewsFreeable(FLAG_CONTENT_VIEW_HEADS_UP);
+
+        // params may be null if the notification was already removed from the collection but we let
+        // it stick around during a launch animation. In this case, the heads up view has already
+        // been unbound, so we don't need to unbind it.
+        // TODO(b/253081345): Change this back to getStageParams and remove null check.
+        RowContentBindParams params = mStage.tryGetStageParams(entry);
+        if (params == null) {
+            mLogger.entryBindStageParamsNullOnUnbind(entry);
+            return;
+        }
+
+        params.markContentViewsFreeable(FLAG_CONTENT_VIEW_HEADS_UP);
         mLogger.entryContentViewMarkedFreeable(entry);
         mStage.requestRebind(entry, e -> mLogger.entryUnbound(e));
     }
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/interruption/HeadsUpViewBinderLogger.kt b/packages/SystemUI/src/com/android/systemui/statusbar/notification/interruption/HeadsUpViewBinderLogger.kt
index d1feaa05c653..5dbec8dcba20 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/interruption/HeadsUpViewBinderLogger.kt
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/interruption/HeadsUpViewBinderLogger.kt
@@ -47,6 +47,14 @@ class HeadsUpViewBinderLogger @Inject constructor(@NotificationHeadsUpLog val bu
             "start unbinding heads up entry $str1 "
         })
     }
+
+    fun entryBindStageParamsNullOnUnbind(entry: NotificationEntry) {
+        buffer.log(TAG, INFO, {
+            str1 = entry.logKey
+        }, {
+            "heads up entry bind stage params null on unbind $str1 "
+        })
+    }
 }
 
 private const val TAG = "HeadsUpViewBinder"
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/BindStage.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/BindStage.java
index 7c41800d880d..d626c18e46f5 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/BindStage.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/BindStage.java
@@ -21,6 +21,7 @@ import android.util.ArrayMap;
 import android.util.Log;
 
 import androidx.annotation.NonNull;
+import androidx.annotation.Nullable;
 
 import com.android.systemui.statusbar.notification.collection.NotificationEntry;
 
@@ -64,7 +65,7 @@ public abstract class BindStage<Params> extends BindRequester {
      * Get the stage parameters for the entry. Clients should use this to modify how the stage
      * handles the notification content.
      */
-    public final Params getStageParams(@NonNull NotificationEntry entry) {
+    public final @NonNull Params getStageParams(@NonNull NotificationEntry entry) {
         Params params = mContentParams.get(entry);
         if (params == null) {
             // TODO: This should throw an exception but there are some cases of re-entrant calls
@@ -79,6 +80,17 @@ public abstract class BindStage<Params> extends BindRequester {
         return params;
     }
 
+    // TODO(b/253081345): Remove this method.
+    /**
+     * Get the stage parameters for the entry, or null if there are no stage parameters for the
+     * entry.
+     *
+     * @see #getStageParams(NotificationEntry)
+     */
+    public final @Nullable Params tryGetStageParams(@NonNull NotificationEntry entry) {
+        return mContentParams.get(entry);
+    }
+
     /**
      * Create a params entry for the notification for this stage.
      */
diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/interruption/HeadsUpViewBinderTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/interruption/HeadsUpViewBinderTest.java
index 3f641df376ed..ca6598726a85 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/interruption/HeadsUpViewBinderTest.java
+++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/interruption/HeadsUpViewBinderTest.java
@@ -91,6 +91,8 @@ public class HeadsUpViewBinderTest extends SysuiTestCase {
         verifyNoMoreInteractions(mLogger);
         clearInvocations(mLogger);
 
+        when(mBindStage.tryGetStageParams(eq(mEntry))).thenReturn(new RowContentBindParams());
+
         mViewBinder.unbindHeadsUpView(mEntry);
         verify(mLogger).entryContentViewMarkedFreeable(eq(mEntry));
         verifyNoMoreInteractions(mLogger);
@@ -139,6 +141,8 @@ public class HeadsUpViewBinderTest extends SysuiTestCase {
         verifyNoMoreInteractions(mLogger);
         clearInvocations(mLogger);
 
+        when(mBindStage.tryGetStageParams(eq(mEntry))).thenReturn(new RowContentBindParams());
+
         mViewBinder.unbindHeadsUpView(mEntry);
         verify(mLogger).currentOngoingBindingAborted(eq(mEntry));
         verify(mLogger).entryContentViewMarkedFreeable(eq(mEntry));
@@ -150,4 +154,30 @@ public class HeadsUpViewBinderTest extends SysuiTestCase {
         verifyNoMoreInteractions(mLogger);
         clearInvocations(mLogger);
     }
+
+    @Test
+    public void testLoggingForLateUnbindFlow() {
+        AtomicReference<NotifBindPipeline.BindCallback> callback = new AtomicReference<>();
+        when(mBindStage.requestRebind(any(), any())).then(i -> {
+            callback.set(i.getArgument(1));
+            return new CancellationSignal();
+        });
+
+        mViewBinder.bindHeadsUpView(mEntry, null);
+        verify(mLogger).startBindingHun(eq(mEntry));
+        verifyNoMoreInteractions(mLogger);
+        clearInvocations(mLogger);
+
+        callback.get().onBindFinished(mEntry);
+        verify(mLogger).entryBoundSuccessfully(eq(mEntry));
+        verifyNoMoreInteractions(mLogger);
+        clearInvocations(mLogger);
+
+        when(mBindStage.tryGetStageParams(eq(mEntry))).thenReturn(null);
+
+        mViewBinder.unbindHeadsUpView(mEntry);
+        verify(mLogger).entryBindStageParamsNullOnUnbind(eq(mEntry));
+        verifyNoMoreInteractions(mLogger);
+        clearInvocations(mLogger);
+    }
 }
diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/RowContentBindStageTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/RowContentBindStageTest.java
index ad3bd711c23f..7c99568ee75f 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/RowContentBindStageTest.java
+++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/RowContentBindStageTest.java
@@ -21,6 +21,10 @@ import static com.android.systemui.statusbar.notification.row.NotificationRowCon
 import static com.android.systemui.statusbar.notification.row.NotificationRowContentBinder.FLAG_CONTENT_VIEW_EXPANDED;
 import static com.android.systemui.statusbar.notification.row.NotificationRowContentBinder.FLAG_CONTENT_VIEW_HEADS_UP;
 
+import static junit.framework.Assert.assertEquals;
+import static junit.framework.Assert.assertNotNull;
+import static junit.framework.Assert.assertNotSame;
+import static junit.framework.Assert.assertNull;
 import static junit.framework.Assert.assertTrue;
 
 import static org.mockito.ArgumentMatchers.any;
@@ -31,6 +35,7 @@ import static org.mockito.Mockito.verify;
 
 import android.testing.AndroidTestingRunner;
 import android.testing.TestableLooper;
+import android.util.Log;
 
 import androidx.test.filters.SmallTest;
 
@@ -100,6 +105,67 @@ public class RowContentBindStageTest extends SysuiTestCase {
         verify(mBinder).unbindContent(eq(mEntry), any(), eq(flags));
     }
 
+    class CountingWtfHandler implements Log.TerribleFailureHandler {
+        private Log.TerribleFailureHandler mOldHandler = null;
+        private int mWtfCount = 0;
+
+        public void register() {
+            mOldHandler = Log.setWtfHandler(this);
+        }
+
+        public void unregister() {
+            Log.setWtfHandler(mOldHandler);
+            mOldHandler = null;
+        }
+
+        @Override
+        public void onTerribleFailure(String tag, Log.TerribleFailure what, boolean system) {
+            mWtfCount++;
+        }
+
+        public int getWtfCount() {
+            return mWtfCount;
+        }
+    }
+
+    @Test
+    public void testGetStageParamsAfterCleanUp() {
+        // GIVEN an entry whose params have already been deleted.
+        RowContentBindParams originalParams = mRowContentBindStage.getStageParams(mEntry);
+        mRowContentBindStage.deleteStageParams(mEntry);
+
+        // WHEN a caller calls getStageParams.
+        CountingWtfHandler countingWtfHandler = new CountingWtfHandler();
+        countingWtfHandler.register();
+
+        RowContentBindParams blankParams = mRowContentBindStage.getStageParams(mEntry);
+
+        countingWtfHandler.unregister();
+
+        // THEN getStageParams logs a WTF and returns blank params created to avoid a crash.
+        assertEquals(1, countingWtfHandler.getWtfCount());
+        assertNotNull(blankParams);
+        assertNotSame(originalParams, blankParams);
+    }
+
+    @Test
+    public void testTryGetStageParamsAfterCleanUp() {
+        // GIVEN an entry whose params have already been deleted.
+        mRowContentBindStage.deleteStageParams(mEntry);
+
+        // WHEN a caller calls getStageParams.
+        CountingWtfHandler countingWtfHandler = new CountingWtfHandler();
+        countingWtfHandler.register();
+
+        RowContentBindParams nullParams = mRowContentBindStage.tryGetStageParams(mEntry);
+
+        countingWtfHandler.unregister();
+
+        // THEN getStageParams does NOT log a WTF and returns null to indicate missing params.
+        assertEquals(0, countingWtfHandler.getWtfCount());
+        assertNull(nullParams);
+    }
+
     @Test
     public void testRebindAllContentViews() {
         // GIVEN a view with content bound.