Merge "Fixed CarUserService when UserHal times out." into tm-dev

3 files changed, 52 insertions, 10 deletions

diff --git a/service/src/com/android/car/user/CarUserService.java b/service/src/com/android/car/user/CarUserService.java
index 77d20284ce..c1e23c9545 100644
--- a/service/src/com/android/car/user/CarUserService.java
+++ b/service/src/com/android/car/user/CarUserService.java
@@ -986,8 +986,8 @@ public final class CarUserService extends ICarUserService.Stub implements CarSer
             Integer androidFailureStatus = null;
 
             synchronized (mLockUser) {
-                if (halCallbackStatus != HalCallback.STATUS_OK) {
-                    Slogf.w(TAG, "invalid callback status (%s) for response %s",
+                if (halCallbackStatus != HalCallback.STATUS_OK || resp == null) {
+                    Slogf.w(TAG, "invalid callback status (%s) or null response (%s)",
                             Integer.toString(halCallbackStatus), resp);
                     sendUserSwitchResult(receiver, isLogout, resultStatus);
                     mUserIdForUserSwitchInProcess = USER_NULL;
@@ -1397,21 +1397,22 @@ public final class CarUserService extends ICarUserService.Stub implements CarSer
 
         try {
             mHal.createUser(request, timeoutMs, (status, resp) -> {
+                String errorMessage = resp != null ? resp.errorMessage : null;
                 int resultStatus = UserCreationResult.STATUS_HAL_INTERNAL_FAILURE;
                 if (DBG) {
                     Slogf.d(TAG, "createUserResponse: status=%s, resp=%s",
                             UserHalHelper.halCallbackStatusToString(status), resp);
                 }
                 UserHandle user = null; // user returned in the result
-                if (status != HalCallback.STATUS_OK) {
-                    Slogf.w(TAG, "invalid callback status (%s) for response %s",
+                if (status != HalCallback.STATUS_OK || resp == null) {
+                    Slogf.w(TAG, "invalid callback status (%s) or null response (%s)",
                             UserHalHelper.halCallbackStatusToString(status), resp);
                     EventLogHelper.writeCarUserServiceCreateUserResp(status, resultStatus,
-                            resp.errorMessage);
+                            errorMessage);
                     removeCreatedUser(newUser, "HAL call failed with "
                             + UserHalHelper.halCallbackStatusToString(status));
                     sendUserCreationResult(receiver, resultStatus, /* androidFailureStatus= */ null,
-                            user, /* errorMessage= */ null,  /* internalErrorMessage= */ null);
+                            user, errorMessage,  /* internalErrorMessage= */ null);
                     return;
                 }
 
@@ -1429,13 +1430,13 @@ public final class CarUserService extends ICarUserService.Stub implements CarSer
                         Slogf.wtf(TAG, "Received invalid user switch status from HAL: %s", resp);
                 }
                 EventLogHelper.writeCarUserServiceCreateUserResp(status, resultStatus,
-                        resp.errorMessage);
+                        errorMessage);
                 if (user == null) {
                     removeCreatedUser(newUser, "HAL returned "
                             + UserCreationResult.statusToString(resultStatus));
                 }
                 sendUserCreationResult(receiver, resultStatus, /* androidFailureStatus= */ null,
-                        user, resp.errorMessage, /* internalErrorMessage= */ null);
+                        user, errorMessage, /* internalErrorMessage= */ null);
             });
         } catch (Exception e) {
             Slogf.w(TAG, e, "mHal.createUser(%s) failed", request);
@@ -1558,7 +1559,7 @@ public final class CarUserService extends ICarUserService.Stub implements CarSer
                 associations.toArray(new UserIdentificationSetAssociation[associations.size()]);
 
         mHal.setUserAssociation(timeoutMs, request, (status, resp) -> {
-            if (status != HalCallback.STATUS_OK) {
+            if (status != HalCallback.STATUS_OK || resp == null) {
                 Slogf.w(TAG, "setUserIdentificationAssociation(): invalid callback status (%s) for "
                         + "response %s", UserHalHelper.halCallbackStatusToString(status), resp);
                 if (resp == null || TextUtils.isEmpty(resp.errorMessage)) {
diff --git a/tests/carservice_unit_test/src/com/android/car/user/BaseCarUserServiceTestCase.java b/tests/carservice_unit_test/src/com/android/car/user/BaseCarUserServiceTestCase.java
index 53873830d6..50dc55b17e 100644
--- a/tests/carservice_unit_test/src/com/android/car/user/BaseCarUserServiceTestCase.java
+++ b/tests/carservice_unit_test/src/com/android/car/user/BaseCarUserServiceTestCase.java
@@ -567,6 +567,12 @@ abstract class BaseCarUserServiceTestCase extends AbstractExtendedMockitoTestCas
             @HalCallbackStatus int callbackStatus, int responseStatus) {
         CreateUserResponse response = new CreateUserResponse();
         response.status = responseStatus;
+        return mockHalCreateUser(callbackStatus, response);
+    }
+
+    @NonNull
+    protected ArgumentCaptor<CreateUserRequest> mockHalCreateUser(
+            @HalCallbackStatus int callbackStatus, CreateUserResponse response) {
         ArgumentCaptor<CreateUserRequest> captor = ArgumentCaptor.forClass(CreateUserRequest.class);
         doAnswer((invocation) -> {
             Log.d(TAG, "Answering " + invocation + " with " + response);
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 68611ec148..fa0e50874a 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
@@ -50,6 +50,7 @@ import static org.mockito.Mockito.when;
 import static org.testng.Assert.assertThrows;
 import static org.testng.Assert.expectThrows;
 
+import android.annotation.Nullable;
 import android.app.ActivityManager;
 import android.car.ICarResultReceiver;
 import android.car.builtin.app.ActivityManagerHelper;
@@ -1024,6 +1025,19 @@ public final class CarUserServiceTest extends BaseCarUserServiceTestCase {
     }
 
     @Test
+    public void testSwitchUser_nullHalResponse() throws Exception {
+        mockExistingUsersAndCurrentUser(mAdminUser);
+        mockHalSwitch(mAdminUserId, mGuestUser, /* response= */ null);
+
+        switchUser(mGuestUserId, mAsyncCallTimeoutMs, mUserSwitchFuture);
+
+        assertUserSwitchResultWithError(getUserSwitchResult(mGuestUserId),
+                UserSwitchResult.STATUS_HAL_INTERNAL_FAILURE, /* expectedErrorMessage= */ null);
+        verifyNoUserSwitch();
+        verifyNoLogoutUser();
+    }
+
+    @Test
     public void testSwitchUser_error_badCallbackStatus() throws Exception {
         mockExistingUsersAndCurrentUser(mAdminUser);
         mSwitchUserResponse.status = SwitchUserStatus.SUCCESS;
@@ -1682,6 +1696,27 @@ public final class CarUserServiceTest extends BaseCarUserServiceTestCase {
     }
 
     @Test
+    public void testCreateUser_halTimeout() throws Exception {
+        UserHandle newUser = UserHandle.of(42);
+        mockUmCreateUser(mMockedUserManager, "dude", "TypeONegative", 108, newUser);
+        mockHalCreateUser(HalCallback.STATUS_HAL_SET_TIMEOUT, /* response= */ null);
+        mockRemoveUser(newUser);
+
+        createUser("dude", "TypeONegative", 108, mAsyncCallTimeoutMs, mUserCreationFuture,
+                NO_CALLER_RESTRICTIONS);
+
+        UserCreationResult result = getUserCreationResult();
+        assertThat(result.getStatus()).isEqualTo(UserCreationResult.STATUS_HAL_INTERNAL_FAILURE);
+        assertThat(result.getAndroidFailureStatus()).isNull();
+        assertThat(result.getUser()).isNull();
+        assertThat(result.getErrorMessage()).isNull();
+        assertThat(result.getInternalErrorMessage()).isNull();
+
+        verifyUserRemoved(newUser.getIdentifier());
+        assertNoHalUserRemoval();
+    }
+
+    @Test
     public void testCreateUser_halServiceThrowsRuntimeException() throws Exception {
         UserHandle newUser = UserHandle.of(42);
         mockUmCreateUser(mMockedUserManager, "dude", "TypeONegative", 108, newUser);
@@ -2502,7 +2537,7 @@ public final class CarUserServiceTest extends BaseCarUserServiceTestCase {
     }
 
     private void assertUserSwitchResultWithError(UserSwitchResult result, int expectedStatus,
-            String expectedErrorMessage) {
+            @Nullable String expectedErrorMessage) {
         assertUserSwitchResult(result.getStatus(), expectedStatus);
         assertWithMessage("error message on %s", result).that(result.getErrorMessage())
                 .isEqualTo(expectedErrorMessage);