Sets the removed user flags on REMOVE_USER UserHal message.

Test: atest CarUserServiceTest
Test: manual verification
Fixes: 155913815

Change-Id: I8a9e3e2008dd03545e1f0a8562754040d87813da

5 files changed, 93 insertions, 68 deletions

diff --git a/car-internal-lib/src/com/android/car/internal/ICarSystemServerClient.aidl b/car-internal-lib/src/com/android/car/internal/ICarSystemServerClient.aidl
index e1a9e68e77..b05c272851 100644
--- a/car-internal-lib/src/com/android/car/internal/ICarSystemServerClient.aidl
+++ b/car-internal-lib/src/com/android/car/internal/ICarSystemServerClient.aidl
@@ -16,10 +16,12 @@
 
 package com.android.car.internal;
 
+import android.content.pm.UserInfo;
+
 /**
  * API to communicate from CarServiceHelperService to car service.
  */
-interface ICarSystemServerClient {
+oneway interface ICarSystemServerClient {
     /**
      * Notify of user lifecycle events.
      *
@@ -27,15 +29,24 @@ interface ICarSystemServerClient {
      * @param fromUserId - user id of previous user when type is SWITCHING (or UserHandle.USER_NULL)
      * @param toUserId - user id of new user.
      */
-    oneway void onUserLifecycleEvent(int eventType, int fromUserId, int toUserId);
+    void onUserLifecycleEvent(int eventType, int fromUserId, int toUserId);
+
+    /**
+     * Nofity when a user is removed.
+     *
+     * NOTE: this is different from onUserLifecycleEvent(), whic is used on user switching events.
+     *
+     * @param user info about the user that was removed.
+     */
+    void onUserRemoved(in UserInfo user);
 
     /**
      * Notify to init boot user.
      */
-    oneway void initBootUser();
+    void initBootUser();
 
     /**
      * Notify to pre-create users.
      */
-    oneway void preCreateUsers();
+    void preCreateUsers();
 }
diff --git a/car-test-lib/src/android/car/test/mocks/AndroidMockitoHelper.java b/car-test-lib/src/android/car/test/mocks/AndroidMockitoHelper.java
index 68cfdf0a4b..efdd4e9595 100644
--- a/car-test-lib/src/android/car/test/mocks/AndroidMockitoHelper.java
+++ b/car-test-lib/src/android/car/test/mocks/AndroidMockitoHelper.java
@@ -24,8 +24,10 @@ import android.annotation.NonNull;
 import android.annotation.Nullable;
 import android.annotation.UserIdInt;
 import android.app.ActivityManager;
+import android.car.test.util.AndroidHelper;
 import android.car.test.util.UserTestingHelper;
 import android.car.test.util.UserTestingHelper.UserInfoBuilder;
+import android.car.test.util.Visitor;
 import android.content.BroadcastReceiver;
 import android.content.Context;
 import android.content.Intent;
@@ -176,17 +178,43 @@ public final class AndroidMockitoHelper {
     }
 
     /**
+     * Mocks a successful call to {@code UserManager#removeUserOrSetEphemeral(int)}.
+     */
+    public static void mockUmRemoveUserOrSetEphemeral(@NonNull UserManager um,
+            @NonNull UserInfo user) {
+        mockUmRemoveUserOrSetEphemeral(um, user, /* listener= */ null);
+    }
+
+    /**
      * Mocks a successful call to {@code UserManager#removeUserOrSetEphemeral(int)}, including the
      * respective {@code receiver} notification.
      */
     public static void mockUmRemoveUserOrSetEphemeral(@NonNull Context context,
             @NonNull UserManager um, @NonNull BroadcastReceiver receiver, @NonNull UserInfo user) {
-        int userId = user.id;
-        when(um.removeUserOrSetEphemeral(userId)).thenAnswer((inv) -> {
+        mockUmRemoveUserOrSetEphemeral(um, user, (u) -> {
+            int userId = u.id;
             Intent intent = new Intent(Intent.ACTION_USER_REMOVED);
+            intent.putExtra(Intent.EXTRA_USER_ID, userId);
             intent.putExtra(Intent.EXTRA_USER_HANDLE, userId);
-            Log.v(TAG, "mockUmRemoveUser(): broadcasting " + intent + " to " + receiver);
+            intent.putExtra(Intent.EXTRA_USER, UserHandle.of(userId));
+            Log.v(TAG, "mockUmRemoveUserOrSetEphemeral(): broadcasting "
+                    + AndroidHelper.toString(intent) + " to " + receiver);
             receiver.onReceive(context, intent);
+        });
+    }
+
+    /**
+     * Mocks a successful call to {@code UserManager#removeUserOrSetEphemeral(int)}, and notifies
+     * {@code listener} when it's called.
+     */
+    public static void mockUmRemoveUserOrSetEphemeral(@NonNull UserManager um,
+            @NonNull UserInfo user, @Nullable Visitor<UserInfo> listener) {
+        int userId = user.id;
+        when(um.removeUserOrSetEphemeral(userId)).thenAnswer((inv) -> {
+            if (listener != null) {
+                Log.v(TAG, "mockUmRemoveUserOrSetEphemeral(" + user + "): notifying " + listener);
+                listener.visit(user);
+            }
             return UserManager.REMOVE_RESULT_REMOVED;
         });
     }
diff --git a/service/src/com/android/car/ICarImpl.java b/service/src/com/android/car/ICarImpl.java
index f58ac9b313..21a4d79a39 100644
--- a/service/src/com/android/car/ICarImpl.java
+++ b/service/src/com/android/car/ICarImpl.java
@@ -28,6 +28,7 @@ import android.car.cluster.renderer.IInstrumentClusterNavigation;
 import android.car.user.CarUserManager;
 import android.content.Context;
 import android.content.pm.PackageManager;
+import android.content.pm.UserInfo;
 import android.content.res.Resources;
 import android.hardware.automotive.vehicle.V2_0.IVehicle;
 import android.hardware.automotive.vehicle.V2_0.VehiclePropValue;
@@ -836,5 +837,13 @@ public class ICarImpl extends ICar.Stub {
             if (DBG) Log.d(TAG, "preCreateUsers(): ");
             mCarUserService.preCreateUsers();
         }
+
+        @Override
+        public void onUserRemoved(UserInfo user) throws RemoteException {
+            assertCallingFromSystemProcess();
+            // TODO(b/160819016): add events log
+            if (DBG) Log.d(TAG, "onUserRemoved(): " + user.toFullString());
+            mCarUserService.onUserRemoved(user);
+        }
     }
 }
diff --git a/service/src/com/android/car/user/CarUserService.java b/service/src/com/android/car/user/CarUserService.java
index a49cbdffff..5f3daf5acf 100644
--- a/service/src/com/android/car/user/CarUserService.java
+++ b/service/src/com/android/car/user/CarUserService.java
@@ -49,11 +49,8 @@ import android.car.user.UserSwitchResult;
 import android.car.userlib.HalCallback;
 import android.car.userlib.UserHalHelper;
 import android.car.userlib.UserHelper;
-import android.content.BroadcastReceiver;
 import android.content.ComponentName;
 import android.content.Context;
-import android.content.Intent;
-import android.content.IntentFilter;
 import android.content.pm.PackageManager;
 import android.content.pm.PackageManager.NameNotFoundException;
 import android.content.pm.UserInfo;
@@ -275,23 +272,6 @@ public final class CarUserService extends ICarUserService.Stub implements CarSer
     @GuardedBy("mLockHelper")
     private ZoneUserBindingHelper mZoneUserBindingHelper;
 
-    private final BroadcastReceiver mUserLifecycleReceiver = new BroadcastReceiver() {
-        @Override
-        public void onReceive(Context context, Intent intent) {
-            if (Log.isLoggable(TAG_USER, Log.DEBUG)) {
-                Log.d(TAG_USER, "onReceive: " + intent);
-            }
-            switch(intent.getAction()) {
-                case Intent.ACTION_USER_REMOVED:
-                    int userId = intent.getIntExtra(Intent.EXTRA_USER_HANDLE, UserHandle.USER_NULL);
-                    notifyHalUserRemoved(userId);
-                    break;
-                default:
-                    Log.w(TAG, "received unexpected intent: " + intent);
-            }
-        }
-    };
-
     /** Map used to avoid calling UserHAL when a user was removed because HAL creation failed. */
     @GuardedBy("mLockUser")
     private final SparseBooleanArray mFailedToCreateUserIds = new SparseBooleanArray(1);
@@ -337,8 +317,6 @@ public final class CarUserService extends ICarUserService.Stub implements CarSer
         if (Log.isLoggable(TAG_USER, Log.DEBUG)) {
             Log.d(TAG_USER, "init()");
         }
-        mContext.registerReceiver(mUserLifecycleReceiver,
-                new IntentFilter(Intent.ACTION_USER_REMOVED));
         mCarUxRestrictionService.registerUxRestrictionsChangeListener(
                 mCarUxRestrictionsChangeListener, Display.DEFAULT_DISPLAY);
 
@@ -350,7 +328,6 @@ public final class CarUserService extends ICarUserService.Stub implements CarSer
         if (Log.isLoggable(TAG_USER, Log.DEBUG)) {
             Log.d(TAG_USER, "release()");
         }
-        mContext.unregisterReceiver(mUserLifecycleReceiver);
         mCarUxRestrictionService
                 .unregisterUxRestrictionsChangeListener(mCarUxRestrictionsChangeListener);
     }
@@ -1192,11 +1169,28 @@ public final class CarUserService extends ICarUserService.Stub implements CarSer
         }
     }
 
-    private void notifyHalUserRemoved(@UserIdInt int userId) {
+    /**
+     * Should be called by {@code ICarImpl} only.
+     */
+    public void onUserRemoved(@NonNull UserInfo user) {
+        if (Log.isLoggable(TAG_USER, Log.DEBUG)) {
+            Log.d(TAG_USER, "onUserRemoved: " + user.toFullString());
+        }
+        notifyHalUserRemoved(user);
+    }
+
+    private void notifyHalUserRemoved(@NonNull UserInfo user) {
         if (!isUserHalSupported()) return;
 
+        if (user == null) {
+            Log.wtf(TAG, "notifyHalUserRemoved() called for null user");
+            return;
+        }
+
+        int userId = user.id;
+
         if (userId == UserHandle.USER_NULL) {
-            Log.wtf(TAG, "notifyHalUserRemoved() Called for UserHandle.USER_NULL");
+            Log.wtf(TAG, "notifyHalUserRemoved() called for UserHandle.USER_NULL");
             return;
         }
 
@@ -1213,10 +1207,7 @@ public final class CarUserService extends ICarUserService.Stub implements CarSer
         android.hardware.automotive.vehicle.V2_0.UserInfo halUser =
                 new android.hardware.automotive.vehicle.V2_0.UserInfo();
         halUser.userId = userId;
-        // TODO(b/155913815): per the REMOVE_USER API, the userFlags should be set as well, but it's
-        // gone. We'll need to either update the documentation, or if that's not possible (as it's
-        // breaking the contract), either change the intent to contain the flags, or keep track of
-        // the flags internally.
+        halUser.flags = UserHalHelper.convertFlags(user);
 
         RemoveUserRequest request = new RemoveUserRequest();
         request.removedUserInfo = halUser;
@@ -1415,6 +1406,8 @@ public final class CarUserService extends ICarUserService.Stub implements CarSer
     }
 
     private void removeCreatedUser(@NonNull UserInfo user, @NonNull String reason) {
+        Log.i(TAG, "removing " + user.toFullString() + "; reason: " + reason);
+
         int userId = user.id;
         EventLog.writeEvent(EventLogTags.CAR_USER_SVC_CREATE_USER_USER_REMOVED, userId, reason);
 
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 b22bcb4b64..e358129576 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
@@ -76,9 +76,7 @@ import android.car.userlib.HalCallback;
 import android.car.userlib.HalCallback.HalCallbackStatus;
 import android.car.userlib.UserHalHelper;
 import android.car.userlib.UserHelper;
-import android.content.BroadcastReceiver;
 import android.content.Context;
-import android.content.Intent;
 import android.content.pm.PackageManager;
 import android.content.pm.UserInfo;
 import android.content.res.Resources;
@@ -107,7 +105,6 @@ import android.os.UserHandle;
 import android.os.UserManager;
 import android.sysprop.CarProperties;
 import android.util.Log;
-import android.util.Pair;
 import android.util.SparseArray;
 import android.view.Display;
 
@@ -251,15 +248,7 @@ public final class CarUserServiceTest extends AbstractExtendedMockitoTestCase {
         new FakeCarOccupantZoneService(mCarUserService);
     }
 
-    private Pair<BroadcastReceiver, ICarUxRestrictionsChangeListener> initService() {
-        ArgumentCaptor<BroadcastReceiver> receiverCaptor =
-                ArgumentCaptor.forClass(BroadcastReceiver.class);
-        when(mMockContext.registerReceiver(receiverCaptor.capture(), argThat((filter) -> {
-            // NOTE: it's assuming there's just one action; if it supports more (like USER_CREATE),
-            // it need to be changed to search through all of them
-            return filter.getAction(0).equals(Intent.ACTION_USER_REMOVED);
-        }))).thenReturn(null);
-
+    private ICarUxRestrictionsChangeListener initService() {
         ArgumentCaptor<ICarUxRestrictionsChangeListener> listenerCaptor =
                 ArgumentCaptor.forClass(ICarUxRestrictionsChangeListener.class);
         doNothing().when(mCarUxRestrictionService).registerUxRestrictionsChangeListener(
@@ -267,35 +256,28 @@ public final class CarUserServiceTest extends AbstractExtendedMockitoTestCase {
 
         mCarUserService.init();
 
-        BroadcastReceiver receiver = receiverCaptor.getValue();
-        assertWithMessage("init() didn't register receiver for %s", Intent.ACTION_USER_REMOVED)
-                .that(receiver).isNotNull();
-
         ICarUxRestrictionsChangeListener listener = listenerCaptor.getValue();
         assertWithMessage("init() didn't register ICarUxRestrictionsChangeListener")
                 .that(listener).isNotNull();
 
-        return new Pair<>(receiver, listener);
+        return listener;
     }
 
     @Test
     public void testInitAndRelease() {
         // init()
-        Pair<BroadcastReceiver, ICarUxRestrictionsChangeListener> pair = initService();
-        assertThat(pair).isNotNull();
-        assertThat(pair.first).isNotNull();
-        assertThat(pair.second).isNotNull();
+        ICarUxRestrictionsChangeListener listener = initService();
+        assertThat(listener).isNotNull();
 
         // release()
         mCarUserService.release();
-        verify(mMockContext).unregisterReceiver(pair.first);
-        verify(mCarUxRestrictionService).unregisterUxRestrictionsChangeListener(pair.second);
+        verify(mCarUxRestrictionService).unregisterUxRestrictionsChangeListener(listener);
     }
 
     @Test
     public void testSetICarServiceHelper_withUxRestrictions() throws Exception {
         mockGetUxRestrictions(/* restricted= */ true);
-        ICarUxRestrictionsChangeListener listener = initService().second;
+        ICarUxRestrictionsChangeListener listener = initService();
 
         mCarUserService.setCarServiceHelper(mICarServiceHelper);
         verify(mICarServiceHelper).setSafetyMode(false);
@@ -307,7 +289,7 @@ public final class CarUserServiceTest extends AbstractExtendedMockitoTestCase {
     @Test
     public void testSetICarServiceHelper_withoutUxRestrictions() throws Exception {
         mockGetUxRestrictions(/* restricted= */ false);
-        ICarUxRestrictionsChangeListener listener = initService().second;
+        ICarUxRestrictionsChangeListener listener = initService();
 
         mCarUserService.setCarServiceHelper(mICarServiceHelper);
         verify(mICarServiceHelper).setSafetyMode(true);
@@ -1160,7 +1142,7 @@ public final class CarUserServiceTest extends AbstractExtendedMockitoTestCase {
         mockAmSwitchUser(mGuestUser, true);
 
         // Should be ok first time...
-        ICarUxRestrictionsChangeListener listener = initService().second;
+        ICarUxRestrictionsChangeListener listener = initService();
         mCarUserService.switchUser(mGuestUser.id, mAsyncCallTimeoutMs, mUserSwitchFuture);
         assertThat(getUserSwitchResult().getStatus()).isEqualTo(UserSwitchResult.STATUS_SUCCESSFUL);
 
@@ -2229,8 +2211,8 @@ public final class CarUserServiceTest extends AbstractExtendedMockitoTestCase {
     }
 
     private void mockRemoveUser(@NonNull UserInfo user) {
-        BroadcastReceiver receiver = initService().first;
-        mockUmRemoveUserOrSetEphemeral(mMockContext, mMockedUserManager, receiver, user);
+        mockUmRemoveUserOrSetEphemeral(mMockedUserManager, user,
+                (u) -> mCarUserService.onUserRemoved(u));
     }
 
     private void mockHalGetInitialInfo(@UserIdInt int currentUserId,
@@ -2601,11 +2583,13 @@ public final class CarUserServiceTest extends AbstractExtendedMockitoTestCase {
 
     private void assertHalRemove(@NonNull UserInfo currentUser, @NonNull UserInfo removeUser) {
         verify(mMockedUserManager).removeUserOrSetEphemeral(removeUser.id);
-        ArgumentCaptor<RemoveUserRequest> request =
+        ArgumentCaptor<RemoveUserRequest> requestCaptor =
                 ArgumentCaptor.forClass(RemoveUserRequest.class);
-        verify(mUserHal).removeUser(request.capture());
-        assertThat(request.getValue().removedUserInfo.userId).isEqualTo(removeUser.id);
-        assertThat(request.getValue().usersInfo.currentUser.userId).isEqualTo(currentUser.id);
+        verify(mUserHal).removeUser(requestCaptor.capture());
+        RemoveUserRequest request = requestCaptor.getValue();
+        assertThat(request.removedUserInfo.userId).isEqualTo(removeUser.id);
+        assertThat(request.removedUserInfo.flags).isEqualTo(UserHalHelper.convertFlags(removeUser));
+        assertThat(request.usersInfo.currentUser.userId).isEqualTo(currentUser.id);
     }
 
     @NonNull