Merge "Limit relaunch retry if it keeps failing" into qt-qpr1-dev
diff --git a/service/src/com/android/car/CarMediaService.java b/service/src/com/android/car/CarMediaService.java
index b69e666..f7c3517 100644
--- a/service/src/com/android/car/CarMediaService.java
+++ b/service/src/com/android/car/CarMediaService.java
@@ -155,11 +155,7 @@
if (Log.isLoggable(CarLog.TAG_MEDIA, Log.DEBUG)) {
Log.d(CarLog.TAG_MEDIA, "Switched to user " + mCurrentUser);
}
- if (mUserManager.isUserUnlocked(mCurrentUser)) {
- initUser();
- } else {
- mPendingInit = true;
- }
+ maybeInitUser();
}
};
@@ -186,20 +182,32 @@
mContext.getResources().getInteger(R.integer.config_mediaSourceChangedAutoplay);
mPlayOnBootConfig = mContext.getResources().getInteger(R.integer.config_mediaBootAutoplay);
mCurrentUser = ActivityManager.getCurrentUser();
- updateMediaSessionCallbackForCurrentUser();
}
@Override
+ // This method is called from ICarImpl after CarMediaService is created.
public void init() {
- // Nothing to do. Reason: this method is only called once after rebooting, but we need to
- // init user state each time a new user is unlocked, so this method is not the right
- // place to call initUser().
+ maybeInitUser();
+ }
+
+ private void maybeInitUser() {
+ if (mUserManager.isUserUnlocked(mCurrentUser)) {
+ initUser();
+ } else {
+ mPendingInit = true;
+ }
}
private void initUser() {
+ // SharedPreferences are shared among different users thus only need initialized once. And
+ // they should be initialized after user 0 is unlocked because SharedPreferences in
+ // credential encrypted storage are not available until after user 0 is unlocked.
+ // initUser() is called when the current foreground user is unlocked, and by that time user
+ // 0 has been unlocked already, so initializing SharedPreferences in initUser() is fine.
if (mSharedPrefs == null) {
mSharedPrefs = mContext.getSharedPreferences(SHARED_PREF, Context.MODE_PRIVATE);
}
+
if (mIsPackageUpdateReceiverRegistered) {
mContext.unregisterReceiver(mPackageUpdateReceiver);
}
@@ -232,6 +240,23 @@
mContext.startForegroundServiceAsUser(serviceStart, currentUser);
}
+ private boolean sharedPrefsInitialized() {
+ if (mSharedPrefs == null) {
+ // It shouldn't reach this but let's be cautious.
+ Log.e(CarLog.TAG_MEDIA, "SharedPreferences are not initialized!");
+ String className = getClass().getName();
+ for (StackTraceElement ste : Thread.currentThread().getStackTrace()) {
+ // Let's print the useful logs only.
+ String log = ste.toString();
+ if (log.contains(className)) {
+ Log.e(CarLog.TAG_MEDIA, log);
+ }
+ }
+ return false;
+ }
+ return true;
+ }
+
private boolean isCurrentUserEphemeral() {
return mUserManager.getUserInfo(mCurrentUser).isEphemeral();
}
@@ -498,18 +523,14 @@
updateActiveMediaController(mMediaSessionManager
.getActiveSessionsForUser(null, ActivityManager.getCurrentUser()));
- if (mSharedPrefs != null) {
- if (mPrimaryMediaComponent != null && !TextUtils.isEmpty(
- mPrimaryMediaComponent.flattenToString())) {
- if (!isCurrentUserEphemeral()) {
- saveLastMediaSource(mPrimaryMediaComponent);
- }
- mRemovedMediaSourcePackage = null;
+ if (mPrimaryMediaComponent != null && !TextUtils.isEmpty(
+ mPrimaryMediaComponent.flattenToString())) {
+ if (!isCurrentUserEphemeral()) {
+ saveLastMediaSource(mPrimaryMediaComponent);
}
- } else {
- // Shouldn't reach this unless there is some other error in CarService
- Log.e(CarLog.TAG_MEDIA, "Error trying to save last media source, prefs uninitialized");
+ mRemovedMediaSourcePackage = null;
}
+
notifyListeners();
if (shouldStartPlayback(mPlayOnMediaSourceChangedConfig)) {
startMediaConnectorService(true, new UserHandle(mCurrentUser));
@@ -587,7 +608,6 @@
return false;
}
-
private boolean isMediaService(@NonNull ComponentName componentName) {
return getMediaService(componentName) != null;
}
@@ -635,6 +655,9 @@
}
private void saveLastMediaSource(@NonNull ComponentName component) {
+ if (!sharedPrefsInitialized()) {
+ return;
+ }
String componentName = component.flattenToString();
String key = SOURCE_KEY + mCurrentUser;
String serialized = mSharedPrefs.getString(key, null);
@@ -650,13 +673,15 @@
}
private ComponentName getLastMediaSource() {
- String key = SOURCE_KEY + mCurrentUser;
- String serialized = mSharedPrefs.getString(key, null);
- if (!TextUtils.isEmpty(serialized)) {
- for (String name : getComponentNameList(serialized)) {
- ComponentName componentName = ComponentName.unflattenFromString(name);
- if (isMediaService(componentName)) {
- return componentName;
+ if (sharedPrefsInitialized()) {
+ String key = SOURCE_KEY + mCurrentUser;
+ String serialized = mSharedPrefs.getString(key, null);
+ if (!TextUtils.isEmpty(serialized)) {
+ for (String name : getComponentNameList(serialized)) {
+ ComponentName componentName = ComponentName.unflattenFromString(name);
+ if (isMediaService(componentName)) {
+ return componentName;
+ }
}
}
}
@@ -682,11 +707,12 @@
}
private void savePlaybackState(PlaybackState playbackState) {
- int state = playbackState != null ? playbackState.getState() : PlaybackState.STATE_NONE;
- if (mSharedPrefs != null) {
- String key = getPlaybackStateKey();
- mSharedPrefs.edit().putInt(key, state).apply();
+ if (!sharedPrefsInitialized()) {
+ return;
}
+ int state = playbackState != null ? playbackState.getState() : PlaybackState.STATE_NONE;
+ String key = getPlaybackStateKey();
+ mSharedPrefs.edit().putInt(key, state).apply();
}
/**
diff --git a/service/src/com/android/car/hal/VmsHalService.java b/service/src/com/android/car/hal/VmsHalService.java
index bba5d5f..99263d7 100644
--- a/service/src/com/android/car/hal/VmsHalService.java
+++ b/service/src/com/android/car/hal/VmsHalService.java
@@ -17,8 +17,6 @@
import static com.android.car.CarServiceUtils.toByteArray;
-import static java.lang.Integer.toHexString;
-
import android.car.VehicleAreaType;
import android.car.vms.IVmsPublisherClient;
import android.car.vms.IVmsPublisherService;
@@ -43,6 +41,7 @@
import android.hardware.automotive.vehicle.V2_0.VmsOfferingMessageIntegerValuesIndex;
import android.hardware.automotive.vehicle.V2_0.VmsPublisherInformationIntegerValuesIndex;
import android.hardware.automotive.vehicle.V2_0.VmsStartSessionMessageIntegerValuesIndex;
+import android.os.Build;
import android.os.Handler;
import android.os.HandlerThread;
import android.os.IBinder;
@@ -54,7 +53,6 @@
import androidx.annotation.VisibleForTesting;
-import com.android.car.CarLog;
import com.android.car.vms.VmsClientManager;
import java.io.FileDescriptor;
@@ -87,6 +85,7 @@
private final int mCoreId;
private final MessageQueue mMessageQueue;
private final int mClientMetricsProperty;
+ private final boolean mPropagatePropertyException;
private volatile boolean mIsSupported = false;
private VmsClientManager mClientManager;
@@ -186,11 +185,7 @@
int messageType = msg.what;
VehiclePropValue vehicleProp = (VehiclePropValue) msg.obj;
if (DBG) Log.d(TAG, "Sending " + VmsMessageType.toString(messageType) + " message");
- try {
- setPropertyValue(vehicleProp);
- } catch (RemoteException e) {
- Log.e(TAG, "While sending " + VmsMessageType.toString(messageType));
- }
+ setPropertyValue(vehicleProp);
return true;
}
}
@@ -199,15 +194,17 @@
* Constructor used by {@link VehicleHal}
*/
VmsHalService(Context context, VehicleHal vehicleHal) {
- this(context, vehicleHal, SystemClock::uptimeMillis);
+ this(context, vehicleHal, SystemClock::uptimeMillis, (Build.IS_ENG || Build.IS_USERDEBUG));
}
@VisibleForTesting
- VmsHalService(Context context, VehicleHal vehicleHal, Supplier<Long> getCoreId) {
+ VmsHalService(Context context, VehicleHal vehicleHal, Supplier<Long> getCoreId,
+ boolean propagatePropertyException) {
mVehicleHal = vehicleHal;
mCoreId = (int) (getCoreId.get() % Integer.MAX_VALUE);
mMessageQueue = new MessageQueue();
mClientMetricsProperty = getClientMetricsProperty(context);
+ mPropagatePropertyException = propagatePropertyException;
}
private static int getClientMetricsProperty(Context context) {
@@ -326,8 +323,9 @@
VehiclePropValue vehicleProp = null;
try {
vehicleProp = mVehicleHal.get(mClientMetricsProperty);
- } catch (PropertyTimeoutException e) {
- Log.e(TAG, "Timeout while reading metrics from client");
+ } catch (PropertyTimeoutException | RuntimeException e) {
+ // Failures to retrieve metrics should be non-fatal
+ Log.e(TAG, "While reading metrics from client", e);
}
if (vehicleProp == null) {
if (DBG) Log.d(TAG, "Metrics unavailable");
@@ -395,7 +393,7 @@
Log.e(TAG, "Unexpected message type: " + messageType);
}
} catch (IndexOutOfBoundsException | RemoteException e) {
- Log.e(TAG, "While handling: " + messageType, e);
+ Log.e(TAG, "While handling " + VmsMessageType.toString(messageType), e);
}
}
}
@@ -425,9 +423,8 @@
mSubscriptionStateSequence = -1;
mAvailableLayersSequence = -1;
- // Enqueue an acknowledgement message
- mMessageQueue.enqueue(VmsMessageType.START_SESSION,
- createStartSessionMessage(mCoreId, clientId));
+ // Send acknowledgement message
+ setPropertyValue(createStartSessionMessage(mCoreId, clientId));
}
// Notify client manager of connection
@@ -678,7 +675,7 @@
mPublisherService.getSubscriptions()));
}
- private void setPropertyValue(VehiclePropValue vehicleProp) throws RemoteException {
+ private void setPropertyValue(VehiclePropValue vehicleProp) {
int messageType = vehicleProp.value.int32Values.get(
VmsBaseMessageIntegerValuesIndex.MESSAGE_TYPE);
@@ -690,11 +687,11 @@
try {
mVehicleHal.set(vehicleProp);
- } catch (PropertyTimeoutException e) {
- Log.e(CarLog.TAG_PROPERTY,
- "set, property not ready 0x" + toHexString(HAL_PROPERTY_ID));
- throw new RemoteException(
- "Timeout while sending " + VmsMessageType.toString(messageType));
+ } catch (PropertyTimeoutException | RuntimeException e) {
+ Log.e(TAG, "While sending " + VmsMessageType.toString(messageType), e.getCause());
+ if (mPropagatePropertyException) {
+ throw new IllegalStateException(e);
+ }
}
}
diff --git a/tests/carservice_unit_test/src/com/android/car/hal/VmsHalServiceTest.java b/tests/carservice_unit_test/src/com/android/car/hal/VmsHalServiceTest.java
index 7571867..093ab9b 100644
--- a/tests/carservice_unit_test/src/com/android/car/hal/VmsHalServiceTest.java
+++ b/tests/carservice_unit_test/src/com/android/car/hal/VmsHalServiceTest.java
@@ -17,7 +17,9 @@
import static org.junit.Assert.assertArrayEquals;
import static org.junit.Assert.assertEquals;
+import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyInt;
+import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.reset;
import static org.mockito.Mockito.verify;
@@ -44,8 +46,6 @@
import android.os.Binder;
import android.os.IBinder;
-import androidx.test.filters.RequiresDevice;
-
import com.android.car.R;
import com.android.car.test.utils.TemporaryFile;
import com.android.car.vms.VmsClientManager;
@@ -103,8 +103,13 @@
@Before
public void setUp() throws Exception {
+ initHalService(true);
+ }
+
+ private void initHalService(boolean propagatePropertyException) throws Exception {
when(mContext.getResources()).thenReturn(mResources);
- mHalService = new VmsHalService(mContext, mVehicleHal, () -> (long) CORE_ID);
+ mHalService = new VmsHalService(mContext, mVehicleHal, () -> (long) CORE_ID,
+ propagatePropertyException);
mHalService.setClientManager(mClientManager);
mHalService.setVmsSubscriberService(mSubscriberService);
@@ -158,6 +163,8 @@
0, // Sequence number
0)); // # of associated layers
+
+ waitForHandlerCompletion();
initOrder.verifyNoMoreInteractions();
reset(mClientManager, mSubscriberService, mVehicleHal);
}
@@ -165,7 +172,7 @@
@Test
public void testCoreId_IntegerOverflow() throws Exception {
mHalService = new VmsHalService(mContext, mVehicleHal,
- () -> (long) Integer.MAX_VALUE + CORE_ID);
+ () -> (long) Integer.MAX_VALUE + CORE_ID, true);
VehiclePropConfig propConfig = new VehiclePropConfig();
propConfig.prop = VehicleProperty.VEHICLE_MAP_SERVICE;
@@ -587,7 +594,6 @@
* </ul>
*/
@Test
- @RequiresDevice
public void testHandleStartSessionEvent() throws Exception {
when(mSubscriberService.getAvailableLayers()).thenReturn(
new VmsAvailableLayers(Collections.emptySet(), 5));
@@ -605,13 +611,18 @@
);
sendHalMessage(request);
+
InOrder inOrder = Mockito.inOrder(mClientManager, mVehicleHal);
inOrder.verify(mClientManager).onHalDisconnected();
inOrder.verify(mVehicleHal).set(response);
+ inOrder.verify(mClientManager).onHalConnected(mPublisherClient, mSubscriberClient);
+
+ waitForHandlerCompletion();
inOrder.verify(mVehicleHal).set(createHalMessage(
VmsMessageType.AVAILABILITY_CHANGE, // Message type
5, // Sequence number
0)); // # of associated layers
+
}
/**
@@ -962,6 +973,33 @@
}
@Test
+ public void testPropertySetExceptionNotPropagated_CoreStartSession() throws Exception {
+ doThrow(new RuntimeException()).when(mVehicleHal).set(any());
+ initHalService(false);
+
+ mHalService.init();
+ waitForHandlerCompletion();
+ }
+
+ @Test
+ public void testPropertySetExceptionNotPropagated_ClientStartSession() throws Exception {
+ initHalService(false);
+
+ when(mSubscriberService.getAvailableLayers()).thenReturn(
+ new VmsAvailableLayers(Collections.emptySet(), 0));
+ doThrow(new RuntimeException()).when(mVehicleHal).set(any());
+
+ VehiclePropValue request = createHalMessage(
+ VmsMessageType.START_SESSION, // Message type
+ -1, // Core ID (unknown)
+ CLIENT_ID // Client ID
+ );
+
+ sendHalMessage(request);
+ waitForHandlerCompletion();
+ }
+
+ @Test
public void testDumpMetrics_DefaultConfig() {
mHalService.dumpMetrics(new FileDescriptor());
verifyZeroInteractions(mVehicleHal);