Ensure the associated call count is accurately tracked for RCS.
This CL ensures that the associatedCallCount of both Connection Services and Remote Connection Services is accurately being tracked. Specifically this CL addresses the following 2 scenarios: (1) when the user merges 2 Fi calls into a conference and (2) when the user disconnects an active TMO call while there is a Fi call on hold. This is primarily accomplished by ensuring that both the primary CS associatedCallCount and the RCS associatedCallCount are incremented when a Fi (or other call dependent on an RCS) call is created.
Bug: 286154316
Test: manual
Change-Id: I05104e89a7620165a6c00e2d276509c055b6324d
diff --git a/flags/Android.bp b/flags/Android.bp
index 6fa147a..4af90ad 100644
--- a/flags/Android.bp
+++ b/flags/Android.bp
@@ -35,7 +35,8 @@
"telecom_calllog_flags.aconfig",
"telecom_resolve_hidden_dependencies.aconfig",
"telecom_bluetoothroutemanager_flags.aconfig",
- "telecom_work_profile_flags.aconfig"
+ "telecom_work_profile_flags.aconfig",
+ "telecom_connection_service_wrapper_flags.aconfig",
],
}
diff --git a/flags/telecom_connection_service_wrapper_flags.aconfig b/flags/telecom_connection_service_wrapper_flags.aconfig
new file mode 100644
index 0000000..5f46c27
--- /dev/null
+++ b/flags/telecom_connection_service_wrapper_flags.aconfig
@@ -0,0 +1,8 @@
+package: "com.android.server.telecom.flags"
+
+flag {
+ name: "updated_rcs_call_count_tracking"
+ namespace: "telecom"
+ description: "Ensure that the associatedCallCount of CS and RCS is accurately being tracked."
+ bug: "286154316"
+}
\ No newline at end of file
diff --git a/src/com/android/server/telecom/Call.java b/src/com/android/server/telecom/Call.java
index 59399b3..d584d62 100644
--- a/src/com/android/server/telecom/Call.java
+++ b/src/com/android/server/telecom/Call.java
@@ -73,6 +73,7 @@
import com.android.internal.telecom.IVideoProvider;
import com.android.internal.util.Preconditions;
import com.android.server.telecom.flags.FeatureFlags;
+import com.android.server.telecom.flags.Flags;
import com.android.server.telecom.stats.CallFailureCause;
import com.android.server.telecom.stats.CallStateChangedAtomWriter;
import com.android.server.telecom.ui.ToastFactory;
@@ -436,6 +437,13 @@
private int mCallerDisplayNamePresentation;
/**
+ * The remote connection service which is attempted or already connecting this call. This is set
+ * to a non-null value only when a connection manager phone account is in use. When set, this
+ * will correspond to the target phone account of the {@link Call}.
+ */
+ private ConnectionServiceWrapper mRemoteConnectionService;
+
+ /**
* The connection service which is attempted or already connecting this call.
*/
private ConnectionServiceWrapper mConnectionService;
@@ -2361,11 +2369,25 @@
@VisibleForTesting
public void setConnectionService(ConnectionServiceWrapper service) {
+ setConnectionService(service, null);
+ }
+
+ @VisibleForTesting
+ public void setConnectionService(
+ ConnectionServiceWrapper service,
+ ConnectionServiceWrapper remoteService
+ ) {
Preconditions.checkNotNull(service);
clearConnectionService();
service.incrementAssociatedCallCount();
+
+ if (mFlags.updatedRcsCallCountTracking() && remoteService != null) {
+ remoteService.incrementAssociatedCallCount();
+ mRemoteConnectionService = remoteService;
+ }
+
mConnectionService = service;
mAnalytics.setCallConnectionService(service.getComponentName().flattenToShortString());
mConnectionService.addCall(this);
@@ -2373,10 +2395,12 @@
/**
* Perform an in-place replacement of the {@link ConnectionServiceWrapper} for this Call.
- * Removes the call from its former {@link ConnectionServiceWrapper}, ensuring that the
- * ConnectionService is NOT unbound if the call count hits zero.
- * This is used by the {@link ConnectionServiceWrapper} when handling {@link Connection} and
- * {@link Conference} additions via a ConnectionManager.
+ * Removes the call from its former {@link ConnectionServiceWrapper}, while still ensuring the
+ * former {@link ConnectionServiceWrapper} is tracked as the mRemoteConnectionService for this
+ * call so that the associatedCallCount of that {@link ConnectionServiceWrapper} is accurately
+ * tracked until it is supposed to be unbound.
+ * This method is used by the {@link ConnectionServiceWrapper} when handling {@link Connection}
+ * and {@link Conference} additions via a ConnectionManager.
* The original {@link android.telecom.ConnectionService} will directly add external calls and
* conferences to Telecom as well as the ConnectionManager, which will add to Telecom. In these
* cases since its first added to via the original CS, we want to change the CS responsible for
@@ -2389,9 +2413,18 @@
if (mConnectionService != null) {
ConnectionServiceWrapper serviceTemp = mConnectionService;
+
+ if (mFlags.updatedRcsCallCountTracking()) {
+ // Continue to track the former CS for this call so that it doesn't unbind early:
+ mRemoteConnectionService = serviceTemp;
+ }
+
mConnectionService = null;
serviceTemp.removeCall(this);
- serviceTemp.decrementAssociatedCallCount(true /*isSuppressingUnbind*/);
+
+ if (!mFlags.updatedRcsCallCountTracking()) {
+ serviceTemp.decrementAssociatedCallCount(true /*isSuppressingUnbind*/);
+ }
}
service.incrementAssociatedCallCount();
@@ -2405,6 +2438,8 @@
void clearConnectionService() {
if (mConnectionService != null) {
ConnectionServiceWrapper serviceTemp = mConnectionService;
+ ConnectionServiceWrapper remoteServiceTemp = mRemoteConnectionService;
+ mRemoteConnectionService = null;
mConnectionService = null;
serviceTemp.removeCall(this);
@@ -2415,6 +2450,10 @@
// necessary, but cleaning up mConnectionService prior to triggering an unbind is good
// to do.
decrementAssociatedCallCount(serviceTemp);
+
+ if (mFlags.updatedRcsCallCountTracking() && remoteServiceTemp != null) {
+ decrementAssociatedCallCount(remoteServiceTemp);
+ }
}
}
@@ -2433,7 +2472,7 @@
return;
}
mCreateConnectionProcessor = new CreateConnectionProcessor(this, mRepository, this,
- phoneAccountRegistrar, mContext);
+ phoneAccountRegistrar, mContext, mFlags);
mCreateConnectionProcessor.process();
}
diff --git a/src/com/android/server/telecom/CallsManager.java b/src/com/android/server/telecom/CallsManager.java
index 9179faf..e1ba484 100755
--- a/src/com/android/server/telecom/CallsManager.java
+++ b/src/com/android/server/telecom/CallsManager.java
@@ -5847,7 +5847,8 @@
return;
}
ConnectionServiceWrapper service = mConnectionServiceRepository.getService(
- phoneAccountHandle.getComponentName(), phoneAccountHandle.getUserHandle());
+ phoneAccountHandle.getComponentName(), phoneAccountHandle.getUserHandle(),
+ mFeatureFlags);
if (service == null) {
Log.i(this, "Found no connection service.");
return;
@@ -5872,7 +5873,8 @@
return;
}
ConnectionServiceWrapper service = mConnectionServiceRepository.getService(
- phoneAccountHandle.getComponentName(), phoneAccountHandle.getUserHandle());
+ phoneAccountHandle.getComponentName(), phoneAccountHandle.getUserHandle(),
+ mFeatureFlags);
if (service == null) {
Log.i(this, "Found no connection service.");
return;
diff --git a/src/com/android/server/telecom/ConnectionServiceRepository.java b/src/com/android/server/telecom/ConnectionServiceRepository.java
index 3991ed5..d6a78d0 100644
--- a/src/com/android/server/telecom/ConnectionServiceRepository.java
+++ b/src/com/android/server/telecom/ConnectionServiceRepository.java
@@ -23,6 +23,7 @@
import com.android.internal.annotations.VisibleForTesting;
import com.android.internal.util.IndentingPrintWriter;
+import com.android.server.telecom.flags.FeatureFlags;
import java.util.HashMap;
@@ -61,7 +62,10 @@
}
@VisibleForTesting
- public ConnectionServiceWrapper getService(ComponentName componentName, UserHandle userHandle) {
+ public ConnectionServiceWrapper getService(
+ ComponentName componentName,
+ UserHandle userHandle,
+ FeatureFlags featureFlags) {
Pair<ComponentName, UserHandle> cacheKey = Pair.create(componentName, userHandle);
ConnectionServiceWrapper service = mServiceCache.get(cacheKey);
if (service == null) {
@@ -72,7 +76,8 @@
mCallsManager,
mContext,
mLock,
- userHandle);
+ userHandle,
+ featureFlags);
service.addListener(mUnbindListener);
mServiceCache.put(cacheKey, service);
}
diff --git a/src/com/android/server/telecom/ConnectionServiceWrapper.java b/src/com/android/server/telecom/ConnectionServiceWrapper.java
index 07b048d..5936730 100644
--- a/src/com/android/server/telecom/ConnectionServiceWrapper.java
+++ b/src/com/android/server/telecom/ConnectionServiceWrapper.java
@@ -64,6 +64,7 @@
import com.android.internal.telecom.IVideoProvider;
import com.android.internal.telecom.RemoteServiceCallback;
import com.android.internal.util.Preconditions;
+import com.android.server.telecom.flags.FeatureFlags;
import com.android.server.telecom.flags.Flags;
import java.util.ArrayList;
@@ -1353,6 +1354,7 @@
private final CallsManager mCallsManager;
private final AppOpsManager mAppOpsManager;
private final Context mContext;
+ private final FeatureFlags mFlags;
private ConnectionServiceFocusManager.ConnectionServiceFocusListener mConnSvrFocusListener;
@@ -1374,8 +1376,10 @@
CallsManager callsManager,
Context context,
TelecomSystem.SyncRoot lock,
- UserHandle userHandle) {
- super(ConnectionService.SERVICE_INTERFACE, componentName, context, lock, userHandle);
+ UserHandle userHandle,
+ FeatureFlags featureFlags) {
+ super(ConnectionService.SERVICE_INTERFACE, componentName, context, lock, userHandle,
+ featureFlags);
mConnectionServiceRepository = connectionServiceRepository;
phoneAccountRegistrar.addListener(new PhoneAccountRegistrar.Listener() {
// TODO -- Upon changes to PhoneAccountRegistrar, need to re-wire connections
@@ -1385,6 +1389,7 @@
mCallsManager = callsManager;
mAppOpsManager = (AppOpsManager) context.getSystemService(Context.APP_OPS_SERVICE);
mContext = context;
+ mFlags = featureFlags;
}
/** See {@link IConnectionService#addConnectionServiceAdapter}. */
@@ -2573,7 +2578,7 @@
isCallerConnectionManager = true;
}
ConnectionServiceWrapper service = mConnectionServiceRepository.getService(
- handle.getComponentName(), handle.getUserHandle());
+ handle.getComponentName(), handle.getUserHandle(), mFlags);
if (service != null && service != this) {
simServices.add(service);
} else {
diff --git a/src/com/android/server/telecom/CreateConnectionProcessor.java b/src/com/android/server/telecom/CreateConnectionProcessor.java
index 19691c1..f5b257d 100644
--- a/src/com/android/server/telecom/CreateConnectionProcessor.java
+++ b/src/com/android/server/telecom/CreateConnectionProcessor.java
@@ -32,6 +32,8 @@
// TODO: Needed for move to system service: import com.android.internal.R;
import com.android.internal.annotations.VisibleForTesting;
+import com.android.server.telecom.flags.Flags;
+import com.android.server.telecom.flags.FeatureFlags;
import java.util.ArrayList;
import java.util.Collection;
@@ -125,6 +127,7 @@
private DisconnectCause mLastErrorDisconnectCause;
private final PhoneAccountRegistrar mPhoneAccountRegistrar;
private final Context mContext;
+ private final FeatureFlags mFlags;
private CreateConnectionTimeout mTimeout;
private ConnectionServiceWrapper mService;
private int mConnectionAttempt;
@@ -132,7 +135,8 @@
@VisibleForTesting
public CreateConnectionProcessor(
Call call, ConnectionServiceRepository repository, CreateConnectionResponse response,
- PhoneAccountRegistrar phoneAccountRegistrar, Context context) {
+ PhoneAccountRegistrar phoneAccountRegistrar, Context context,
+ FeatureFlags featureFlags) {
Log.v(this, "CreateConnectionProcessor created for Call = %s", call);
mCall = call;
mRepository = repository;
@@ -140,6 +144,7 @@
mPhoneAccountRegistrar = phoneAccountRegistrar;
mContext = context;
mConnectionAttempt = 0;
+ mFlags = featureFlags;
}
boolean isProcessingComplete() {
@@ -239,7 +244,7 @@
Log.i(this, "Trying attempt %s", attempt);
PhoneAccountHandle phoneAccount = attempt.connectionManagerPhoneAccount;
mService = mRepository.getService(phoneAccount.getComponentName(),
- phoneAccount.getUserHandle());
+ phoneAccount.getUserHandle(), mFlags);
if (mService == null) {
Log.i(this, "Found no connection service for attempt %s", attempt);
attemptNextPhoneAccount();
@@ -247,7 +252,25 @@
mConnectionAttempt++;
mCall.setConnectionManagerPhoneAccount(attempt.connectionManagerPhoneAccount);
mCall.setTargetPhoneAccount(attempt.targetPhoneAccount);
- mCall.setConnectionService(mService);
+ if (mFlags.updatedRcsCallCountTracking()) {
+ if (Objects.equals(attempt.connectionManagerPhoneAccount,
+ attempt.targetPhoneAccount)) {
+ mCall.setConnectionService(mService);
+ } else {
+ PhoneAccountHandle remotePhoneAccount = attempt.targetPhoneAccount;
+ ConnectionServiceWrapper mRemoteService =
+ mRepository.getService(remotePhoneAccount.getComponentName(),
+ remotePhoneAccount.getUserHandle(), mFlags);
+ if (mRemoteService == null) {
+ mCall.setConnectionService(mService);
+ } else {
+ Log.v(this, "attemptNextPhoneAccount Setting RCS = %s", mRemoteService);
+ mCall.setConnectionService(mService, mRemoteService);
+ }
+ }
+ } else {
+ mCall.setConnectionService(mService);
+ }
setTimeoutIfNeeded(mService, attempt);
if (mCall.isIncoming()) {
if (mCall.isAdhocConferenceCall()) {
diff --git a/src/com/android/server/telecom/ServiceBinder.java b/src/com/android/server/telecom/ServiceBinder.java
index 7274993..77f7b2e 100644
--- a/src/com/android/server/telecom/ServiceBinder.java
+++ b/src/com/android/server/telecom/ServiceBinder.java
@@ -29,6 +29,7 @@
import com.android.internal.annotations.VisibleForTesting;
import com.android.internal.util.Preconditions;
+import com.android.server.telecom.flags.FeatureFlags;
import java.util.Collections;
import java.util.Set;
@@ -240,6 +241,8 @@
* Abbreviated form of the package name from {@link #mComponentName}; used for session logging.
*/
protected final String mPackageAbbreviation;
+ private final FeatureFlags mFlags;
+
/** The set of callbacks waiting for notification of the binding's success or failure. */
private final Set<BindCallback> mCallbacks = new ArraySet<>();
@@ -282,7 +285,7 @@
* @param userHandle The {@link UserHandle} to use for binding.
*/
protected ServiceBinder(String serviceAction, ComponentName componentName, Context context,
- TelecomSystem.SyncRoot lock, UserHandle userHandle) {
+ TelecomSystem.SyncRoot lock, UserHandle userHandle, FeatureFlags featureFlags) {
Preconditions.checkState(!TextUtils.isEmpty(serviceAction));
Preconditions.checkNotNull(componentName);
@@ -292,6 +295,7 @@
mComponentName = componentName;
mPackageAbbreviation = Log.getPackageAbbreviation(componentName);
mUserHandle = userHandle;
+ mFlags = featureFlags;
}
final UserHandle getUserHandle() {
@@ -305,10 +309,16 @@
}
final void decrementAssociatedCallCount() {
- decrementAssociatedCallCount(false /*isSuppressingUnbind*/);
+ if (mFlags.updatedRcsCallCountTracking()) {
+ decrementAssociatedCallCountUpdated();
+ } else {
+ decrementAssociatedCallCount(false /*isSuppressingUnbind*/);
+ }
}
final void decrementAssociatedCallCount(boolean isSuppressingUnbind) {
+ // This is the legacy method - will be removed after the Flags.updatedRcsCallCountTracking
+ // mendel study completes.
if (mAssociatedCallCount > 0) {
mAssociatedCallCount--;
Log.v(this, "Call count decrement %d, %s", mAssociatedCallCount,
@@ -323,6 +333,21 @@
}
}
+ final void decrementAssociatedCallCountUpdated() {
+ if (mAssociatedCallCount > 0) {
+ mAssociatedCallCount--;
+ Log.i(this, "Call count decrement %d, %s", mAssociatedCallCount,
+ mComponentName.flattenToShortString());
+
+ if (mAssociatedCallCount == 0) {
+ unbind();
+ }
+ } else {
+ Log.wtf(this, "%s: ignoring a request to decrement mAssociatedCallCount below zero",
+ mComponentName.getClassName());
+ }
+ }
+
final int getAssociatedCallCount() {
return mAssociatedCallCount;
}
diff --git a/tests/src/com/android/server/telecom/tests/CallsManagerTest.java b/tests/src/com/android/server/telecom/tests/CallsManagerTest.java
index 649f435..6e0e660 100644
--- a/tests/src/com/android/server/telecom/tests/CallsManagerTest.java
+++ b/tests/src/com/android/server/telecom/tests/CallsManagerTest.java
@@ -2815,8 +2815,9 @@
public void testQueryCurrentLocationCheckOnReceiveResult() throws Exception {
ConnectionServiceWrapper service = new ConnectionServiceWrapper(
new ComponentName(mContext.getPackageName(),
- mContext.getPackageName().getClass().getName()),
- null, mPhoneAccountRegistrar, mCallsManager, mContext, mLock, null);
+ mContext.getPackageName().getClass().getName()), null,
+ mPhoneAccountRegistrar, mCallsManager, mContext, mLock, null,
+ mFeatureFlags);
CompletableFuture<String> resultFuture = new CompletableFuture<>();
try {
@@ -2841,7 +2842,8 @@
mSetRlagsRule.enableFlags(Flags.FLAG_UNBIND_TIMEOUT_CONNECTIONS);
ConnectionServiceWrapper service = new ConnectionServiceWrapper(
SIM_1_ACCOUNT.getAccountHandle().getComponentName(), null,
- mPhoneAccountRegistrar, mCallsManager, mContext, mLock, null);
+ mPhoneAccountRegistrar, mCallsManager, mContext, mLock, null,
+ mFeatureFlags);
TestScheduledExecutorService scheduledExecutorService = new TestScheduledExecutorService();
service.setScheduledExecutorService(scheduledExecutorService);
Call call = addSpyCall();
diff --git a/tests/src/com/android/server/telecom/tests/CreateConnectionProcessorTest.java b/tests/src/com/android/server/telecom/tests/CreateConnectionProcessorTest.java
index 8a85a87..c356b8f 100644
--- a/tests/src/com/android/server/telecom/tests/CreateConnectionProcessorTest.java
+++ b/tests/src/com/android/server/telecom/tests/CreateConnectionProcessorTest.java
@@ -38,6 +38,7 @@
import com.android.server.telecom.CreateConnectionProcessor;
import com.android.server.telecom.CreateConnectionResponse;
import com.android.server.telecom.PhoneAccountRegistrar;
+import com.android.server.telecom.flags.FeatureFlags;
import org.junit.After;
import org.junit.Before;
@@ -123,7 +124,7 @@
mTestCreateConnectionProcessor = new CreateConnectionProcessor(mMockCall,
mMockConnectionServiceRepository, mMockCreateConnectionResponse,
- mMockAccountRegistrar, mContext);
+ mMockAccountRegistrar, mContext, mFeatureFlags);
mAccountToSub = new HashMap<>();
phoneAccounts = new ArrayList<>();
@@ -842,7 +843,7 @@
ConnectionServiceWrapper wrapper = mock(ConnectionServiceWrapper.class);
when(mMockConnectionServiceRepository.getService(
eq(makeQuickConnectionServiceComponentName()),
- any(UserHandle.class))).thenReturn(wrapper);
+ any(UserHandle.class), any(FeatureFlags.class))).thenReturn(wrapper);
return wrapper;
}