Merge "[Ongoing Call] Use the notification's key to identify when to remove the ongoing call chip, rather than the notification's template." into sc-dev

packages/SystemUI/src/com/android/systemui/statusbar/phone/ongoingcall/OngoingCallController.kt

@@ -53,8 +53,8 @@ class OngoingCallController @Inject constructor(
     private val logger: OngoingCallLogger
 ) : CallbackController<OngoingCallListener> {
 
-    /** Null if there's no ongoing call. */
+    /** Non-null if there's an active call notification. */
-    private var ongoingCallInfo: OngoingCallInfo? = null
+    private var callNotificationInfo: CallNotificationInfo? = null
     /** True if the application managing the call is visible to the user. */
     private var isCallAppVisible: Boolean = true
     private var chipView: View? = null
@@ -76,19 +76,34 @@ class OngoingCallController @Inject constructor(
         }
 
         override fun onEntryUpdated(entry: NotificationEntry) {
-            if (isOngoingCallNotification(entry)) {
+            // We have a new call notification or our existing call notification has been updated.
-                ongoingCallInfo = OngoingCallInfo(
+            // TODO(b/183229367): This likely won't work if you take a call from one app then
-                    entry.sbn.notification.`when`,
+            //  switch to a call from another app.
-                    entry.sbn.notification.contentIntent.intent,
+            if (callNotificationInfo == null && isCallNotification(entry) ||
-                    entry.sbn.uid)
+                    (entry.sbn.key == callNotificationInfo?.key)) {
-                updateChip()
+                val newOngoingCallInfo = CallNotificationInfo(
-            } else if (isCallNotification(entry)) {
+                        entry.sbn.key,
-                removeChip()
+                        entry.sbn.notification.`when`,
+                        entry.sbn.notification.contentIntent.intent,
+                        entry.sbn.uid,
+                        entry.sbn.notification.extras.getInt(
+                                Notification.EXTRA_CALL_TYPE, -1) == CALL_TYPE_ONGOING
+                )
+                if (newOngoingCallInfo == callNotificationInfo) {
+                    return
+                }
+
+                callNotificationInfo = newOngoingCallInfo
+                if (newOngoingCallInfo.isOngoing) {
+                    updateChip()
+                } else {
+                    removeChip()
+                }
             }
         }
 
         override fun onEntryRemoved(entry: NotificationEntry, reason: Int) {
-            if (isOngoingCallNotification(entry)) {
+            if (entry.sbn.key == callNotificationInfo?.key) {
                 removeChip()
             }
         }
@@ -126,7 +141,7 @@ class OngoingCallController @Inject constructor(
      * Returns true if there's an active ongoing call that should be displayed in a status bar chip.
      */
     fun hasOngoingCall(): Boolean {
-        return ongoingCallInfo != null &&
+        return callNotificationInfo?.isOngoing == true &&
                 // When the user is in the phone app, don't show the chip.
                 !isCallAppVisible
     }
@@ -146,7 +161,7 @@ class OngoingCallController @Inject constructor(
     }
 
     private fun updateChip() {
-        val currentOngoingCallInfo = ongoingCallInfo ?: return
+        val currentCallNotificationInfo = callNotificationInfo ?: return
 
         val currentChipView = chipView
         val timeView =
@@ -155,7 +170,7 @@ class OngoingCallController @Inject constructor(
             currentChipView?.findViewById<View>(R.id.ongoing_call_chip_background)
 
         if (currentChipView != null && timeView != null && backgroundView != null) {
-            timeView.base = currentOngoingCallInfo.callStartTime -
+            timeView.base = currentCallNotificationInfo.callStartTime -
                     System.currentTimeMillis() +
                     systemClock.elapsedRealtime()
             timeView.start()
@@ -163,17 +178,17 @@ class OngoingCallController @Inject constructor(
             currentChipView.setOnClickListener {
                 logger.logChipClicked()
                 activityStarter.postStartActivityDismissingKeyguard(
-                        currentOngoingCallInfo.intent, 0,
+                        currentCallNotificationInfo.intent, 0,
                         ActivityLaunchAnimator.Controller.fromView(backgroundView))
             }
 
-            setUpUidObserver(currentOngoingCallInfo)
+            setUpUidObserver(currentCallNotificationInfo)
 
             mListeners.forEach { l -> l.onOngoingCallStateChanged(animate = true) }
         } else {
-            // If we failed to update the chip, don't store the ongoing call info. Then
+            // If we failed to update the chip, don't store the call info. Then [hasOngoingCall]
-            // [hasOngoingCall] will return false and we fall back to typical notification handling.
+            // will return false and we fall back to typical notification handling.
-            ongoingCallInfo = null
+            callNotificationInfo = null
 
             if (DEBUG) {
                 Log.w(TAG, "Ongoing call chip view could not be found; " +
@@ -185,14 +200,14 @@ class OngoingCallController @Inject constructor(
     /**
      * Sets up an [IUidObserver] to monitor the status of the application managing the ongoing call.
      */
-    private fun setUpUidObserver(currentOngoingCallInfo: OngoingCallInfo) {
+    private fun setUpUidObserver(currentCallNotificationInfo: CallNotificationInfo) {
         isCallAppVisible = isProcessVisibleToUser(
-                iActivityManager.getUidProcessState(currentOngoingCallInfo.uid, null))
+                iActivityManager.getUidProcessState(currentCallNotificationInfo.uid, null))
 
         uidObserver = object : IUidObserver.Stub() {
             override fun onUidStateChanged(
                     uid: Int, procState: Int, procStateSeq: Long, capability: Int) {
-                if (uid == currentOngoingCallInfo.uid) {
+                if (uid == currentCallNotificationInfo.uid) {
                     val oldIsCallAppVisible = isCallAppVisible
                     isCallAppVisible = isProcessVisibleToUser(procState)
                     if (oldIsCallAppVisible != isCallAppVisible) {
@@ -225,26 +240,23 @@ class OngoingCallController @Inject constructor(
     }
 
     private fun removeChip() {
-        ongoingCallInfo = null
+        callNotificationInfo = null
         mListeners.forEach { l -> l.onOngoingCallStateChanged(animate = true) }
         if (uidObserver != null) {
             iActivityManager.unregisterUidObserver(uidObserver)
         }
     }
 
-    private class OngoingCallInfo(
+    private data class CallNotificationInfo(
+        val key: String,
         val callStartTime: Long,
         val intent: Intent,
-        val uid: Int
+        val uid: Int,
+        /** True if the call is currently ongoing (as opposed to incoming, screening, etc.). */
+        val isOngoing: Boolean
     )
 }
 
-private fun isOngoingCallNotification(entry: NotificationEntry): Boolean {
-    val extras = entry.sbn.notification.extras
-    return isCallNotification(entry) &&
-            extras.getInt(Notification.EXTRA_CALL_TYPE, -1) == CALL_TYPE_ONGOING
-}
-
 private fun isCallNotification(entry: NotificationEntry): Boolean {
     return entry.sbn.notification.isStyle(Notification.CallStyle::class.java)
 }

packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/ongoingcall/OngoingCallControllerTest.kt

@@ -55,6 +55,8 @@ import org.mockito.Mockito.mock
 import org.mockito.Mockito.never
 import org.mockito.Mockito.times
 import org.mockito.Mockito.verify
+import org.mockito.Mockito.reset
+
 import org.mockito.MockitoAnnotations
 
 private const val CALL_UID = 900
@@ -138,15 +140,51 @@ class OngoingCallControllerTest : SysuiTestCase() {
                 .onOngoingCallStateChanged(anyBoolean())
     }
 
+    /**
+     * If a call notification is never added before #onEntryRemoved is called, then the listener
+     * should never be notified.
+     */
     @Test
-    fun onEntryRemoved_ongoingCallNotif_listenerNotified() {
+    fun onEntryRemoved_callNotifNeverAddedBeforehand_listenerNotNotified() {
         notifCollectionListener.onEntryRemoved(createOngoingCallNotifEntry(), REASON_USER_STOPPED)
 
+        verify(mockOngoingCallListener, never()).onOngoingCallStateChanged(anyBoolean())
+    }
+
+    @Test
+    fun onEntryRemoved_callNotifAddedThenRemoved_listenerNotified() {
+        val ongoingCallNotifEntry = createOngoingCallNotifEntry()
+        notifCollectionListener.onEntryAdded(ongoingCallNotifEntry)
+        reset(mockOngoingCallListener)
+
+        notifCollectionListener.onEntryRemoved(ongoingCallNotifEntry, REASON_USER_STOPPED)
+
         verify(mockOngoingCallListener).onOngoingCallStateChanged(anyBoolean())
     }
 
+    /** Regression test for b/188491504. */
     @Test
-    fun onEntryRemoved_notOngoingCallNotif_listenerNotNotified() {
+    fun onEntryRemoved_removedNotifHasSameKeyAsAddedNotif_listenerNotified() {
+        val ongoingCallNotifEntry = createOngoingCallNotifEntry()
+        notifCollectionListener.onEntryAdded(ongoingCallNotifEntry)
+        reset(mockOngoingCallListener)
+
+        // Create another notification based on the ongoing call one, but remove the features that
+        // made it a call notification.
+        val removedEntryBuilder = NotificationEntryBuilder(ongoingCallNotifEntry)
+        removedEntryBuilder.modifyNotification(context).style = null
+
+        notifCollectionListener.onEntryRemoved(removedEntryBuilder.build(), REASON_USER_STOPPED)
+
+        verify(mockOngoingCallListener).onOngoingCallStateChanged(anyBoolean())
+    }
+
+
+    @Test
+    fun onEntryRemoved_notifKeyDoesNotMatchOngoingCallNotif_listenerNotNotified() {
+        notifCollectionListener.onEntryAdded(createOngoingCallNotifEntry())
+        reset(mockOngoingCallListener)
+
         notifCollectionListener.onEntryRemoved(createNotCallNotifEntry(), REASON_USER_STOPPED)
 
         verify(mockOngoingCallListener, never()).onOngoingCallStateChanged(anyBoolean())
@@ -157,6 +195,20 @@ class OngoingCallControllerTest : SysuiTestCase() {
         assertThat(controller.hasOngoingCall()).isFalse()
     }
 
+    @Test
+    fun hasOngoingCall_unrelatedNotifSent_returnsFalse() {
+        notifCollectionListener.onEntryUpdated(createNotCallNotifEntry())
+
+        assertThat(controller.hasOngoingCall()).isFalse()
+    }
+
+    @Test
+    fun hasOngoingCall_screeningCallNotifSent_returnsFalse() {
+        notifCollectionListener.onEntryUpdated(createScreeningCallNotifEntry())
+
+        assertThat(controller.hasOngoingCall()).isFalse()
+    }
+
     @Test
     fun hasOngoingCall_ongoingCallNotifSentAndCallAppNotVisible_returnsTrue() {
         `when`(mockIActivityManager.getUidProcessState(eq(CALL_UID), nullable(String::class.java)))
@@ -224,6 +276,7 @@ class OngoingCallControllerTest : SysuiTestCase() {
     fun setChipView_whenHasOngoingCallIsTrue_listenerNotifiedWithNewView() {
         // Start an ongoing call.
         notifCollectionListener.onEntryUpdated(createOngoingCallNotifEntry())
+        reset(mockOngoingCallListener)
 
         lateinit var newChipView: View
         TestableLooper.get(this).runWithLooper {
@@ -233,10 +286,7 @@ class OngoingCallControllerTest : SysuiTestCase() {
         // Change the chip view associated with the controller.
         controller.setChipView(newChipView)
 
-        // Verify the listener was notified once for the initial call and again when the new view
+        verify(mockOngoingCallListener).onOngoingCallStateChanged(anyBoolean())
-        // was set.
-        verify(mockOngoingCallListener, times(2))
-                .onOngoingCallStateChanged(anyBoolean())
     }
 
     @Test
@@ -245,6 +295,7 @@ class OngoingCallControllerTest : SysuiTestCase() {
         `when`(mockIActivityManager.getUidProcessState(eq(CALL_UID), nullable(String::class.java)))
                 .thenReturn(PROC_STATE_INVISIBLE)
         notifCollectionListener.onEntryUpdated(createOngoingCallNotifEntry())
+        reset(mockOngoingCallListener)
 
         val captor = ArgumentCaptor.forClass(IUidObserver.Stub::class.java)
         verify(mockIActivityManager).registerUidObserver(
@@ -256,9 +307,7 @@ class OngoingCallControllerTest : SysuiTestCase() {
         mainExecutor.advanceClockToLast()
         mainExecutor.runAllReady();
 
-        // Once for when the call was started, and another time when the process visibility changes.
+        verify(mockOngoingCallListener).onOngoingCallStateChanged(anyBoolean())
-        verify(mockOngoingCallListener, times(2))
-                .onOngoingCallStateChanged(anyBoolean())
     }
 
     @Test
@@ -267,6 +316,7 @@ class OngoingCallControllerTest : SysuiTestCase() {
         `when`(mockIActivityManager.getUidProcessState(eq(CALL_UID), nullable(String::class.java)))
                 .thenReturn(PROC_STATE_VISIBLE)
         notifCollectionListener.onEntryUpdated(createOngoingCallNotifEntry())
+        reset(mockOngoingCallListener)
 
         val captor = ArgumentCaptor.forClass(IUidObserver.Stub::class.java)
         verify(mockIActivityManager).registerUidObserver(
@@ -278,9 +328,7 @@ class OngoingCallControllerTest : SysuiTestCase() {
         mainExecutor.advanceClockToLast()
         mainExecutor.runAllReady();
 
-        // Once for when the call was started, and another time when the process visibility changes.
+        verify(mockOngoingCallListener).onOngoingCallStateChanged(anyBoolean())
-        verify(mockOngoingCallListener, times(2))
-                .onOngoingCallStateChanged(anyBoolean())
     }
 
     @Test