Fix a car mode tracking leak upon uninstall
When a package gets uninstalled while it's requesting car mode, a race
condition might lead Telecom to receive its car mode cancellation later
than the uninstall, at which point the check to see whether it's a valid
InCallService would fail. If this happens, CarModeTracker will leak the
package and it'll take a restart to clear.
This hasn't been observed in the wild, but it's been causing some
flakiness in Telecom's CTS tests.
Bug: 162647577
Test: atest TelecomUnitTests CtsTelecomTestCases
Change-Id: I549e3d4fe14ad40b89d788d7304821b16764d642
diff --git a/src/com/android/server/telecom/CarModeTracker.java b/src/com/android/server/telecom/CarModeTracker.java
index 0ec4917..e64ef5d 100644
--- a/src/com/android/server/telecom/CarModeTracker.java
+++ b/src/com/android/server/telecom/CarModeTracker.java
@@ -28,6 +28,7 @@
import java.util.Comparator;
import java.util.List;
import java.util.Objects;
+import java.util.Optional;
import java.util.PriorityQueue;
import java.util.stream.Collectors;
@@ -144,6 +145,28 @@
}
/**
+ * Force-removes a package from the car mode tracking list, no matter at which priority.
+ *
+ * This handles the case where packages are disabled or uninstalled. In those case, remove them
+ * from the tracking list so they don't cause a leak.
+ * @param packageName Package name of the app to force-remove
+ */
+ public void forceExitCarMode(@NonNull String packageName) {
+ Optional<CarModeApp> forcedApp = mCarModeApps.stream()
+ .filter(c -> c.getPackageName().equals(packageName))
+ .findAny();
+ if (forcedApp.isPresent()) {
+ String logString = String.format("forceExitCarMode: packageName=%s, was at priority=%s",
+ packageName, forcedApp.get().getPriority());
+ Log.i(this, logString);
+ mCarModeChangeLog.log(logString);
+ mCarModeApps.removeIf(c -> c.getPackageName().equals(packageName));
+ } else {
+ Log.i(this, "Package %s is not tracked as requesting car mode", packageName);
+ }
+ }
+
+ /**
* Retrieves a list of the apps which are currently in car mode, ordered by priority such that
* the highest priority app is first.
* @return List of apps in car mode.
diff --git a/src/com/android/server/telecom/InCallController.java b/src/com/android/server/telecom/InCallController.java
index 0b6b55e..4718871 100644
--- a/src/com/android/server/telecom/InCallController.java
+++ b/src/com/android/server/telecom/InCallController.java
@@ -901,9 +901,18 @@
}
};
- private final SystemStateListener mSystemStateListener =
- (priority, packageName, isCarMode) -> InCallController.this.handleCarModeChange(
- priority, packageName, isCarMode);
+ private final SystemStateListener mSystemStateListener = new SystemStateListener() {
+ @Override
+ public void onCarModeChanged(int priority, String packageName, boolean isCarMode) {
+ InCallController.this.handleCarModeChange(priority, packageName, isCarMode);
+ }
+
+ @Override
+ public void onPackageUninstalled(String packageName) {
+ mCarModeTracker.forceExitCarMode(packageName);
+ updateCarModeForSwitchingConnection();
+ }
+ };
private static final int IN_CALL_SERVICE_TYPE_INVALID = 0;
private static final int IN_CALL_SERVICE_TYPE_DIALER_UI = 1;
@@ -1859,7 +1868,8 @@
public void handleCarModeChange(int priority, String packageName, boolean isCarMode) {
Log.i(this, "handleCarModeChange: packageName=%s, priority=%d, isCarMode=%b",
packageName, priority, isCarMode);
- if (!isCarModeInCallService(packageName)) {
+ // Don't ignore the signal if we are disabling car mode; package may be uninstalled.
+ if (isCarMode && !isCarModeInCallService(packageName)) {
Log.i(this, "handleCarModeChange: not a valid InCallService; packageName=%s",
packageName);
return;
@@ -1871,8 +1881,12 @@
mCarModeTracker.handleExitCarMode(priority, packageName);
}
+ updateCarModeForSwitchingConnection();
+ }
+
+ public void updateCarModeForSwitchingConnection() {
if (mInCallServiceConnection != null) {
- Log.i(this, "handleCarModeChange: car mode apps: %s",
+ Log.i(this, "updateCarModeForSwitchingConnection: car mode apps: %s",
mCarModeTracker.getCarModeApps().stream().collect(Collectors.joining(", ")));
if (shouldUseCarModeUI()) {
mInCallServiceConnection.changeCarModeApp(
diff --git a/src/com/android/server/telecom/SystemStateHelper.java b/src/com/android/server/telecom/SystemStateHelper.java
index 073b081..3be3d5e 100644
--- a/src/com/android/server/telecom/SystemStateHelper.java
+++ b/src/com/android/server/telecom/SystemStateHelper.java
@@ -26,6 +26,7 @@
import android.hardware.SensorEvent;
import android.hardware.SensorEventListener;
import android.hardware.SensorManager;
+import android.net.Uri;
import android.telecom.Log;
import java.util.Set;
@@ -38,7 +39,7 @@
* Provides various system states to the rest of the telecom codebase.
*/
public class SystemStateHelper {
- public static interface SystemStateListener {
+ public interface SystemStateListener {
/**
* Listener method to inform interested parties when a package name requests to enter or
* exit car mode.
@@ -48,6 +49,12 @@
* otherwise.
*/
void onCarModeChanged(int priority, String packageName, boolean isCarMode);
+
+ /**
+ * Notifies when a package has been uninstalled.
+ * @param packageName the package name of the uninstalled package
+ */
+ void onPackageUninstalled(String packageName);
}
private final Context mContext;
@@ -62,7 +69,7 @@
UiModeManager.DEFAULT_PRIORITY);
String callingPackage = intent.getStringExtra(
UiModeManager.EXTRA_CALLING_PACKAGE);
- Log.i(SystemStateHelper.this, "ENTER_CAR_MODE_PRIVILEGED; priority=%d, pkg=%s",
+ Log.i(SystemStateHelper.this, "ENTER_CAR_MODE_PRIORITIZED; priority=%d, pkg=%s",
priority, callingPackage);
onEnterCarMode(priority, callingPackage);
} else if (UiModeManager.ACTION_EXIT_CAR_MODE_PRIORITIZED.equals(action)) {
@@ -70,11 +77,21 @@
UiModeManager.DEFAULT_PRIORITY);
String callingPackage = intent.getStringExtra(
UiModeManager.EXTRA_CALLING_PACKAGE);
- Log.i(SystemStateHelper.this, "EXIT_CAR_MODE_PRIVILEGED; priority=%d, pkg=%s",
+ Log.i(SystemStateHelper.this, "EXIT_CAR_MODE_PRIORITIZED; priority=%d, pkg=%s",
priority, callingPackage);
onExitCarMode(priority, callingPackage);
+ } else if (Intent.ACTION_PACKAGE_REMOVED.equals(action)) {
+ Uri data = intent.getData();
+ if (data == null) {
+ Log.w(SystemStateHelper.this,
+ "Got null data for package removed, ignoring");
+ return;
+ }
+ mListeners.forEach(
+ l -> l.onPackageUninstalled(data.getEncodedSchemeSpecificPart()));
} else {
- Log.w(this, "Unexpected intent received: %s", intent.getAction());
+ Log.w(SystemStateHelper.this,
+ "Unexpected intent received: %s", intent.getAction());
}
} finally {
Log.endSession();
@@ -88,11 +105,16 @@
public SystemStateHelper(Context context) {
mContext = context;
- IntentFilter intentFilter = new IntentFilter(
+ IntentFilter intentFilter1 = new IntentFilter(
UiModeManager.ACTION_ENTER_CAR_MODE_PRIORITIZED);
- intentFilter.addAction(UiModeManager.ACTION_EXIT_CAR_MODE_PRIORITIZED);
- mContext.registerReceiver(mBroadcastReceiver, intentFilter);
- Log.i(this, "Registering car mode receiver: %s", intentFilter);
+ intentFilter1.addAction(UiModeManager.ACTION_EXIT_CAR_MODE_PRIORITIZED);
+
+ IntentFilter intentFilter2 = new IntentFilter(Intent.ACTION_PACKAGE_REMOVED);
+ intentFilter2.addDataScheme("package");
+ mContext.registerReceiver(mBroadcastReceiver, intentFilter1);
+ mContext.registerReceiver(mBroadcastReceiver, intentFilter2);
+ Log.i(this, "Registering broadcast receiver: %s", intentFilter1);
+ Log.i(this, "Registering broadcast receiver: %s", intentFilter2);
mIsCarMode = getSystemCarMode();
}
diff --git a/tests/src/com/android/server/telecom/tests/CarModeTrackerTest.java b/tests/src/com/android/server/telecom/tests/CarModeTrackerTest.java
index 4ef4596..dbfcdb1 100644
--- a/tests/src/com/android/server/telecom/tests/CarModeTrackerTest.java
+++ b/tests/src/com/android/server/telecom/tests/CarModeTrackerTest.java
@@ -17,6 +17,7 @@
package com.android.server.telecom.tests;
import static junit.framework.Assert.assertEquals;
+import static junit.framework.Assert.assertFalse;
import static junit.framework.Assert.assertTrue;
import static junit.framework.TestCase.assertNull;
@@ -104,6 +105,17 @@
}
/**
+ * Ensure that we don't keep a package around after it's been removed from the device
+ */
+ @Test
+ public void testForceExitCarMode() {
+ testEnterCarModeBasic();
+ mCarModeTracker.forceExitCarMode(CAR_MODE_APP1_PACKAGE_NAME);
+ assertFalse(mCarModeTracker.isInCarMode());
+ assertNull(mCarModeTracker.getCurrentCarModePackage());
+ }
+
+ /**
* Verifies only the first app at the default priority gets tracked.
*/
@Test
diff --git a/tests/src/com/android/server/telecom/tests/InCallControllerTests.java b/tests/src/com/android/server/telecom/tests/InCallControllerTests.java
index 2b461f4..0e5ac27 100644
--- a/tests/src/com/android/server/telecom/tests/InCallControllerTests.java
+++ b/tests/src/com/android/server/telecom/tests/InCallControllerTests.java
@@ -35,6 +35,7 @@
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
@@ -66,6 +67,7 @@
import android.telecom.TelecomManager;
import android.test.mock.MockContext;
import android.test.suitebuilder.annotation.MediumTest;
+import android.test.suitebuilder.annotation.SmallTest;
import android.text.TextUtils;
import com.android.internal.telecom.IInCallAdapter;
@@ -146,6 +148,8 @@
private InCallController mInCallController;
private TelecomSystem.SyncRoot mLock = new TelecomSystem.SyncRoot() {};
private EmergencyCallHelper mEmergencyCallHelper;
+ private SystemStateHelper.SystemStateListener mSystemStateListener;
+ private CarModeTracker mCarModeTracker = spy(new CarModeTracker());
@Override
@Before
@@ -166,7 +170,13 @@
when(mMockCallsManager.getRoleManagerAdapter()).thenReturn(mMockRoleManagerAdapter);
mInCallController = new InCallController(mMockContext, mLock, mMockCallsManager,
mMockSystemStateHelper, mDefaultDialerCache, mTimeoutsAdapter,
- mEmergencyCallHelper, new CarModeTracker(), mClockProxy);
+ mEmergencyCallHelper, mCarModeTracker, mClockProxy);
+
+ ArgumentCaptor<SystemStateHelper.SystemStateListener> systemStateListenerArgumentCaptor
+ = ArgumentCaptor.forClass(SystemStateHelper.SystemStateListener.class);
+ verify(mMockSystemStateHelper).addListener(systemStateListenerArgumentCaptor.capture());
+ mSystemStateListener = systemStateListenerArgumentCaptor.getValue();
+
when(mMockContext.getSystemService(eq(Context.NOTIFICATION_SERVICE)))
.thenReturn(mNotificationManager);
// Companion Apps don't have CONTROL_INCALL_EXPERIENCE permission.
@@ -210,6 +220,24 @@
super.tearDown();
}
+ @SmallTest
+ @Test
+ public void testCarModeAppRemoval() {
+ setupMockPackageManager(true /* default */, true /* system */, true /* external calls */);
+ when(mMockCallsManager.getCurrentUserHandle()).thenReturn(mUserHandle);
+ when(mMockContext.getPackageManager()).thenReturn(mMockPackageManager);
+
+ when(mMockSystemStateHelper.isCarMode()).thenReturn(true);
+
+ mSystemStateListener.onCarModeChanged(666, CAR_PKG, true);
+ verify(mCarModeTracker).handleEnterCarMode(666, CAR_PKG);
+ assertTrue(mCarModeTracker.isInCarMode());
+
+ mSystemStateListener.onPackageUninstalled(CAR_PKG);
+ verify(mCarModeTracker).forceExitCarMode(CAR_PKG);
+ assertFalse(mCarModeTracker.isInCarMode());
+ }
+
@MediumTest
@Test
public void testBindToService_NoServicesFound_IncomingCall() throws Exception {
diff --git a/tests/src/com/android/server/telecom/tests/SystemStateHelperTest.java b/tests/src/com/android/server/telecom/tests/SystemStateHelperTest.java
index 2de3c83..893ae3d 100644
--- a/tests/src/com/android/server/telecom/tests/SystemStateHelperTest.java
+++ b/tests/src/com/android/server/telecom/tests/SystemStateHelperTest.java
@@ -16,6 +16,8 @@
package com.android.server.telecom.tests;
+import static junit.framework.Assert.fail;
+
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
@@ -23,9 +25,11 @@
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.ArgumentMatchers.isNull;
import static org.mockito.Matchers.any;
+import static org.mockito.Mockito.atLeastOnce;
import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
@@ -39,6 +43,7 @@
import android.hardware.SensorEvent;
import android.hardware.SensorEventListener;
import android.hardware.SensorManager;
+import android.net.Uri;
import android.test.suitebuilder.annotation.SmallTest;
import com.android.server.telecom.SystemStateHelper;
@@ -53,11 +58,10 @@
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;
import org.mockito.internal.util.reflection.FieldSetter;
-import org.mockito.invocation.InvocationOnMock;
-import org.mockito.stubbing.Answer;
-import java.lang.reflect.Constructor;
-import java.lang.reflect.Method;
+import java.util.List;
+import java.util.function.Predicate;
+import java.util.stream.Collectors;
/**
* Unit tests for SystemStateHelper
@@ -128,16 +132,53 @@
@SmallTest
@Test
- public void testReceiverAndIntentFilter() {
- ArgumentCaptor<IntentFilter> intentFilter = ArgumentCaptor.forClass(IntentFilter.class);
- new SystemStateHelper(mContext);
- verify(mContext).registerReceiver(any(BroadcastReceiver.class), intentFilter.capture());
+ public void testPackageRemoved() {
+ ArgumentCaptor<BroadcastReceiver> receiver =
+ ArgumentCaptor.forClass(BroadcastReceiver.class);
+ new SystemStateHelper(mContext).addListener(mSystemStateListener);
+ verify(mContext, atLeastOnce())
+ .registerReceiver(receiver.capture(), any(IntentFilter.class));
+ Intent packageRemovedIntent = new Intent(Intent.ACTION_PACKAGE_REMOVED);
+ packageRemovedIntent.setData(Uri.fromParts("package", "com.android.test", null));
+ receiver.getValue().onReceive(mContext, packageRemovedIntent);
+ verify(mSystemStateListener).onPackageUninstalled("com.android.test");
+ }
- assertEquals(2, intentFilter.getValue().countActions());
- assertEquals(UiModeManager.ACTION_ENTER_CAR_MODE_PRIORITIZED,
- intentFilter.getValue().getAction(0));
- assertEquals(UiModeManager.ACTION_EXIT_CAR_MODE_PRIORITIZED,
- intentFilter.getValue().getAction(1));
+ @SmallTest
+ @Test
+ public void testReceiverAndIntentFilter() {
+ ArgumentCaptor<IntentFilter> intentFilterCaptor =
+ ArgumentCaptor.forClass(IntentFilter.class);
+ new SystemStateHelper(mContext);
+ verify(mContext, times(2)).registerReceiver(
+ any(BroadcastReceiver.class), intentFilterCaptor.capture());
+
+ Predicate<IntentFilter> carModeFilterTest = (intentFilter) ->
+ 2 == intentFilter.countActions()
+ && intentFilter.hasAction(UiModeManager.ACTION_ENTER_CAR_MODE_PRIORITIZED)
+ && intentFilter.hasAction(UiModeManager.ACTION_EXIT_CAR_MODE_PRIORITIZED);
+
+ Predicate<IntentFilter> packageRemovedFilterTest = (intentFilter) ->
+ 1 == intentFilter.countActions()
+ && intentFilter.hasAction(Intent.ACTION_PACKAGE_REMOVED)
+ && intentFilter.hasDataScheme("package");
+
+ List<IntentFilter> capturedFilters = intentFilterCaptor.getAllValues();
+ assertEquals(2, capturedFilters.size());
+ for (IntentFilter filter : capturedFilters) {
+ if (carModeFilterTest.test(filter)) {
+ carModeFilterTest = (i) -> false;
+ continue;
+ }
+ if (packageRemovedFilterTest.test(filter)) {
+ packageRemovedFilterTest = (i) -> false;
+ continue;
+ }
+ String failString = String.format("Registered intent filters not correct. Got %s",
+ capturedFilters.stream().map(IntentFilter::toString)
+ .collect(Collectors.joining("\n")));
+ fail(failString);
+ }
}
@SmallTest
@@ -147,7 +188,8 @@
ArgumentCaptor.forClass(BroadcastReceiver.class);
new SystemStateHelper(mContext).addListener(mSystemStateListener);
- verify(mContext).registerReceiver(receiver.capture(), any(IntentFilter.class));
+ verify(mContext, atLeastOnce())
+ .registerReceiver(receiver.capture(), any(IntentFilter.class));
when(mIntentEnter.getAction()).thenReturn(UiModeManager.ACTION_ENTER_CAR_MODE_PRIORITIZED);
receiver.getValue().onReceive(mContext, mIntentEnter);