[go: nahoru, domu]

[123 merge] Fix race condition crash when initializing settings on the login screen

Adds check that a user pod has been selected on the login screen before
attempting to initialize settings from local_state.

(cherry picked from commit 9a042194191c93f5174c8174442f84d554a6bbb5)

Bug: b/327283600
Change-Id: I8ca3fabd3391813bbc4ae79dd1d72309e329f3f7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5336182
Reviewed-by: Jimmy Gong <jimmyxgong@chromium.org>
Commit-Queue: David Padlipsky <dpad@google.com>
Cr-Original-Commit-Position: refs/heads/main@{#1267263}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5357016
Cr-Commit-Position: refs/branch-heads/6312@{#486}
Cr-Branched-From: 6711dcdae48edaf98cbc6964f90fac85b7d9986e-refs/heads/main@{#1262506}
diff --git a/ash/system/input_device_settings/input_device_settings_controller_impl.cc b/ash/system/input_device_settings/input_device_settings_controller_impl.cc
index 8fdde22..e3cf3d93 100644
--- a/ash/system/input_device_settings/input_device_settings_controller_impl.cc
+++ b/ash/system/input_device_settings/input_device_settings_controller_impl.cc
@@ -1756,6 +1756,14 @@
     return;
   }
 
+  // If there is no active account id or local state isn't initialized, use
+  // default graphics tablet settings. This can happen as an uncommon race
+  // condition when first signing in on the login screen.
+  if (!active_account_id_ || !local_state_) {
+    graphics_tablet->settings = mojom::GraphicsTabletSettings::New();
+    return;
+  }
+
   graphics_tablet_pref_handler_->InitializeLoginScreenGraphicsTabletSettings(
       local_state_, active_account_id_.value(), graphics_tablet);
 }
diff --git a/ash/system/input_device_settings/input_device_settings_controller_unittest.cc b/ash/system/input_device_settings/input_device_settings_controller_unittest.cc
index 61b5cb4..dab5e130 100644
--- a/ash/system/input_device_settings/input_device_settings_controller_unittest.cc
+++ b/ash/system/input_device_settings/input_device_settings_controller_unittest.cc
@@ -286,6 +286,7 @@
   void InitializeWithDefaultKeyboardSettings(
       const mojom::KeyboardPolicies& keyboard_policies,
       mojom::Keyboard* keyboard) override {
+    keyboard->settings = CreateNewKeyboardSettings();
     num_initialize_default_keyboard_settings_calls_++;
   }
 
@@ -488,22 +489,27 @@
     auto user_2_prefs = std::make_unique<TestingPrefServiceSimple>();
     RegisterUserProfilePrefs(user_2_prefs->registry(), /*country=*/"",
                              /*for_test=*/true);
-    session_controller->AddUserSession(kUserEmail1,
-                                       user_manager::UserType::kRegular,
-                                       /*provide_pref_service=*/false);
-    session_controller->SetUserPrefService(account_id_1,
-                                           std::move(user_1_prefs));
-    session_controller->AddUserSession(kUserEmail2,
-                                       user_manager::UserType::kRegular,
-                                       /*provide_pref_service=*/false);
-    session_controller->SetUserPrefService(account_id_2,
-                                           std::move(user_2_prefs));
-    session_controller->AddUserSession(kUserEmail3,
-                                       user_manager::UserType::kRegular,
-                                       /*provide_pref_service=*/false);
 
-    session_controller->SwitchActiveUser(account_id_1);
-    session_controller->SetSessionState(session_manager::SessionState::ACTIVE);
+    if (should_sign_in_) {
+      session_controller->AddUserSession(kUserEmail1,
+                                         user_manager::UserType::kRegular,
+                                         /*provide_pref_service=*/false);
+      session_controller->SetUserPrefService(account_id_1,
+                                             std::move(user_1_prefs));
+      session_controller->AddUserSession(kUserEmail2,
+                                         user_manager::UserType::kRegular,
+                                         /*provide_pref_service=*/false);
+      session_controller->SetUserPrefService(account_id_2,
+                                             std::move(user_2_prefs));
+      session_controller->AddUserSession(kUserEmail3,
+                                         user_manager::UserType::kRegular,
+                                         /*provide_pref_service=*/false);
+
+      session_controller->SwitchActiveUser(account_id_1);
+      session_controller->SetSessionState(
+          session_manager::SessionState::ACTIVE);
+    }
+
     // Reset the `num_keyboard_settings_initialized_` to account for the
     // `InitializeKeyboardSettings` call made after test setup where we
     // simualate
@@ -541,6 +547,11 @@
       scoped_resetter_;
   raw_ptr<FakeKeyboardPrefHandler, DanglingUntriaged> keyboard_pref_handler_ =
       nullptr;
+
+  // Used by other instances of the InputDeviceSettingsControllerTest to control
+  // whether or not to sign in within the SetUp() function. Configured to sign
+  // in by default.
+  bool should_sign_in_ = true;
 };
 
 TEST_F(InputDeviceSettingsControllerTest, KeyboardAddingOne) {
@@ -1559,4 +1570,59 @@
   EXPECT_EQ(2u, observer_->num_pointing_stick_settings_updated());
 }
 
+class InputDeviceSettingsControllerNoSignInTest
+    : public InputDeviceSettingsControllerTest {
+ public:
+  InputDeviceSettingsControllerNoSignInTest() { should_sign_in_ = false; }
+};
+
+TEST_F(InputDeviceSettingsControllerNoSignInTest,
+       NoCrashOnPodSelectionRaceConditionMouse) {
+  ui::DeviceDataManagerTestApi().SetMouseDevices({kSampleMouseUsb});
+
+  auto* mouse = controller_->GetMouse(kSampleMouseUsb.id);
+  ASSERT_TRUE(mouse);
+  ASSERT_TRUE(mouse->settings);
+}
+
+TEST_F(InputDeviceSettingsControllerNoSignInTest,
+       NoCrashOnPodSelectionRaceConditionKeyboard) {
+  ui::DeviceDataManagerTestApi().SetKeyboardDevices({kSampleKeyboardInternal});
+
+  auto* keyboard = controller_->GetKeyboard(kSampleKeyboardInternal.id);
+  ASSERT_TRUE(keyboard);
+  ASSERT_TRUE(keyboard->settings);
+}
+
+TEST_F(InputDeviceSettingsControllerNoSignInTest,
+       NoCrashOnPodSelectionRaceConditionTouchpad) {
+  ui::DeviceDataManagerTestApi().SetTouchpadDevices({kSampleTouchpadInternal});
+
+  auto* touchpad = controller_->GetTouchpad(kSampleTouchpadInternal.id);
+  ASSERT_TRUE(touchpad);
+  ASSERT_TRUE(touchpad->settings);
+}
+
+TEST_F(InputDeviceSettingsControllerNoSignInTest,
+       NoCrashOnPodSelectionRaceConditionPointingStick) {
+  ui::DeviceDataManagerTestApi().SetPointingStickDevices(
+      {kSamplePointingStickExternal});
+
+  auto* pointing_stick =
+      controller_->GetPointingStick(kSamplePointingStickExternal.id);
+  ASSERT_TRUE(pointing_stick);
+  ASSERT_TRUE(pointing_stick->settings);
+}
+
+TEST_F(InputDeviceSettingsControllerNoSignInTest,
+       NoCrashOnPodSelectionRaceConditionGraphicsTablet) {
+  ui::DeviceDataManagerTestApi().SetGraphicsTabletDevices(
+      {kSampleGraphicsTablet});
+
+  auto* graphics_tablet =
+      controller_->GetGraphicsTablet(kSampleGraphicsTablet.id);
+  ASSERT_TRUE(graphics_tablet);
+  ASSERT_TRUE(graphics_tablet->settings);
+}
+
 }  // namespace ash