Fix wrong assumption about external keyboard names

The code used to assume that a keyboard device name
must include the string "keyboard" in it.
CL at crrev.com/c/1074336 built on this wrong assumption
to detect whether to show external keyboard key remap
settings in the system keyboard settings.

This CL fixes this assumption.

BUG=1074336
TEST=manually, unit test.

Change-Id: I174173097e6da0159c655f724690060ebe74529a
Reviewed-on: https://chromium-review.googlesource.com/1214322Reviewed-by: Kevin Schoedel <kpschoedel@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: Ahmed Fakhry <afakhry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#589722}

chrome/browser/ui/webui/settings/chromeos/device_keyboard_handler.cc

@@ -30,10 +30,13 @@ KeyboardsStateResult GetKeyboardsState() {
        ui::InputDeviceManager::GetInstance()->GetKeyboardDevices()) {
     const ui::EventRewriterChromeOS::DeviceType type =
         ui::EventRewriterChromeOS::GetDeviceType(keyboard);
-    if (type == ui::EventRewriterChromeOS::kDeviceAppleKeyboard)
+    if (type == ui::EventRewriterChromeOS::kDeviceAppleKeyboard) {
       result.has_apple_keyboard = true;
-    else if (type == ui::EventRewriterChromeOS::kDeviceExternalNonAppleKeyboard)
+    } else if (type ==
+                   ui::EventRewriterChromeOS::kDeviceExternalNonAppleKeyboard ||
+               type == ui::EventRewriterChromeOS::kDeviceExternalUnknown) {
       result.has_external_non_apple_keyboard = true;
+    }
   }
 
   return result;

chrome/browser/ui/webui/settings/chromeos/device_keyboard_handler_unittest.cc

@@ -210,6 +210,17 @@ TEST_F(KeyboardHandlerTest, ExternalKeyboard) {
   EXPECT_TRUE(HasExternalMetaKey());
   EXPECT_TRUE(HasAppleCommandKey());
 
+  // Some keyboard devices don't report the string "keyboard" as part of their
+  // device names. Those should also be detcted as external keyboards, and
+  // should show the capslock and external meta remapping.
+  // https://crbug.com/834594.
+  input_device_client_test_api_.SetKeyboardDevices(std::vector<ui::InputDevice>{
+      {4, ui::INPUT_DEVICE_EXTERNAL, "Topre Corporation Realforce 87"}});
+  EXPECT_TRUE(HasCapsLock());
+  EXPECT_FALSE(HasDiamondKey());
+  EXPECT_TRUE(HasExternalMetaKey());
+  EXPECT_FALSE(HasAppleCommandKey());
+
   // Disconnect the external keyboard and check that the key goes away.
   input_device_client_test_api_.SetKeyboardDevices({});
   EXPECT_FALSE(HasCapsLock());

ui/chromeos/events/event_rewriter_chromeos.cc

@@ -145,6 +145,7 @@ const ModifierRemapping* GetSearchRemappedKey(
       break;
 
     case EventRewriterChromeOS::kDeviceExternalNonAppleKeyboard:
+    case EventRewriterChromeOS::kDeviceExternalUnknown:
       pref_name = prefs::kLanguageRemapExternalMetaKeyTo;
       break;
 
@@ -301,9 +302,12 @@ EventRewriterChromeOS::DeviceType EventRewriterChromeOS::GetDeviceType(
       return EventRewriterChromeOS::kDeviceAppleKeyboard;
   }
 
-  if (!found_apple && found_keyboard &&
-      keyboard_device.type == INPUT_DEVICE_EXTERNAL) {
-    return EventRewriterChromeOS::kDeviceExternalNonAppleKeyboard;
+  if (!found_apple && keyboard_device.type == INPUT_DEVICE_EXTERNAL) {
+    // ui::InputDevice is a generic input device, and we're not sure if it's
+    // actually a keyboard.
+    return found_keyboard
+               ? EventRewriterChromeOS::kDeviceExternalNonAppleKeyboard
+               : EventRewriterChromeOS::kDeviceExternalUnknown;
   }
 
   return EventRewriterChromeOS::kDeviceUnknown;

ui/chromeos/events/event_rewriter_chromeos.h

@@ -43,6 +43,7 @@ class EventRewriterChromeOS : public ui::EventRewriter {
     kDeviceUnknown = 0,
     kDeviceAppleKeyboard,
     kDeviceExternalNonAppleKeyboard,
+    kDeviceExternalUnknown,
     kDeviceHotrodRemote,
     kDeviceVirtualCoreKeyboard,  // X-server generated events.
   };