Merge "Skip duplicate USER_SYSTEM start in SystemServiceManager." into tm-qpr-dev am: 6303152e0a

Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/base/+/20457786

Change-Id: I53c460fa04166232ff8ccedbf3d76bf099092af9
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

2 files changed, 31 insertions, 15 deletions

diff --git a/services/core/java/com/android/server/SystemServiceManager.java b/services/core/java/com/android/server/SystemServiceManager.java
index 78df983c83f7..166806b31cd1 100644
--- a/services/core/java/com/android/server/SystemServiceManager.java
+++ b/services/core/java/com/android/server/SystemServiceManager.java
@@ -357,13 +357,24 @@ public final class SystemServiceManager implements Dumpable {
      * Starts the given user.
      */
     public void onUserStarting(@NonNull TimingsTraceAndSlog t, @UserIdInt int userId) {
-        EventLog.writeEvent(EventLogTags.SSM_USER_STARTING, userId);
-
         final TargetUser targetUser = newTargetUser(userId);
         synchronized (mTargetUsers) {
+            // On Automotive / Headless System User Mode, the system user will be started twice:
+            // - Once by some external or local service that switches the system user to
+            //   the background.
+            // - Once by the ActivityManagerService, when the system is marked ready.
+            // These two events are not synchronized and the order of execution is
+            // non-deterministic. To avoid starting the system user twice, verify whether
+            // the system user has already been started by checking the mTargetUsers.
+            // TODO(b/242195409): this workaround shouldn't be necessary once we move
+            // the headless-user start logic to UserManager-land.
+            if (userId == UserHandle.USER_SYSTEM && mTargetUsers.contains(userId)) {
+                Slog.e(TAG, "Skipping starting system user twice");
+                return;
+            }
             mTargetUsers.put(userId, targetUser);
         }
-
+        EventLog.writeEvent(EventLogTags.SSM_USER_STARTING, userId);
         onUser(t, USER_STARTING, /* prevUser= */ null, targetUser);
     }
 
diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java
index 4c7151a91cc0..5db736d43969 100644
--- a/services/core/java/com/android/server/am/ActivityManagerService.java
+++ b/services/core/java/com/android/server/am/ActivityManagerService.java
@@ -8164,15 +8164,12 @@ public class ActivityManagerService extends IActivityManager.Stub
         mBatteryStatsService.noteEvent(BatteryStats.HistoryItem.EVENT_USER_FOREGROUND_START,
                 Integer.toString(currentUserId), currentUserId);
 
-        // On Automotive, at this point the system user has already been started and unlocked,
-        // and some of the tasks we do here have already been done. So skip those in that case.
-        // TODO(b/132262830, b/203885241): this workdound shouldn't be necessary once we move the
-        // headless-user start logic to UserManager-land
-        final boolean bootingSystemUser = currentUserId == UserHandle.USER_SYSTEM;
-
-        if (bootingSystemUser) {
-            mSystemServiceManager.onUserStarting(t, currentUserId);
-        }
+        // On Automotive / Headless System User Mode, at this point the system user has already been
+        // started and unlocked, and some of the tasks we do here have already been done. So skip
+        // those in that case. The duplicate system user start is guarded in SystemServiceManager.
+        // TODO(b/242195409): this workaround shouldn't be necessary once we move the headless-user
+        // start logic to UserManager-land.
+        mSystemServiceManager.onUserStarting(t, currentUserId);
 
         synchronized (this) {
             // Only start up encryption-aware persistent apps; once user is
@@ -8202,7 +8199,15 @@ public class ActivityManagerService extends IActivityManager.Stub
                 t.traceEnd();
             }
 
-            if (bootingSystemUser) {
+            // Some systems - like automotive - will explicitly unlock system user then switch
+            // to a secondary user. Hence, we don't want to send duplicate broadcasts for
+            // the system user here.
+            // TODO(b/242195409): this workaround shouldn't be necessary once we move
+            // the headless-user start logic to UserManager-land.
+            final boolean isBootingSystemUser = (currentUserId == UserHandle.USER_SYSTEM)
+                    && !UserManager.isHeadlessSystemUserMode();
+
+            if (isBootingSystemUser) {
                 t.traceBegin("startHomeOnAllDisplays");
                 mAtmInternal.startHomeOnAllDisplays(currentUserId, "systemReady");
                 t.traceEnd();
@@ -8213,7 +8218,7 @@ public class ActivityManagerService extends IActivityManager.Stub
             t.traceEnd();
 
 
-            if (bootingSystemUser) {
+            if (isBootingSystemUser) {
                 t.traceBegin("sendUserStartBroadcast");
                 final int callingUid = Binder.getCallingUid();
                 final int callingPid = Binder.getCallingPid();
@@ -8254,7 +8259,7 @@ public class ActivityManagerService extends IActivityManager.Stub
             mAtmInternal.resumeTopActivities(false /* scheduleIdle */);
             t.traceEnd();
 
-            if (bootingSystemUser) {
+            if (isBootingSystemUser) {
                 t.traceBegin("sendUserSwitchBroadcasts");
                 mUserController.sendUserSwitchBroadcasts(-1, currentUserId);
                 t.traceEnd();