From 4c910f625aafa00bbc5671f10aaf78e131bc9002 Mon Sep 17 00:00:00 2001 From: Caitlin Cassidy Date: Thu, 17 Feb 2022 20:37:43 +0000 Subject: [PATCH] [Ongoing Call] Make the uidObserver a val so that we're guaranteed to not accidentally register it multiple times in a row. This is a follow-up fix from comments on ag/15503430. Fixes: 216520671 Test: Verify the call chip still disappears when you're in a call and you open the call app. Test: atest OngoingCallControllerTest Change-Id: I9454b27961d9277a6c85e16c196d6ed0ddc6273e Change-Id: I8073f178566754037e25af57d2b9949f0d6a396d Merged-In: I8073f178566754037e25af57d2b9949f0d6a396d --- .../ongoingcall/OngoingCallController.kt | 150 ++++++++++-------- .../ongoingcall/OngoingCallControllerTest.kt | 12 +- 2 files changed, 87 insertions(+), 75 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/ongoingcall/OngoingCallController.kt b/packages/SystemUI/src/com/android/systemui/statusbar/phone/ongoingcall/OngoingCallController.kt index de05eb1e42caa..982ccfd4dffac 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/ongoingcall/OngoingCallController.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/ongoingcall/OngoingCallController.kt @@ -69,13 +69,10 @@ class OngoingCallController @Inject constructor( private var isFullscreen: Boolean = false /** Non-null if there's an active call notification. */ private var callNotificationInfo: CallNotificationInfo? = null - /** True if the application managing the call is visible to the user. */ - private var isCallAppVisible: Boolean = false private var chipView: View? = null - private var uidObserver: IUidObserver.Stub? = null private val mListeners: MutableList = mutableListOf() - + private val uidObserver = CallAppUidObserver() private val notifListener = object : NotifCollectionListener { // Temporary workaround for b/178406514 for testing purposes. // @@ -160,7 +157,7 @@ class OngoingCallController @Inject constructor( fun hasOngoingCall(): Boolean { return callNotificationInfo?.isOngoing == true && // When the user is in the phone app, don't show the chip. - !isCallAppVisible + !uidObserver.isCallAppVisible } override fun addCallback(listener: OngoingCallListener) { @@ -196,7 +193,7 @@ class OngoingCallController @Inject constructor( } updateChipClickListener() - setUpUidObserver(currentCallNotificationInfo) + uidObserver.registerWithUid(currentCallNotificationInfo.uid) if (!currentCallNotificationInfo.statusBarSwipedAway) { statusBarWindowController.ifPresent { it.setOngoingProcessRequiresStatusBarVisible(true) @@ -240,64 +237,6 @@ class OngoingCallController @Inject constructor( } } - /** - * Sets up an [IUidObserver] to monitor the status of the application managing the ongoing call. - */ - private fun setUpUidObserver(currentCallNotificationInfo: CallNotificationInfo) { - try { - isCallAppVisible = isProcessVisibleToUser( - iActivityManager.getUidProcessState( - currentCallNotificationInfo.uid, context.opPackageName - ) - ) - } catch (se: SecurityException) { - Log.e(TAG, "Security exception when trying to get process state: $se") - return - } - - if (uidObserver != null) { - iActivityManager.unregisterUidObserver(uidObserver) - } - - uidObserver = object : IUidObserver.Stub() { - override fun onUidStateChanged( - uid: Int, - procState: Int, - procStateSeq: Long, - capability: Int - ) { - if (uid == currentCallNotificationInfo.uid) { - val oldIsCallAppVisible = isCallAppVisible - isCallAppVisible = isProcessVisibleToUser(procState) - if (oldIsCallAppVisible != isCallAppVisible) { - // Animations may be run as a result of the call's state change, so ensure - // the listener is notified on the main thread. - mainExecutor.execute { - mListeners.forEach { l -> l.onOngoingCallStateChanged(animate = true) } - } - } - } - } - - override fun onUidGone(uid: Int, disabled: Boolean) {} - override fun onUidActive(uid: Int) {} - override fun onUidIdle(uid: Int, disabled: Boolean) {} - override fun onUidCachedChanged(uid: Int, cached: Boolean) {} - } - - try { - iActivityManager.registerUidObserver( - uidObserver, - ActivityManager.UID_OBSERVER_PROCSTATE, - ActivityManager.PROCESS_STATE_UNKNOWN, - context.opPackageName - ) - } catch (se: SecurityException) { - Log.e(TAG, "Security exception when trying to register uid observer: $se") - return - } - } - /** Returns true if the given [procState] represents a process that's visible to the user. */ private fun isProcessVisibleToUser(procState: Int): Boolean { return procState <= ActivityManager.PROCESS_STATE_TOP @@ -321,9 +260,7 @@ class OngoingCallController @Inject constructor( statusBarWindowController.ifPresent { it.setOngoingProcessRequiresStatusBarVisible(false) } swipeStatusBarAwayGestureHandler.ifPresent { it.removeOnGestureDetectedCallback(TAG) } mListeners.forEach { l -> l.onOngoingCallStateChanged(animate = true) } - if (uidObserver != null) { - iActivityManager.unregisterUidObserver(uidObserver) - } + uidObserver.unregister() } /** Tear down anything related to the chip view to prevent leaks. */ @@ -380,7 +317,84 @@ class OngoingCallController @Inject constructor( override fun dump(fd: FileDescriptor, pw: PrintWriter, args: Array) { pw.println("Active call notification: $callNotificationInfo") - pw.println("Call app visible: $isCallAppVisible") + pw.println("Call app visible: ${uidObserver.isCallAppVisible}") + } + + /** Our implementation of a [IUidObserver]. */ + inner class CallAppUidObserver : IUidObserver.Stub() { + /** True if the application managing the call is visible to the user. */ + var isCallAppVisible: Boolean = false + private set + + /** The UID of the application managing the call. Null if there is no active call. */ + private var callAppUid: Int? = null + + /** + * True if this observer is currently registered with the activity manager and false + * otherwise. + */ + private var isRegistered = false + + + /** Register this observer with the activity manager and the given [uid]. */ + fun registerWithUid(uid: Int) { + if (callAppUid == uid) { + return + } + callAppUid = uid + + try { + isCallAppVisible = isProcessVisibleToUser( + iActivityManager.getUidProcessState(uid, context.opPackageName) + ) + if (isRegistered) { + return + } + iActivityManager.registerUidObserver( + uidObserver, + ActivityManager.UID_OBSERVER_PROCSTATE, + ActivityManager.PROCESS_STATE_UNKNOWN, + context.opPackageName + ) + isRegistered = true + } catch (se: SecurityException) { + Log.e(TAG, "Security exception when trying to set up uid observer: $se") + } + } + + /** Unregister this observer with the activity manager. */ + fun unregister() { + callAppUid = null + isRegistered = false + iActivityManager.unregisterUidObserver(uidObserver) + } + + override fun onUidStateChanged( + uid: Int, + procState: Int, + procStateSeq: Long, + capability: Int + ) { + val currentCallAppUid = callAppUid ?: return + if (uid != currentCallAppUid) { + return + } + + val oldIsCallAppVisible = isCallAppVisible + isCallAppVisible = isProcessVisibleToUser(procState) + if (oldIsCallAppVisible != isCallAppVisible) { + // Animations may be run as a result of the call's state change, so ensure + // the listener is notified on the main thread. + mainExecutor.execute { + mListeners.forEach { l -> l.onOngoingCallStateChanged(animate = true) } + } + } + } + + override fun onUidGone(uid: Int, disabled: Boolean) {} + override fun onUidActive(uid: Int) {} + override fun onUidIdle(uid: Int, disabled: Boolean) {} + override fun onUidCachedChanged(uid: Int, cached: Boolean) {} } } diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/ongoingcall/OngoingCallControllerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/ongoingcall/OngoingCallControllerTest.kt index ada0453acd86c..1c48eca797a30 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/ongoingcall/OngoingCallControllerTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/ongoingcall/OngoingCallControllerTest.kt @@ -205,17 +205,15 @@ class OngoingCallControllerTest : SysuiTestCase() { /** Regression test for b/194731244. */ @Test - fun onEntryUpdated_calledManyTimes_uidObserverUnregisteredManyTimes() { - val numCalls = 4 - - for (i in 0 until numCalls) { + fun onEntryUpdated_calledManyTimes_uidObserverOnlyRegisteredOnce() { + for (i in 0 until 4) { // Re-create the notification each time so that it's considered a different object and - // observers will get re-registered (and hopefully unregistered). + // will re-trigger the whole flow. notifCollectionListener.onEntryUpdated(createOngoingCallNotifEntry()) } - // There should be 1 observer still registered, so we should unregister n-1 times. - verify(mockIActivityManager, times(numCalls - 1)).unregisterUidObserver(any()) + verify(mockIActivityManager, times(1)) + .registerUidObserver(any(), any(), any(), any()) } /** Regression test for b/216248574. */