Merge "Fixes newUsersInfo() to ignore exceptions converting user flags." into tm-dev

3 files changed, 57 insertions, 17 deletions

diff --git a/service/src/com/android/car/hal/UserHalHelper.java b/service/src/com/android/car/hal/UserHalHelper.java
index 06185ba6a4..7c6f13e72e 100644
--- a/service/src/com/android/car/hal/UserHalHelper.java
+++ b/service/src/com/android/car/hal/UserHalHelper.java
@@ -594,21 +594,26 @@ public final class UserHalHelper {
         UsersInfo usersInfo = emptyUsersInfo();
         usersInfo.currentUser.userId = userId;
         UserHandle currentUser = null;
-        usersInfo.numberUsers = users.size();
-
-        ArrayList<UserInfo> halUsers = new ArrayList<>();
-        for (int i = 0; i < usersInfo.numberUsers; i++) {
+        int allUsersSize = users.size();
+        ArrayList<UserInfo> halUsers = new ArrayList<>(allUsersSize);
+        for (int i = 0; i < allUsersSize; i++) {
             UserHandle user = users.get(i);
-            if (user.getIdentifier() == usersInfo.currentUser.userId) {
-                currentUser = user;
+            try {
+                if (user.getIdentifier() == usersInfo.currentUser.userId) {
+                    currentUser = user;
+                }
+                UserInfo halUser = new UserInfo();
+                halUser.userId = user.getIdentifier();
+                halUser.flags = convertFlags(userHandleHelper, user);
+                halUsers.add(halUser);
+            } catch (Exception e) {
+                // Most likely the user was removed
+                Log.w(TAG, "newUsersInfo(): ignoring user " + user + " due to exception", e);
             }
-            UserInfo halUser = new UserInfo();
-            halUser.userId = user.getIdentifier();
-            halUser.flags = convertFlags(userHandleHelper, user);
-            halUsers.add(halUser);
         }
-
-        usersInfo.existingUsers = halUsers.toArray(new UserInfo[halUsers.size()]);
+        int existingUsersSize = halUsers.size();
+        usersInfo.numberUsers = existingUsersSize;
+        usersInfo.existingUsers = halUsers.toArray(new UserInfo[existingUsersSize]);
 
         if (currentUser != null) {
             usersInfo.currentUser.flags = convertFlags(userHandleHelper, currentUser);
diff --git a/tests/carservice_unit_test/src/com/android/car/hal/UserHalHelperTest.java b/tests/carservice_unit_test/src/com/android/car/hal/UserHalHelperTest.java
index 8c9c34c43e..888c7c8816 100644
--- a/tests/carservice_unit_test/src/com/android/car/hal/UserHalHelperTest.java
+++ b/tests/carservice_unit_test/src/com/android/car/hal/UserHalHelperTest.java
@@ -41,6 +41,7 @@ import static com.android.car.user.MockedUserHandleBuilder.expectEphemeralUserEx
 import static com.android.car.user.MockedUserHandleBuilder.expectGuestUserExists;
 import static com.android.car.user.MockedUserHandleBuilder.expectRegularUserExists;
 import static com.android.car.user.MockedUserHandleBuilder.expectSystemUserExists;
+import static com.android.car.user.MockedUserHandleBuilder.expectUserExistsButGettersFail;
 
 import static com.google.common.truth.Truth.assertThat;
 import static com.google.common.truth.Truth.assertWithMessage;
@@ -1347,6 +1348,30 @@ public final class UserHalHelperTest extends AbstractExtendedMockitoTestCase {
     }
 
     @Test
+    public void testNewUsersInfo_flagConversionFails() {
+        UserHandle user100 = expectAdminUserExists(mUserHandleHelper, 100);
+        UserHandle user200 = expectUserExistsButGettersFail(mUserHandleHelper, 200);
+        UserHandle user300 = expectRegularUserExists(mUserHandleHelper, 300);
+
+        mockGetAllUsers(user100, user200, user300);
+        mockAmGetCurrentUser(300); // just to make sure it's not used
+
+        UsersInfo usersInfo = UserHalHelper.newUsersInfo(mUm, mUserHandleHelper);
+
+        assertThat(usersInfo).isNotNull();
+        assertThat(usersInfo.currentUser.userId).isEqualTo(300);
+        assertThat(usersInfo.currentUser.flags).isEqualTo(0);
+
+        assertThat(usersInfo.numberUsers).isEqualTo(2);
+        assertThat(usersInfo.existingUsers.length).isEqualTo(2);
+
+        assertThat(usersInfo.existingUsers[0].userId).isEqualTo(100);
+        assertThat(usersInfo.existingUsers[0].flags).isEqualTo(UserInfo.USER_FLAG_ADMIN);
+        assertThat(usersInfo.existingUsers[1].userId).isEqualTo(300);
+        assertThat(usersInfo.existingUsers[1].flags).isEqualTo(0);
+    }
+
+    @Test
     public void testCheckValidUsersInfo_null() {
         assertThrows(IllegalArgumentException.class, () -> UserHalHelper.checkValid(null));
     }
diff --git a/tests/carservice_unit_test/src/com/android/car/user/MockedUserHandleBuilder.java b/tests/carservice_unit_test/src/com/android/car/user/MockedUserHandleBuilder.java
index 18e42bcdbe..3ca951244a 100644
--- a/tests/carservice_unit_test/src/com/android/car/user/MockedUserHandleBuilder.java
+++ b/tests/carservice_unit_test/src/com/android/car/user/MockedUserHandleBuilder.java
@@ -54,11 +54,6 @@ public final class MockedUserHandleBuilder {
         return this;
     }
 
-    private MockedUserHandleBuilder setProfile() {
-        when(mUserHandleHelper.isProfileUser(mUser)).thenReturn(true);
-        return this;
-    }
-
     private MockedUserHandleBuilder setManagedProfile() {
         when(mUserHandleHelper.isManagedProfile(mUser)).thenReturn(true);
         return this;
@@ -74,6 +69,16 @@ public final class MockedUserHandleBuilder {
         return this;
     }
 
+    private MockedUserHandleBuilder expectGettersFail() {
+        RuntimeException exception = new RuntimeException("D'OH!");
+        when(mUserHandleHelper.isAdminUser(mUser)).thenThrow(exception);
+        when(mUserHandleHelper.isEnabledUser(mUser)).thenThrow(exception);
+        when(mUserHandleHelper.isProfileUser(mUser)).thenThrow(exception);
+        when(mUserHandleHelper.isPreCreatedUser(mUser)).thenThrow(exception);
+        when(mUserHandleHelper.isInitializedUser(mUser)).thenThrow(exception);
+        return this;
+    }
+
     private UserHandle build() {
         return mUser;
     }
@@ -83,6 +88,11 @@ public final class MockedUserHandleBuilder {
         return new MockedUserHandleBuilder(userHandleHelper, userId).build();
     }
 
+    public static UserHandle expectUserExistsButGettersFail(
+            @NonNull UserHandleHelper userHandleHelper, @UserIdInt int userId) {
+        return new MockedUserHandleBuilder(userHandleHelper, userId).expectGettersFail().build();
+    }
+
     public static UserHandle expectSystemUserExists(@NonNull UserHandleHelper userHandleHelper,
             @UserIdInt int userId) {
         return new MockedUserHandleBuilder(userHandleHelper, userId).build();