Skip to content

Commit

Permalink
accelerators: Refactor EventRewriterChromeOS (Move out enums)
Browse files Browse the repository at this point in the history
Move two enums DeviceType and KeyboardTopRowLayout to the new
KeyboardCapability API.

chrome/browser/ash/events/event_rewriter_unittest.cc
chrome/browser/ash/events/keyboard_capability_unittest.cc
input_data_provider_keyboard_unittest.cc
input_data_provider_unittest.cc

Bug: b:216049298
Test: accelerator_configuration_provider_unittest.cc
Change-Id: Ia0cbbc2f71750cc2fe1964faf796f50986b35569
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4086953
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>
Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Commit-Queue: Wenyu Zhang <zhangwenyu@google.com>
Cr-Commit-Position: refs/heads/main@{#1085242}
  • Loading branch information
wenyu zhang authored and Chromium LUCI CQ committed Dec 20, 2022
1 parent 3397d86 commit 06508ae
Show file tree
Hide file tree
Showing 11 changed files with 207 additions and 167 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ InputDataProviderKeyboard::AuxData::~AuxData() = default;

void InputDataProviderKeyboard::ProcessKeyboardTopRowLayout(
const InputDeviceInformation* device_info,
ui::EventRewriterChromeOS::KeyboardTopRowLayout top_row_layout,
ui::KeyboardCapability::KeyboardTopRowLayout top_row_layout,
const base::flat_map<uint32_t, ui::EventRewriterChromeOS::MutableKeyState>&
scan_code_map,
std::vector<mojom::TopRowKey>* out_top_row_keys,
Expand All @@ -287,15 +287,15 @@ void InputDataProviderKeyboard::ProcessKeyboardTopRowLayout(
base::flat_map<uint32_t, uint32_t> top_row_key_scancode_indexes;

switch (top_row_layout) {
case ui::EventRewriterChromeOS::kKbdTopRowLayoutWilco:
case ui::KeyboardCapability::KeyboardTopRowLayout::kKbdTopRowLayoutWilco:
top_row_keys.assign(std::begin(kSystemKeysWilco),
std::end(kSystemKeysWilco));

for (size_t i = 0; i < top_row_keys.size(); i++)
top_row_key_scancode_indexes[kScancodesWilco[i]] = i;
break;

case ui::EventRewriterChromeOS::kKbdTopRowLayoutDrallion:
case ui::KeyboardCapability::KeyboardTopRowLayout::kKbdTopRowLayoutDrallion:
top_row_keys.assign(std::begin(kSystemKeysDrallion),
std::end(kSystemKeysDrallion));

Expand All @@ -318,7 +318,7 @@ void InputDataProviderKeyboard::ProcessKeyboardTopRowLayout(

break;

case ui::EventRewriterChromeOS::kKbdTopRowLayoutCustom: {
case ui::KeyboardCapability::KeyboardTopRowLayout::kKbdTopRowLayoutCustom: {
// Process scan-code map generated from custom top-row key layout: it maps
// from physical scan codes to several things, including VKEY key-codes,
// which we will use to derive a linear index.
Expand Down Expand Up @@ -365,14 +365,14 @@ void InputDataProviderKeyboard::ProcessKeyboardTopRowLayout(
}
break;
}
case ui::EventRewriterChromeOS::kKbdTopRowLayout2:
case ui::KeyboardCapability::KeyboardTopRowLayout::kKbdTopRowLayout2:
top_row_keys.assign(std::begin(kSystemKeys2), std::end(kSystemKeys2));
// No specific top_row_key_scancode_indexes are needed
// for classic ChromeOS keyboards, as they do not have an /Fn/ key and
// only emit /F[0-9]+/ keys.
break;

case ui::EventRewriterChromeOS::kKbdTopRowLayout1:
case ui::KeyboardCapability::KeyboardTopRowLayout::kKbdTopRowLayout1:
default:
top_row_keys.assign(std::begin(kSystemKeys1), std::end(kSystemKeys1));
// No specific top_row_key_scancode_indexes are needed for classic
Expand Down Expand Up @@ -408,19 +408,19 @@ mojom::KeyboardInfoPtr InputDataProviderKeyboard::ConstructKeyboard(

// Work out the physical layout.
if (device_info->keyboard_type ==
ui::EventRewriterChromeOS::DeviceType::kDeviceInternalKeyboard) {
ui::KeyboardCapability::DeviceType::kDeviceInternalKeyboard) {
// Reven boards have unknown keyboard layouts and should not be considered
// internal keyboards for the purposes of diagnostics.
if (chromeos::switches::IsRevenBranding()) {
result->physical_layout = mojom::PhysicalLayout::kUnknown;
result->connection_type = mojom::ConnectionType::kUnknown;
} else if (device_info->keyboard_top_row_layout ==
ui::EventRewriterChromeOS::KeyboardTopRowLayout::
ui::KeyboardCapability::KeyboardTopRowLayout::
kKbdTopRowLayoutWilco) {
result->physical_layout =
mojom::PhysicalLayout::kChromeOSDellEnterpriseWilco;
} else if (device_info->keyboard_top_row_layout ==
ui::EventRewriterChromeOS::KeyboardTopRowLayout::
ui::KeyboardCapability::KeyboardTopRowLayout::
kKbdTopRowLayoutDrallion) {
result->physical_layout =
mojom::PhysicalLayout::kChromeOSDellEnterpriseDrallion;
Expand Down Expand Up @@ -455,7 +455,7 @@ mojom::KeyboardInfoPtr InputDataProviderKeyboard::ConstructKeyboard(
LOG(ERROR) << "OS believes internal numberpad is implemented, but "
"evdev disagrees.";
} else if (device_info->keyboard_top_row_layout ==
ui::EventRewriterChromeOS::KeyboardTopRowLayout::
ui::KeyboardCapability::KeyboardTopRowLayout::
kKbdTopRowLayoutCustom) {
// If keyboard has WWCB top row custom layout (vivaldi) then we can trust
// the HID descriptor to be accurate about presence of keys.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

#include "ash/webui/diagnostics_ui/mojom/input_data_provider.mojom.h"
#include "ui/chromeos/events/event_rewriter_chromeos.h"
#include "ui/chromeos/events/keyboard_capability.h"

namespace ash {
namespace diagnostics {
Expand Down Expand Up @@ -53,7 +54,7 @@ class InputDataProviderKeyboard {
private:
void ProcessKeyboardTopRowLayout(
const InputDeviceInformation* device_info,
ui::EventRewriterChromeOS::KeyboardTopRowLayout top_row_layout,
ui::KeyboardCapability::KeyboardTopRowLayout top_row_layout,
const base::flat_map<uint32_t,
ui::EventRewriterChromeOS::MutableKeyState>&
scan_code_map,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,9 +153,9 @@ class InputDataProviderKeyboardTest : public ash::AshTestBase {
ui::CapabilitiesToDeviceInfo(capabilities,
&device_information.event_device_info);
device_information.keyboard_type =
ui::EventRewriterChromeOS::kDeviceInternalKeyboard;
ui::KeyboardCapability::DeviceType::kDeviceInternalKeyboard;
device_information.keyboard_top_row_layout =
ui::EventRewriterChromeOS::kKbdTopRowLayoutCustom;
ui::KeyboardCapability::KeyboardTopRowLayout::kKbdTopRowLayoutCustom;

device_information.input_device =
InputDeviceFromCapabilities(kEvdevId, capabilities);
Expand All @@ -175,9 +175,9 @@ class VivaldiKeyboardTestBase : public InputDataProviderKeyboardTest {
InputDataProviderKeyboardTest::SetUp();

device_information.keyboard_type =
ui::EventRewriterChromeOS::kDeviceInternalKeyboard;
ui::KeyboardCapability::DeviceType::kDeviceInternalKeyboard;
device_information.keyboard_top_row_layout =
ui::EventRewriterChromeOS::kKbdTopRowLayoutCustom;
ui::KeyboardCapability::KeyboardTopRowLayout::kKbdTopRowLayoutCustom;
}

void TearDown() override {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
#include "testing/gtest/include/gtest/gtest.h"
#include "ui/aura/client/aura_constants.h"
#include "ui/chromeos/events/event_rewriter_chromeos.h"
#include "ui/chromeos/events/keyboard_capability.h"
#include "ui/events/devices/device_data_manager.h"
#include "ui/events/devices/device_data_manager_test_api.h"
#include "ui/events/devices/touch_device_transform.h"
Expand Down Expand Up @@ -449,9 +450,9 @@ class FakeInputDeviceInfoHelper : public InputDeviceInfoHelper {
if (base_name == "event0") {
device_caps = ui::kLinkKeyboard;
info->keyboard_type =
ui::EventRewriterChromeOS::DeviceType::kDeviceInternalKeyboard;
ui::KeyboardCapability::DeviceType::kDeviceInternalKeyboard;
info->keyboard_top_row_layout =
ui::EventRewriterChromeOS::KeyboardTopRowLayout::kKbdTopRowLayout1;
ui::KeyboardCapability::KeyboardTopRowLayout::kKbdTopRowLayout1;
EXPECT_EQ(0, id);
} else if (base_name == "event1") {
device_caps = ui::kLinkTouchpad;
Expand All @@ -465,45 +466,45 @@ class FakeInputDeviceInfoHelper : public InputDeviceInfoHelper {
} else if (base_name == "event4") {
device_caps = ui::kHpUsbKeyboard;
info->keyboard_type =
ui::EventRewriterChromeOS::DeviceType::kDeviceExternalGenericKeyboard;
info->keyboard_top_row_layout = ui::EventRewriterChromeOS::
KeyboardTopRowLayout::kKbdTopRowLayoutDefault;
ui::KeyboardCapability::DeviceType::kDeviceExternalGenericKeyboard;
info->keyboard_top_row_layout =
ui::KeyboardCapability::KeyboardTopRowLayout::kKbdTopRowLayoutDefault;
EXPECT_EQ(4, id);
} else if (base_name == "event5") {
device_caps = ui::kSarienKeyboard; // Wilco
info->keyboard_type =
ui::EventRewriterChromeOS::DeviceType::kDeviceInternalKeyboard;
info->keyboard_top_row_layout = ui::EventRewriterChromeOS::
KeyboardTopRowLayout::kKbdTopRowLayoutWilco;
ui::KeyboardCapability::DeviceType::kDeviceInternalKeyboard;
info->keyboard_top_row_layout =
ui::KeyboardCapability::KeyboardTopRowLayout::kKbdTopRowLayoutWilco;
EXPECT_EQ(5, id);
} else if (base_name == "event6") {
device_caps = ui::kEveKeyboard;
info->keyboard_type =
ui::EventRewriterChromeOS::DeviceType::kDeviceInternalKeyboard;
ui::KeyboardCapability::DeviceType::kDeviceInternalKeyboard;
info->keyboard_top_row_layout =
ui::EventRewriterChromeOS::KeyboardTopRowLayout::kKbdTopRowLayout2;
ui::KeyboardCapability::KeyboardTopRowLayout::kKbdTopRowLayout2;
EXPECT_EQ(6, id);
} else if (base_name == "event7") {
device_caps = ui::kJinlonKeyboard;
info->keyboard_type =
ui::EventRewriterChromeOS::DeviceType::kDeviceInternalKeyboard;
info->keyboard_top_row_layout = ui::EventRewriterChromeOS::
KeyboardTopRowLayout::kKbdTopRowLayoutCustom;
ui::KeyboardCapability::DeviceType::kDeviceInternalKeyboard;
info->keyboard_top_row_layout =
ui::KeyboardCapability::KeyboardTopRowLayout::kKbdTopRowLayoutCustom;
info->keyboard_scan_code_map = kInternalJinlonScanCodeMap;
EXPECT_EQ(7, id);
} else if (base_name == "event8") {
device_caps = ui::kMicrosoftBluetoothNumberPad;
info->keyboard_type =
ui::EventRewriterChromeOS::DeviceType::kDeviceExternalGenericKeyboard;
info->keyboard_top_row_layout = ui::EventRewriterChromeOS::
KeyboardTopRowLayout::kKbdTopRowLayoutDefault;
ui::KeyboardCapability::DeviceType::kDeviceExternalGenericKeyboard;
info->keyboard_top_row_layout =
ui::KeyboardCapability::KeyboardTopRowLayout::kKbdTopRowLayoutDefault;
EXPECT_EQ(8, id);
} else if (base_name == "event9") {
device_caps = ui::kLogitechTouchKeyboardK400;
info->keyboard_type =
ui::EventRewriterChromeOS::DeviceType::kDeviceExternalGenericKeyboard;
info->keyboard_top_row_layout = ui::EventRewriterChromeOS::
KeyboardTopRowLayout::kKbdTopRowLayoutDefault;
ui::KeyboardCapability::DeviceType::kDeviceExternalGenericKeyboard;
info->keyboard_top_row_layout =
ui::KeyboardCapability::KeyboardTopRowLayout::kKbdTopRowLayoutDefault;
EXPECT_EQ(9, id);
} else if (base_name == "event10") {
device_caps = ui::kDrallionKeyboard;
Expand All @@ -513,9 +514,9 @@ class FakeInputDeviceInfoHelper : public InputDeviceInfoHelper {
device_caps = ui::kJinlonKeyboard;
device_caps.kbd_function_row_physmap = kModifiedJinlonDescriptor;
info->keyboard_type =
ui::EventRewriterChromeOS::DeviceType::kDeviceInternalKeyboard;
info->keyboard_top_row_layout = ui::EventRewriterChromeOS::
KeyboardTopRowLayout::kKbdTopRowLayoutCustom;
ui::KeyboardCapability::DeviceType::kDeviceInternalKeyboard;
info->keyboard_top_row_layout =
ui::KeyboardCapability::KeyboardTopRowLayout::kKbdTopRowLayoutCustom;
info->keyboard_scan_code_map = kInternalJinlonScanCodeMap;
info->keyboard_scan_code_map.erase(0x96);
info->keyboard_scan_code_map[0xC4] = {ui::EF_NONE, ui::DomCode::F8,
Expand All @@ -527,9 +528,9 @@ class FakeInputDeviceInfoHelper : public InputDeviceInfoHelper {
} else if (base_name == "event13") {
device_caps = ui::kHammerKeyboard;
info->keyboard_type =
ui::EventRewriterChromeOS::DeviceType::kDeviceInternalKeyboard;
ui::KeyboardCapability::DeviceType::kDeviceInternalKeyboard;
info->keyboard_top_row_layout =
ui::EventRewriterChromeOS::KeyboardTopRowLayout::kKbdTopRowLayout2;
ui::KeyboardCapability::KeyboardTopRowLayout::kKbdTopRowLayout2;
EXPECT_EQ(13, id);
} else if (base_name == "event14") {
device_caps = ui::kBaskingTouchScreen;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

#include "ash/webui/diagnostics_ui/mojom/input_data_provider.mojom.h"
#include "ui/chromeos/events/event_rewriter_chromeos.h"
#include "ui/chromeos/events/keyboard_capability.h"
#include "ui/events/devices/input_device.h"
#include "ui/events/ozone/evdev/event_device_info.h"

Expand All @@ -29,8 +30,8 @@ class InputDeviceInformation {
base::FilePath path;

// Keyboard-only fields:
ui::EventRewriterChromeOS::DeviceType keyboard_type;
ui::EventRewriterChromeOS::KeyboardTopRowLayout keyboard_top_row_layout;
ui::KeyboardCapability::DeviceType keyboard_type;
ui::KeyboardCapability::KeyboardTopRowLayout keyboard_top_row_layout;
base::flat_map<uint32_t, ui::EventRewriterChromeOS::MutableKeyState>
keyboard_scan_code_map;
};
Expand Down
40 changes: 27 additions & 13 deletions chrome/browser/ash/events/event_rewriter_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
#include "ui/base/ime/ash/fake_ime_keyboard.h"
#include "ui/base/ui_base_features.h"
#include "ui/chromeos/events/event_rewriter_chromeos.h"
#include "ui/chromeos/events/keyboard_capability.h"
#include "ui/chromeos/events/modifier_key.h"
#include "ui/chromeos/events/pref_names.h"
#include "ui/events/devices/device_data_manager.h"
Expand Down Expand Up @@ -80,9 +81,9 @@ constexpr char kKbdInvalidCustomTopRowLayout[] = "X X X";
// Values for enum output parameters to IdentifyKeyboard() that do not
// match any enum types.
const auto kImpossibleDeviceType =
static_cast<ui::EventRewriterChromeOS::DeviceType>(-1);
static_cast<ui::KeyboardCapability::DeviceType>(-1);
const auto kImpossibleKeyboardTopRowLayout =
static_cast<ui::EventRewriterChromeOS::KeyboardTopRowLayout>(-1);
static_cast<ui::KeyboardCapability::KeyboardTopRowLayout>(-1);

class TestEventRewriterContinuation
: public ui::test::TestEventRewriterContinuation {
Expand Down Expand Up @@ -4213,8 +4214,11 @@ TEST_F(EventRewriterTest, IdentifyKeyboardUnspecified) {
bool result = ui::EventRewriterChromeOS::IdentifyKeyboard(
input_device, &out_type, &out_layout, &scan_code_map);
EXPECT_TRUE(result);
EXPECT_EQ(out_type, ui::EventRewriterChromeOS::kDeviceInternalKeyboard);
EXPECT_EQ(out_layout, ui::EventRewriterChromeOS::kKbdTopRowLayoutDefault);
EXPECT_EQ(out_type,
ui::KeyboardCapability::DeviceType::kDeviceInternalKeyboard);
EXPECT_EQ(
out_layout,
ui::KeyboardCapability::KeyboardTopRowLayout::kKbdTopRowLayoutDefault);
EXPECT_TRUE(scan_code_map.empty());
}

Expand All @@ -4231,8 +4235,10 @@ TEST_F(EventRewriterTest, IdentifyKeyboardInvalidLayoutTag) {
bool result = ui::EventRewriterChromeOS::IdentifyKeyboard(
input_device, &out_type, &out_layout, &scan_code_map);
EXPECT_FALSE(result);
EXPECT_EQ(out_type, ui::EventRewriterChromeOS::kDeviceUnknown);
EXPECT_EQ(out_layout, ui::EventRewriterChromeOS::kKbdTopRowLayoutDefault);
EXPECT_EQ(out_type, ui::KeyboardCapability::DeviceType::kDeviceUnknown);
EXPECT_EQ(
out_layout,
ui::KeyboardCapability::KeyboardTopRowLayout::kKbdTopRowLayoutDefault);
EXPECT_TRUE(scan_code_map.empty());
}

Expand All @@ -4251,8 +4257,11 @@ TEST_F(EventRewriterTest, IdentifyKeyboardInvalidCustomLayout) {
// Unparsable custom top row layout attributes are ignored and the
// keyboard treated as default layout.
EXPECT_TRUE(result);
EXPECT_EQ(out_type, ui::EventRewriterChromeOS::kDeviceInternalKeyboard);
EXPECT_EQ(out_layout, ui::EventRewriterChromeOS::kKbdTopRowLayoutDefault);
EXPECT_EQ(out_type,
ui::KeyboardCapability::DeviceType::kDeviceInternalKeyboard);
EXPECT_EQ(
out_layout,
ui::KeyboardCapability::KeyboardTopRowLayout::kKbdTopRowLayoutDefault);
EXPECT_TRUE(scan_code_map.empty());
}

Expand All @@ -4269,9 +4278,11 @@ TEST_F(EventRewriterTest, IdentifyKeyboardExternalChrome) {
bool result = ui::EventRewriterChromeOS::IdentifyKeyboard(
input_device, &out_type, &out_layout, &scan_code_map);
EXPECT_TRUE(result);
EXPECT_EQ(out_type,
ui::EventRewriterChromeOS::kDeviceExternalChromeOsKeyboard);
EXPECT_EQ(out_layout, ui::EventRewriterChromeOS::kKbdTopRowLayout2);
EXPECT_EQ(
out_type,
ui::KeyboardCapability::DeviceType::kDeviceExternalChromeOsKeyboard);
EXPECT_EQ(out_layout,
ui::KeyboardCapability::KeyboardTopRowLayout::kKbdTopRowLayout2);
EXPECT_TRUE(scan_code_map.empty());
}

Expand All @@ -4289,8 +4300,11 @@ TEST_F(EventRewriterTest, IdentifyKeyboardCustomLayout) {
input_device, &out_type, &out_layout, &scan_code_map);

EXPECT_TRUE(result);
EXPECT_EQ(out_type, ui::EventRewriterChromeOS::kDeviceInternalKeyboard);
EXPECT_EQ(out_layout, ui::EventRewriterChromeOS::kKbdTopRowLayoutCustom);
EXPECT_EQ(out_type,
ui::KeyboardCapability::DeviceType::kDeviceInternalKeyboard);
EXPECT_EQ(
out_layout,
ui::KeyboardCapability::KeyboardTopRowLayout::kKbdTopRowLayoutCustom);

// Basic inspection to match kKbdDefaultCustomTopRowLayout
EXPECT_EQ(15u, scan_code_map.size());
Expand Down
17 changes: 9 additions & 8 deletions chrome/browser/ui/webui/settings/ash/device_keyboard_handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "base/values.h"
#include "content/public/browser/web_ui.h"
#include "ui/chromeos/events/event_rewriter_chromeos.h"
#include "ui/chromeos/events/keyboard_capability.h"
#include "ui/chromeos/events/keyboard_layout_util.h"

namespace {
Expand All @@ -28,22 +29,22 @@ KeyboardsStateResult GetKeyboardsState() {
for (const ui::InputDevice& keyboard :
ui::DeviceDataManager::GetInstance()->GetKeyboardDevices()) {
switch (ui::EventRewriterChromeOS::GetDeviceType(keyboard)) {
case ui::EventRewriterChromeOS::kDeviceInternalKeyboard:
case ui::KeyboardCapability::DeviceType::kDeviceInternalKeyboard:
result.has_launcher_key = true;
break;
case ui::EventRewriterChromeOS::kDeviceExternalAppleKeyboard:
case ui::KeyboardCapability::DeviceType::kDeviceExternalAppleKeyboard:
result.has_external_apple_keyboard = true;
break;
case ui::EventRewriterChromeOS::kDeviceExternalChromeOsKeyboard:
case ui::KeyboardCapability::DeviceType::kDeviceExternalChromeOsKeyboard:
result.has_external_chromeos_keyboard = true;
break;
case ui::EventRewriterChromeOS::kDeviceExternalGenericKeyboard:
case ui::EventRewriterChromeOS::kDeviceExternalUnknown:
case ui::KeyboardCapability::DeviceType::kDeviceExternalGenericKeyboard:
case ui::KeyboardCapability::DeviceType::kDeviceExternalUnknown:
result.has_external_generic_keyboard = true;
break;
case ui::EventRewriterChromeOS::kDeviceHotrodRemote:
case ui::EventRewriterChromeOS::kDeviceVirtualCoreKeyboard:
case ui::EventRewriterChromeOS::kDeviceUnknown:
case ui::KeyboardCapability::DeviceType::kDeviceHotrodRemote:
case ui::KeyboardCapability::DeviceType::kDeviceVirtualCoreKeyboard:
case ui::KeyboardCapability::DeviceType::kDeviceUnknown:
break;
}
}
Expand Down

0 comments on commit 06508ae

Please sign in to comment.