Lock screen: Use focused user preference for 24 hours clock

Also postpone focused pod hooks during screen locking until after the
session state changes.

Fixed: 914972
Change-Id: I8fc68a0f07be8a3eea9cde973eb0e203e47edd4f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2215326Reviewed-by: Alexander Alekseev <alemate@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Roman Sorokin [CET] <rsorokin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#772576}

chrome/browser/chromeos/login/screens/user_selection_screen.cc

@@ -378,6 +378,7 @@ class UserSelectionScreen::DircryptoMigrationChecker {
 UserSelectionScreen::UserSelectionScreen(const std::string& display_type)
     : BaseScreen(UserBoardView::kScreenId, OobeScreenPriority::DEFAULT),
       display_type_(display_type) {
+  session_manager::SessionManager::Get()->AddObserver(this);
   if (display_type_ != OobeUI::kLoginDisplay)
     return;
   allowed_input_methods_subscription_ =
@@ -393,6 +394,7 @@ UserSelectionScreen::~UserSelectionScreen() {
   ui::UserActivityDetector* activity_detector = ui::UserActivityDetector::Get();
   if (activity_detector && activity_detector->HasObserver(this))
     activity_detector->RemoveObserver(this);
+  session_manager::SessionManager::Get()->RemoveObserver(this);
 }
 
 void UserSelectionScreen::InitEasyUnlock() {
@@ -692,6 +694,14 @@ void UserSelectionScreen::CheckUserStatus(const AccountId& account_id) {
 }
 
 void UserSelectionScreen::HandleFocusPod(const AccountId& account_id) {
+  DCHECK(!pending_focused_account_id_.has_value());
+  const session_manager::SessionState session_state =
+      session_manager::SessionManager::Get()->session_state();
+  if (session_state == session_manager::SessionState::ACTIVE) {
+    // Wait for the session state change before actual work.
+    pending_focused_account_id_ = account_id;
+    return;
+  }
   proximity_auth::ScreenlockBridge::Get()->SetFocusedUser(account_id);
   if (focused_pod_account_id_ == account_id)
     return;
@@ -832,6 +842,16 @@ void UserSelectionScreen::AttemptEasySignin(const AccountId& account_id,
   }
 }
 
+void UserSelectionScreen::OnSessionStateChanged() {
+  if (!pending_focused_account_id_.has_value())
+    return;
+  DCHECK(session_manager::SessionManager::Get()->IsUserSessionBlocked());
+
+  AccountId focused_pod(pending_focused_account_id_.value());
+  pending_focused_account_id_.reset();
+  HandleFocusPod(focused_pod);
+}
+
 void UserSelectionScreen::ShowImpl() {}
 
 void UserSelectionScreen::HideImpl() {}

chrome/browser/chromeos/login/screens/user_selection_screen.h

@@ -21,6 +21,7 @@
 #include "chrome/browser/chromeos/settings/cros_settings.h"
 #include "chromeos/components/proximity_auth/screenlock_bridge.h"
 #include "components/account_id/account_id.h"
+#include "components/session_manager/core/session_manager_observer.h"
 #include "components/user_manager/user.h"
 #include "ui/base/ime/chromeos/ime_keyboard.h"
 #include "ui/base/ime/chromeos/input_method_manager.h"
@@ -38,7 +39,8 @@ class UserBoardView;
 class UserSelectionScreen
     : public ui::UserActivityObserver,
       public proximity_auth::ScreenlockBridge::LockHandler,
-      public BaseScreen {
+      public BaseScreen,
+      public session_manager::SessionManagerObserver {
  public:
   explicit UserSelectionScreen(const std::string& display_type);
   ~UserSelectionScreen() override;
@@ -98,6 +100,9 @@ class UserSelectionScreen
                          const std::string& secret,
                          const std::string& key_label) override;
 
+  // session_manager::SessionManagerObserver
+  void OnSessionStateChanged() override;
+
   // Fills |user_dict| with information about |user|.
   static void FillUserDictionary(
       const user_manager::User* user,
@@ -173,6 +178,12 @@ class UserSelectionScreen
   user_manager::UserList users_to_send_;
 
   AccountId focused_pod_account_id_;
+
+  // Sometimes we might get focused pod while user session is still active. e.g.
+  // while creating lock screen. So postpone any work until after the session
+  // state changes.
+  base::Optional<AccountId> pending_focused_account_id_;
+
   // Input Method Engine state used at the user selection screen.
   scoped_refptr<input_method::InputMethodManager::State> ime_state_;
 

chrome/browser/chromeos/system/system_clock.cc

@@ -19,6 +19,7 @@
 #include "chromeos/settings/cros_settings_names.h"
 #include "components/prefs/pref_change_registrar.h"
 #include "components/prefs/pref_service.h"
+#include "components/session_manager/core/session_manager.h"
 #include "components/user_manager/user.h"
 #include "components/user_manager/user_manager.h"
 #include "content/public/browser/notification_service.h"
@@ -120,15 +121,19 @@ void SystemClock::SetLastFocusedPodHourClockType(
 }
 
 bool SystemClock::ShouldUse24HourClock() const {
+  const session_manager::SessionState session_state =
+      session_manager::SessionManager::Get()->session_state();
+  if (session_state == session_manager::SessionState::LOGIN_PRIMARY ||
+      session_state == session_manager::SessionState::LOCKED) {
+    if (user_pod_was_focused_)
+      return last_focused_pod_hour_clock_type_ == base::k24HourClock;
+  }
   // default is used for kUse24HourClock preference on login screen and whenever
   // set so in user's preference
   const chromeos::LoginState::LoggedInUserType status =
       LoginState::IsInitialized() ? LoginState::Get()->GetLoggedInUserType()
                                   : LoginState::LOGGED_IN_USER_NONE;
 
-  if (status == LoginState::LOGGED_IN_USER_NONE && user_pod_was_focused_)
-    return last_focused_pod_hour_clock_type_ == base::k24HourClock;
-
   const CrosSettings* const cros_settings = CrosSettings::Get();
   bool system_use_24_hour_clock = true;
   const bool system_value_found = cros_settings->GetBoolean(

chrome/browser/ui/ash/system_tray_client_browsertest.cc

@@ -5,7 +5,9 @@
 #include "chrome/browser/ui/ash/system_tray_client.h"
 
 #include "ash/public/cpp/ash_view_ids.h"
+#include "ash/public/cpp/login_screen_test_api.h"
 #include "ash/public/cpp/system_tray_test_api.h"
+#include "chrome/browser/chromeos/login/lock/screen_locker_tester.h"
 #include "chrome/browser/chromeos/login/login_manager_test.h"
 #include "chrome/browser/chromeos/login/test/login_manager_mixin.h"
 #include "chrome/browser/chromeos/login/ui/user_adding_screen.h"
@@ -100,3 +102,40 @@ IN_PROC_BROWSER_TEST_F(SystemTrayClientClockTest, TestMultiProfile24HourClock) {
   content::RunAllPendingInMessageLoop();
   EXPECT_TRUE(tray_test_api->Is24HourClock());
 }
+
+// Test that on the login and lock screen clock type is taken from user profile
+// of the focused pod.
+IN_PROC_BROWSER_TEST_F(SystemTrayClientClockTest, PRE_FocusedPod24HourClock) {
+  auto tray_test_api = ash::SystemTrayTestApi::Create();
+
+  // Login a user with a 24-hour clock.
+  LoginUser(account_id1_);
+  SetupUserProfile(account_id1_, true /* use_24_hour_clock */);
+  EXPECT_TRUE(tray_test_api->Is24HourClock());
+
+  // Add a user with a 12-hour clock.
+  chromeos::UserAddingScreen::Get()->Start();
+  AddUser(account_id2_);
+  SetupUserProfile(account_id2_, false /* use_24_hour_clock */);
+  EXPECT_FALSE(tray_test_api->Is24HourClock());
+
+  // Test lock screen.
+  chromeos::ScreenLockerTester locker;
+  locker.Lock();
+
+  EXPECT_TRUE(ash::LoginScreenTestApi::FocusUser(account_id1_));
+  EXPECT_TRUE(tray_test_api->Is24HourClock());
+
+  EXPECT_TRUE(ash::LoginScreenTestApi::FocusUser(account_id2_));
+  EXPECT_FALSE(tray_test_api->Is24HourClock());
+}
+
+IN_PROC_BROWSER_TEST_F(SystemTrayClientClockTest, FocusedPod24HourClock) {
+  auto tray_test_api = ash::SystemTrayTestApi::Create();
+  // Test login screen.
+  EXPECT_TRUE(ash::LoginScreenTestApi::FocusUser(account_id1_));
+  EXPECT_TRUE(tray_test_api->Is24HourClock());
+
+  EXPECT_TRUE(ash::LoginScreenTestApi::FocusUser(account_id2_));
+  EXPECT_FALSE(tray_test_api->Is24HourClock());
+}