[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