Changed UserLifecycleEvent to use (int) user ids internally.
It must expose UserHandle to the (@System) public methods, but as this
event will also used by intenral services, it's better to keep the
user id as the "cannonical" value and create the UserHandles on demand.
Test: atest UserLifecycleEventTest
Fixes: 152043575
Change-Id: I8ed5aa239565774b3c14d634585d049d4df3f13b
diff --git a/car-lib/src/android/car/user/CarUserManager.java b/car-lib/src/android/car/user/CarUserManager.java
index b2515ed..1fc0393 100644
--- a/car-lib/src/android/car/user/CarUserManager.java
+++ b/car-lib/src/android/car/user/CarUserManager.java
@@ -143,7 +143,7 @@
/** @hide */
public static final String BUNDLE_PARAM_ACTION = "action";
/** @hide */
- public static final String BUNDLE_PARAM_PREVIOUS_USER_HANDLE = "previous_user";
+ public static final String BUNDLE_PARAM_PREVIOUS_USER_ID = "previous_user";
private final Object mLock = new Object();
private final ICarUserService mService;
@@ -418,10 +418,10 @@
Log.w(TAG, "Received result (" + resultCode + ") without data");
return;
}
- UserHandle toHandle = new UserHandle(resultCode);
- UserHandle fromHandle = resultData.getParcelable(BUNDLE_PARAM_PREVIOUS_USER_HANDLE);
+ int from = resultData.getInt(BUNDLE_PARAM_PREVIOUS_USER_ID, UserHandle.USER_NULL);
+ int to = resultCode;
int eventType = resultData.getInt(BUNDLE_PARAM_ACTION);
- UserLifecycleEvent event = new UserLifecycleEvent(eventType, fromHandle, toHandle);
+ UserLifecycleEvent event = new UserLifecycleEvent(eventType, from, to);
ArrayMap<UserLifecycleListener, Executor> listeners;
synchronized (mLock) {
listeners = mListeners;
@@ -522,15 +522,20 @@
@TestApi
public static final class UserLifecycleEvent {
private final @UserLifecycleEventType int mEventType;
- private final @NonNull UserHandle mUserHandle;
- private final @Nullable UserHandle mPreviousUserHandle;
+ private final @UserIdInt int mUserId;
+ private final @UserIdInt int mPreviousUserId;
/** @hide */
public UserLifecycleEvent(@UserLifecycleEventType int eventType,
- @NonNull UserHandle from, @Nullable UserHandle to) {
+ @UserIdInt int from, @UserIdInt int to) {
mEventType = eventType;
- mPreviousUserHandle = from;
- mUserHandle = to;
+ mPreviousUserId = from;
+ mUserId = to;
+ }
+
+ /** @hide */
+ public UserLifecycleEvent(@UserLifecycleEventType int eventType, @UserIdInt int to) {
+ this(eventType, UserHandle.USER_NULL, to);
}
/**
@@ -549,11 +554,34 @@
}
/**
+ * Gets the id of the user whose event is being reported.
+ *
+ * @hide
+ */
+ @UserIdInt
+ public int getUserId() {
+ return mUserId;
+ }
+
+ /**
* Gets the handle of the user whose event is being reported.
*/
@NonNull
public UserHandle getUserHandle() {
- return mUserHandle;
+ return UserHandle.of(mUserId);
+ }
+
+ /**
+ * Gets the id of the user being switched from.
+ *
+ * <p>This method returns {@link UserHandle#USER_NULL} for all event types but
+ * {@link CarUserManager#USER_LIFECYCLE_EVENT_TYPE_SWITCHING}.
+ *
+ * @hide
+ */
+ @UserIdInt
+ public int getPreviousUserId() {
+ return mPreviousUserId;
}
/**
@@ -564,19 +592,19 @@
*/
@Nullable
public UserHandle getPreviousUserHandle() {
- return mPreviousUserHandle;
+ return mPreviousUserId == UserHandle.USER_NULL ? null : UserHandle.of(mPreviousUserId);
}
@Override
public String toString() {
StringBuilder builder = new StringBuilder("Event[type=")
.append(lifecycleEventTypeToString(mEventType));
- if (mPreviousUserHandle != null) {
+ if (mPreviousUserId != UserHandle.USER_NULL) {
builder
- .append(",from=").append(mPreviousUserHandle)
- .append(",to=").append(mUserHandle);
+ .append(",from=").append(mPreviousUserId)
+ .append(",to=").append(mUserId);
} else {
- builder.append(",user=").append(mUserHandle);
+ builder.append(",user=").append(mUserId);
}
return builder.append(']').toString();
diff --git a/service/src/com/android/car/CarMediaService.java b/service/src/com/android/car/CarMediaService.java
index 3b111d8..e5f8ea5 100644
--- a/service/src/com/android/car/CarMediaService.java
+++ b/service/src/com/android/car/CarMediaService.java
@@ -178,7 +178,7 @@
Log.d(CarLog.TAG_MEDIA, "CarMediaService.onEvent(" + event + ")");
}
if (CarUserManager.USER_LIFECYCLE_EVENT_TYPE_SWITCHING == event.getEventType()) {
- maybeInitUser(event.getUserHandle().getIdentifier());
+ maybeInitUser(event.getUserId());
}
};
diff --git a/service/src/com/android/car/PerUserCarServiceHelper.java b/service/src/com/android/car/PerUserCarServiceHelper.java
index cbe701e..1343d92 100644
--- a/service/src/com/android/car/PerUserCarServiceHelper.java
+++ b/service/src/com/android/car/PerUserCarServiceHelper.java
@@ -81,7 +81,7 @@
}
if (CarUserManager.USER_LIFECYCLE_EVENT_TYPE_SWITCHING == event.getEventType()) {
List<ServiceCallback> callbacks;
- int userId = event.getUserHandle().getIdentifier();
+ int userId = event.getUserId();
if (DBG) {
Log.d(TAG, "User Switch Happened. New User" + userId);
}
diff --git a/service/src/com/android/car/pm/VendorServiceController.java b/service/src/com/android/car/pm/VendorServiceController.java
index be1b6a8..ffe4558 100644
--- a/service/src/com/android/car/pm/VendorServiceController.java
+++ b/service/src/com/android/car/pm/VendorServiceController.java
@@ -132,14 +132,14 @@
if (CarUserManager.USER_LIFECYCLE_EVENT_TYPE_UNLOCKING == event.getEventType()) {
Message msg = mHandler.obtainMessage(
MSG_USER_LOCK_CHANGED,
- event.getUserHandle().getIdentifier(),
+ event.getUserId(),
/* unlocked= */ 1);
mHandler.executeOrSendMessage(msg);
} else if (CarUserManager.USER_LIFECYCLE_EVENT_TYPE_SWITCHING == event.getEventType()) {
mHandler.removeMessages(MSG_SWITCH_USER);
Message msg = mHandler.obtainMessage(
MSG_SWITCH_USER,
- event.getUserHandle().getIdentifier(),
+ event.getUserId(),
/* unlocked= */ 0);
mHandler.executeOrSendMessage(msg);
}
diff --git a/service/src/com/android/car/user/CarUserNoticeService.java b/service/src/com/android/car/user/CarUserNoticeService.java
index 698f4ce..3e0891c 100644
--- a/service/src/com/android/car/user/CarUserNoticeService.java
+++ b/service/src/com/android/car/user/CarUserNoticeService.java
@@ -119,7 +119,7 @@
stopUi(/* clearUiShown= */ true);
synchronized (mLock) {
// This should be the only place to change user
- mUserId = event.getUserHandle().getIdentifier();
+ mUserId = event.getUserId();
}
startNoticeUiIfNecessary();
});
diff --git a/service/src/com/android/car/user/CarUserService.java b/service/src/com/android/car/user/CarUserService.java
index 0becaf7..114d307 100644
--- a/service/src/com/android/car/user/CarUserService.java
+++ b/service/src/com/android/car/user/CarUserService.java
@@ -643,12 +643,8 @@
public void setUserLockStatus(@UserIdInt int userId, boolean unlocked) {
TimingsTraceLog t = new TimingsTraceLog(TAG_USER,
Trace.TRACE_TAG_SYSTEM_SERVER);
- // TODO(b/152043575): we should overload the UserLifecycleEvent constructor with a
- // /* hidden */ method that takes int userId directly.
- notifyUserLifecycleListeners(t, new UserLifecycleEvent(
- CarUserManager.USER_LIFECYCLE_EVENT_TYPE_UNLOCKING,
- /* from= */ null,
- /* to= */ UserHandle.of(userId)));
+ notifyUserLifecycleListeners(t,
+ new UserLifecycleEvent(CarUserManager.USER_LIFECYCLE_EVENT_TYPE_UNLOCKING, userId));
if (!unlocked) { // nothing else to do when it is locked back.
return;
@@ -821,7 +817,7 @@
data.putInt(CarUserManager.BUNDLE_PARAM_ACTION,
CarUserManager.USER_LIFECYCLE_EVENT_TYPE_SWITCHING);
// TODO(b/144120654): should pass currentId from CarServiceHelperService so it
- // can set BUNDLE_PARAM_PREVIOUS_USER_HANDLE
+ // can set BUNDLE_PARAM_PREVIOUS_USER_ID (and unit test it)
if (Log.isLoggable(TAG_USER, Log.DEBUG)) {
Log.d(TAG_USER, "Notifying listener for uid " + uid);
}
@@ -837,13 +833,10 @@
}, "SwitchUser-" + userId + "-Listeners").start();
}
- // TODO(b/145689885): passing null for `from` parameter until it gets properly replaced
+ // TODO(b/145689885): not passing `from` parameter until it gets properly replaced
// the expected Binder call.
- // TODO(b/152043575): we should overload the UserLifecycleEvent constructor with a
- // /* hidden */ method that takes int userId directly.
- notifyUserLifecycleListeners(t, new UserLifecycleEvent(
- /* eventType= */ CarUserManager.USER_LIFECYCLE_EVENT_TYPE_SWITCHING,
- /* from= */ null, /* to= */ new UserHandle(userId)));
+ notifyUserLifecycleListeners(t,
+ new UserLifecycleEvent(CarUserManager.USER_LIFECYCLE_EVENT_TYPE_SWITCHING, userId));
}
private void notifyUserLifecycleListeners(TimingsTraceLog t, UserLifecycleEvent event) {
diff --git a/tests/carservice_unit_test/src/com/android/car/CarOccupantZoneServiceTest.java b/tests/carservice_unit_test/src/com/android/car/CarOccupantZoneServiceTest.java
index 3517e13..c5bba05 100644
--- a/tests/carservice_unit_test/src/com/android/car/CarOccupantZoneServiceTest.java
+++ b/tests/carservice_unit_test/src/com/android/car/CarOccupantZoneServiceTest.java
@@ -457,9 +457,7 @@
final int newUserId = 100;
doReturn(newUserId).when(mService).getCurrentUser();
mService.mUserLifecycleListener.onEvent(new UserLifecycleEvent(
- CarUserManager.USER_LIFECYCLE_EVENT_TYPE_SWITCHING,
- /* from= */ null,
- /* to= */ UserHandle.of(newUserId)));
+ CarUserManager.USER_LIFECYCLE_EVENT_TYPE_SWITCHING, newUserId));
// key : zone id
HashMap<Integer, OccupantConfig> configs = mService.getActiveOccupantConfigs();
@@ -676,9 +674,7 @@
final int newUserId = 100;
doReturn(newUserId).when(mService).getCurrentUser();
mService.mUserLifecycleListener.onEvent(new UserLifecycleEvent(
- CarUserManager.USER_LIFECYCLE_EVENT_TYPE_SWITCHING,
- /* from= */ null,
- /* to= */ UserHandle.of(newUserId)));
+ CarUserManager.USER_LIFECYCLE_EVENT_TYPE_SWITCHING, newUserId));
assertThat(newUserId).isEqualTo(mManager.getUserForOccupant(mZoneDriverLHD));
//TODO update this after secondary user handling
@@ -698,9 +694,7 @@
resetConfigChangeEventWait();
mService.mUserLifecycleListener.onEvent(new UserLifecycleEvent(
- CarUserManager.USER_LIFECYCLE_EVENT_TYPE_SWITCHING,
- /* from= */ null,
- /* to= */ UserHandle.of(0))); // user id does not matter.
+ CarUserManager.USER_LIFECYCLE_EVENT_TYPE_SWITCHING, 0)); // user id does not matter.
assertThat(waitForConfigChangeEventAndAssertFlag(eventWaitTimeMs,
CarOccupantZoneManager.ZONE_CONFIG_CHANGE_FLAG_USER)).isTrue();
@@ -713,9 +707,7 @@
resetConfigChangeEventWait();
mManager.unregisterOccupantZoneConfigChangeListener(mChangeListener);
mService.mUserLifecycleListener.onEvent(new UserLifecycleEvent(
- CarUserManager.USER_LIFECYCLE_EVENT_TYPE_SWITCHING,
- /* from= */ null,
- /* to= */ UserHandle.of(0)));
+ CarUserManager.USER_LIFECYCLE_EVENT_TYPE_SWITCHING, 0));
assertThat(waitForConfigChangeEventAndAssertFlag(eventWaitTimeMs, 0)).isFalse();
}
diff --git a/tests/carservice_unit_test/src/com/android/car/user/CarUserNoticeServiceTest.java b/tests/carservice_unit_test/src/com/android/car/user/CarUserNoticeServiceTest.java
index d9b9b14..842af3a 100644
--- a/tests/carservice_unit_test/src/com/android/car/user/CarUserNoticeServiceTest.java
+++ b/tests/carservice_unit_test/src/com/android/car/user/CarUserNoticeServiceTest.java
@@ -237,9 +237,7 @@
private void switchUser(int userId) throws Exception {
// Notify listeners about user switch.
mUserLifecycleListenerArgumentCaptor.getValue().onEvent(new UserLifecycleEvent(
- CarUserManager.USER_LIFECYCLE_EVENT_TYPE_SWITCHING,
- /* from= */ null,
- /* to= */ UserHandle.of(userId)));
+ CarUserManager.USER_LIFECYCLE_EVENT_TYPE_SWITCHING, userId));
}
private CountDownLatch mockBindService() {
diff --git a/tests/carservice_unit_test/src/com/android/car/user/CarUserServiceTest.java b/tests/carservice_unit_test/src/com/android/car/user/CarUserServiceTest.java
index cee943b..4cbaa33 100644
--- a/tests/carservice_unit_test/src/com/android/car/user/CarUserServiceTest.java
+++ b/tests/carservice_unit_test/src/com/android/car/user/CarUserServiceTest.java
@@ -262,7 +262,7 @@
verify(mUserLifecycleListener).onEvent(mLifeCycleEventCaptor.capture());
UserLifecycleEvent actualEvent = mLifeCycleEventCaptor.getValue();
assertThat(actualEvent.getEventType()).isEqualTo(expectedEventType);
- assertThat(actualEvent.getUserHandle().getIdentifier()).isEqualTo(expectedNewUserId);
+ assertThat(actualEvent.getUserId()).isEqualTo(expectedNewUserId);
}
/**
diff --git a/tests/carservice_unit_test/src/com/android/car/user/UserLifecycleEventTest.java b/tests/carservice_unit_test/src/com/android/car/user/UserLifecycleEventTest.java
new file mode 100644
index 0000000..28bffde
--- /dev/null
+++ b/tests/carservice_unit_test/src/com/android/car/user/UserLifecycleEventTest.java
@@ -0,0 +1,53 @@
+/*
+ * Copyright (C) 2020 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.car.user;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import android.car.user.CarUserManager.UserLifecycleEvent;
+import android.os.UserHandle;
+
+import org.junit.Test;
+
+public final class UserLifecycleEventTest {
+
+ @Test
+ public void testFullConstructor() {
+ int eventType = 42;
+ int from = 10;
+ int to = 20;
+ UserLifecycleEvent event = new UserLifecycleEvent(eventType, from, to);
+
+ assertThat(event.getEventType()).isEqualTo(eventType);
+ assertThat(event.getUserId()).isEqualTo(to);
+ assertThat(event.getUserHandle().getIdentifier()).isEqualTo(to);
+ assertThat(event.getPreviousUserId()).isEqualTo(from);
+ assertThat(event.getPreviousUserHandle().getIdentifier()).isEqualTo(from);
+ }
+
+ @Test
+ public void testAlternativeConstructor() {
+ int eventType = 42;
+ int to = 20;
+ UserLifecycleEvent event = new UserLifecycleEvent(eventType, to);
+
+ assertThat(event.getEventType()).isEqualTo(eventType);
+ assertThat(event.getUserId()).isEqualTo(to);
+ assertThat(event.getUserHandle().getIdentifier()).isEqualTo(to);
+ assertThat(event.getPreviousUserId()).isEqualTo(UserHandle.USER_NULL);
+ assertThat(event.getPreviousUserHandle()).isNull();
+ }
+}