Fix broken test and in-call controller bug
1. testBindToService_DefaultDialer_FallBackToSystem was broken because
InCallController#onConnected returned false due to an empty list of
calls in the mock CallsManager. Added a return stub to prevent this.
2. If an incoming call disconnects remotely before onServiceConnected is
called, no services will be bound to InCallController, but
InCallController will also not attempt to bind to an InCallService for
future calls. This is because the failure case in
InCallServiceBindingConnection#onConnected calls disconnect() directly,
not setting mInCallServiceConnection to null. This means that
isBoundToServices will continue to return true, so bindToServices will
never be called again until something else calls unbindFromServices.
Fix was to not disconnect if no calls are found when onServiceConnected
is called.
3. A test was added to test the fix in #2.
Change-Id: I95caba2e3169be33d89424269a09b3ad292be3de
Fix: 28743971
diff --git a/src/com/android/server/telecom/CallsManager.java b/src/com/android/server/telecom/CallsManager.java
index 0cc3b3b..1a216ff 100644
--- a/src/com/android/server/telecom/CallsManager.java
+++ b/src/com/android/server/telecom/CallsManager.java
@@ -266,7 +266,7 @@
RingtoneFactory ringtoneFactory = new RingtoneFactory(this, context);
SystemVibrator systemVibrator = new SystemVibrator(context);
mInCallController = new InCallController(
- context, mLock, this, systemStateProvider, defaultDialerAdapter);
+ context, mLock, this, systemStateProvider, defaultDialerAdapter, mTimeoutsAdapter);
mRinger = new Ringer(playerFactory, context, systemSettingsUtil, asyncRingtonePlayer,
ringtoneFactory, systemVibrator, mInCallController);
@@ -629,7 +629,8 @@
return false;
}
- CallAudioState getAudioState() {
+ @VisibleForTesting
+ public CallAudioState getAudioState() {
return mCallAudioManager.getCallAudioState();
}
@@ -1481,7 +1482,8 @@
/**
* Returns true if telecom supports adding another top-level call.
*/
- boolean canAddCall() {
+ @VisibleForTesting
+ public boolean canAddCall() {
boolean isDeviceProvisioned = Settings.Global.getInt(mContext.getContentResolver(),
Settings.Global.DEVICE_PROVISIONED, 0) != 0;
if (!isDeviceProvisioned) {
diff --git a/src/com/android/server/telecom/InCallController.java b/src/com/android/server/telecom/InCallController.java
index 5e1b887..87eb1d0 100644
--- a/src/com/android/server/telecom/InCallController.java
+++ b/src/com/android/server/telecom/InCallController.java
@@ -217,7 +217,7 @@
InCallController.this.onConnected(mInCallServiceInfo, service);
if (!shouldRemainConnected) {
// Sometimes we can opt to disconnect for certain reasons, like if the
- // InCallService rejected our intialization step, or the calls went away
+ // InCallService rejected our initialization step, or the calls went away
// in the time it took us to bind to the InCallService. In such cases, we go
// ahead and disconnect ourselves.
disconnect();
@@ -599,17 +599,19 @@
private final CallsManager mCallsManager;
private final SystemStateProvider mSystemStateProvider;
private final DefaultDialerManagerAdapter mDefaultDialerAdapter;
+ private final Timeouts.Adapter mTimeoutsAdapter;
private CarSwappingInCallServiceConnection mInCallServiceConnection;
private NonUIInCallServiceConnectionCollection mNonUIInCallServiceConnections;
public InCallController(Context context, TelecomSystem.SyncRoot lock, CallsManager callsManager,
SystemStateProvider systemStateProvider,
- DefaultDialerManagerAdapter defaultDialerAdapter) {
+ DefaultDialerManagerAdapter defaultDialerAdapter, Timeouts.Adapter timeoutsAdapter) {
mContext = context;
mLock = lock;
mCallsManager = callsManager;
mSystemStateProvider = systemStateProvider;
mDefaultDialerAdapter = defaultDialerAdapter;
+ mTimeoutsAdapter = timeoutsAdapter;
Resources resources = mContext.getResources();
mSystemInCallComponentName = new ComponentName(
@@ -671,7 +673,7 @@
}
}
}
- }.prepare(), Timeouts.getCallRemoveUnbindInCallServicesDelay(
+ }.prepare(), mTimeoutsAdapter.getCallRemoveUnbindInCallServicesDelay(
mContext.getContentResolver()));
}
call.removeListener(mCallListener);
@@ -842,10 +844,14 @@
*/
private void unbindFromServices() {
if (isBoundToServices()) {
- mInCallServiceConnection.disconnect();
- mInCallServiceConnection = null;
- mNonUIInCallServiceConnections.disconnect();
- mNonUIInCallServiceConnections = null;
+ if (mInCallServiceConnection != null) {
+ mInCallServiceConnection.disconnect();
+ mInCallServiceConnection = null;
+ }
+ if (mNonUIInCallServiceConnections != null) {
+ mNonUIInCallServiceConnections.disconnect();
+ mNonUIInCallServiceConnections = null;
+ }
}
}
@@ -1079,34 +1085,32 @@
// Upon successful connection, send the state of the world to the service.
List<Call> calls = orderCallsWithChildrenFirst(mCallsManager.getCalls());
- if (!calls.isEmpty()) {
- Log.i(this, "Adding %s calls to InCallService after onConnected: %s", calls.size(),
- info.getComponentName());
- for (Call call : calls) {
- try {
- if (call.isExternalCall() && !info.isExternalCallsSupported()) {
- continue;
- }
-
- // Track the call if we don't already know about it.
- addCall(call);
-
- inCallService.addCall(ParcelableCallUtils.toParcelableCall(
- call,
- true /* includeVideoProvider */,
- mCallsManager.getPhoneAccountRegistrar(),
- info.isExternalCallsSupported()));
- } catch (RemoteException ignored) {
- }
- }
+ Log.i(this, "Adding %s calls to InCallService after onConnected: %s, including external " +
+ "calls", calls.size(), info.getComponentName());
+ int numCallsSent = 0;
+ for (Call call : calls) {
try {
- inCallService.onCallAudioStateChanged(mCallsManager.getAudioState());
- inCallService.onCanAddCallChanged(mCallsManager.canAddCall());
+ if (call.isExternalCall() && !info.isExternalCallsSupported()) {
+ continue;
+ }
+
+ // Track the call if we don't already know about it.
+ addCall(call);
+ numCallsSent += 1;
+ inCallService.addCall(ParcelableCallUtils.toParcelableCall(
+ call,
+ true /* includeVideoProvider */,
+ mCallsManager.getPhoneAccountRegistrar(),
+ info.isExternalCallsSupported()));
} catch (RemoteException ignored) {
}
- } else {
- return false;
}
+ try {
+ inCallService.onCallAudioStateChanged(mCallsManager.getAudioState());
+ inCallService.onCanAddCallChanged(mCallsManager.canAddCall());
+ } catch (RemoteException ignored) {
+ }
+ Log.i(this, "%s calls sent to InCallService.", numCallsSent);
Trace.endSection();
return true;
}
diff --git a/src/com/android/server/telecom/ParcelableCallUtils.java b/src/com/android/server/telecom/ParcelableCallUtils.java
index b64ce9c..2061ce4 100644
--- a/src/com/android/server/telecom/ParcelableCallUtils.java
+++ b/src/com/android/server/telecom/ParcelableCallUtils.java
@@ -97,7 +97,7 @@
}
// If this is a single-SIM device, the "default SIM" will always be the only SIM.
- boolean isDefaultSmsAccount =
+ boolean isDefaultSmsAccount = phoneAccountRegistrar != null &&
phoneAccountRegistrar.isUserSelectedSmsPhoneAccount(call.getTargetPhoneAccount());
if (call.isRespondViaSmsCapable() && isDefaultSmsAccount) {
capabilities |= android.telecom.Call.Details.CAPABILITY_RESPOND_VIA_TEXT;
diff --git a/src/com/android/server/telecom/Timeouts.java b/src/com/android/server/telecom/Timeouts.java
index 08d7410..92df388 100644
--- a/src/com/android/server/telecom/Timeouts.java
+++ b/src/com/android/server/telecom/Timeouts.java
@@ -32,6 +32,10 @@
public long getCallScreeningTimeoutMillis(ContentResolver cr) {
return Timeouts.getCallScreeningTimeoutMillis(cr);
}
+
+ public long getCallRemoveUnbindInCallServicesDelay(ContentResolver cr) {
+ return Timeouts.getCallRemoveUnbindInCallServicesDelay(cr);
+ }
}
/** A prefix to use for all keys so to not clobber the global namespace. */
@@ -53,15 +57,6 @@
}
/**
- * Returns the longest period, in milliseconds, to wait for the query for direct-to-voicemail
- * to complete. If the query goes beyond this timeout, the incoming call screen is shown to the
- * user.
- */
- public static long getDirectToVoicemailMillis(ContentResolver contentResolver) {
- return get(contentResolver, "direct_to_voicemail_ms", 500L);
- }
-
- /**
* Returns the amount of time to wait before disconnecting a call that was canceled via
* NEW_OUTGOING_CALL broadcast. This timeout allows apps which repost the call using a gateway
* to reuse the existing call, preventing the call from causing a start->end->start jank in the
@@ -141,11 +136,4 @@
public static long getCallScreeningTimeoutMillis(ContentResolver contentResolver) {
return get(contentResolver, "call_screening_timeout", 5000L /* 5 seconds */);
}
-
- /**
- * Returns the amount of time to wait for the block checker to allow or disallow a call.
- */
- public static long getBlockCheckTimeoutMillis(ContentResolver contentResolver) {
- return get(contentResolver, "block_check_timeout_millis", 500L);
- }
}
diff --git a/tests/src/com/android/server/telecom/tests/InCallControllerTests.java b/tests/src/com/android/server/telecom/tests/InCallControllerTests.java
index 0173ed8..cf72225 100644
--- a/tests/src/com/android/server/telecom/tests/InCallControllerTests.java
+++ b/tests/src/com/android/server/telecom/tests/InCallControllerTests.java
@@ -18,6 +18,7 @@
import android.Manifest;
import android.content.ComponentName;
+import android.content.ContentResolver;
import android.content.Context;
import android.content.Intent;
import android.content.ServiceConnection;
@@ -29,6 +30,7 @@
import android.os.IBinder;
import android.os.UserHandle;
import android.telecom.InCallService;
+import android.telecom.ParcelableCall;
import android.telecom.PhoneAccountHandle;
import android.telecom.TelecomManager;
import android.test.mock.MockContext;
@@ -46,6 +48,7 @@
import com.android.server.telecom.SystemStateProvider;
import com.android.server.telecom.TelecomServiceImpl.DefaultDialerManagerAdapter;
import com.android.server.telecom.TelecomSystem;
+import com.android.server.telecom.Timeouts;
import org.mockito.ArgumentCaptor;
import org.mockito.Mock;
@@ -53,6 +56,7 @@
import org.mockito.invocation.InvocationOnMock;
import org.mockito.stubbing.Answer;
+import java.util.Collections;
import java.util.LinkedList;
import static org.mockito.Matchers.any;
@@ -79,6 +83,7 @@
@Mock Resources mMockResources;
@Mock MockContext mMockContext;
@Mock DefaultDialerManagerAdapter mMockDefaultDialerAdapter;
+ @Mock Timeouts.Adapter mTimeoutsAdapter;
private static final int CURRENT_USER_ID = 900973;
private static final String DEF_PKG = "defpkg";
@@ -100,7 +105,7 @@
doReturn(SYS_PKG).when(mMockResources).getString(R.string.ui_default_package);
doReturn(SYS_CLASS).when(mMockResources).getString(R.string.incall_default_class);
mInCallController = new InCallController(mMockContext, mLock, mMockCallsManager,
- mMockSystemStateProvider, mMockDefaultDialerAdapter);
+ mMockSystemStateProvider, mMockDefaultDialerAdapter, mTimeoutsAdapter);
}
@Override
@@ -278,10 +283,14 @@
when(mMockCallsManager.getCurrentUserHandle()).thenReturn(mUserHandle);
when(mMockContext.getPackageManager()).thenReturn(mMockPackageManager);
when(mMockCallsManager.hasEmergencyCall()).thenReturn(false);
+ when(mMockCallsManager.getCalls()).thenReturn(Collections.singletonList(mMockCall));
+ when(mMockCallsManager.getAudioState()).thenReturn(null);
+ when(mMockCallsManager.canAddCall()).thenReturn(false);
when(mMockCall.isIncoming()).thenReturn(false);
when(mMockCall.getTargetPhoneAccount()).thenReturn(PA_HANDLE);
when(mMockCall.getIntentExtras()).thenReturn(callExtras);
when(mMockCall.isExternalCall()).thenReturn(false);
+ when(mMockCall.getConferenceableCalls()).thenReturn(Collections.emptyList());
when(mMockDefaultDialerAdapter.getDefaultDialerApplication(mMockContext, CURRENT_USER_ID))
.thenReturn(DEF_PKG);
when(mMockContext.bindServiceAsUser(
@@ -386,6 +395,61 @@
assertEquals(DEF_CLASS, bindIntent.getComponent().getClassName());
}
+ /**
+ * Make sure that if a call goes away before the in-call service finishes binding and another
+ * call gets connected soon after, the new call will still be sent to the in-call service.
+ */
+ @MediumTest
+ public void testUnbindDueToCallDisconnect() throws Exception {
+ when(mMockCallsManager.getCurrentUserHandle()).thenReturn(mUserHandle);
+ when(mMockContext.getPackageManager()).thenReturn(mMockPackageManager);
+ when(mMockCallsManager.hasEmergencyCall()).thenReturn(false);
+ when(mMockCall.isIncoming()).thenReturn(true);
+ when(mMockCall.isExternalCall()).thenReturn(false);
+ when(mMockDefaultDialerAdapter.getDefaultDialerApplication(mMockContext, CURRENT_USER_ID))
+ .thenReturn(DEF_PKG);
+ when(mMockContext.bindServiceAsUser(
+ any(Intent.class), any(ServiceConnection.class), anyInt(), any(UserHandle.class)))
+ .thenReturn(true);
+ when(mTimeoutsAdapter.getCallRemoveUnbindInCallServicesDelay(any(ContentResolver.class)))
+ .thenReturn(500L);
+
+ when(mMockCallsManager.getCalls()).thenReturn(Collections.singletonList(mMockCall));
+ setupMockPackageManager(true /* default */, true /* system */, false /* external calls */);
+ mInCallController.bindToServices(mMockCall);
+
+ ArgumentCaptor<Intent> bindIntentCaptor = ArgumentCaptor.forClass(Intent.class);
+ ArgumentCaptor<ServiceConnection> serviceConnectionCaptor =
+ ArgumentCaptor.forClass(ServiceConnection.class);
+ verify(mMockContext, times(1)).bindServiceAsUser(
+ bindIntentCaptor.capture(),
+ serviceConnectionCaptor.capture(),
+ eq(Context.BIND_AUTO_CREATE | Context.BIND_FOREGROUND_SERVICE),
+ eq(UserHandle.CURRENT));
+
+ // Pretend that the call has gone away.
+ when(mMockCallsManager.getCalls()).thenReturn(Collections.emptyList());
+ mInCallController.onCallRemoved(mMockCall);
+
+ // Start the connection, make sure we don't unbind, and make sure that we don't send
+ // anything to the in-call service yet.
+ ServiceConnection serviceConnection = serviceConnectionCaptor.getValue();
+ ComponentName defDialerComponentName = new ComponentName(DEF_PKG, DEF_CLASS);
+ IBinder mockBinder = mock(IBinder.class);
+ IInCallService mockInCallService = mock(IInCallService.class);
+ when(mockBinder.queryLocalInterface(anyString())).thenReturn(mockInCallService);
+
+ serviceConnection.onServiceConnected(defDialerComponentName, mockBinder);
+ verify(mockInCallService).setInCallAdapter(any(IInCallAdapter.class));
+ verify(mMockContext, never()).unbindService(serviceConnection);
+ verify(mockInCallService, never()).addCall(any(ParcelableCall.class));
+
+ // Now, we add in the call again and make sure that it's sent to the InCallService.
+ when(mMockCallsManager.getCalls()).thenReturn(Collections.singletonList(mMockCall));
+ mInCallController.onCallAdded(mMockCall);
+ verify(mockInCallService).addCall(any(ParcelableCall.class));
+ }
+
private void setupMocks(boolean isExternalCall) {
when(mMockCallsManager.getCurrentUserHandle()).thenReturn(mUserHandle);
when(mMockContext.getPackageManager()).thenReturn(mMockPackageManager);
