Fixed calls to onUserVisibilityChanged() for system user.

When the system user is switched out, it should call
SystemServiceManager.onUserVisibilityChanged(0, false), but it was instead
calling SystemServiceManager.onUserStarting(0, false).

Test: manual verification
Test: atest FrameworksServicesTests:UserControllerTest

Fixes: 255895655
Bug: 244333150

Change-Id: Ib15d89b77b9ff247e9d92411cf806f74c6aa0b3c

4 files changed, 43 insertions, 13 deletions

diff --git a/services/core/java/com/android/server/SystemServiceManager.java b/services/core/java/com/android/server/SystemServiceManager.java
index 83d86cdc05c6..953e85014535 100644
--- a/services/core/java/com/android/server/SystemServiceManager.java
+++ b/services/core/java/com/android/server/SystemServiceManager.java
@@ -371,6 +371,7 @@ public final class SystemServiceManager implements Dumpable {
             // 2. When a user is switched from bg to fg, the onUserVisibilityChanged() callback is
             // called onUserSwitching(), so calling it before onUserStarting() make it more
             // consistent with that
+            EventLog.writeEvent(EventLogTags.SSM_USER_VISIBILITY_CHANGED, userId, /* visible= */ 1);
             onUser(t, USER_VISIBLE, /* prevUser= */ null, targetUser);
         }
         onUser(t, USER_STARTING, /* prevUser= */ null, targetUser);
@@ -381,14 +382,30 @@ public final class SystemServiceManager implements Dumpable {
      *
      * <p><b>NOTE: </b>this method should only be called when a user that is already running become
      * visible; if the user is starting visible, callers should call
-     * {@link #onUserStarting(TimingsTraceAndSlog, int, boolean)} instead
+     * {@link #onUserStarting(TimingsTraceAndSlog, int, boolean)} instead.
      */
     public void onUserVisible(@UserIdInt int userId) {
-        EventLog.writeEvent(EventLogTags.SSM_USER_VISIBLE, userId);
+        EventLog.writeEvent(EventLogTags.SSM_USER_VISIBILITY_CHANGED, userId, /* visible= */ 1);
         onUser(USER_VISIBLE, userId);
     }
 
     /**
+     * Updates the visibility of the system user.
+     *
+     * <p>Since the system user never stops, this method must be called when it's switched from / to
+     * foreground.
+     */
+    public void onSystemUserVisibilityChanged(boolean visible) {
+        int userId = UserHandle.USER_SYSTEM;
+        EventLog.writeEvent(EventLogTags.SSM_USER_VISIBILITY_CHANGED, userId, visible ? 1 : 0);
+        if (visible) {
+            onUser(USER_VISIBLE, userId);
+        } else {
+            onUser(USER_INVISIBLE, userId);
+        }
+    }
+
+    /**
      * Unlocks the given user.
      */
     public void onUserUnlocking(@UserIdInt int userId) {
diff --git a/services/core/java/com/android/server/am/EventLogTags.logtags b/services/core/java/com/android/server/am/EventLogTags.logtags
index dec8b62de2ec..60e6754f4b6f 100644
--- a/services/core/java/com/android/server/am/EventLogTags.logtags
+++ b/services/core/java/com/android/server/am/EventLogTags.logtags
@@ -116,7 +116,7 @@ option java_package com.android.server.am
 30086 ssm_user_stopping (userId|1|5),(visibilityChanged|1)
 30087 ssm_user_stopped (userId|1|5)
 30088 ssm_user_completed_event (userId|1|5),(eventFlag|1|5)
-30089 ssm_user_visible (userId|1|5)
+30089 ssm_user_visibility_changed (userId|1|5),(visible|1)
 
 # Foreground service start/stop events.
 30100 am_foreground_service_start (User|1|5),(Component Name|3),(allowWhileInUse|1),(startReasonCode|3),(targetSdk|1|1),(callerTargetSdk|1|1),(notificationWasDeferred|1),(notificationShown|1),(durationMs|1|3),(startForegroundCount|1|1),(stopReason|3)
diff --git a/services/core/java/com/android/server/am/UserController.java b/services/core/java/com/android/server/am/UserController.java
index 8d3890ce9d5a..af559808d880 100644
--- a/services/core/java/com/android/server/am/UserController.java
+++ b/services/core/java/com/android/server/am/UserController.java
@@ -2190,7 +2190,7 @@ class UserController implements Handler.Callback {
         if (oldUserId == UserHandle.USER_SYSTEM) {
             // System user is never stopped, but its visibility is changed (as it is brought to the
             // background)
-            updateSystemUserVisibility(/* visible= */ false);
+            updateSystemUserVisibility(t, /* visible= */ false);
         }
 
         t.traceEnd(); // end continueUserSwitch
@@ -2549,10 +2549,15 @@ class UserController implements Handler.Callback {
 
     // TODO(b/242195409): remove this method if initial system user boot logic is refactored?
     void onSystemUserStarting() {
-        updateSystemUserVisibility(/* visible= */ !UserManager.isHeadlessSystemUserMode());
+        if (!UserManager.isHeadlessSystemUserMode()) {
+            // Don't need to call on HSUM because it will be called when the system user is
+            // restarted on background
+            mInjector.onUserStarting(UserHandle.USER_SYSTEM, /* visible= */ true);
+        }
     }
 
-    private void updateSystemUserVisibility(boolean visible) {
+    private void updateSystemUserVisibility(TimingsTraceAndSlog t, boolean visible) {
+        t.traceBegin("update-system-userVisibility-" + visible);
         if (DEBUG_MU) {
             Slogf.d(TAG, "updateSystemUserVisibility(): visible=%b", visible);
         }
@@ -2564,7 +2569,8 @@ class UserController implements Handler.Callback {
                 mVisibleUsers.delete(userId);
             }
         }
-        mInjector.onUserStarting(userId, visible);
+        mInjector.notifySystemUserVisibilityChanged(visible);
+        t.traceEnd();
     }
 
     /**
@@ -3673,5 +3679,8 @@ class UserController implements Handler.Callback {
             getSystemServiceManager().onUserStarting(TimingsTraceAndSlog.newAsyncLog(), userId,
                     visible);
         }
+        void notifySystemUserVisibilityChanged(boolean visible) {
+            getSystemServiceManager().onSystemUserVisibilityChanged(visible);
+        }
     }
 }
diff --git a/services/tests/servicestests/src/com/android/server/am/UserControllerTest.java b/services/tests/servicestests/src/com/android/server/am/UserControllerTest.java
index fe92a1dbdac1..935d1d880a2a 100644
--- a/services/tests/servicestests/src/com/android/server/am/UserControllerTest.java
+++ b/services/tests/servicestests/src/com/android/server/am/UserControllerTest.java
@@ -24,7 +24,6 @@ import static android.app.ActivityManagerInternal.ALLOW_NON_FULL;
 import static android.app.ActivityManagerInternal.ALLOW_NON_FULL_IN_PROFILE;
 import static android.app.ActivityManagerInternal.ALLOW_PROFILES_OR_NON_FULL;
 import static android.content.pm.PackageManager.PERMISSION_GRANTED;
-import static android.os.UserHandle.USER_SYSTEM;
 import static android.testing.DexmakerShareClassLoaderRule.runWithDexmakerShareClassLoader;
 
 import static androidx.test.platform.app.InstrumentationRegistry.getInstrumentation;
@@ -403,7 +402,7 @@ public class UserControllerTest {
         verify(mInjector, times(0)).dismissKeyguard(any(), anyString());
         verify(mInjector.getWindowManager(), times(1)).stopFreezingScreen();
         continueUserSwitchAssertions(TEST_USER_ID, false);
-        verifyOnUserStarting(USER_SYSTEM, /* visible= */ false);
+        verifySystemUserVisibilityChangedNotified(/* visible= */ false);
     }
 
     @Test
@@ -424,7 +423,7 @@ public class UserControllerTest {
         verify(mInjector, times(1)).dismissKeyguard(any(), anyString());
         verify(mInjector.getWindowManager(), times(1)).stopFreezingScreen();
         continueUserSwitchAssertions(TEST_USER_ID, false);
-        verifyOnUserStarting(USER_SYSTEM, /* visible= */ false);
+        verifySystemUserVisibilityChangedNotified(/* visible= */ false);
     }
 
     @Test
@@ -531,7 +530,7 @@ public class UserControllerTest {
         assertFalse(mUserController.canStartMoreUsers());
         assertEquals(Arrays.asList(new Integer[] {0, TEST_USER_ID1, TEST_USER_ID2}),
                 mUserController.getRunningUsersLU());
-        verifyOnUserStarting(USER_SYSTEM, /* visible= */ false);
+        verifySystemUserVisibilityChangedNotified(/* visible= */ false);
     }
 
     /**
@@ -964,8 +963,8 @@ public class UserControllerTest {
         verify(mInjector.getUserManagerInternal(), never()).unassignUserFromDisplay(userId);
     }
 
-    private void verifyOnUserStarting(@UserIdInt int userId, boolean visible) {
-        verify(mInjector).onUserStarting(userId, visible);
+    private void verifySystemUserVisibilityChangedNotified(boolean visible) {
+        verify(mInjector).notifySystemUserVisibilityChanged(visible);
     }
 
     // Should be public to allow mocking
@@ -1108,6 +1107,11 @@ public class UserControllerTest {
         void onUserStarting(@UserIdInt int userId, boolean visible) {
             Log.i(TAG, "onUserStarting(" + userId + ", " + visible + ")");
         }
+
+        @Override
+        void notifySystemUserVisibilityChanged(boolean visible) {
+            Log.i(TAG, "notifySystemUserVisibilityChanged(" + visible + ")");
+        }
     }
 
     private static class TestHandler extends Handler {