DO NOT MERGE
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
(cherry picked from commit 45c65d16d0b178744372a62134fe5ba0c4487cdd)
diff --git a/src/com/android/server/telecom/Call.java b/src/com/android/server/telecom/Call.java
index dd8e7e8..16f7400 100644
--- a/src/com/android/server/telecom/Call.java
+++ b/src/com/android/server/telecom/Call.java
@@ -406,6 +406,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;
@@ -2313,11 +2320,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 (remoteService != null) {
+ remoteService.incrementAssociatedCallCount();
+ mRemoteConnectionService = remoteService;
+ }
+
mConnectionService = service;
mAnalytics.setCallConnectionService(service.getComponentName().flattenToShortString());
mConnectionService.addCall(this);
@@ -2325,10 +2346,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
@@ -2341,9 +2364,12 @@
if (mConnectionService != null) {
ConnectionServiceWrapper serviceTemp = mConnectionService;
+
+ // 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*/);
}
service.incrementAssociatedCallCount();
@@ -2357,6 +2383,8 @@
void clearConnectionService() {
if (mConnectionService != null) {
ConnectionServiceWrapper serviceTemp = mConnectionService;
+ ConnectionServiceWrapper remoteServiceTemp = mRemoteConnectionService;
+ mRemoteConnectionService = null;
mConnectionService = null;
serviceTemp.removeCall(this);
@@ -2367,6 +2395,10 @@
// necessary, but cleaning up mConnectionService prior to triggering an unbind is good
// to do.
decrementAssociatedCallCount(serviceTemp);
+
+ if (remoteServiceTemp != null) {
+ decrementAssociatedCallCount(remoteServiceTemp);
+ }
}
}
diff --git a/src/com/android/server/telecom/CreateConnectionProcessor.java b/src/com/android/server/telecom/CreateConnectionProcessor.java
index 19691c1..dea070c 100644
--- a/src/com/android/server/telecom/CreateConnectionProcessor.java
+++ b/src/com/android/server/telecom/CreateConnectionProcessor.java
@@ -247,7 +247,21 @@
mConnectionAttempt++;
mCall.setConnectionManagerPhoneAccount(attempt.connectionManagerPhoneAccount);
mCall.setTargetPhoneAccount(attempt.targetPhoneAccount);
- mCall.setConnectionService(mService);
+ if (Objects.equals(attempt.connectionManagerPhoneAccount,
+ attempt.targetPhoneAccount)) {
+ mCall.setConnectionService(mService);
+ } else {
+ PhoneAccountHandle remotePhoneAccount = attempt.targetPhoneAccount;
+ ConnectionServiceWrapper mRemoteService =
+ mRepository.getService(remotePhoneAccount.getComponentName(),
+ remotePhoneAccount.getUserHandle());
+ if (mRemoteService == null) {
+ mCall.setConnectionService(mService);
+ } else {
+ Log.v(this, "attemptNextPhoneAccount Setting RCS = %s", mRemoteService);
+ mCall.setConnectionService(mService, mRemoteService);
+ }
+ }
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..bf3b488 100644
--- a/src/com/android/server/telecom/ServiceBinder.java
+++ b/src/com/android/server/telecom/ServiceBinder.java
@@ -305,16 +305,16 @@
}
final void decrementAssociatedCallCount() {
- decrementAssociatedCallCount(false /*isSuppressingUnbind*/);
+ decrementAssociatedCallCountUpdated();
}
- final void decrementAssociatedCallCount(boolean isSuppressingUnbind) {
+ final void decrementAssociatedCallCountUpdated() {
if (mAssociatedCallCount > 0) {
mAssociatedCallCount--;
- Log.v(this, "Call count decrement %d, %s", mAssociatedCallCount,
+ Log.i(this, "Call count decrement %d, %s", mAssociatedCallCount,
mComponentName.flattenToShortString());
- if (!isSuppressingUnbind && mAssociatedCallCount == 0) {
+ if (mAssociatedCallCount == 0) {
unbind();
}
} else {
diff --git a/tests/src/com/android/server/telecom/tests/CreateConnectionProcessorTest.java b/tests/src/com/android/server/telecom/tests/CreateConnectionProcessorTest.java
index 8a85a87..0b30656 100644
--- a/tests/src/com/android/server/telecom/tests/CreateConnectionProcessorTest.java
+++ b/tests/src/com/android/server/telecom/tests/CreateConnectionProcessorTest.java
@@ -205,7 +205,12 @@
// Include a Connection Manager
PhoneAccountHandle callManagerPAHandle = getNewConnectionMangerHandleForCall(mMockCall,
"cm_acct");
- ConnectionServiceWrapper service = makeConnectionServiceWrapper();
+
+ // Need a separate CSW for the connection mgr and the target phone acct.
+ ConnectionServiceWrapper targetCsw = configureConnectionServiceWrapper(pAHandle);
+ ConnectionServiceWrapper connectionMgrCsw = configureConnectionServiceWrapper(
+ callManagerPAHandle);
+
// Make sure the target phone account has the correct permissions
PhoneAccount mFakeTargetPhoneAccount = makeQuickAccount("cm_acct",
PhoneAccount.CAPABILITY_SIM_SUBSCRIPTION, null);
@@ -216,8 +221,13 @@
verify(mMockCall).setConnectionManagerPhoneAccount(eq(callManagerPAHandle));
verify(mMockCall).setTargetPhoneAccount(eq(pAHandle));
- verify(mMockCall).setConnectionService(eq(service));
- verify(service).createConnection(eq(mMockCall),
+ // TODO: This test requires refactoring; it should be targetCsw for the remote CS.
+ // However, this test uses phone accounts from all the same component meaning that there
+ // is no distinction between the target and connection mgr service. Ideally they should use
+ // different packages.
+ verify(mMockCall).setConnectionService(eq(connectionMgrCsw) /* primary cs */,
+ eq(connectionMgrCsw) /* remote CS */);
+ verify(connectionMgrCsw).createConnection(eq(mMockCall),
any(CreateConnectionResponse.class));
// Notify successful connection to call
CallIdMapper mockCallIdMapper = mock(CallIdMapper.class);
@@ -667,13 +677,22 @@
// Include a connection Manager for the user with the capability to make calls
PhoneAccount emerCallManagerPA = getNewEmergencyConnectionManagerPhoneAccount("cm_acct",
PhoneAccount.CAPABILITY_PLACE_EMERGENCY_CALLS);
- ConnectionServiceWrapper service = makeConnectionServiceWrapper();
+
+ ConnectionServiceWrapper targetCsw =
+ configureConnectionServiceWrapper(regularAccount.getAccountHandle());
+ ConnectionServiceWrapper connectionMgrCsw =
+ configureConnectionServiceWrapper(callManagerPA.getAccountHandle());
+ ConnectionServiceWrapper emergencyConnectionMgrCsw =
+ configureConnectionServiceWrapper(emerCallManagerPA.getAccountHandle());
+
PhoneAccount emergencyPhoneAccount = makeEmergencyPhoneAccount("tel_emer", 0, null);
phoneAccounts.add(emergencyPhoneAccount);
mapToSubSlot(regularAccount, 2 /*subId*/, 1 /*slotId*/);
mTestCreateConnectionProcessor.process();
reset(mMockCall);
- reset(service);
+ reset(targetCsw);
+ reset(connectionMgrCsw);
+ reset(emergencyConnectionMgrCsw);
when(mMockCall.getConnectionServiceFocusManager()).thenReturn(
mConnectionServiceFocusManager);
@@ -686,8 +705,11 @@
verify(mMockCall).setConnectionManagerPhoneAccount(
eq(emerCallManagerPA.getAccountHandle()));
verify(mMockCall).setTargetPhoneAccount(eq(regularAccount.getAccountHandle()));
- verify(mMockCall).setConnectionService(eq(service));
- verify(service).createConnection(eq(mMockCall), any(CreateConnectionResponse.class));
+ // Fallback was to the emergency connection mgr, so that CSW should have been set.
+ verify(mMockCall).setConnectionService(eq(emergencyConnectionMgrCsw) /* primary */,
+ eq(emergencyConnectionMgrCsw) /* remote (ie original target) */);
+ verify(emergencyConnectionMgrCsw).createConnection(eq(mMockCall),
+ any(CreateConnectionResponse.class));
}
/**
@@ -868,5 +890,18 @@
.setShortDescription("desc" + idx)
.setIsEnabled(true)
.build();
+ }
+
+ /**
+ * Configures a mock ConnectionServiceWrapper for the passed in phone account handle.
+ * @param account The phone account handle to use.
+ * @return The configured mock.
+ */
+ private ConnectionServiceWrapper configureConnectionServiceWrapper(PhoneAccountHandle account) {
+ ConnectionServiceWrapper wrapper = mock(ConnectionServiceWrapper.class);
+ when(mMockConnectionServiceRepository.getService(
+ eq(account.getComponentName()),
+ any(UserHandle.class))).thenReturn(wrapper);
+ return wrapper;
}
}
\ No newline at end of file