commit 98900c8184a8e7b7f64417efea55ded028a0a810
author felipeal <felipeal@google.com> Thu Apr 09 09:05:02 2020 -0700
committer felipeal <felipeal@google.com> Thu Apr 09 11:24:24 2020 -0700
tree 2b4cbc28a881caae6dd8c87e930af1c51841cf49
parent 140b15266268555772b88e85a9b3559fa51283bb

Minor changes on CarUserService user lifecycle handling.

- Remove duplicated call to notifyUserLifecycleListeners()
- Rename unlockUser() on onUserUnlocked().
- Rename onUserSwitch() on onUserSwitching().
- Call onUserUnlocked() after the user was unlocked.
- Remove irrelevant tracing.
- Added missing t.traceEnd() on onUserSwitching().
- Rename some unit test methods to be more generic.

Test: atest CarServiceUnitTest:com.android.car.user.CarUserServiceTest
Bug: 145689885

Change-Id: Ia775d4e53a896f236359278f5642465d51a66644

service/src/com/android/car/user/CarUserService.java

diff --git a/service/src/com/android/car/user/CarUserService.java b/service/src/com/android/car/user/CarUserService.java
index 64e9ec4..bc6fc58 100644
--- a/service/src/com/android/car/user/CarUserService.java
+++ b/service/src/com/android/car/user/CarUserService.java
@@ -755,11 +755,7 @@
         }
     }
 
-    private void unlockUser(@UserIdInt int userId) {
-        TimingsTraceLog t = new TimingsTraceLog(TAG_USER, Trace.TRACE_TAG_SYSTEM_SERVER);
-        notifyUserLifecycleListeners(
-                new UserLifecycleEvent(CarUserManager.USER_LIFECYCLE_EVENT_TYPE_UNLOCKING, userId));
-        t.traceBegin("UnlockTasks-" + userId);
+    private void onUserUnlocked(@UserIdInt int userId) {
         ArrayList<Runnable> tasks = null;
         synchronized (mLockUser) {
             if (userId == UserHandle.USER_SYSTEM) {
@@ -795,7 +791,6 @@
                 r.run();
             }
         }
-        t.traceEnd();
     }
 
     /**
@@ -887,21 +882,19 @@
      */
     public void onUserLifecycleEvent(UserLifecycleEvent event) {
         int userId = event.getUserId();
-        if (event.getEventType() == CarUserManager.USER_LIFECYCLE_EVENT_TYPE_SWITCHING) {
-            onSwitchUser(userId);
-        } else if (event.getEventType() == CarUserManager.USER_LIFECYCLE_EVENT_TYPE_UNLOCKING) {
-            unlockUser(userId);
+        int eventType = event.getEventType();
+
+        // Handle special cases first, then notify listeners
+        if (eventType == CarUserManager.USER_LIFECYCLE_EVENT_TYPE_SWITCHING) {
+            onUserSwitching(userId);
+        } else if (eventType == CarUserManager.USER_LIFECYCLE_EVENT_TYPE_UNLOCKED) {
+            onUserUnlocked(userId);
         }
 
-        // TODO(b/144120654): right now just the app listeners are running in the background so the
-        // CTS tests pass (as otherwise they might fail if a car service callback takes too long),
-        // but once we refactor the car service callback into lifecycle listeners, we should use a
-        // proper thread management (like a Threadpool / executor);
-
-        // Notify all user listeners
+        // Notify internal service listeners
         notifyUserLifecycleListeners(event);
 
-        // Notify all app listeners
+        // Notify external app listeners
         notifyAppLifecycleListeners(event);
     }
 
@@ -958,10 +951,10 @@
         t.traceEnd();
     }
 
-    private void onSwitchUser(@UserIdInt int userId) {
+    private void onUserSwitching(@UserIdInt int userId) {
         Log.i(TAG_USER, "onSwitchUser() callback for user " + userId);
         TimingsTraceLog t = new TimingsTraceLog(TAG_USER, Trace.TRACE_TAG_SYSTEM_SERVER);
-        t.traceBegin("onSwitchUser-" + userId);
+        t.traceBegin("onUserSwitching-" + userId);
 
         if (!isSystemUser(userId)) {
             mCarUserManagerHelper.setLastActiveUser(userId);
@@ -973,6 +966,7 @@
             setupPassengerUser();
             startFirstPassenger(userId);
         }
+        t.traceEnd();
     }
 
     /**