isInSelfManagedCall for (package,user) should return true for calls being setup
Duo reported a regression that outgoing and incoming call notifications
became dismissiable. Upon investigation, this turned out to be a timing
issue where NotificationManagerService was querying Telecom to determine
if a self-managed call was added but Telecom had not yet added the call
to tracking.
The fix is to cover the time window where Telecom is first made aware of
the call in CallsManager until the call is offically added to
CallsManager. The time window is almost at a second which is a
significant amount of time.
Fixes: 282997779
Test: 4 new unit tests + manual testing:
(m1) Flashing Duo's latest apk and verifying INCOMING call
notifications are NON-dismissable
(m2) Flashing Duo's latest apk and verifying OUTGOING call
notifications are NON-dissmissable
(m3) Flashing Duo's latest apk and verifying ONGOING call
notifications are NON-dissmissable
(m4) Testing the above cases on Telecom Transactional VoIP
app to ensure the change is general to all VoIP apps
Change-Id: If0cbbec59410824e0c00e51d4db3de571dcd41f2
diff --git a/src/com/android/server/telecom/CallsManager.java b/src/com/android/server/telecom/CallsManager.java
index eb86ad0..4d2f273 100644
--- a/src/com/android/server/telecom/CallsManager.java
+++ b/src/com/android/server/telecom/CallsManager.java
@@ -353,6 +353,16 @@
new ConcurrentHashMap<Call, Boolean>(8, 0.9f, 1));
/**
+ * List of self-managed calls that have been initialized but not yet added to
+ * CallsManager#addCall(Call). There is a window of time when a Call has been added to Telecom
+ * (e.g. TelecomManager#addNewIncomingCall) to actually added in CallsManager#addCall(Call).
+ * This list is helpful for the NotificationManagerService to know that Telecom is currently
+ * setting up a call which is an important set in making notifications non-dismissible.
+ */
+ private final Set<Call> mSelfManagedCallsBeingSetup = Collections.newSetFromMap(
+ new ConcurrentHashMap<Call, Boolean>(8, 0.9f, 1));
+
+ /**
* A pending call is one which requires user-intervention in order to be placed.
* Used by {@link #startCallConfirmation}.
*/
@@ -1398,6 +1408,8 @@
// Required for backwards compatibility
handle = extras.getParcelable(TelephonyManager.EXTRA_INCOMING_NUMBER);
}
+ PhoneAccount phoneAccount = mPhoneAccountRegistrar.getPhoneAccountUnchecked(
+ phoneAccountHandle);
Call call = new Call(
generateNextCallId(extras),
mContext,
@@ -1414,6 +1426,15 @@
isConference, /* isConference */
mClockProxy,
mToastFactory);
+ // Ensure new calls related to self-managed calls/connections are set as such. This will
+ // be overridden when the actual connection is returned in startCreateConnection, however
+ // doing this now ensures the logs and any other logic will treat this call as self-managed
+ // from the moment it is created.
+ boolean isSelfManaged = phoneAccount != null && phoneAccount.isSelfManaged();
+ call.setIsSelfManaged(isSelfManaged);
+ // It's important to start tracking self-managed calls as soon as the Call object is
+ // initialized so NotificationManagerService is aware Telecom is setting up a call
+ if (isSelfManaged) mSelfManagedCallsBeingSetup.add(call);
// set properties for transactional call
if (extras.containsKey(TelecomManager.TRANSACTION_CALL_ID_KEY)) {
@@ -1434,15 +1455,8 @@
call.setAssociatedUser(phoneAccountHandle.getUserHandle());
}
- // Ensure new calls related to self-managed calls/connections are set as such. This will
- // be overridden when the actual connection is returned in startCreateConnection, however
- // doing this now ensures the logs and any other logic will treat this call as self-managed
- // from the moment it is created.
- PhoneAccount phoneAccount = mPhoneAccountRegistrar.getPhoneAccountUnchecked(
- phoneAccountHandle);
if (phoneAccount != null) {
Bundle phoneAccountExtras = phoneAccount.getExtras();
- call.setIsSelfManaged(phoneAccount.isSelfManaged());
if (call.isSelfManaged()) {
// Self managed calls will always be voip audio mode.
call.setIsVoipAudioMode(true);
@@ -1745,7 +1759,7 @@
isConference ? participants : null,
null /* gatewayInfo */,
null /* connectionManagerPhoneAccount */,
- null /* requestedAccountHandle */,
+ requestedAccountHandle /* targetPhoneAccountHandle */,
Call.CALL_DIRECTION_OUTGOING /* callDirection */,
false /* forceAttachToExistingConnection */,
isConference, /* isConference */
@@ -1766,7 +1780,6 @@
TelecomManager.PRESENTATION_ALLOWED);
}
}
- call.setTargetPhoneAccount(requestedAccountHandle);
}
call.initAnalytics(callingPackage, creationLogs.toString());
@@ -1805,6 +1818,9 @@
} else {
isReusedCall = true;
}
+ // It's important to start tracking self-managed calls as soon as the Call object is
+ // initialized so NotificationManagerService is aware Telecom is setting up a call
+ if (isSelfManaged) mSelfManagedCallsBeingSetup.add(call);
int videoState = VideoProfile.STATE_AUDIO_ONLY;
if (extras != null) {
@@ -4241,6 +4257,7 @@
Log.i(this, "addCall(%s)", call);
call.addListener(this);
mCalls.add(call);
+ mSelfManagedCallsBeingSetup.remove(call);
// Specifies the time telecom finished routing the call. This is used by the dialer for
// analytics.
@@ -4284,6 +4301,7 @@
mCalls.remove(call);
shouldNotify = true;
}
+ mSelfManagedCallsBeingSetup.remove(call);
call.destroy();
updateExternalCallCanPullSupport();
@@ -4541,8 +4559,10 @@
* @return {@code true} if the app has ongoing calls, or {@code false} otherwise.
*/
public boolean isInSelfManagedCall(String packageName, UserHandle userHandle) {
- return mCalls.stream().anyMatch(
- c -> c.isSelfManaged()
+ return mSelfManagedCallsBeingSetup.stream().anyMatch(c -> c.isSelfManaged()
+ && c.getTargetPhoneAccount().getComponentName().getPackageName().equals(packageName)
+ && c.getTargetPhoneAccount().getUserHandle().equals(userHandle)) ||
+ mCalls.stream().anyMatch(c -> c.isSelfManaged()
&& c.getTargetPhoneAccount().getComponentName().getPackageName().equals(packageName)
&& c.getTargetPhoneAccount().getUserHandle().equals(userHandle));
}
@@ -4734,11 +4754,14 @@
}
/**
- * Determines if there are any self-managed calls.
+ * Note: isInSelfManagedCall(packageName, UserHandle) should always be used in favor or this
+ * method. This method determines if there are any self-managed calls globally.
* @return {@code true} if there are self-managed calls, {@code false} otherwise.
*/
+ @VisibleForTesting
public boolean hasSelfManagedCalls() {
- return mCalls.stream().filter(call -> call.isSelfManaged()).count() > 0;
+ return mSelfManagedCallsBeingSetup.size() > 0 ||
+ mCalls.stream().filter(call -> call.isSelfManaged()).count() > 0;
}
/**
@@ -6493,4 +6516,14 @@
}
call.getTransactionServiceWrapper().stopCallStreaming(call);
}
+
+ @VisibleForTesting
+ public Set<Call> getSelfManagedCallsBeingSetup() {
+ return mSelfManagedCallsBeingSetup;
+ }
+
+ @VisibleForTesting
+ public void addCallBeingSetup(Call call) {
+ mSelfManagedCallsBeingSetup.add(call);
+ }
}
diff --git a/testapps/transactionalVoipApp/src/com/android/server/telecom/transactionalVoipApp/InCallActivity.java b/testapps/transactionalVoipApp/src/com/android/server/telecom/transactionalVoipApp/InCallActivity.java
index 707c325..50556a1 100644
--- a/testapps/transactionalVoipApp/src/com/android/server/telecom/transactionalVoipApp/InCallActivity.java
+++ b/testapps/transactionalVoipApp/src/com/android/server/telecom/transactionalVoipApp/InCallActivity.java
@@ -251,6 +251,7 @@
@Override
public void onResult(CallControl callControl) {
Log.i(TAG, "addCall: onResult: callback fired");
+ Utils.postIncomingCallStyleNotification(getApplicationContext());
mVoipCall.onAddCallControl(callControl);
updateCallId();
updateCurrentEndpoint();
@@ -275,7 +276,8 @@
mAudioRecord.stop();
try {
mAudioRecord.unregisterAudioRecordingCallback(mAudioRecordingCallback);
- } catch (IllegalArgumentException e) {
+ Utils.clearNotification(getApplicationContext());
+ } catch (Exception e) {
// pass through
}
}
diff --git a/testapps/transactionalVoipApp/src/com/android/server/telecom/transactionalVoipApp/Utils.java b/testapps/transactionalVoipApp/src/com/android/server/telecom/transactionalVoipApp/Utils.java
index 0de2b19..ec448b2 100644
--- a/testapps/transactionalVoipApp/src/com/android/server/telecom/transactionalVoipApp/Utils.java
+++ b/testapps/transactionalVoipApp/src/com/android/server/telecom/transactionalVoipApp/Utils.java
@@ -77,11 +77,16 @@
pendingAnswer, pendingReject)
)
.setFullScreenIntent(pendingAnswer, true)
+ .setOngoing(true)
.build();
return callStyleNotification;
}
+ public static void postIncomingCallStyleNotification(Context context) {
+ NotificationManager nm = context.getSystemService(NotificationManager.class);
+ nm.notify(Utils.CALL_NOTIFICATION_ID, createCallStyleNotification(context));
+ }
public static void updateCallStyleNotification_toOngoingCall(Context context) {
PendingIntent ongoingCall = PendingIntent.getActivity(context, 0,
@@ -97,6 +102,7 @@
ongoingCall)
)
.setFullScreenIntent(ongoingCall, true)
+ .setOngoing(true)
.build();
NotificationManager notificationManager =
@@ -105,6 +111,14 @@
notificationManager.notify(CALL_NOTIFICATION_ID, callStyleNotification);
}
+ public static void clearNotification(Context context) {
+ NotificationManager notificationManager =
+ context.getSystemService(NotificationManager.class);
+ if (notificationManager != null) {
+ notificationManager.cancel(CALL_NOTIFICATION_ID);
+ }
+ }
+
public static MediaPlayer createMediaPlayer(Context context) {
int audioToPlay = (Math.random() > 0.5f) ?
com.android.server.telecom.transactionalVoipApp.R.raw.sample_audio :
diff --git a/testapps/transactionalVoipApp/src/com/android/server/telecom/transactionalVoipApp/VoipAppMainActivity.java b/testapps/transactionalVoipApp/src/com/android/server/telecom/transactionalVoipApp/VoipAppMainActivity.java
index 7578b9d..72a3906 100644
--- a/testapps/transactionalVoipApp/src/com/android/server/telecom/transactionalVoipApp/VoipAppMainActivity.java
+++ b/testapps/transactionalVoipApp/src/com/android/server/telecom/transactionalVoipApp/VoipAppMainActivity.java
@@ -99,8 +99,6 @@
}
private void startInCallActivity(int direction) {
- mNotificationManager.notify(Utils.CALL_NOTIFICATION_ID,
- Utils.createCallStyleNotification(getApplicationContext()));
Bundle extras = new Bundle();
extras.putInt(Utils.sCALL_DIRECTION_KEY, direction);
Intent intent = new Intent(getApplicationContext(), InCallActivity.class);
@@ -142,6 +140,7 @@
protected void onDestroy() {
Log.i(TAG, ACT_STATE_TAG + " onDestroy: is called before the activity is"
+ " destroyed. ");
+ Utils.clearNotification(getApplicationContext());
super.onDestroy();
}
}
diff --git a/tests/src/com/android/server/telecom/tests/CallsManagerTest.java b/tests/src/com/android/server/telecom/tests/CallsManagerTest.java
index 7f252bc..c42a2ca 100644
--- a/tests/src/com/android/server/telecom/tests/CallsManagerTest.java
+++ b/tests/src/com/android/server/telecom/tests/CallsManagerTest.java
@@ -62,7 +62,6 @@
import android.os.UserHandle;
import android.os.UserManager;
import android.provider.BlockedNumberContract;
-import android.provider.Telephony;
import android.telecom.CallException;
import android.telecom.CallScreeningService;
import android.telecom.CallerInfo;
@@ -156,6 +155,8 @@
private static final int TEST_TIMEOUT = 5000; // milliseconds
private static final long STATE_TIMEOUT = 5000L;
private static final int SECONDARY_USER_ID = 12;
+ private static final UserHandle TEST_USER_HANDLE = UserHandle.of(123);
+ private static final String TEST_PACKAGE_NAME = "GoogleMeet";
private static final PhoneAccountHandle SIM_1_HANDLE = new PhoneAccountHandle(
ComponentName.unflattenFromString("com.foo/.Blah"), "Sim1");
private static final PhoneAccountHandle SIM_1_HANDLE_SECONDARY = new PhoneAccountHandle(
@@ -173,6 +174,8 @@
ComponentName.unflattenFromString("com.baz/.Self"), "Self");
private static final PhoneAccountHandle SELF_MANAGED_2_HANDLE = new PhoneAccountHandle(
ComponentName.unflattenFromString("com.baz/.Self2"), "Self2");
+ private static final PhoneAccountHandle SELF_MANAGED_W_CUSTOM_HANDLE = new PhoneAccountHandle(
+ new ComponentName(TEST_PACKAGE_NAME, "class"), "1", TEST_USER_HANDLE);
private static final PhoneAccount SIM_1_ACCOUNT = new PhoneAccount.Builder(SIM_1_HANDLE, "Sim1")
.setCapabilities(PhoneAccount.CAPABILITY_SIM_SUBSCRIPTION
| PhoneAccount.CAPABILITY_CALL_PROVIDER
@@ -202,6 +205,11 @@
.setCapabilities(PhoneAccount.CAPABILITY_SELF_MANAGED)
.setIsEnabled(true)
.build();
+ private static final PhoneAccount SM_W_DIFFERENT_PACKAGE_AND_USER = new PhoneAccount.Builder(
+ SELF_MANAGED_W_CUSTOM_HANDLE, "Self")
+ .setCapabilities(PhoneAccount.CAPABILITY_SELF_MANAGED)
+ .setIsEnabled(true)
+ .build();
private static final Uri TEST_ADDRESS = Uri.parse("tel:555-1212");
private static final Uri TEST_ADDRESS2 = Uri.parse("tel:555-1213");
private static final Uri TEST_ADDRESS3 = Uri.parse("tel:555-1214");
@@ -3078,6 +3086,109 @@
eq(false));
}
+ /**
+ * Verify CallsManager#isInSelfManagedCall(packageName, userHandle) returns true when
+ * CallsManager is first made aware of the incoming call in processIncomingCallIntent.
+ */
+ @SmallTest
+ @Test
+ public void testAddNewIncomingCall_IsInSelfManagedCall() {
+ // GIVEN
+ assertEquals(0, mCallsManager.getSelfManagedCallsBeingSetup().size());
+ assertFalse(mCallsManager.isInSelfManagedCall(TEST_PACKAGE_NAME, TEST_USER_HANDLE));
+
+ // WHEN
+ when(mPhoneAccountRegistrar.getPhoneAccountUnchecked(any()))
+ .thenReturn(SM_W_DIFFERENT_PACKAGE_AND_USER);
+
+ // THEN
+ mCallsManager.processIncomingCallIntent(SELF_MANAGED_W_CUSTOM_HANDLE, new Bundle(), false);
+
+ assertEquals(1, mCallsManager.getSelfManagedCallsBeingSetup().size());
+ assertTrue(mCallsManager.isInSelfManagedCall(TEST_PACKAGE_NAME, TEST_USER_HANDLE));
+ assertEquals(0, mCallsManager.getCalls().size());
+ }
+
+ /**
+ * Verify CallsManager#isInSelfManagedCall(packageName, userHandle) returns true when
+ * CallsManager is first made aware of the outgoing call in StartOutgoingCall.
+ */
+ @SmallTest
+ @Test
+ public void testStartOutgoing_IsInSelfManagedCall() {
+ // GIVEN
+ assertEquals(0, mCallsManager.getSelfManagedCallsBeingSetup().size());
+ assertFalse(mCallsManager.isInSelfManagedCall(TEST_PACKAGE_NAME, TEST_USER_HANDLE));
+
+ // WHEN
+ when(mPhoneAccountRegistrar.getPhoneAccount(any(), any()))
+ .thenReturn(SM_W_DIFFERENT_PACKAGE_AND_USER);
+ // Ensure contact info lookup succeeds
+ doAnswer(invocation -> {
+ Uri handle = invocation.getArgument(0);
+ CallerInfo info = new CallerInfo();
+ CompletableFuture<Pair<Uri, CallerInfo>> callerInfoFuture = new CompletableFuture<>();
+ callerInfoFuture.complete(new Pair<>(handle, info));
+ return callerInfoFuture;
+ }).when(mCallerInfoLookupHelper).startLookup(any(Uri.class));
+ // Ensure we have candidate phone account handle info.
+ when(mPhoneAccountRegistrar.getOutgoingPhoneAccountForScheme(any(), any())).thenReturn(
+ SELF_MANAGED_W_CUSTOM_HANDLE);
+ when(mPhoneAccountRegistrar.getCallCapablePhoneAccounts(any(), anyBoolean(),
+ any(), anyInt(), anyInt(), anyBoolean())).thenReturn(
+ new ArrayList<>(List.of(SELF_MANAGED_W_CUSTOM_HANDLE)));
+
+ // THEN
+ mCallsManager.startOutgoingCall(TEST_ADDRESS, SELF_MANAGED_W_CUSTOM_HANDLE, new Bundle(),
+ TEST_USER_HANDLE, new Intent(), TEST_PACKAGE_NAME);
+
+ assertEquals(1, mCallsManager.getSelfManagedCallsBeingSetup().size());
+ assertTrue(mCallsManager.isInSelfManagedCall(TEST_PACKAGE_NAME, TEST_USER_HANDLE));
+ assertEquals(0, mCallsManager.getCalls().size());
+ }
+
+ /**
+ * Verify SelfManagedCallsBeingSetup is being cleaned up in CallsManager#addCall and
+ * CallsManager#removeCall. This ensures no memory leaks.
+ */
+ @SmallTest
+ @Test
+ public void testCallsBeingSetupCleanup() {
+ Call spyCall = addSpyCall();
+ assertEquals(0, mCallsManager.getSelfManagedCallsBeingSetup().size());
+ // verify CallsManager#removeCall removes the call from SelfManagedCallsBeingSetup
+ mCallsManager.addCallBeingSetup(spyCall);
+ mCallsManager.removeCall(spyCall);
+ assertEquals(0, mCallsManager.getSelfManagedCallsBeingSetup().size());
+ // verify CallsManager#addCall removes the call from SelfManagedCallsBeingSetup
+ mCallsManager.addCallBeingSetup(spyCall);
+ mCallsManager.addCall(spyCall);
+ assertEquals(0, mCallsManager.getSelfManagedCallsBeingSetup().size());
+ }
+
+ /**
+ * Verify isInSelfManagedCall returns false if there is a self-managed call, but it is for a
+ * different package and user
+ */
+ @SmallTest
+ @Test
+ public void testIsInSelfManagedCall_PackageUserQueryIsWorkingAsIntended() {
+ // start an active call
+ Call randomCall = createSpyCall(SELF_MANAGED_HANDLE, CallState.ACTIVE);
+ mCallsManager.addCallBeingSetup(randomCall);
+ assertEquals(1, mCallsManager.getSelfManagedCallsBeingSetup().size());
+ // query isInSelfManagedCall for a package that is NOT in a call; expect false
+ assertFalse(mCallsManager.isInSelfManagedCall(TEST_PACKAGE_NAME, TEST_USER_HANDLE));
+ // start another call
+ Call targetCall = addSpyCall(SELF_MANAGED_W_CUSTOM_HANDLE, CallState.DIALING);
+ when(targetCall.getTargetPhoneAccount()).thenReturn(SELF_MANAGED_W_CUSTOM_HANDLE);
+ when(targetCall.isSelfManaged()).thenReturn(true);
+ mCallsManager.addCallBeingSetup(targetCall);
+ // query isInSelfManagedCall for a package that is in a call
+ assertTrue(mCallsManager.isInSelfManagedCall(TEST_PACKAGE_NAME, TEST_USER_HANDLE));
+ }
+
+
private Call addSpyCall() {
return addSpyCall(SIM_2_HANDLE, CallState.ACTIVE);
}