From 6306f47df7eef4af262352f126154062ca709e6c Mon Sep 17 00:00:00 2001 From: Grace Jia Date: Fri, 24 Jan 2020 12:10:20 -0800 Subject: [PATCH] Capture metrics on InCallService failures and inform user of failures Test: Manual test on telecom testapps, atest TelecomUnitTest Bug: 147883088 Change-Id: Id0e11558c16d770156fd01e470969c99e956be45 --- proto/telecom.proto | 7 ++ res/values/strings.xml | 11 ++ src/com/android/server/telecom/Analytics.java | 14 ++- .../server/telecom/InCallController.java | 108 ++++++++++++++++-- .../android/server/telecom/TelecomSystem.java | 2 +- .../ui/NotificationChannelManager.java | 9 ++ .../testapps/TestInCallServiceImpl.java | 1 - .../telecom/tests/InCallControllerTests.java | 83 +++++++++++--- 8 files changed, 205 insertions(+), 30 deletions(-) diff --git a/proto/telecom.proto b/proto/telecom.proto index 73eba876b..411b8e239 100644 --- a/proto/telecom.proto +++ b/proto/telecom.proto @@ -177,6 +177,13 @@ message InCallServiceInfo { // The type of the in-call service optional InCallServiceType in_call_service_type = 2; + + // The number of milliseconds that the in call service remained bound between binding and + // disconnection. + optional int64 bound_duration_millis = 3; + + // True if the in call service has ever crashed during a call. + optional bool is_null_binding = 4; } // Information about each call. diff --git a/res/values/strings.xml b/res/values/strings.xml index 93c578220..a0a518fad 100644 --- a/res/values/strings.xml +++ b/res/values/strings.xml @@ -68,6 +68,15 @@ background. This app may be accessing and playing audio over the call. + + Crashed phone app + + + Your Phone app %s has crashed. + You call was continued using the Phone app that came with your device. + + Call muted. @@ -300,6 +309,8 @@ Background calls Disconnected calls + + Crashed phone apps diff --git a/src/com/android/server/telecom/Analytics.java b/src/com/android/server/telecom/Analytics.java index 299745458..410660e74 100644 --- a/src/com/android/server/telecom/Analytics.java +++ b/src/com/android/server/telecom/Analytics.java @@ -201,7 +201,8 @@ public class Analytics { public void addVideoEvent(int eventId, int videoState) { } - public void addInCallService(String serviceName, int type) { + public void addInCallService(String serviceName, int type, long boundDuration, + boolean isNullBinding) { } public void addCallProperties(int properties) { @@ -370,10 +371,13 @@ public class Analytics { } @Override - public void addInCallService(String serviceName, int type) { + public void addInCallService(String serviceName, int type, long boundDuration, + boolean isNullBinding) { inCallServiceInfos.add(new TelecomLogClass.InCallServiceInfo() .setInCallServiceName(serviceName) - .setInCallServiceType(type)); + .setInCallServiceType(type) + .setBoundDurationMillis(boundDuration) + .setIsNullBinding(isNullBinding)); } @Override @@ -533,6 +537,10 @@ public class Analytics { s.append(service.getInCallServiceName()); s.append(" type: "); s.append(service.getInCallServiceType()); + s.append(" is crashed: "); + s.append(service.getIsNullBinding()); + s.append(" service last time in ms: "); + s.append(service.getBoundDurationMillis()); s.append("\n"); } s.append("]"); diff --git a/src/com/android/server/telecom/InCallController.java b/src/com/android/server/telecom/InCallController.java index b18c6fffb..797a442bf 100644 --- a/src/com/android/server/telecom/InCallController.java +++ b/src/com/android/server/telecom/InCallController.java @@ -18,6 +18,8 @@ package com.android.server.telecom; import android.Manifest; import android.annotation.NonNull; +import android.app.Notification; +import android.app.NotificationManager; import android.content.ComponentName; import android.content.Context; import android.content.Intent; @@ -47,6 +49,7 @@ import com.android.internal.annotations.VisibleForTesting; import com.android.internal.telecom.IInCallService; import com.android.internal.util.IndentingPrintWriter; import com.android.server.telecom.SystemStateHelper.SystemStateListener; +import com.android.server.telecom.ui.NotificationChannelManager; import java.util.ArrayList; import java.util.Arrays; @@ -65,6 +68,8 @@ import java.util.stream.Collectors; * a binding to the {@link IInCallService} (implemented by the in-call app). */ public class InCallController extends CallsManagerListenerBase { + public static final int IN_CALL_SERVICE_NOTIFICATION_ID = 3; + public static final String NOTIFICATION_TAG = InCallController.class.getSimpleName(); public class InCallServiceConnection { /** @@ -83,7 +88,7 @@ public class InCallController extends CallsManagerListenerBase { public static final int CONNECTION_NOT_SUPPORTED = 3; public class Listener { - public void onDisconnect(InCallServiceConnection conn) {} + public void onDisconnect(InCallServiceConnection conn, Call call) {} } protected Listener mListener; @@ -97,6 +102,7 @@ public class InCallController extends CallsManagerListenerBase { } public InCallServiceInfo getInfo() { return null; } public void dump(IndentingPrintWriter pw) {} + public Call mCall; } private class InCallServiceInfo { @@ -104,6 +110,8 @@ public class InCallController extends CallsManagerListenerBase { private boolean mIsExternalCallsSupported; private boolean mIsSelfManagedCallsSupported; private final int mType; + private long mBindingStartTime; + private long mDisconnectTime; public InCallServiceInfo(ComponentName componentName, boolean isExternalCallsSupported, @@ -131,6 +139,22 @@ public class InCallController extends CallsManagerListenerBase { return mType; } + public long getBindingStartTime() { + return mBindingStartTime; + } + + public long getDisconnectTime() { + return mDisconnectTime; + } + + public void setBindingStartTime(long bindingStartTime) { + mBindingStartTime = bindingStartTime; + } + + public void setDisconnectTime(long disconnectTime) { + mDisconnectTime = disconnectTime; + } + @Override public boolean equals(Object o) { if (this == o) { @@ -198,14 +222,47 @@ public class InCallController extends CallsManagerListenerBase { } } } + + @Override + public void onNullBinding(ComponentName name) { + Log.startSession("ICSBC.oNB"); + synchronized (mLock) { + try { + Log.d(this, "onNullBinding: %s", name); + mIsNullBinding = true; + mIsBound = false; + onDisconnected(); + } finally { + Log.endSession(); + } + } + } + + @Override + public void onBindingDied(ComponentName name) { + Log.startSession("ICSBC.oBD"); + synchronized (mLock) { + try { + Log.d(this, "onBindingDied: %s", name); + mIsBound = false; + onDisconnected(); + } finally { + Log.endSession(); + } + } + } }; private final InCallServiceInfo mInCallServiceInfo; private boolean mIsConnected = false; private boolean mIsBound = false; + private boolean mIsNullBinding = false; + private NotificationManager mNotificationManager; public InCallServiceBindingConnection(InCallServiceInfo info) { mInCallServiceInfo = info; + mNotificationManager = + (NotificationManager) mContext.getSystemService(Context.NOTIFICATION_SERVICE); } @Override @@ -234,6 +291,7 @@ public class InCallController extends CallsManagerListenerBase { Log.i(this, "Attempting to bind to InCall %s, with %s", mInCallServiceInfo, intent); mIsConnected = true; + mInCallServiceInfo.setBindingStartTime(mClockProxy.elapsedRealtime()); if (!mContext.bindServiceAsUser(intent, mServiceConnection, Context.BIND_AUTO_CREATE | Context.BIND_FOREGROUND_SERVICE | Context.BIND_ALLOW_BACKGROUND_ACTIVITY_STARTS, @@ -242,11 +300,10 @@ public class InCallController extends CallsManagerListenerBase { mIsConnected = false; } - if (call != null && mIsConnected) { - call.getAnalytics().addInCallService( - mInCallServiceInfo.getComponentName().flattenToShortString(), - mInCallServiceInfo.getType()); + if (mIsConnected && call != null) { + mCall = call; } + Log.i(this, "mCall: %s, mIsConnected: %s", mCall, mIsConnected); return mIsConnected ? CONNECTION_SUCCEEDED : CONNECTION_FAILED; } @@ -259,10 +316,36 @@ public class InCallController extends CallsManagerListenerBase { @Override public void disconnect() { if (mIsConnected) { - Log.i(InCallController.this, "ICSBC#disconnect: unbinding; %s", - mInCallServiceInfo); + mInCallServiceInfo.setDisconnectTime(mClockProxy.elapsedRealtime()); + Log.i(InCallController.this, "ICSBC#disconnect: unbinding after %s ms;" + + "%s. isCrashed: %s", mInCallServiceInfo.mDisconnectTime + - mInCallServiceInfo.mBindingStartTime, + mInCallServiceInfo, mIsNullBinding); + String packageName = mInCallServiceInfo.getComponentName().getPackageName(); mContext.unbindService(mServiceConnection); mIsConnected = false; + if (mIsNullBinding) { + Notification.Builder builder = new Notification.Builder(mContext, + NotificationChannelManager.CHANNEL_ID_IN_CALL_SERVICE_CRASH); + builder.setSmallIcon(R.drawable.ic_phone) + .setColor(mContext.getResources().getColor(R.color.theme_color)) + .setContentTitle( + mContext.getText( + R.string.notification_crashedInCallService_title)) + .setStyle(new Notification.BigTextStyle() + .bigText(mContext.getString( + R.string.notification_crashedInCallService_body, + packageName))); + mNotificationManager.notify(NOTIFICATION_TAG, IN_CALL_SERVICE_NOTIFICATION_ID, + builder.build()); + } + if (mCall != null) { + mCall.getAnalytics().addInCallService( + mInCallServiceInfo.getComponentName().flattenToShortString(), + mInCallServiceInfo.getType(), + mInCallServiceInfo.getDisconnectTime() + - mInCallServiceInfo.getBindingStartTime(), mIsNullBinding); + } } else { Log.i(InCallController.this, "ICSBC#disconnect: already disconnected; %s", mInCallServiceInfo); @@ -302,7 +385,7 @@ public class InCallController extends CallsManagerListenerBase { InCallController.this.onDisconnected(mInCallServiceInfo); disconnect(); // Unbind explicitly if we get disconnected. if (mListener != null) { - mListener.onDisconnect(InCallServiceBindingConnection.this); + mListener.onDisconnect(InCallServiceBindingConnection.this, mCall); } } } @@ -319,7 +402,7 @@ public class InCallController extends CallsManagerListenerBase { private Listener mSubListener = new Listener() { @Override - public void onDisconnect(InCallServiceConnection subConnection) { + public void onDisconnect(InCallServiceConnection subConnection, Call call) { if (subConnection == mSubConnection) { if (mIsConnected && mIsProxying) { // At this point we know that we need to be connected to the InCallService @@ -327,7 +410,7 @@ public class InCallController extends CallsManagerListenerBase { // just died so we need to stop proxying and connect to the system in-call // service instead. mIsProxying = false; - connect(null); + connect(call); } } } @@ -799,6 +882,7 @@ public class InCallController extends CallsManagerListenerBase { private final Handler mHandler = new Handler(Looper.getMainLooper()); private CarSwappingInCallServiceConnection mInCallServiceConnection; private NonUIInCallServiceConnectionCollection mNonUIInCallServiceConnections; + private final ClockProxy mClockProxy; // Future that's in a completed state unless we're in the middle of binding to a service. // The future will complete with true if binding succeeds, false if it timed out. @@ -809,7 +893,8 @@ public class InCallController extends CallsManagerListenerBase { public InCallController(Context context, TelecomSystem.SyncRoot lock, CallsManager callsManager, SystemStateHelper systemStateHelper, DefaultDialerCache defaultDialerCache, Timeouts.Adapter timeoutsAdapter, - EmergencyCallHelper emergencyCallHelper, CarModeTracker carModeTracker) { + EmergencyCallHelper emergencyCallHelper, CarModeTracker carModeTracker, + ClockProxy clockProxy) { mContext = context; mLock = lock; mCallsManager = callsManager; @@ -819,6 +904,7 @@ public class InCallController extends CallsManagerListenerBase { mEmergencyCallHelper = emergencyCallHelper; mCarModeTracker = carModeTracker; mSystemStateHelper.addListener(mSystemStateListener); + mClockProxy = clockProxy; } @Override diff --git a/src/com/android/server/telecom/TelecomSystem.java b/src/com/android/server/telecom/TelecomSystem.java index a8e16aef0..4bc61e064 100644 --- a/src/com/android/server/telecom/TelecomSystem.java +++ b/src/com/android/server/telecom/TelecomSystem.java @@ -270,7 +270,7 @@ public class TelecomSystem { EmergencyCallHelper emergencyCallHelper) { return new InCallController(context, lock, callsManager, systemStateProvider, defaultDialerCache, timeoutsAdapter, emergencyCallHelper, - new CarModeTracker()); + new CarModeTracker(), clockProxy); } }; diff --git a/src/com/android/server/telecom/ui/NotificationChannelManager.java b/src/com/android/server/telecom/ui/NotificationChannelManager.java index 360239b03..58794a602 100644 --- a/src/com/android/server/telecom/ui/NotificationChannelManager.java +++ b/src/com/android/server/telecom/ui/NotificationChannelManager.java @@ -39,6 +39,7 @@ public class NotificationChannelManager { public static final String CHANNEL_ID_CALL_BLOCKING = "TelecomCallBlocking"; public static final String CHANNEL_ID_AUDIO_PROCESSING = "TelecomBackgroundAudioProcessing"; public static final String CHANNEL_ID_DISCONNECTED_CALLS = "TelecomDisconnectedCalls"; + public static final String CHANNEL_ID_IN_CALL_SERVICE_CRASH = "TelecomInCallServiceCrash"; private BroadcastReceiver mLocaleChangeReceiver = new BroadcastReceiver() { @Override @@ -61,6 +62,7 @@ public class NotificationChannelManager { createOrUpdateChannel(context, CHANNEL_ID_CALL_BLOCKING); createOrUpdateChannel(context, CHANNEL_ID_AUDIO_PROCESSING); createOrUpdateChannel(context, CHANNEL_ID_DISCONNECTED_CALLS); + createOrUpdateChannel(context, CHANNEL_ID_IN_CALL_SERVICE_CRASH); } private void createOrUpdateChannel(Context context, String channelId) { @@ -118,6 +120,13 @@ public class NotificationChannelManager { vibration = true; sound = silentRingtone; break; + case CHANNEL_ID_IN_CALL_SERVICE_CRASH: + name = context.getText(R.string.notification_channel_in_call_service_crash); + importance = NotificationManager.IMPORTANCE_DEFAULT; + canShowBadge = true; + lights = true; + vibration = true; + sound = null; } NotificationChannel channel = new NotificationChannel(channelId, name, importance); diff --git a/testapps/src/com/android/server/telecom/testapps/TestInCallServiceImpl.java b/testapps/src/com/android/server/telecom/testapps/TestInCallServiceImpl.java index 78b8bcc74..fb15e1d24 100644 --- a/testapps/src/com/android/server/telecom/testapps/TestInCallServiceImpl.java +++ b/testapps/src/com/android/server/telecom/testapps/TestInCallServiceImpl.java @@ -16,7 +16,6 @@ package com.android.server.telecom.testapps; -import android.content.Context; import android.content.Intent; import android.telecom.Call; import android.telecom.CallAudioState; diff --git a/tests/src/com/android/server/telecom/tests/InCallControllerTests.java b/tests/src/com/android/server/telecom/tests/InCallControllerTests.java index 23d22e9b1..38a1798d1 100644 --- a/tests/src/com/android/server/telecom/tests/InCallControllerTests.java +++ b/tests/src/com/android/server/telecom/tests/InCallControllerTests.java @@ -16,8 +16,14 @@ package com.android.server.telecom.tests; +import static com.android.server.telecom.InCallController.IN_CALL_SERVICE_NOTIFICATION_ID; +import static com.android.server.telecom.InCallController.NOTIFICATION_TAG; + import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.ArgumentMatchers.matches; import static org.mockito.ArgumentMatchers.nullable; import static org.mockito.Matchers.any; @@ -33,6 +39,8 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import android.Manifest; +import android.app.Notification; +import android.app.NotificationManager; import android.app.UiModeManager; import android.content.ComponentName; import android.content.ContentResolver; @@ -44,12 +52,14 @@ import android.content.pm.PackageManager; import android.content.pm.ResolveInfo; import android.content.pm.ServiceInfo; import android.content.res.Resources; +import android.os.Build; import android.os.Bundle; import android.os.Handler; import android.os.IBinder; import android.os.Looper; import android.os.UserHandle; import android.telecom.InCallService; +import android.telecom.Log; import android.telecom.ParcelableCall; import android.telecom.PhoneAccountHandle; import android.telecom.TelecomManager; @@ -64,6 +74,7 @@ import com.android.server.telecom.BluetoothHeadsetProxy; import com.android.server.telecom.Call; import com.android.server.telecom.CallsManager; import com.android.server.telecom.CarModeTracker; +import com.android.server.telecom.ClockProxy; import com.android.server.telecom.DefaultDialerCache; import com.android.server.telecom.EmergencyCallHelper; import com.android.server.telecom.InCallController; @@ -85,23 +96,10 @@ import org.mockito.MockitoAnnotations; import org.mockito.invocation.InvocationOnMock; import org.mockito.stubbing.Answer; -import java.util.ArrayList; import java.util.Collections; import java.util.LinkedList; -import java.util.List; import java.util.concurrent.CompletableFuture; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; -import static org.mockito.ArgumentMatchers.matches; -import static org.mockito.Matchers.any; -import static org.mockito.Matchers.eq; -import static org.mockito.Mockito.doReturn; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verify; - @RunWith(JUnit4.class) public class InCallControllerTests extends TelecomTestCase { @Mock CallsManager mMockCallsManager; @@ -115,6 +113,9 @@ public class InCallControllerTests extends TelecomTestCase { @Mock Timeouts.Adapter mTimeoutsAdapter; @Mock DefaultDialerCache mDefaultDialerCache; @Mock RoleManagerAdapter mMockRoleManagerAdapter; + @Mock ClockProxy mClockProxy; + @Mock Analytics.CallInfoImpl mCallInfo; + @Mock NotificationManager mNotificationManager; private static final int CURRENT_USER_ID = 900973; private static final String DEF_PKG = "defpkg"; @@ -159,7 +160,9 @@ public class InCallControllerTests extends TelecomTestCase { when(mMockCallsManager.getRoleManagerAdapter()).thenReturn(mMockRoleManagerAdapter); mInCallController = new InCallController(mMockContext, mLock, mMockCallsManager, mMockSystemStateHelper, mDefaultDialerCache, mTimeoutsAdapter, - mEmergencyCallHelper, new CarModeTracker()); + mEmergencyCallHelper, new CarModeTracker(), mClockProxy); + when(mMockContext.getSystemService(eq(Context.NOTIFICATION_SERVICE))) + .thenReturn(mNotificationManager); // Companion Apps don't have CONTROL_INCALL_EXPERIENCE permission. doAnswer(invocation -> { int uid = invocation.getArgument(0); @@ -472,6 +475,58 @@ public class InCallControllerTests extends TelecomTestCase { assertEquals(SYS_CLASS, bindIntent.getComponent().getClassName()); } + @Test + public void testBindToService_NullBinding_FallBackToSystem() throws Exception { + ApplicationInfo applicationInfo = new ApplicationInfo(); + applicationInfo.targetSdkVersion = Build.VERSION_CODES.R; + when(mMockCallsManager.isInEmergencyCall()).thenReturn(false); + when(mMockContext.getPackageManager()).thenReturn(mMockPackageManager); + when(mMockCall.isIncoming()).thenReturn(false); + when(mMockCall.isExternalCall()).thenReturn(false); + when(mMockCallsManager.getCurrentUserHandle()).thenReturn(mUserHandle); + when(mMockCall.getAnalytics()).thenReturn(mCallInfo); + when(mMockContext.bindServiceAsUser( + any(Intent.class), any(ServiceConnection.class), anyInt(), any(UserHandle.class))) + .thenReturn(true); + when(mMockContext.getApplicationInfo()).thenReturn(applicationInfo); + + setupMockPackageManager(true /* default */, true /* system */, false /* external calls */); + mInCallController.bindToServices(mMockCall); + + ArgumentCaptor bindIntentCaptor = ArgumentCaptor.forClass(Intent.class); + ArgumentCaptor serviceConnectionCaptor = + ArgumentCaptor.forClass(ServiceConnection.class); + verify(mMockContext, times(1)).bindServiceAsUser( + bindIntentCaptor.capture(), + serviceConnectionCaptor.capture(), + anyInt(), + any(UserHandle.class)); + ServiceConnection serviceConnection = serviceConnectionCaptor.getValue(); + ComponentName defDialerComponentName = new ComponentName(DEF_PKG, DEF_CLASS); + ComponentName sysDialerComponentName = new ComponentName(SYS_PKG, SYS_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)); + serviceConnection.onNullBinding(defDialerComponentName); + + verify(mNotificationManager).notify(eq(NOTIFICATION_TAG), + eq(IN_CALL_SERVICE_NOTIFICATION_ID), any(Notification.class)); + verify(mCallInfo).addInCallService(eq(sysDialerComponentName.flattenToShortString()), + anyInt(), anyLong(), eq(true)); + + ArgumentCaptor bindIntentCaptor2 = ArgumentCaptor.forClass(Intent.class); + verify(mMockContext, times(2)).bindServiceAsUser( + bindIntentCaptor2.capture(), + any(ServiceConnection.class), + eq(Context.BIND_AUTO_CREATE | Context.BIND_FOREGROUND_SERVICE + | Context.BIND_ALLOW_BACKGROUND_ACTIVITY_STARTS), + eq(UserHandle.CURRENT)); + } + /** * Ensures that the {@link InCallController} will bind to an {@link InCallService} which * supports external calls.