Disable secret key checks if certain condition is met
See b/360396368 for explanation of why this change is needed.
Bug: b/360396368
Change-Id: Ia75699c42ccc245c679a0e44b3b74e365672a311
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5789218
Reviewed-by: Darren Shen <shend@chromium.org>
Commit-Queue: Darren Shen <shend@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1343305}
diff --git a/ash/BUILD.gn b/ash/BUILD.gn
index ad56b43..bc160e23 100644
--- a/ash/BUILD.gn
+++ b/ash/BUILD.gn
@@ -1917,6 +1917,8 @@
"system/input_device_settings/input_device_tracker.h",
"system/input_device_settings/keyboard_modifier_metrics_recorder.cc",
"system/input_device_settings/keyboard_modifier_metrics_recorder.h",
+ "system/input_device_settings/modifier_split_bypass_checker.cc",
+ "system/input_device_settings/modifier_split_bypass_checker.h",
"system/input_device_settings/pref_handlers/graphics_tablet_pref_handler.h",
"system/input_device_settings/pref_handlers/graphics_tablet_pref_handler_impl.cc",
"system/input_device_settings/pref_handlers/graphics_tablet_pref_handler_impl.h",
diff --git a/ash/accelerators/accelerator_alias_converter.cc b/ash/accelerators/accelerator_alias_converter.cc
index a1fd51d5..846acb9 100644
--- a/ash/accelerators/accelerator_alias_converter.cc
+++ b/ash/accelerators/accelerator_alias_converter.cc
@@ -614,8 +614,7 @@
return std::vector<ui::Accelerator>();
}
- if (features::IsModifierSplitEnabled() &&
- IsSplitModifierKeyboard(device_id.value())) {
+ if (device_id.has_value() && IsSplitModifierKeyboard(device_id.value())) {
const auto iter = ui::kSixPackKeyToFnKeyMap.find(accelerator.key_code());
// [Insert] is technically a six pack key but has no Fn based rewrite. Need
// to make sure we return no aliased accelerator for this case.
diff --git a/ash/accelerators/accelerator_commands.cc b/ash/accelerators/accelerator_commands.cc
index ae99b3c..4417898 100644
--- a/ash/accelerators/accelerator_commands.cc
+++ b/ash/accelerators/accelerator_commands.cc
@@ -710,7 +710,8 @@
bool CanTogglePicker() {
CHECK(Shell::HasInstance());
- return features::IsPickerUpdateEnabled() && Shell::Get()->picker_controller();
+ return features::IsPickerUpdateEnabled() &&
+ Shell::Get()->picker_controller()->IsFeatureEnabled();
}
bool CanTogglePrivacyScreen() {
diff --git a/ash/accelerators/accelerator_controller_impl.cc b/ash/accelerators/accelerator_controller_impl.cc
index 01e2995..ee77a198 100644
--- a/ash/accelerators/accelerator_controller_impl.cc
+++ b/ash/accelerators/accelerator_controller_impl.cc
@@ -301,7 +301,7 @@
if (accelerator.key_code() == ui::VKEY_F13 &&
Shell::Get()->keyboard_capability()->HasFunctionKey(
accelerator.source_device_id())) {
- CHECK(features::IsModifierSplitEnabled());
+ CHECK(Shell::Get()->keyboard_capability()->IsModifierSplitEnabled());
return false;
}
return true;
@@ -324,7 +324,7 @@
// Check if from modifier split keyboard. if not, show notification.
if (Shell::Get()->keyboard_capability()->HasFunctionKey(
accelerator.source_device_id())) {
- CHECK(features::IsModifierSplitEnabled());
+ CHECK(Shell::Get()->keyboard_capability()->IsModifierSplitEnabled());
notification_controller->ShowCapsLockRewritingNudge();
return false;
}
diff --git a/ash/accelerators/accelerator_controller_unittest.cc b/ash/accelerators/accelerator_controller_unittest.cc
index 285dfc4c..480b82fa 100644
--- a/ash/accelerators/accelerator_controller_unittest.cc
+++ b/ash/accelerators/accelerator_controller_unittest.cc
@@ -100,6 +100,7 @@
#include "ui/base/accelerators/accelerator.h"
#include "ui/base/accelerators/media_keys_util.h"
#include "ui/base/accelerators/test_accelerator_target.h"
+#include "ui/base/ime/ash/fake_ime_keyboard.h"
#include "ui/base/ime/ash/mock_input_method_manager.h"
#include "ui/base/ime/init/input_method_factory.h"
#include "ui/base/ime/mock_input_method.h"
@@ -2874,6 +2875,11 @@
return use_positional_shortcuts_;
}
+ input_method::ImeKeyboard* GetImeKeyboard() override {
+ return &ime_keyboard_;
+ }
+
+ input_method::FakeImeKeyboard ime_keyboard_;
base::ObserverList<InputMethodManager::Observer>::Unchecked observers_;
bool use_positional_shortcuts_ = false;
};
diff --git a/ash/auth/test/active_session_auth_controller_unittest.cc b/ash/auth/test/active_session_auth_controller_unittest.cc
index cb5a0db..808e6e6 100644
--- a/ash/auth/test/active_session_auth_controller_unittest.cc
+++ b/ash/auth/test/active_session_auth_controller_unittest.cc
@@ -34,16 +34,14 @@
using OnAuthComplete =
base::test::TestFuture<bool, const ash::AuthProofToken&, base::TimeDelta>;
- ActiveSessionAuthControllerTest() {
+ void SetUp() override {
InitializeUserManager();
AddUserToUserManager();
SystemSaltGetter::Initialize();
CryptohomeMiscClient::InitializeFake();
UserDataAuthClient::InitializeFake();
auth_parts_ = AuthParts::Create(&local_state_);
- }
- void SetUp() override {
AshTestBase::SetUp();
GetSessionControllerClient()->DisableAutomaticallyProvideSigninPref();
@@ -59,6 +57,7 @@
auth_parts_.reset();
user_manager_->Destroy();
+ user_manager_.reset();
SystemSaltGetter::Shutdown();
CryptohomeMiscClient::Shutdown();
UserDataAuthClient::Shutdown();
diff --git a/ash/constants/ash_features.cc b/ash/constants/ash_features.cc
index 2a4f6dce..b4a45b83 100644
--- a/ash/constants/ash_features.cc
+++ b/ash/constants/ash_features.cc
@@ -1944,6 +1944,12 @@
// Enables to split left and right modifiers in settings.
BASE_FEATURE(kModifierSplit, "ModifierSplit", base::FEATURE_ENABLED_BY_DEFAULT);
+// Enables the ability to potentially override the secret key based on some
+// factors.
+BASE_FEATURE(kModifierSplitDeviceEnabled,
+ "ModifierSplitDeviceEnabled",
+ base::FEATURE_DISABLED_BY_DEFAULT);
+
// Enables to split left and right modifiers in settings for dogfood.
BASE_FEATURE(kModifierSplitDogfood,
"ModifierSplitDogfood",
@@ -2506,7 +2512,7 @@
base::FEATURE_ENABLED_BY_DEFAULT);
// Enables the new picker feature.
-BASE_FEATURE(kPicker, "Picker", base::FEATURE_DISABLED_BY_DEFAULT);
+BASE_FEATURE(kPicker, "Picker", base::FEATURE_ENABLED_BY_DEFAULT);
// Always show the feature tour for Picker for debugging purposes.
BASE_FEATURE(kPickerAlwaysShowFeatureTour,
@@ -4300,9 +4306,7 @@
bool IsModifierSplitEnabled() {
return IsInputDeviceSettingsSplitEnabled() &&
- base::FeatureList::IsEnabled(kModifierSplit) &&
- (IsModifierSplitDogfoodEnabled() ||
- switches::IsModifierSplitSecretKeyMatched());
+ base::FeatureList::IsEnabled(kModifierSplit);
}
bool IsModifierSplitDogfoodEnabled() {
diff --git a/ash/constants/ash_features.h b/ash/constants/ash_features.h
index 68ca36e..6be6e2f6 100644
--- a/ash/constants/ash_features.h
+++ b/ash/constants/ash_features.h
@@ -649,6 +649,8 @@
COMPONENT_EXPORT(ASH_CONSTANTS) BASE_DECLARE_FEATURE(kModifierSplit);
COMPONENT_EXPORT(ASH_CONSTANTS) BASE_DECLARE_FEATURE(kModifierSplitDogfood);
COMPONENT_EXPORT(ASH_CONSTANTS)
+BASE_DECLARE_FEATURE(kModifierSplitDeviceEnabled);
+COMPONENT_EXPORT(ASH_CONSTANTS)
BASE_DECLARE_FEATURE(kMouseImposterCheck);
COMPONENT_EXPORT(ASH_CONSTANTS) BASE_DECLARE_FEATURE(kMojoDBusRelay);
COMPONENT_EXPORT(ASH_CONSTANTS) BASE_DECLARE_FEATURE(kMultiCalendarSupport);
diff --git a/ash/picker/picker_controller.cc b/ash/picker/picker_controller.cc
index bd6e175c..5752306 100644
--- a/ash/picker/picker_controller.cc
+++ b/ash/picker/picker_controller.cc
@@ -71,6 +71,8 @@
#include "ui/base/ime/text_input_client.h"
#include "ui/base/l10n/l10n_util.h"
#include "ui/display/screen.h"
+#include "ui/events/ash/keyboard_capability.h"
+#include "ui/events/devices/device_data_manager.h"
#include "ui/gfx/geometry/point.h"
#include "ui/gfx/geometry/rect.h"
@@ -107,19 +109,23 @@
constexpr float kCapsLockRatioThresholdForBottom = 0.2;
PickerFeatureKeyType MatchPickerFeatureKeyHash() {
- // Command line looks like:
- // out/Default/chrome --user-data-dir=/tmp/tmp123
- // --picker-feature-key="INSERT KEY HERE" --enable-features=PickerFeature
- const std::string provided_key_hash = base::SHA1HashString(
- base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
- switches::kPickerFeatureKey));
- if (provided_key_hash == kPickerFeatureDevKeyHash) {
- return PickerFeatureKeyType::kDev;
- }
- if (provided_key_hash == kPickerFeatureTestKeyHash) {
- return PickerFeatureKeyType::kTest;
- }
- return PickerFeatureKeyType::kNone;
+ static const PickerFeatureKeyType key_type = []() {
+ // Command line looks like:
+ // out/Default/chrome --user-data-dir=/tmp/tmp123
+ // --picker-feature-key="INSERT KEY HERE" --enable-features=PickerFeature
+ const std::string provided_key_hash = base::SHA1HashString(
+ base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
+ switches::kPickerFeatureKey));
+ if (provided_key_hash == kPickerFeatureDevKeyHash) {
+ return PickerFeatureKeyType::kDev;
+ }
+ if (provided_key_hash == kPickerFeatureTestKeyHash) {
+ return PickerFeatureKeyType::kTest;
+ }
+ return PickerFeatureKeyType::kNone;
+ }();
+
+ return key_type;
}
ui::TextInputClient* GetFocusedTextInputClient() {
@@ -297,14 +303,17 @@
CloseCapsLockStateView();
}
-bool PickerController::IsFeatureKeyMatched() {
+bool PickerController::IsFeatureEnabled() {
+ if (!features::IsPickerUpdateEnabled()) {
+ return false;
+ }
+
if (!g_should_check_key) {
return true;
}
- if (base::FeatureList::IsEnabled(ash::features::kPickerDogfood)) {
- // This flag allows PickerController to be created, but ToggleWidget will
- // still check if the feature is allowed by the client.
+ if (base::FeatureList::IsEnabled(ash::features::kPickerDogfood) &&
+ client_->IsFeatureAllowedForDogfood()) {
return true;
}
@@ -316,8 +325,7 @@
return true;
}
-void PickerController::DisableFeatureKeyCheckForTesting() {
- CHECK_IS_TEST();
+void PickerController::DisableFeatureKeyCheck() {
g_should_check_key = false;
}
@@ -351,10 +359,7 @@
void PickerController::ToggleWidget(
const base::TimeTicks trigger_event_timestamp) {
- CHECK(client_);
- if (base::FeatureList::IsEnabled(ash::features::kPickerDogfood) &&
- !client_->IsFeatureAllowedForDogfood()) {
- LOG(ERROR) << "Picker feature is blocked";
+ if (!IsFeatureEnabled()) {
return;
}
diff --git a/ash/picker/picker_controller.h b/ash/picker/picker_controller.h
index df6271a..a9c1b586 100644
--- a/ash/picker/picker_controller.h
+++ b/ash/picker/picker_controller.h
@@ -26,6 +26,8 @@
#include "base/timer/timer.h"
#include "ui/base/emoji/emoji_panel_helper.h"
#include "ui/base/ime/ash/ime_keyboard.h"
+#include "ui/events/devices/device_data_manager.h"
+#include "ui/events/devices/input_device_event_observer.h"
#include "ui/views/view_observer.h"
#include "ui/views/widget/unique_widget_ptr.h"
@@ -53,9 +55,6 @@
PickerController& operator=(const PickerController&) = delete;
~PickerController() override;
- // Whether the provided feature key for Picker can enable the feature.
- static bool IsFeatureKeyMatched();
-
// Maximum time to wait for focus to be regained after completing the feature
// tour. If this timeout is reached, we stop waiting for focus and show the
// Picker widget regardless of the focus state.
@@ -69,6 +68,13 @@
// published.
static constexpr base::TimeDelta kBurnInPeriod = base::Milliseconds(200);
+ // Disables the feature key checking.
+ static void DisableFeatureKeyCheck();
+
+ // Whether the feature is currently enabled or not based on the secret key and
+ // other factors.
+ bool IsFeatureEnabled();
+
// Sets the `client` used by this class and the widget to communicate with the
// browser. `client` may be set to null, which will close the Widget if it's
// open, and may call "stop search" methods on the PREVIOUS client.
@@ -137,8 +143,6 @@
void OnCapsLockChanged(bool enabled) override;
void OnLayoutChanging(const std::string& layout_name) override;
- // Disables the feature key checking. Only works in tests.
- static void DisableFeatureKeyCheckForTesting();
// Disables the feature tour. Only works in tests.
static void DisableFeatureTourForTesting();
diff --git a/ash/picker/picker_controller_unittest.cc b/ash/picker/picker_controller_unittest.cc
index 319b256..a949666 100644
--- a/ash/picker/picker_controller_unittest.cc
+++ b/ash/picker/picker_controller_unittest.cc
@@ -174,9 +174,9 @@
raw_ptr<sync_preferences::TestingPrefServiceSyncable> prefs_ = nullptr;
};
-class PickerControllerTest : public AshTestBase {
+class PickerControllerTestBase : public AshTestBase {
public:
- PickerControllerTest()
+ PickerControllerTestBase()
: AshTestBase(base::test::TaskEnvironment::TimeSource::MOCK_TIME) {
auto delegate = std::make_unique<MockNewWindowDelegate>();
new_window_delegate_ = delegate.get();
@@ -229,6 +229,13 @@
metrics_recorder_;
};
+class PickerControllerTest : public PickerControllerTestBase {
+ void SetUp() override {
+ PickerControllerTestBase::SetUp();
+ PickerController::DisableFeatureKeyCheck();
+ }
+};
+
TEST_F(PickerControllerTest, ToggleWidgetShowsWidgetIfClosed) {
controller().ToggleWidget();
@@ -520,28 +527,6 @@
EXPECT_FALSE(controller().widget_for_testing());
}
-TEST_F(PickerControllerTest,
- ToggleWidgetShowsWidgetForDogfoodWhenClientAllowed) {
- base::test::ScopedFeatureList features(ash::features::kPickerDogfood);
-
- EXPECT_CALL(client(), IsFeatureAllowedForDogfood).WillOnce(Return(true));
-
- controller().ToggleWidget();
-
- EXPECT_TRUE(controller().widget_for_testing());
-}
-
-TEST_F(PickerControllerTest,
- ToggleWidgetDoesNotShowWidgetWhenClientDisallowsDogfood) {
- base::test::ScopedFeatureList features(ash::features::kPickerDogfood);
-
- EXPECT_CALL(client(), IsFeatureAllowedForDogfood).WillOnce(Return(false));
-
- controller().ToggleWidget();
-
- EXPECT_FALSE(controller().widget_for_testing());
-}
-
TEST_F(PickerControllerTest, SetClientToNullKeepsWidget) {
controller().ToggleWidget();
@@ -1139,6 +1124,30 @@
PickerCapsLockPosition::kBottom);
}
+class PickerControllerKeyEnabledTest : public PickerControllerTestBase {};
+
+TEST_F(PickerControllerKeyEnabledTest,
+ ToggleWidgetShowsWidgetForDogfoodWhenClientAllowed) {
+ base::test::ScopedFeatureList features(ash::features::kPickerDogfood);
+
+ EXPECT_CALL(client(), IsFeatureAllowedForDogfood).WillOnce(Return(true));
+
+ controller().ToggleWidget();
+
+ EXPECT_TRUE(controller().widget_for_testing());
+}
+
+TEST_F(PickerControllerKeyEnabledTest,
+ ToggleWidgetDoesNotShowWidgetWhenClientDisallowsDogfood) {
+ base::test::ScopedFeatureList features(ash::features::kPickerDogfood);
+
+ EXPECT_CALL(client(), IsFeatureAllowedForDogfood).WillOnce(Return(false));
+
+ controller().ToggleWidget();
+
+ EXPECT_FALSE(controller().widget_for_testing());
+}
+
struct ActionTestCase {
PickerSearchResult result;
std::optional<PickerActionType> unfocused_action;
diff --git a/ash/shell.cc b/ash/shell.cc
index d8633e4..cd0c51e 100644
--- a/ash/shell.cc
+++ b/ash/shell.cc
@@ -837,6 +837,7 @@
// since the latters destructor triggers events that the former is listening
// to but no longer cares about.
keyboard_controller_->DestroyVirtualKeyboard();
+ picker_controller_.reset();
// Depends on |tablet_mode_controller_|.
window_restore_controller_.reset();
@@ -1816,8 +1817,7 @@
coral_controller_ = std::make_unique<CoralController>();
}
- if (features::IsPickerUpdateEnabled() &&
- PickerController::IsFeatureKeyMatched()) {
+ if (features::IsPickerUpdateEnabled()) {
picker_controller_ = std::make_unique<PickerController>();
}
diff --git a/ash/shell_unittest.cc b/ash/shell_unittest.cc
index 9ca590a..35222e5 100644
--- a/ash/shell_unittest.cc
+++ b/ash/shell_unittest.cc
@@ -596,39 +596,6 @@
EXPECT_TRUE(status_area_widget->GetNativeView()->HasFocus());
}
-class ShellPickerIncorrectKeyTest : public AshTestBase {
- public:
- ShellPickerIncorrectKeyTest() {
- feature_list_.InitWithFeatures({features::kPicker},
- {features::kPickerDogfood});
-
- base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
- command_line->AppendSwitchASCII(switches::kPickerFeatureKey, "hello");
- }
-
- private:
- base::test::ScopedFeatureList feature_list_;
-};
-
-TEST_F(ShellPickerIncorrectKeyTest, NoPickerControllerIfFeatureKeyIsWrong) {
- EXPECT_FALSE(Shell::Get()->picker_controller());
-}
-
-class ShellPickerDogfoodTest : public AshTestBase {
- public:
- ShellPickerDogfoodTest() {
- feature_list_.InitWithFeatures(
- {features::kPicker, features::kPickerDogfood}, {});
- }
-
- private:
- base::test::ScopedFeatureList feature_list_;
-};
-
-TEST_F(ShellPickerDogfoodTest, PickerControllerExistsIfDogfooding) {
- EXPECT_TRUE(Shell::Get()->picker_controller());
-}
-
// This verifies WindowObservers are removed when a window is destroyed after
// the Shell is destroyed. This scenario (aura::Windows being deleted after the
// Shell) occurs if someone is holding a reference to an unparented Window, as
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 80ca52f..6e08703 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
@@ -32,6 +32,7 @@
#include "ash/system/input_device_settings/input_device_settings_policy_handler.h"
#include "ash/system/input_device_settings/input_device_settings_pref_names.h"
#include "ash/system/input_device_settings/input_device_settings_utils.h"
+#include "ash/system/input_device_settings/modifier_split_bypass_checker.h"
#include "ash/system/input_device_settings/pref_handlers/graphics_tablet_pref_handler_impl.h"
#include "ash/system/input_device_settings/pref_handlers/keyboard_pref_handler_impl.h"
#include "ash/system/input_device_settings/pref_handlers/mouse_pref_handler_impl.h"
@@ -40,6 +41,7 @@
#include "base/command_line.h"
#include "base/containers/contains.h"
#include "base/containers/flat_map.h"
+#include "base/feature_list.h"
#include "base/functional/bind.h"
#include "base/functional/callback_forward.h"
#include "base/hash/sha1.h"
@@ -672,6 +674,12 @@
CHECK(input_method::InputMethodManager::Get());
input_method::InputMethodManager::Get()->AddObserver(this);
+ if (features::IsModifierSplitEnabled() &&
+ base::FeatureList::IsEnabled(features::kModifierSplitDeviceEnabled)) {
+ modifier_split_bypass_checker_ =
+ std::make_unique<ModifierSplitBypassChecker>();
+ }
+
if (features::IsWelcomeExperienceEnabled()) {
message_center::MessageCenter::Get()->AddObserver(this);
device::BluetoothAdapterFactory::Get()->GetAdapter(base::BindOnce(
@@ -1130,6 +1138,16 @@
}
}
+void InputDeviceSettingsControllerImpl::
+ ForceKeyboardSettingRefreshWhenFeatureEnabled() {
+ sequenced_task_runner_->PostTask(
+ FROM_HERE,
+ base::BindOnce(
+ &InputDeviceSettingsControllerImpl::RefreshMetaAndModifierKeys,
+ weak_ptr_factory_.GetWeakPtr()));
+ ScheduleDeviceSettingsRefresh();
+}
+
void InputDeviceSettingsControllerImpl::ScheduleDeviceSettingsRefresh() {
if (settings_refresh_pending_) {
return;
@@ -1141,8 +1159,9 @@
Shell::Get()->keyboard_capability()->IsModifierSplitEnabled()) {
sequenced_task_runner_->PostTask(
FROM_HERE,
- base::BindOnce(&InputDeviceSettingsControllerImpl::RefreshModifierKeys,
- weak_ptr_factory_.GetWeakPtr()));
+ base::BindOnce(
+ &InputDeviceSettingsControllerImpl::RefreshMetaAndModifierKeys,
+ weak_ptr_factory_.GetWeakPtr()));
}
settings_refresh_pending_ = true;
@@ -2932,8 +2951,10 @@
RefreshBatteryInfoForConnectedDevices();
}
-void InputDeviceSettingsControllerImpl::RefreshModifierKeys() {
+void InputDeviceSettingsControllerImpl::RefreshMetaAndModifierKeys() {
for (auto& [_, keyboard] : keyboards_) {
+ keyboard->meta_key =
+ Shell::Get()->keyboard_capability()->GetMetaKey(keyboard->id);
keyboard->modifier_keys =
Shell::Get()->keyboard_capability()->GetModifierKeys(keyboard->id);
}
diff --git a/ash/system/input_device_settings/input_device_settings_controller_impl.h b/ash/system/input_device_settings/input_device_settings_controller_impl.h
index faadd151..f5b917107 100644
--- a/ash/system/input_device_settings/input_device_settings_controller_impl.h
+++ b/ash/system/input_device_settings/input_device_settings_controller_impl.h
@@ -22,6 +22,7 @@
#include "ash/system/input_device_settings/input_device_settings_metrics_manager.h"
#include "ash/system/input_device_settings/input_device_settings_notification_controller.h"
#include "ash/system/input_device_settings/input_device_settings_policy_handler.h"
+#include "ash/system/input_device_settings/modifier_split_bypass_checker.h"
#include "ash/system/input_device_settings/pref_handlers/graphics_tablet_pref_handler.h"
#include "ash/system/input_device_settings/pref_handlers/keyboard_pref_handler.h"
#include "ash/system/input_device_settings/pref_handlers/mouse_pref_handler.h"
@@ -71,6 +72,10 @@
static void RegisterProfilePrefs(PrefRegistrySimple* pref_registry);
+ // Refreshes keyboard info and settings. To be used when the feature is first
+ // forcibly enabled.
+ void ForceKeyboardSettingRefreshWhenFeatureEnabled();
+
// InputDeviceSettingsController:
std::vector<mojom::KeyboardPtr> GetConnectedKeyboards() override;
std::vector<mojom::TouchpadPtr> GetConnectedTouchpads() override;
@@ -297,9 +302,9 @@
// current input method.
void RefreshKeyDisplay();
- // Refresh modifier keys when they potentially changed due to flags being
- // enabled.
- void RefreshModifierKeys();
+ // Refresh meta and modifier keys when they potentially changed due to flags
+ // being enabled.
+ void RefreshMetaAndModifierKeys();
// Get the mouse button config based on the mouse metadata. Return
// kDefault by default if there is no mouse metadata.
@@ -351,6 +356,8 @@
raw_ptr<PrefService> local_state_ = nullptr; // Not owned.
+ std::unique_ptr<ModifierSplitBypassChecker> modifier_split_bypass_checker_;
+
std::unique_ptr<KeyboardPrefHandler> keyboard_pref_handler_;
std::unique_ptr<TouchpadPrefHandler> touchpad_pref_handler_;
std::unique_ptr<MousePrefHandler> mouse_pref_handler_;
diff --git a/ash/system/input_device_settings/modifier_split_bypass_checker.cc b/ash/system/input_device_settings/modifier_split_bypass_checker.cc
new file mode 100644
index 0000000..ff7c634
--- /dev/null
+++ b/ash/system/input_device_settings/modifier_split_bypass_checker.cc
@@ -0,0 +1,70 @@
+// Copyright 2024 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "ash/system/input_device_settings/modifier_split_bypass_checker.h"
+
+#include "ash/constants/ash_features.h"
+#include "ash/picker/picker_controller.h"
+#include "ash/shell.h"
+#include "ash/system/input_device_settings/input_device_settings_controller_impl.h"
+#include "ui/events/ash/keyboard_capability.h"
+#include "ui/events/devices/device_data_manager.h"
+#include "ui/events/devices/input_device_event_observer.h"
+
+namespace ash {
+
+ModifierSplitBypassChecker::ModifierSplitBypassChecker() {
+ CHECK(features::IsModifierSplitEnabled());
+ CHECK(base::FeatureList::IsEnabled(features::kModifierSplitDeviceEnabled));
+ if (Shell::Get()->keyboard_capability()->IsModifierSplitEnabled()) {
+ return;
+ }
+
+ StartCheckingToEnableFeature();
+}
+ModifierSplitBypassChecker::~ModifierSplitBypassChecker() = default;
+
+void ModifierSplitBypassChecker::OnInputDeviceConfigurationChanged(
+ uint8_t input_device_type) {
+ if (input_device_type & ui::InputDeviceEventObserver::kKeyboard) {
+ CheckIfFeaturesShouldbeEnabled();
+ }
+}
+
+void ModifierSplitBypassChecker::OnDeviceListsComplete() {
+ CheckIfFeaturesShouldbeEnabled();
+}
+
+void ModifierSplitBypassChecker::StartCheckingToEnableFeature() {
+ CheckIfFeaturesShouldbeEnabled();
+ input_device_event_observation_.Observe(ui::DeviceDataManager::GetInstance());
+}
+
+void ModifierSplitBypassChecker::CheckIfFeaturesShouldbeEnabled() {
+ const auto& keyboards =
+ ui::DeviceDataManager::GetInstance()->GetKeyboardDevices();
+ for (const auto& keyboard : keyboards) {
+ if (Shell::Get()->keyboard_capability()->IsSplitModifierKeyboardForOverride(
+ keyboard)) {
+ ForceEnableFeatures();
+ break;
+ }
+ }
+}
+
+void ModifierSplitBypassChecker::ForceEnableFeatures() {
+ Shell::Get()->keyboard_capability()->ForceEnableFeature();
+ Shell::Get()
+ ->input_device_settings_controller()
+ ->ForceKeyboardSettingRefreshWhenFeatureEnabled();
+ if (features::IsPickerUpdateEnabled()) {
+ Shell::Get()->picker_controller()->DisableFeatureKeyCheck();
+ }
+
+ // Reset observing as we are no longer interested in seeing when new keyboards
+ // connect.
+ input_device_event_observation_.Reset();
+}
+
+} // namespace ash
diff --git a/ash/system/input_device_settings/modifier_split_bypass_checker.h b/ash/system/input_device_settings/modifier_split_bypass_checker.h
new file mode 100644
index 0000000..195dc81
--- /dev/null
+++ b/ash/system/input_device_settings/modifier_split_bypass_checker.h
@@ -0,0 +1,37 @@
+// Copyright 2024 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef ASH_SYSTEM_INPUT_DEVICE_SETTINGS_MODIFIER_SPLIT_BYPASS_CHECKER_H_
+#define ASH_SYSTEM_INPUT_DEVICE_SETTINGS_MODIFIER_SPLIT_BYPASS_CHECKER_H_
+
+#include "base/scoped_observation.h"
+#include "ui/events/devices/device_data_manager.h"
+#include "ui/events/devices/input_device_event_observer.h"
+
+namespace ash {
+
+class ModifierSplitBypassChecker : public ui::InputDeviceEventObserver {
+ public:
+ ModifierSplitBypassChecker();
+ ModifierSplitBypassChecker(const ModifierSplitBypassChecker&) = delete;
+ ModifierSplitBypassChecker& operator=(const ModifierSplitBypassChecker&) =
+ delete;
+ ~ModifierSplitBypassChecker() override;
+
+ // ui::InputDeviceEventObserver
+ void OnInputDeviceConfigurationChanged(uint8_t input_device_type) override;
+ void OnDeviceListsComplete() override;
+
+ private:
+ void StartCheckingToEnableFeature();
+ void CheckIfFeaturesShouldbeEnabled();
+ void ForceEnableFeatures();
+
+ base::ScopedObservation<ui::DeviceDataManager, ui::InputDeviceEventObserver>
+ input_device_event_observation_{this};
+};
+
+} // namespace ash
+
+#endif // ASH_SYSTEM_INPUT_DEVICE_SETTINGS_MODIFIER_SPLIT_BYPASS_CHECKER_H_
diff --git a/ash/webui/shortcut_customization_ui/backend/accelerator_configuration_provider.cc b/ash/webui/shortcut_customization_ui/backend/accelerator_configuration_provider.cc
index 394c35a..fc62898c 100644
--- a/ash/webui/shortcut_customization_ui/backend/accelerator_configuration_provider.cc
+++ b/ash/webui/shortcut_customization_ui/backend/accelerator_configuration_provider.cc
@@ -616,7 +616,7 @@
#if BUILDFLAG(GOOGLE_CHROME_BRANDING)
if (layout_id == AcceleratorAction::kTogglePicker &&
- features::IsModifierSplitEnabled()) {
+ Shell::Get()->keyboard_capability()->IsModifierSplitEnabled()) {
layout->description_string_id = IDS_ASH_ACCELERATOR_DESCRIPTION_RIGHT_ALT;
}
#endif // BUILDFLAG(GOOGLE_CHROME_BRANDING)
diff --git a/ash/webui/shortcut_customization_ui/backend/accelerator_configuration_provider_unittest.cc b/ash/webui/shortcut_customization_ui/backend/accelerator_configuration_provider_unittest.cc
index ab03174..1efee1d8 100644
--- a/ash/webui/shortcut_customization_ui/backend/accelerator_configuration_provider_unittest.cc
+++ b/ash/webui/shortcut_customization_ui/backend/accelerator_configuration_provider_unittest.cc
@@ -45,6 +45,8 @@
#include "mojo/public/cpp/bindings/pending_remote.h"
#include "mojo/public/cpp/bindings/receiver.h"
#include "testing/gtest/include/gtest/gtest.h"
+#include "ui/base/ime/ash/fake_ime_keyboard.h"
+#include "ui/base/ime/ash/ime_keyboard.h"
#include "ui/base/ime/ash/input_method_manager.h"
#include "ui/base/ime/ash/mock_input_method_manager.h"
#include "ui/base/l10n/l10n_util.h"
@@ -350,6 +352,11 @@
}
}
+ input_method::ImeKeyboard* GetImeKeyboard() override {
+ return &ime_keyboard_;
+ }
+
+ input_method::FakeImeKeyboard ime_keyboard_;
base::ObserverList<InputMethodManager::Observer>::Unchecked observers_;
};
diff --git a/chrome/browser/ui/ash/picker/picker_accessibility_browsertest.cc b/chrome/browser/ui/ash/picker/picker_accessibility_browsertest.cc
index 53e395f4..c7d0351 100644
--- a/chrome/browser/ui/ash/picker/picker_accessibility_browsertest.cc
+++ b/chrome/browser/ui/ash/picker/picker_accessibility_browsertest.cc
@@ -749,6 +749,7 @@
IN_PROC_BROWSER_TEST_F(PickerAccessibilityBrowserTest,
InsertingAnnouncesInsertionBeforeTextfieldRefocus) {
ash::PickerController controller;
+ controller.DisableFeatureKeyCheck();
PickerClientImpl client(&controller, user_manager::UserManager::Get());
std::unique_ptr<views::Widget> textfield_widget =
ash::TestWidgetBuilder()
diff --git a/chrome/browser/ui/ash/picker/picker_interactive_uitest.cc b/chrome/browser/ui/ash/picker/picker_interactive_uitest.cc
index 36c7d67..ed37433 100644
--- a/chrome/browser/ui/ash/picker/picker_interactive_uitest.cc
+++ b/chrome/browser/ui/ash/picker/picker_interactive_uitest.cc
@@ -72,7 +72,7 @@
};
PickerInteractiveUiTest() {
- ash::PickerController::DisableFeatureKeyCheckForTesting();
+ ash::PickerController::DisableFeatureKeyCheck();
ash::PickerController::DisableFeatureTourForTesting();
}
diff --git a/ui/events/ash/keyboard_capability.cc b/ui/events/ash/keyboard_capability.cc
index 2e82c5a..cd22b9e 100644
--- a/ui/events/ash/keyboard_capability.cc
+++ b/ui/events/ash/keyboard_capability.cc
@@ -1090,6 +1090,17 @@
return HasRightAltKeyForOobe(*keyboard);
}
+bool KeyboardCapability::IsSplitModifierKeyboardForOverride(
+ const KeyboardDevice& keyboard) const {
+ if (kRightAltBlocklist.contains(board_name_)) {
+ return false;
+ }
+
+ return ash::features::IsModifierSplitEnabled() &&
+ keyboard.type == InputDeviceType::INPUT_DEVICE_INTERNAL &&
+ keyboard.has_function_key && keyboard.has_assistant_key;
+}
+
ui::mojom::MetaKey KeyboardCapability::GetMetaKey(
const KeyboardDevice& keyboard) const {
const auto device_type = GetDeviceType(keyboard);
@@ -1130,6 +1141,15 @@
}
}
+ui::mojom::MetaKey KeyboardCapability::GetMetaKey(int device_id) const {
+ auto keyboard = FindKeyboardWithId(device_id);
+ if (!keyboard) {
+ return mojom::MetaKey::kLauncher;
+ }
+
+ return GetMetaKey(*keyboard);
+}
+
ui::mojom::MetaKey KeyboardCapability::GetMetaKeyToDisplay() const {
ui::mojom::MetaKey current_best = ui::mojom::MetaKey::kExternalMeta;
for (const ui::KeyboardDevice& keyboard :
@@ -1297,6 +1317,10 @@
board_name_ = board_name;
}
+void KeyboardCapability::ForceEnableFeature() {
+ modifier_split_dogfood_controller_->ForceEnableFeature();
+}
+
void KeyboardCapability::ResetModifierSplitDogfoodControllerForTesting() {
modifier_split_dogfood_controller_ =
std::make_unique<ModifierSplitDogfoodController>(); // IN-TEST
diff --git a/ui/events/ash/keyboard_capability.h b/ui/events/ash/keyboard_capability.h
index f8e9b5d..79835ff6 100644
--- a/ui/events/ash/keyboard_capability.h
+++ b/ui/events/ash/keyboard_capability.h
@@ -344,6 +344,7 @@
// Returns the appropriate meta key present on the given keyboard.
ui::mojom::MetaKey GetMetaKey(const KeyboardDevice& keyboard) const;
+ ui::mojom::MetaKey GetMetaKey(int device_id) const;
// Returns the meta key to display in the UI to represent the overall current
// keyboard situation. This will only return either Launcher, Search, or
@@ -377,6 +378,10 @@
const KeyboardDevice& keyboard) const;
const std::vector<TopRowActionKey>* GetTopRowActionKeys(int device_id) const;
+ // Whether or not the given keyboard is a split modifier keyboard and
+ // qualifies to forcibly enable features.
+ bool IsSplitModifierKeyboardForOverride(const KeyboardDevice& keyboard) const;
+
void SetBoardNameForTesting(const std::string& board_name);
const base::flat_map<int, KeyboardInfo>& keyboard_info_map() const {
@@ -387,6 +392,8 @@
return modifier_split_dogfood_controller_->IsEnabled();
}
+ void ForceEnableFeature();
+
void ResetModifierSplitDogfoodControllerForTesting();
private:
diff --git a/ui/events/ash/keyboard_capability_unittest.cc b/ui/events/ash/keyboard_capability_unittest.cc
index 6d0dd59..a653993 100644
--- a/ui/events/ash/keyboard_capability_unittest.cc
+++ b/ui/events/ash/keyboard_capability_unittest.cc
@@ -171,7 +171,7 @@
FakeDeviceManager() = default;
FakeDeviceManager(const FakeDeviceManager&) = delete;
FakeDeviceManager& operator=(const FakeDeviceManager&) = delete;
- ~FakeDeviceManager() = default;
+ ~FakeDeviceManager() { RemoveAllDevices(); }
// Add a fake keyboard to DeviceDataManagerTestApi and provide layout info to
// fake udev.
diff --git a/ui/events/ash/modifier_split_dogfood_controller.cc b/ui/events/ash/modifier_split_dogfood_controller.cc
index 3a8271e..dcc7b0e 100644
--- a/ui/events/ash/modifier_split_dogfood_controller.cc
+++ b/ui/events/ash/modifier_split_dogfood_controller.cc
@@ -6,7 +6,9 @@
#include "ash/constants/ash_features.h"
#include "ash/constants/ash_switches.h"
+#include "base/location.h"
#include "base/strings/string_util.h"
+#include "base/system/sys_info.h"
#include "components/user_manager/user_manager.h"
#include "google_apis/gaia/gaia_auth_util.h"
@@ -16,8 +18,7 @@
// Dogfood flag should be ignored and not considered if the secret key
// matches.
modifier_split_enabled_ = ash::features::IsModifierSplitEnabled() &&
- (ash::switches::IsModifierSplitSecretKeyMatched() ||
- !ash::features::IsModifierSplitDogfoodEnabled());
+ ash::switches::IsModifierSplitSecretKeyMatched();
if (user_manager::UserManager::IsInitialized() &&
ash::features::IsModifierSplitEnabled()) {
@@ -32,6 +33,10 @@
}
}
+void ModifierSplitDogfoodController::ForceEnableFeature() {
+ modifier_split_enabled_ = true;
+}
+
void ModifierSplitDogfoodController::OnUserLoggedIn(
const user_manager::User& user) {
if (modifier_split_enabled_) {
diff --git a/ui/events/ash/modifier_split_dogfood_controller.h b/ui/events/ash/modifier_split_dogfood_controller.h
index dcb1dab..ac24b649 100644
--- a/ui/events/ash/modifier_split_dogfood_controller.h
+++ b/ui/events/ash/modifier_split_dogfood_controller.h
@@ -25,6 +25,9 @@
bool IsEnabled() const { return modifier_split_enabled_; }
+ void ForceEnableFeature();
+
+ // user_manager::UserManager::Observer:
void OnUserLoggedIn(const user_manager::User& user) override;
private: