Merge "Fix holding behavior across self-manged ConnectionServices." into udc-dev am: 390a9d136f am: 8ca48d5b87
Original change: https://googleplex-android-review.googlesource.com/c/platform/packages/services/Telecomm/+/23036398
Change-Id: I87533112b30f564f4d21a6bed9769915eaa94e3b
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
diff --git a/src/com/android/server/telecom/CallsManager.java b/src/com/android/server/telecom/CallsManager.java
index b9f8b40..d457cc8 100644
--- a/src/com/android/server/telecom/CallsManager.java
+++ b/src/com/android/server/telecom/CallsManager.java
@@ -3430,10 +3430,11 @@
*/
boolean holdActiveCallForNewCall(Call call) {
Call activeCall = (Call) mConnectionSvrFocusMgr.getCurrentFocusCall();
- Log.i(this, "holdActiveCallForNewCall, newCall: %s, activeCall: %s", call, activeCall);
+ Log.i(this, "holdActiveCallForNewCall, newCall: %s, activeCall: %s", call.getId(),
+ (activeCall == null ? "<none>" : activeCall.getId()));
if (activeCall != null && activeCall != call) {
if (canHold(activeCall)) {
- activeCall.hold();
+ activeCall.hold("swap to " + call.getId());
return true;
} else if (supportsHold(activeCall)
&& areFromSameSource(activeCall, call)) {
@@ -4918,12 +4919,25 @@
liveCallPhoneAccount);
}
- // First thing, if we are trying to make a call with the same phone account as the live
- // call, then allow it so that the connection service can make its own decision about
- // how to handle the new call relative to the current one.
+ // First thing, for managed calls, if we are trying to make a call with the same phone
+ // account as the live call, then allow it so that the connection service can make its own
+ // decision about how to handle the new call relative to the current one.
+ // Note: This behavior is primarily in place because Telephony historically manages the
+ // state of the calls it tracks by itself, holding and unholding as needed. Self-managed
+ // calls, even though from the same package are normally held/unheld automatically by
+ // Telecom. Calls within a single ConnectionService get held/unheld automatically during
+ // "swap" operations by CallsManager#holdActiveCallForNewCall. There is, however, a quirk
+ // in that if an app declares TWO different ConnectionServices, holdActiveCallForNewCall
+ // would not work correctly because focus switches between ConnectionServices, yet we
+ // tended to assume that if the calls are from the same package that the hold/unhold should
+ // be done by the app. That was a bad assumption as it meant that we could have two active
+ // calls.
+ // TODO(b/280826075): We need to come back and revisit all this logic in a holistic manner.
if (PhoneAccountHandle.areFromSamePackage(liveCallPhoneAccount,
- call.getTargetPhoneAccount())) {
- Log.i(this, "makeRoomForOutgoingCall: phoneAccount matches.");
+ call.getTargetPhoneAccount())
+ && !call.isSelfManaged()
+ && !liveCall.isSelfManaged()) {
+ Log.i(this, "makeRoomForOutgoingCall: managed phoneAccount matches");
call.getAnalytics().setCallIsAdditional(true);
liveCall.getAnalytics().setCallIsInterrupted(true);
return true;
@@ -5503,6 +5517,13 @@
impl.dump(pw);
pw.decreaseIndent();
}
+
+ if (mConnectionSvrFocusMgr != null) {
+ pw.println("mConnectionSvrFocusMgr:");
+ pw.increaseIndent();
+ mConnectionSvrFocusMgr.dump(pw);
+ pw.decreaseIndent();
+ }
}
/**
diff --git a/src/com/android/server/telecom/ConnectionServiceFocusManager.java b/src/com/android/server/telecom/ConnectionServiceFocusManager.java
index 6fbc494..3694727 100644
--- a/src/com/android/server/telecom/ConnectionServiceFocusManager.java
+++ b/src/com/android/server/telecom/ConnectionServiceFocusManager.java
@@ -25,8 +25,10 @@
import android.telecom.Log;
import android.telecom.Logging.Session;
import android.text.TextUtils;
+import android.util.LocalLog;
import com.android.internal.annotations.VisibleForTesting;
+import com.android.internal.util.IndentingPrintWriter;
import java.util.ArrayList;
import java.util.List;
@@ -41,6 +43,7 @@
public class ConnectionServiceFocusManager {
private static final String TAG = "ConnectionSvrFocusMgr";
private static final int GET_CURRENT_FOCUS_TIMEOUT_MILLIS = 1000;
+ private final LocalLog mLocalLog = new LocalLog(20);
/** Factory interface used to create the {@link ConnectionServiceFocusManager} instance. */
public interface ConnectionServiceFocusManagerFactory {
@@ -124,6 +127,11 @@
* @return {@code True} if this call can receive focus, {@code false} otherwise.
*/
boolean isFocusable();
+
+ /**
+ * @return the ID of the focusable for debug purposes.
+ */
+ String getId();
}
/** Interface define a call back for focus request event. */
@@ -361,10 +369,11 @@
}
private void updateCurrentFocusCall() {
+ CallFocus previousFocus = mCurrentFocusCall;
mCurrentFocusCall = null;
if (mCurrentFocus == null) {
- Log.d(this, "updateCurrentFocusCall: mCurrentFocus is null");
+ Log.i(this, "updateCurrentFocusCall: mCurrentFocus is null");
return;
}
@@ -377,11 +386,16 @@
for (CallFocus call : calls) {
if (PRIORITY_FOCUS_CALL_STATE.contains(call.getState())) {
mCurrentFocusCall = call;
+ if (previousFocus != call) {
+ mLocalLog.log(call.getId());
+ }
Log.i(this, "updateCurrentFocusCall %s", mCurrentFocusCall);
return;
}
}
-
+ if (previousFocus != null) {
+ mLocalLog.log("<none>");
+ }
Log.i(this, "updateCurrentFocusCall = null");
}
@@ -477,6 +491,11 @@
}
}
+ public void dump(IndentingPrintWriter pw) {
+ pw.println("Call Focus History:");
+ mLocalLog.dump(pw);
+ }
+
private final class FocusManagerHandler extends Handler {
FocusManagerHandler(Looper looper) {
super(looper);
diff --git a/testapps/AndroidManifest.xml b/testapps/AndroidManifest.xml
index dd8258a..645a42b 100644
--- a/testapps/AndroidManifest.xml
+++ b/testapps/AndroidManifest.xml
@@ -259,6 +259,15 @@
</intent-filter>
</service>
+ <service android:name="com.android.server.telecom.testapps.OtherSelfManagedConnectionService"
+ android:permission="android.permission.BIND_TELECOM_CONNECTION_SERVICE"
+ android:process="com.android.server.telecom.testapps.SelfMangingCallingApp"
+ android:exported="true">
+ <intent-filter>
+ <action android:name="android.telecom.ConnectionService"/>
+ </intent-filter>
+ </service>
+
<receiver android:exported="false"
android:process="com.android.server.telecom.testapps.SelfMangingCallingApp"
android:name="com.android.server.telecom.testapps.SelfManagedCallNotificationReceiver"/>
diff --git a/testapps/res/layout/self_managed_sample_main.xml b/testapps/res/layout/self_managed_sample_main.xml
index d26d629..98b879a 100644
--- a/testapps/res/layout/self_managed_sample_main.xml
+++ b/testapps/res/layout/self_managed_sample_main.xml
@@ -55,6 +55,12 @@
android:layout_height="wrap_content"
android:background="@color/test_call_b_color"
android:text="2"/>
+ <RadioButton
+ android:id="@+id/useAcct3Button"
+ android:layout_width="wrap_content"
+ android:layout_height="wrap_content"
+ android:background="@color/test_call_c_color"
+ android:text="3"/>
</RadioGroup>
<TextView
android:id="@+id/hasFocus"
diff --git a/testapps/res/values/colors.xml b/testapps/res/values/colors.xml
index 3939e78..9447ac8 100644
--- a/testapps/res/values/colors.xml
+++ b/testapps/res/values/colors.xml
@@ -17,4 +17,5 @@
<resources>
<color name="test_call_a_color">#f2eebf</color>
<color name="test_call_b_color">#afc5e6</color>
+ <color name="test_call_c_color">#c5afe6</color>
</resources>
diff --git a/testapps/src/com/android/server/telecom/testapps/OtherSelfManagedConnectionService.java b/testapps/src/com/android/server/telecom/testapps/OtherSelfManagedConnectionService.java
new file mode 100644
index 0000000..7bb9830
--- /dev/null
+++ b/testapps/src/com/android/server/telecom/testapps/OtherSelfManagedConnectionService.java
@@ -0,0 +1,20 @@
+/*
+ * Copyright (C) 2023 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.server.telecom.testapps;
+
+public class OtherSelfManagedConnectionService extends SelfManagedConnectionService {
+}
diff --git a/testapps/src/com/android/server/telecom/testapps/SelfManagedCallList.java b/testapps/src/com/android/server/telecom/testapps/SelfManagedCallList.java
index d4661ff..273b060 100644
--- a/testapps/src/com/android/server/telecom/testapps/SelfManagedCallList.java
+++ b/testapps/src/com/android/server/telecom/testapps/SelfManagedCallList.java
@@ -46,20 +46,27 @@
public static String SELF_MANAGED_ACCOUNT_1 = "1";
public static String SELF_MANAGED_ACCOUNT_2 = "2";
+ public static String SELF_MANAGED_ACCOUNT_1A = "1A";
public static String SELF_MANAGED_ACCOUNT_3 = "3";
public static String SELF_MANAGED_NAME_1 = "SuperCall";
public static String SELF_MANAGED_NAME_2 = "Mega Call";
- public static String SELF_MANAGED_NAME_3 = "SM Call";
+ public static String SELF_MANAGED_NAME_1A = "SM Call";
+ public static String SELF_MANAGED_NAME_3 = "Sep Process";
public static String CUSTOM_URI_SCHEME = "custom";
private static SelfManagedCallList sInstance;
private static ComponentName COMPONENT_NAME = new ComponentName(
SelfManagedCallList.class.getPackage().getName(),
SelfManagedConnectionService.class.getName());
+ private static ComponentName OTHER_COMPONENT_NAME = new ComponentName(
+ SelfManagedCallList.class.getPackage().getName(),
+ OtherSelfManagedConnectionService.class.getName());
private static Uri SELF_MANAGED_ADDRESS_1 = Uri.fromParts(PhoneAccount.SCHEME_TEL, "555-1212",
"");
private static Uri SELF_MANAGED_ADDRESS_2 = Uri.fromParts(PhoneAccount.SCHEME_SIP,
"me@test.org", "");
+ private static Uri SELF_MANAGED_ADDRESS_3 = Uri.fromParts(PhoneAccount.SCHEME_SIP,
+ "hilda@test.org", "");
private static Map<String, PhoneAccountHandle> mPhoneAccounts = new ArrayMap();
public static SelfManagedCallList getInstance() {
@@ -101,20 +108,29 @@
SELF_MANAGED_NAME_1, true /* areCallsLogged */);
registerPhoneAccount(context, SELF_MANAGED_ACCOUNT_2, SELF_MANAGED_ADDRESS_2,
SELF_MANAGED_NAME_2, false /* areCallsLogged */);
- registerPhoneAccount(context, SELF_MANAGED_ACCOUNT_3, SELF_MANAGED_ADDRESS_1,
- SELF_MANAGED_NAME_3, true /* areCallsLogged */);
+ registerPhoneAccount(context, SELF_MANAGED_ACCOUNT_1A, SELF_MANAGED_ADDRESS_1,
+ SELF_MANAGED_NAME_1A, true /* areCallsLogged */);
+ registerPhoneAccount(context, SELF_MANAGED_ACCOUNT_1A, SELF_MANAGED_ADDRESS_1,
+ SELF_MANAGED_NAME_1A, true /* areCallsLogged */);
+ registerPhoneAccount(context, OTHER_COMPONENT_NAME, SELF_MANAGED_ACCOUNT_3,
+ SELF_MANAGED_ADDRESS_3, SELF_MANAGED_NAME_3, false /* areCallsLogged */);
}
public void registerPhoneAccount(Context context, String id, Uri address, String name,
- boolean areCallsLogged) {
- PhoneAccountHandle handle = new PhoneAccountHandle(COMPONENT_NAME, id);
+ boolean areCallsLogged) {
+ registerPhoneAccount(context, COMPONENT_NAME, id, address, name, areCallsLogged);
+ }
+
+ public void registerPhoneAccount(Context context, ComponentName componentName, String id,
+ Uri address, String name, boolean areCallsLogged) {
+ PhoneAccountHandle handle = new PhoneAccountHandle(componentName, id);
mPhoneAccounts.put(id, handle);
Bundle extras = new Bundle();
extras.putBoolean(PhoneAccount.EXTRA_SUPPORTS_HANDOVER_TO, true);
if (areCallsLogged) {
extras.putBoolean(PhoneAccount.EXTRA_LOG_SELF_MANAGED_CALLS, true);
}
- if (id.equals(SELF_MANAGED_ACCOUNT_3)) {
+ if (id.equals(SELF_MANAGED_ACCOUNT_1A)) {
extras.putBoolean(PhoneAccount.EXTRA_ADD_SELF_MANAGED_CALLS_TO_INCALLSERVICE, true);
}
PhoneAccount.Builder builder = PhoneAccount.builder(handle, name)
diff --git a/testapps/src/com/android/server/telecom/testapps/SelfManagedCallListAdapter.java b/testapps/src/com/android/server/telecom/testapps/SelfManagedCallListAdapter.java
index 75ceb62..475f255 100644
--- a/testapps/src/com/android/server/telecom/testapps/SelfManagedCallListAdapter.java
+++ b/testapps/src/com/android/server/telecom/testapps/SelfManagedCallListAdapter.java
@@ -166,8 +166,10 @@
SelfManagedConnection.EXTRA_PHONE_ACCOUNT_HANDLE);
if (phoneAccountHandle.getId().equals(SelfManagedCallList.SELF_MANAGED_ACCOUNT_1)) {
result.setBackgroundColor(result.getContext().getColor(R.color.test_call_a_color));
- } else {
+ } else if (phoneAccountHandle.getId().equals(SelfManagedCallList.SELF_MANAGED_ACCOUNT_2)) {
result.setBackgroundColor(result.getContext().getColor(R.color.test_call_b_color));
+ } else {
+ result.setBackgroundColor(result.getContext().getColor(R.color.test_call_c_color));
}
CallAudioState audioState = connection.getCallAudioState();
diff --git a/testapps/src/com/android/server/telecom/testapps/SelfManagedCallingActivity.java b/testapps/src/com/android/server/telecom/testapps/SelfManagedCallingActivity.java
index 44410d2..5cdaf3d 100644
--- a/testapps/src/com/android/server/telecom/testapps/SelfManagedCallingActivity.java
+++ b/testapps/src/com/android/server/telecom/testapps/SelfManagedCallingActivity.java
@@ -43,8 +43,6 @@
import android.widget.TextView;
import android.widget.Toast;
-import com.android.server.telecom.testapps.R;
-
import java.util.Objects;
/**
@@ -66,6 +64,7 @@
private Button mDisableCarMode;
private RadioButton mUseAcct1Button;
private RadioButton mUseAcct2Button;
+ private RadioButton mUseAcct3Button;
private CheckBox mHoldableCheckbox;
private CheckBox mVideoCallCheckbox;
private EditText mNumber;
@@ -165,6 +164,7 @@
}));
mUseAcct1Button = findViewById(R.id.useAcct1Button);
mUseAcct2Button = findViewById(R.id.useAcct2Button);
+ mUseAcct3Button = findViewById(R.id.useAcct3Button);
mHasFocus = findViewById(R.id.hasFocus);
mVideoCallCheckbox = findViewById(R.id.videoCall);
mHoldableCheckbox = findViewById(R.id.holdable);
@@ -183,6 +183,8 @@
return mCallList.getPhoneAccountHandle(SelfManagedCallList.SELF_MANAGED_ACCOUNT_1);
} else if (mUseAcct2Button.isChecked()) {
return mCallList.getPhoneAccountHandle(SelfManagedCallList.SELF_MANAGED_ACCOUNT_2);
+ } else if (mUseAcct3Button.isChecked()) {
+ return mCallList.getPhoneAccountHandle(SelfManagedCallList.SELF_MANAGED_ACCOUNT_3);
}
return null;
}
@@ -214,8 +216,7 @@
private void placeSelfManagedOutgoingCall() {
TelecomManager tm = TelecomManager.from(this);
- PhoneAccountHandle phoneAccountHandle = mCallList.getPhoneAccountHandle(
- SelfManagedCallList.SELF_MANAGED_ACCOUNT_3);
+ PhoneAccountHandle phoneAccountHandle = getSelectedPhoneAccountHandle();
if (mCheckIfPermittedBeforeCalling.isChecked()) {
Toast.makeText(this, R.string.outgoingCallNotPermitted, Toast.LENGTH_SHORT).show();
@@ -264,7 +265,7 @@
private void placeSelfManagedIncomingCall() {
TelecomManager tm = TelecomManager.from(this);
PhoneAccountHandle phoneAccountHandle = mCallList.getPhoneAccountHandle(
- SelfManagedCallList.SELF_MANAGED_ACCOUNT_3);
+ SelfManagedCallList.SELF_MANAGED_ACCOUNT_1A);
if (mCheckIfPermittedBeforeCalling.isChecked()) {
if (!tm.isIncomingCallPermitted(phoneAccountHandle)) {
diff --git a/tests/src/com/android/server/telecom/tests/CallsManagerTest.java b/tests/src/com/android/server/telecom/tests/CallsManagerTest.java
index 98fedc3..22a850f 100644
--- a/tests/src/com/android/server/telecom/tests/CallsManagerTest.java
+++ b/tests/src/com/android/server/telecom/tests/CallsManagerTest.java
@@ -170,6 +170,8 @@
ComponentName.unflattenFromString("com.voip/.Stuff"), "Voip1");
private static final PhoneAccountHandle SELF_MANAGED_HANDLE = new PhoneAccountHandle(
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 PhoneAccount SIM_1_ACCOUNT = new PhoneAccount.Builder(SIM_1_HANDLE, "Sim1")
.setCapabilities(PhoneAccount.CAPABILITY_SIM_SUBSCRIPTION
| PhoneAccount.CAPABILITY_CALL_PROVIDER
@@ -194,6 +196,11 @@
.setCapabilities(PhoneAccount.CAPABILITY_SELF_MANAGED)
.setIsEnabled(true)
.build();
+ private static final PhoneAccount SELF_MANAGED_2_ACCOUNT = new PhoneAccount.Builder(
+ SELF_MANAGED_2_HANDLE, "Self2")
+ .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");
@@ -822,7 +829,7 @@
mCallsManager.answerCall(incomingCall, VideoProfile.STATE_AUDIO_ONLY);
// THEN the ongoing call is held and the focus request for incoming call is sent
- verify(ongoingCall).hold();
+ verify(ongoingCall).hold(anyString());
verifyFocusRequestAndExecuteCallback(incomingCall);
// and the incoming call is answered.
@@ -1030,7 +1037,7 @@
mCallsManager.markCallAsActive(newCall);
// THEN the ongoing call is held
- verify(ongoingCall).hold();
+ verify(ongoingCall).hold(anyString());
verifyFocusRequestAndExecuteCallback(newCall);
// and the new call is active
@@ -1687,6 +1694,57 @@
assertTrue(mCallsManager.makeRoomForOutgoingCall(ongoingCall2));
}
+ /**
+ * Test where a VoIP app adds another new call and has one active already; ensure we hold the
+ * active call. This assumes same connection service in the same app.
+ */
+ @SmallTest
+ @Test
+ public void testMakeRoomForOutgoingCallForSameVoipApp() {
+ Call activeCall = addSpyCall(SELF_MANAGED_HANDLE, null /* connMgr */,
+ CallState.ACTIVE, Connection.CAPABILITY_HOLD | Connection.CAPABILITY_SUPPORT_HOLD,
+ 0 /* properties */);
+ Call newDialingCall = createCall(SELF_MANAGED_HANDLE, CallState.DIALING);
+ newDialingCall.setConnectionProperties(Connection.CAPABILITY_HOLD
+ | Connection.CAPABILITY_SUPPORT_HOLD);
+ assertTrue(mCallsManager.makeRoomForOutgoingCall(newDialingCall));
+ verify(activeCall).hold(anyString());
+ }
+
+ /**
+ * Test where a VoIP app adds another new call and has one active already; ensure we hold the
+ * active call. This assumes different connection services in the same app.
+ */
+ @SmallTest
+ @Test
+ public void testMakeRoomForOutgoingCallForSameVoipAppDifferentConnectionService() {
+ Call activeCall = addSpyCall(SELF_MANAGED_HANDLE, null /* connMgr */,
+ CallState.ACTIVE, Connection.CAPABILITY_HOLD | Connection.CAPABILITY_SUPPORT_HOLD,
+ 0 /* properties */);
+ Call newDialingCall = createCall(SELF_MANAGED_2_HANDLE, CallState.DIALING);
+ newDialingCall.setConnectionProperties(Connection.CAPABILITY_HOLD
+ | Connection.CAPABILITY_SUPPORT_HOLD);
+ assertTrue(mCallsManager.makeRoomForOutgoingCall(newDialingCall));
+ verify(activeCall).hold(anyString());
+ }
+
+ /**
+ * Test where a VoIP app adds another new call and has one active already; ensure we hold the
+ * active call. This assumes different connection services in the same app.
+ */
+ @SmallTest
+ @Test
+ public void testMakeRoomForOutgoingCallForSameNonVoipApp() {
+ Call activeCall = addSpyCall(SIM_1_HANDLE, null /* connMgr */,
+ CallState.ACTIVE, Connection.CAPABILITY_HOLD | Connection.CAPABILITY_SUPPORT_HOLD,
+ 0 /* properties */);
+ Call newDialingCall = createCall(SIM_1_HANDLE, CallState.DIALING);
+ newDialingCall.setConnectionProperties(Connection.CAPABILITY_HOLD
+ | Connection.CAPABILITY_SUPPORT_HOLD);
+ assertTrue(mCallsManager.makeRoomForOutgoingCall(newDialingCall));
+ verify(activeCall, never()).hold(anyString());
+ }
+
@SmallTest
@Test
public void testMakeRoomForOutgoingCallHasOutgoingCallSelectingAccount() {
@@ -3031,6 +3089,10 @@
mClockProxy,
mToastFactory);
ongoingCall.setState(initialState, "just cuz");
+ if (targetPhoneAccount == SELF_MANAGED_HANDLE
+ || targetPhoneAccount == SELF_MANAGED_2_HANDLE) {
+ ongoingCall.setIsSelfManaged(true);
+ }
return ongoingCall;
}