Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 815f5ca1 authored by Felipe Leme's avatar Felipe Leme
Browse files

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
parent a956334f

services/core/java/com/android/server/SystemServiceManager.java

+19 −2
Original line number Diff line number Diff line
@@ -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,13 +382,29 @@ 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.

     */

services/core/java/com/android/server/am/EventLogTags.logtags

+1 −1
Original line number Diff line number Diff line
@@ -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)

services/core/java/com/android/server/am/UserController.java

+13 −4
Original line number Diff line number Diff line
@@ -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);

        }

    }

}

services/tests/servicestests/src/com/android/server/am/UserControllerTest.java

+10 −6
Original line number Diff line number Diff line
@@ -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 {