Merge "VoipCallMonitor updates" into main
diff --git a/src/com/android/server/telecom/CallsManager.java b/src/com/android/server/telecom/CallsManager.java
index 0a38720..8425a10 100644
--- a/src/com/android/server/telecom/CallsManager.java
+++ b/src/com/android/server/telecom/CallsManager.java
@@ -775,7 +775,10 @@
mCallStreamingNotification = callStreamingNotification;
mFeatureFlags = featureFlags;
if (mFeatureFlags.voipCallMonitorRefactor()) {
- mVoipCallMonitor = new VoipCallMonitor(mContext, mLock);
+ mVoipCallMonitor = new VoipCallMonitor(
+ mContext,
+ new Handler(Looper.getMainLooper()),
+ mLock);
mVoipCallMonitorLegacy = null;
} else {
mVoipCallMonitor = null;
diff --git a/src/com/android/server/telecom/callsequencing/voip/VoipCallMonitor.java b/src/com/android/server/telecom/callsequencing/voip/VoipCallMonitor.java
index bd0b932..8c74510 100644
--- a/src/com/android/server/telecom/callsequencing/voip/VoipCallMonitor.java
+++ b/src/com/android/server/telecom/callsequencing/voip/VoipCallMonitor.java
@@ -30,9 +30,7 @@
import android.content.Context;
import android.content.ServiceConnection;
import android.os.Handler;
-import android.os.HandlerThread;
import android.os.IBinder;
-import android.os.Looper;
import android.os.RemoteException;
import android.service.notification.NotificationListenerService;
import android.service.notification.StatusBarNotification;
@@ -53,14 +51,16 @@
import java.util.Objects;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentLinkedQueue;
public class VoipCallMonitor extends CallsManagerListenerBase {
- private static final long NOTIFICATION_NOT_POSTED_IN_TIME_TIMEOUT = 5000L;
- private static final long NOTIFICATION_REMOVED_BUT_CALL_IS_STILL_ONGOING_TIMEOUT = 5000L;
+ public static final long NOTIFICATION_NOT_POSTED_IN_TIME_TIMEOUT = 5000L;
+ public static final long NOTIFICATION_REMOVED_BUT_CALL_IS_STILL_ONGOING_TIMEOUT = 5000L;
+ private static final String TAG = VoipCallMonitor.class.getSimpleName();
private static final String DElIMITER = "#";
// This list caches calls that are added to the VoipCallMonitor and need an accompanying
// Call-Style Notification!
- private final List<Call> mNewCallsMissingCallStyleNotification;
+ private final ConcurrentLinkedQueue<Call> mNewCallsMissingCallStyleNotification;
private final ConcurrentHashMap<String, Call> mNotificationIdToCall;
private final ConcurrentHashMap<PhoneAccountHandle, Set<Call>> mAccountHandleToCallMap;
private final ConcurrentHashMap<PhoneAccountHandle, ServiceConnection> mServices;
@@ -70,11 +70,11 @@
private final Context mContext;
private final TelecomSystem.SyncRoot mSyncRoot;
- public VoipCallMonitor(Context context, TelecomSystem.SyncRoot lock) {
+ public VoipCallMonitor(Context context, Handler handler, TelecomSystem.SyncRoot lock) {
mSyncRoot = lock;
mContext = context;
- mHandlerForClass = new Handler(Looper.getMainLooper());
- mNewCallsMissingCallStyleNotification = new ArrayList<>();
+ mHandlerForClass = handler;
+ mNewCallsMissingCallStyleNotification = new ConcurrentLinkedQueue<>();
mNotificationIdToCall = new ConcurrentHashMap<>();
mServices = new ConcurrentHashMap<>();
mAccountHandleToCallMap = new ConcurrentHashMap<>();
@@ -83,21 +83,18 @@
@Override
public void onNotificationPosted(StatusBarNotification sbn) {
if (isCallStyleNotification(sbn)) {
- Log.i(this, "onNotificationPosted: sbn=[%s]", sbn);
- boolean foundCallForNotification = false;
+ Log.i(TAG, "onNotificationPosted: sbn=[%s]", sbn);
// Case 1: Call added to this class (via onCallAdded) BEFORE Call-Style
// Notification is posted by the app (only supported scenario)
- // --> remove the newly added call from
- // mNewCallsMissingCallStyleNotification so FGS is not revoked.
- for (Call call : new ArrayList<>(mNewCallsMissingCallStyleNotification)) {
+ Call newCallNoLongerAwaitingNotification = null;
+ for (Call call : mNewCallsMissingCallStyleNotification) {
if (isNotificationForCall(sbn, call)) {
- Log.i(this, "onNotificationPosted: found a pending "
+ Log.i(TAG, "onNotificationPosted: found a pending "
+ "call=[%s] for sbn.id=[%s]", call, sbn.getId());
mNotificationIdToCall.put(
getNotificationIdToCallKey(sbn),
call);
- removeFromNotificationTracking(call);
- foundCallForNotification = true;
+ newCallNoLongerAwaitingNotification = call;
break;
}
}
@@ -105,11 +102,19 @@
// --> Currently do not support this
// Case 3: Call-Style Notification was updated (ex. incoming -> ongoing)
// --> do nothing
- if (!foundCallForNotification) {
- Log.i(this, "onNotificationPosted: could not find a call for the"
+ if (newCallNoLongerAwaitingNotification == null) {
+ Log.i(TAG, "onNotificationPosted: could not find a call for the"
+ " sbn.id=[%s]. This could mean the notification posted"
+ " BEFORE the call is added (error) or it's an update from"
+ " incoming to ongoing (ok).", sbn.getId());
+ } else {
+ // --> remove the newly added call from
+ // mNewCallsMissingCallStyleNotification so FGS is not revoked when the
+ // timeout is hit in VoipCallMonitor#startMonitoringNotification(...). The
+ // timeout ensures the voip app posts a call-style notification within
+ // 5 seconds!
+ mNewCallsMissingCallStyleNotification
+ .remove(newCallNoLongerAwaitingNotification);
}
}
}
@@ -119,14 +124,17 @@
if (!isCallStyleNotification(sbn)) {
return;
}
- Log.i(this, "onNotificationRemoved: Call-Style notification=[%s] removed", sbn);
+ Log.i(TAG, "onNotificationRemoved: Call-Style notification=[%s] removed", sbn);
Call call = getCallFromStatusBarNotificationId(sbn);
if (call != null) {
- PhoneAccountHandle handle = getTargetPhoneAccount(call);
if (!isCallDisconnected(call)) {
mHandlerForClass.postDelayed(() -> {
if (isCallStillBeingTracked(call)) {
- stopFGSDelegation(call, handle);
+ Log.w(TAG,
+ "onNotificationRemoved: notification has been removed for"
+ + " more than 5 seconds but call still ongoing "
+ + "c=[%s]", call);
+ // TODO:: stopFGSDelegation(call, handle) when b/383403913 is fixed
}
}, NOTIFICATION_REMOVED_BUT_CALL_IS_STILL_ONGOING_TIMEOUT);
}
@@ -185,7 +193,7 @@
new ComponentName(this.getClass().getPackageName(),
this.getClass().getCanonicalName()), ActivityManager.getCurrentUser());
} catch (RemoteException e) {
- Log.e(this, e, "Cannot register notification listener");
+ Log.e(TAG, e, "Cannot register notification listener");
}
}
@@ -193,7 +201,7 @@
try {
mNotificationListener.unregisterAsSystemService();
} catch (RemoteException e) {
- Log.e(this, e, "Cannot unregister notification listener");
+ Log.e(TAG, e, "Cannot unregister notification listener");
}
}
@@ -217,11 +225,10 @@
if (!isTransactional(call) || handle == null) {
return;
}
- removeFromNotificationTracking(call);
Set<Call> ongoingCalls = mAccountHandleToCallMap
.computeIfAbsent(handle, k -> new HashSet<>());
ongoingCalls.remove(call);
- Log.d(this, "onCallRemoved: callList.size=[%d]", ongoingCalls.size());
+ Log.d(TAG, "onCallRemoved: callList.size=[%d]", ongoingCalls.size());
if (ongoingCalls.isEmpty()) {
stopFGSDelegation(call, handle);
} else {
@@ -230,7 +237,7 @@
}
private void maybeStartFGSDelegation(int pid, int uid, PhoneAccountHandle handle, Call call) {
- Log.i(this, "maybeStartFGSDelegation for call=[%s]", call);
+ Log.i(TAG, "maybeStartFGSDelegation for call=[%s]", call);
if (mActivityManagerInternal != null) {
if (mServices.containsKey(handle)) {
Log.addEvent(call, LogUtils.Events.ALREADY_HAS_FGS_DELEGATION);
@@ -262,34 +269,38 @@
try {
if (mActivityManagerInternal
.startForegroundServiceDelegate(options, fgsConnection)) {
- Log.i(this, "maybeStartFGSDelegation: startForegroundServiceDelegate success");
+ Log.i(TAG, "maybeStartFGSDelegation: startForegroundServiceDelegate success");
} else {
Log.addEvent(call, LogUtils.Events.GAIN_FGS_DELEGATION_FAILED);
}
} catch (Exception e) {
- Log.i(this, "startForegroundServiceDelegate failed due to: " + e);
+ Log.i(TAG, "startForegroundServiceDelegate failed due to: " + e);
}
}
}
@VisibleForTesting
public void stopFGSDelegation(Call call, PhoneAccountHandle handle) {
- Log.i(this, "stopFGSDelegation of call=[%s]", call);
+ Log.i(TAG, "stopFGSDelegation of call=[%s]", call);
if (handle == null) {
return;
}
- // In the event this class is waiting for any new calls to post a notification, remove
- // the call from the notification tracking container!
- Set<Call> ongoingCalls = mAccountHandleToCallMap.get(handle);
- if (ongoingCalls != null) {
- for (Call c : new ArrayList<>(ongoingCalls)) {
- removeFromNotificationTracking(c);
+
+ // In the event this class is waiting for any new calls to post a notification, cleanup
+ List<Call> toRemove = new ArrayList<>();
+ for (Call callAwaitingNotification : mNewCallsMissingCallStyleNotification) {
+ if (handle.equals(callAwaitingNotification.getTargetPhoneAccount())) {
+ Log.d(TAG, "stopFGSDelegation: removing call from notification tracking c=[%s]",
+ callAwaitingNotification);
+ toRemove.add(callAwaitingNotification);
}
}
+ mNewCallsMissingCallStyleNotification.removeAll(toRemove);
+
if (mActivityManagerInternal != null) {
ServiceConnection fgsConnection = mServices.get(handle);
if (fgsConnection != null) {
- Log.i(this, "stopFGSDelegation: requesting stopForegroundServiceDelegate");
+ Log.i(TAG, "stopFGSDelegation: requesting stopForegroundServiceDelegate");
mActivityManagerInternal.stopForegroundServiceDelegate(fgsConnection);
}
}
@@ -301,17 +312,17 @@
String callId = getCallId(call);
// Wait 5 seconds for a CallStyle notification to be posted for the call.
// If the Call-Style Notification is not posted, FGS delegation needs to be revoked!
- Log.i(this, "startMonitoringNotification: starting timeout for call.id=[%s]", callId);
- addToNotificationTracking(call);
+ Log.i(TAG, "startMonitoringNotification: starting timeout for call.id=[%s]", callId);
+ mNewCallsMissingCallStyleNotification.add(call);
// If no notification is posted, stop foreground service delegation!
mHandlerForClass.postDelayed(() -> {
- if (isStillMissingNotification(call)) {
- Log.i(this, "startMonitoringNotification: A Call-Style-Notification"
+ if (mNewCallsMissingCallStyleNotification.contains(call)) {
+ Log.i(TAG, "startMonitoringNotification: A Call-Style-Notification"
+ " for voip-call=[%s] hasn't posted in time,"
+ " stopping delegation for app=[%s].", call, packageName);
stopFGSDelegation(call, handle);
} else {
- Log.i(this, "startMonitoringNotification: found a call-style"
+ Log.i(TAG, "startMonitoringNotification: found a call-style"
+ " notification for call.id[%s] at timeout", callId);
}
}, NOTIFICATION_NOT_POSTED_IN_TIME_TIMEOUT);
@@ -321,24 +332,6 @@
* Helpers
*/
- private void addToNotificationTracking(Call call) {
- synchronized (mNewCallsMissingCallStyleNotification) {
- mNewCallsMissingCallStyleNotification.add(call);
- }
- }
-
- private boolean isStillMissingNotification(Call call) {
- synchronized (mNewCallsMissingCallStyleNotification) {
- return mNewCallsMissingCallStyleNotification.contains(call);
- }
- }
-
- private void removeFromNotificationTracking(Call call) {
- synchronized (mNewCallsMissingCallStyleNotification) {
- mNewCallsMissingCallStyleNotification.remove(call);
- }
- }
-
private PhoneAccountHandle getTargetPhoneAccount(Call call) {
synchronized (mSyncRoot) {
if (call == null) {
@@ -426,7 +419,7 @@
public boolean hasForegroundServiceDelegation(PhoneAccountHandle handle) {
boolean hasFgs = mServices.containsKey(handle);
- Log.i(this, "hasForegroundServiceDelegation: handle=[%s], hasFgs=[%b]", handle, hasFgs);
+ Log.i(TAG, "hasForegroundServiceDelegation: handle=[%s], hasFgs=[%b]", handle, hasFgs);
return hasFgs;
}
@@ -434,4 +427,9 @@
public ConcurrentHashMap<PhoneAccountHandle, Set<Call>> getAccountToCallsMapping() {
return mAccountHandleToCallMap;
}
+
+ @VisibleForTesting
+ public ConcurrentLinkedQueue<Call> getNewCallsMissingCallStyleNotificationQueue(){
+ return mNewCallsMissingCallStyleNotification;
+ }
}
diff --git a/tests/src/com/android/server/telecom/tests/VoipCallMonitorTest.java b/tests/src/com/android/server/telecom/tests/VoipCallMonitorTest.java
index 1444b09..1b3856c 100644
--- a/tests/src/com/android/server/telecom/tests/VoipCallMonitorTest.java
+++ b/tests/src/com/android/server/telecom/tests/VoipCallMonitorTest.java
@@ -22,9 +22,13 @@
import static android.content.pm.ServiceInfo.FOREGROUND_SERVICE_TYPE_PHONE_CALL;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.atLeastOnce;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.timeout;
import static org.mockito.Mockito.times;
@@ -40,6 +44,7 @@
import android.content.Intent;
import android.content.ServiceConnection;
import android.os.Bundle;
+import android.os.Handler;
import android.os.IBinder;
import android.os.UserHandle;
import android.service.notification.StatusBarNotification;
@@ -54,6 +59,7 @@
import org.junit.After;
import org.junit.Before;
+import org.junit.Ignore;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
@@ -75,6 +81,7 @@
private static final UserHandle USER_HANDLE_1 = new UserHandle(1);
private static final long TIMEOUT = 6000L;
+ @Mock private Handler mHandler;
@Mock private TelecomSystem.SyncRoot mLock;
@Mock private ActivityManagerInternal mActivityManagerInternal;
@Mock private IBinder mServiceConnection;
@@ -88,7 +95,8 @@
@Before
public void setUp() throws Exception {
super.setUp();
- mMonitor = new VoipCallMonitor(mContext, mLock);
+ mHandler = mock(Handler.class);
+ mMonitor = new VoipCallMonitor(mContext, mHandler, mLock);
mActivityManagerInternal = mock(ActivityManagerInternal.class);
mMonitor.setActivityManagerInternal(mActivityManagerInternal);
mMonitor.registerNotificationListener();
@@ -158,16 +166,18 @@
public void testStartMonitorForOneCall() {
// GIVEN - a single call and notification for a voip app
Call call = createTestCall("testCall", mHandle1User1);
- StatusBarNotification sbn = createStatusBarNotificationFromHandle(mHandle1User1);
+ StatusBarNotification sbn = createStatusBarNotificationFromHandle(mHandle1User1, 1);
// WHEN - the Voip call is added and a notification is posted, verify FGS is gained
addCallAndVerifyFgsIsGained(call);
mMonitor.postNotification(sbn);
+ assertNotificationTimeoutTriggered();
+ assertFalse(mMonitor.getNewCallsMissingCallStyleNotificationQueue().contains(call));
// THEN - when the Voip call is removed, verify that FGS is revoked for the app
mMonitor.onCallRemoved(call);
mMonitor.removeNotification(sbn);
- verify(mActivityManagerInternal, timeout(TIMEOUT))
+ verify(mActivityManagerInternal, times(1))
.stopForegroundServiceDelegate(any(ServiceConnection.class));
}
@@ -179,25 +189,34 @@
public void testStopDelegation_SameApp() {
// GIVEN - 2 consecutive calls for a single Voip app
Call call1 = createTestCall("testCall1", mHandle1User1);
- StatusBarNotification sbn1 = createStatusBarNotificationFromHandle(mHandle1User1);
+ StatusBarNotification sbn1 = createStatusBarNotificationFromHandle(mHandle1User1, 1);
Call call2 = createTestCall("testCall2", mHandle1User1);
- StatusBarNotification sbn2 = createStatusBarNotificationFromHandle(mHandle1User1);
+ StatusBarNotification sbn2 = createStatusBarNotificationFromHandle(mHandle1User1, 2);
// WHEN - the second call is added and the first is disconnected
- mMonitor.postNotification(sbn1);
+ // -- add the first all and post the corresponding notification
addCallAndVerifyFgsIsGained(call1);
- mMonitor.postNotification(sbn2);
+ assertTrue(mMonitor.getNewCallsMissingCallStyleNotificationQueue().contains(call1));
+ mMonitor.postNotification(sbn1);
+ assertNotificationTimeoutTriggered();
+ assertFalse(mMonitor.getNewCallsMissingCallStyleNotificationQueue().contains(call1));
+ // -- add the second call and post the corresponding notification
mMonitor.onCallAdded(call2);
- mMonitor.onCallRemoved(call1);
+ assertTrue(mMonitor.getNewCallsMissingCallStyleNotificationQueue().contains(call2));
+ mMonitor.postNotification(sbn2);
+ assertNotificationTimeoutTriggered();
+ assertFalse(mMonitor.getNewCallsMissingCallStyleNotificationQueue().contains(call2));
// THEN - assert FGS is maintained for the process since there is still an ongoing call
- verify(mActivityManagerInternal, timeout(TIMEOUT).times(0))
- .stopForegroundServiceDelegate(any(ServiceConnection.class));
+ mMonitor.onCallRemoved(call1);
mMonitor.removeNotification(sbn1);
+ assertNotificationTimeoutTriggered();
+ verify(mActivityManagerInternal, times(0))
+ .stopForegroundServiceDelegate(any(ServiceConnection.class));
// once all calls are removed, verify FGS is stopped
mMonitor.onCallRemoved(call2);
mMonitor.removeNotification(sbn2);
- verify(mActivityManagerInternal, timeout(TIMEOUT).times(1))
+ verify(mActivityManagerInternal, times(1))
.stopForegroundServiceDelegate(any(ServiceConnection.class));
}
@@ -245,9 +264,10 @@
*/
@SmallTest
@Test
+ @Ignore("b/383403913") // when b/383403913 is fixed, remove the @Ignore
public void testStopFgsIfCallNotificationIsRemoved_PostedAfterFgsIsGained() {
// GIVEN
- StatusBarNotification sbn = createStatusBarNotificationFromHandle(mHandle1User1);
+ StatusBarNotification sbn = createStatusBarNotificationFromHandle(mHandle1User1, 1);
// WHEN
// FGS is gained after the call is added to VoipCallMonitor
@@ -259,7 +279,65 @@
// shortly after posting the notification, simulate the user dismissing it
mMonitor.removeNotification(sbn);
// FGS should be removed once the notification is removed
- verify(mActivityManagerInternal, timeout(TIMEOUT)).stopForegroundServiceDelegate(c);
+ assertNotificationTimeoutTriggered();
+ verify(mActivityManagerInternal, times(1)).stopForegroundServiceDelegate(c);
+ }
+
+
+ /**
+ * Tests the behavior of foreground service (FGS) delegation for a VoIP app during a scenario
+ * with two consecutive calls. In this scenario, the first call is disconnected shortly after
+ * being created but the second call continues. The apps foreground service should be
+ * maintained.
+ *
+ * GIVEN: Two calls (call1 and call2) are created for the same VoIP app.
+ * WHEN:
+ * - call1 is added, starting the FGS.
+ * - call2 is added immediately after.
+ * - call1 is removed.
+ * - call1 notification is finally posted (late)
+ * - call1 notification is removed shortly after since the call was disconnected
+ * THEN:
+ * - Verifies that the FGS is NOT stopped while call2 is still active.
+ * - Verifies that the FGS IS stopped after call2 is removed and its notification is gone.
+ */
+ @SmallTest
+ @Test
+ public void test2CallsInQuickSuccession() {
+ // GIVEN - 2 consecutive calls for a single Voip app
+ Call call1 = createTestCall("testCall1", mHandle1User1);
+ StatusBarNotification sbn1 = createStatusBarNotificationFromHandle(mHandle1User1, 1);
+ Call call2 = createTestCall("testCall2", mHandle1User1);
+ StatusBarNotification sbn2 = createStatusBarNotificationFromHandle(mHandle1User1, 2);
+
+ // WHEN - add the calls to the VoipCallMonitor class
+ addCallAndVerifyFgsIsGained(call1);
+ mMonitor.onCallAdded(call2);
+ assertTrue(mMonitor.getNewCallsMissingCallStyleNotificationQueue().contains(call1));
+ assertTrue(mMonitor.getNewCallsMissingCallStyleNotificationQueue().contains(call2));
+ // -- mock the app disconnecting the first
+ mMonitor.onCallRemoved(call1);
+ // Shortly after, simulate the notification updates coming in to the class
+ // -- post and remove the first call-style notification
+ mMonitor.postNotification(sbn1);
+ assertFalse(mMonitor.getNewCallsMissingCallStyleNotificationQueue().contains(call1));
+ mMonitor.removeNotification(sbn1);
+ assertNotificationTimeoutTriggered();
+
+ // -- keep the second notification up since the call will continue
+ mMonitor.postNotification(sbn2);
+ assertFalse(mMonitor.getNewCallsMissingCallStyleNotificationQueue().contains(call2));
+
+ // THEN - assert FGS is maintained for the process since there is still an ongoing call
+ assertNotificationTimeoutTriggered();
+ verify(mActivityManagerInternal, times(0))
+ .stopForegroundServiceDelegate(any(ServiceConnection.class));
+
+ // once all calls are removed, verify FGS is stopped
+ mMonitor.onCallRemoved(call2);
+ mMonitor.removeNotification(sbn2);
+ verify(mActivityManagerInternal, timeout(TIMEOUT).times(1))
+ .stopForegroundServiceDelegate(any(ServiceConnection.class));
}
/**
@@ -291,9 +369,10 @@
.build();
}
- private StatusBarNotification createStatusBarNotificationFromHandle(PhoneAccountHandle handle) {
+ private StatusBarNotification createStatusBarNotificationFromHandle(
+ PhoneAccountHandle handle, int id) {
return new StatusBarNotification(
- handle.getComponentName().getPackageName(), "", 0, "", 0, 0,
+ handle.getComponentName().getPackageName(), "", id, "", 0, 0,
createCallStyleNotification(), handle.getUserHandle(), "", 0);
}
@@ -314,4 +393,17 @@
mServiceConnection);
return serviceConnection;
}
+
+ /**
+ * Verifies that a delayed runnable is posted to the handler to handle the notification timeout.
+ * This also executes the captured runnable to simulate the timeout occurring.
+ */
+ private void assertNotificationTimeoutTriggered() {
+ ArgumentCaptor<Runnable> runnableCaptor = ArgumentCaptor.forClass(Runnable.class);
+ verify(mHandler, atLeastOnce()).postDelayed(
+ runnableCaptor.capture(),
+ eq(VoipCallMonitor.NOTIFICATION_NOT_POSTED_IN_TIME_TIMEOUT));
+ Runnable capturedRunnable = runnableCaptor.getValue();
+ capturedRunnable.run();
+ }
}