Limit alt+click -> right-click remapping to touchpads only

Many media and creative suite applications make use of
Alt+Click actions, but we always rewrite it to Right-Click
without giving the users a way to turn this behavior off.
Users of such applications often use external mice, which
already have an independent right click button.

This CL limits this behavior to events coming from touchpad
devices.

BUG=890648
TEST=Expanded tests.

Change-Id: I1b72eb4d9986e9139d7c6193cc2f808a6d9cad06
Reviewed-on: https://chromium-review.googlesource.com/c/1255605Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: Ahmed Fakhry <afakhry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#599783}

chrome/browser/chromeos/DEPS

@@ -33,4 +33,7 @@ specific_include_rules = {
     "+ash/shell.h",
     "+ash/sticky_keys/sticky_keys_controller.h",
   ],
+  "event_rewriter_unittest.cc": [
+    "+ui/events/devices/device_data_manager.h",
+  ],
 }

chrome/browser/chromeos/events/event_rewriter_unittest.cc

@@ -29,6 +29,7 @@
 #include "ui/chromeos/events/event_rewriter_chromeos.h"
 #include "ui/chromeos/events/modifier_key.h"
 #include "ui/chromeos/events/pref_names.h"
+#include "ui/events/devices/device_data_manager.h"
 #include "ui/events/event.h"
 #include "ui/events/event_rewriter.h"
 #include "ui/events/event_utils.h"
@@ -2475,6 +2476,21 @@ TEST_F(EventRewriterAshTest, TopRowKeysAreFunctionKeys) {
 }
 
 TEST_F(EventRewriterTest, DontRewriteIfNotRewritten) {
+  ui::DeviceDataManager* device_data_manager =
+      ui::DeviceDataManager::GetInstance();
+  std::vector<ui::InputDevice> touchpad_devices(2);
+  constexpr int kTouchpadId1 = 10;
+  constexpr int kTouchpadId2 = 11;
+  touchpad_devices[0].id = kTouchpadId1;
+  touchpad_devices[1].id = kTouchpadId2;
+  static_cast<ui::DeviceHotplugEventObserver*>(device_data_manager)
+      ->OnTouchpadDevicesUpdated(touchpad_devices);
+  std::vector<ui::InputDevice> mouse_devices(1);
+  constexpr int kMouseId = 12;
+  touchpad_devices[0].id = kMouseId;
+  static_cast<ui::DeviceHotplugEventObserver*>(device_data_manager)
+      ->OnMouseDevicesUpdated(mouse_devices);
+
   const int kLeftAndAltFlag = ui::EF_LEFT_MOUSE_BUTTON | ui::EF_ALT_DOWN;
 
   // Test Alt + Left click.
@@ -2483,7 +2499,7 @@ TEST_F(EventRewriterTest, DontRewriteIfNotRewritten) {
                          ui::EventTimeForNow(), kLeftAndAltFlag,
                          ui::EF_LEFT_MOUSE_BUTTON);
     ui::EventTestApi test_press(&press);
-    test_press.set_source_device_id(10);
+    test_press.set_source_device_id(kTouchpadId1);
     // Sanity check.
     EXPECT_EQ(ui::ET_MOUSE_PRESSED, press.type());
     EXPECT_EQ(kLeftAndAltFlag, press.flags());
@@ -2498,7 +2514,7 @@ TEST_F(EventRewriterTest, DontRewriteIfNotRewritten) {
                            ui::EventTimeForNow(), kLeftAndAltFlag,
                            ui::EF_LEFT_MOUSE_BUTTON);
     ui::EventTestApi test_release(&release);
-    test_release.set_source_device_id(10);
+    test_release.set_source_device_id(kTouchpadId1);
     std::unique_ptr<ui::Event> new_event;
     const ui::MouseEvent* result = RewriteMouseButtonEvent(release, &new_event);
     EXPECT_TRUE(ui::EF_RIGHT_MOUSE_BUTTON & result->flags());
@@ -2512,7 +2528,7 @@ TEST_F(EventRewriterTest, DontRewriteIfNotRewritten) {
                          ui::EventTimeForNow(), ui::EF_LEFT_MOUSE_BUTTON,
                          ui::EF_LEFT_MOUSE_BUTTON);
     ui::EventTestApi test_press(&press);
-    test_press.set_source_device_id(10);
+    test_press.set_source_device_id(kTouchpadId1);
     std::unique_ptr<ui::Event> new_event;
     const ui::MouseEvent* result = RewriteMouseButtonEvent(press, &new_event);
     EXPECT_TRUE(ui::EF_LEFT_MOUSE_BUTTON & result->flags());
@@ -2523,7 +2539,7 @@ TEST_F(EventRewriterTest, DontRewriteIfNotRewritten) {
                            ui::EventTimeForNow(), kLeftAndAltFlag,
                            ui::EF_LEFT_MOUSE_BUTTON);
     ui::EventTestApi test_release(&release);
-    test_release.set_source_device_id(10);
+    test_release.set_source_device_id(kTouchpadId1);
     std::unique_ptr<ui::Event> new_event;
     const ui::MouseEvent* result = RewriteMouseButtonEvent(release, &new_event);
     EXPECT_TRUE((ui::EF_LEFT_MOUSE_BUTTON | ui::EF_ALT_DOWN) & result->flags());
@@ -2536,7 +2552,7 @@ TEST_F(EventRewriterTest, DontRewriteIfNotRewritten) {
                          ui::EventTimeForNow(), kLeftAndAltFlag,
                          ui::EF_LEFT_MOUSE_BUTTON);
     ui::EventTestApi test_press(&press);
-    test_press.set_source_device_id(11);
+    test_press.set_source_device_id(kTouchpadId2);
     std::unique_ptr<ui::Event> new_event;
     const ui::MouseEvent* result = RewriteMouseButtonEvent(press, &new_event);
     EXPECT_TRUE(ui::EF_RIGHT_MOUSE_BUTTON & result->flags());
@@ -2548,7 +2564,7 @@ TEST_F(EventRewriterTest, DontRewriteIfNotRewritten) {
                            ui::EventTimeForNow(), kLeftAndAltFlag,
                            ui::EF_LEFT_MOUSE_BUTTON);
     ui::EventTestApi test_release(&release);
-    test_release.set_source_device_id(10);
+    test_release.set_source_device_id(kTouchpadId1);
     std::unique_ptr<ui::Event> new_event;
     const ui::MouseEvent* result = RewriteMouseButtonEvent(release, &new_event);
     EXPECT_TRUE((ui::EF_LEFT_MOUSE_BUTTON | ui::EF_ALT_DOWN) & result->flags());
@@ -2559,13 +2575,39 @@ TEST_F(EventRewriterTest, DontRewriteIfNotRewritten) {
                            ui::EventTimeForNow(), kLeftAndAltFlag,
                            ui::EF_LEFT_MOUSE_BUTTON);
     ui::EventTestApi test_release(&release);
-    test_release.set_source_device_id(11);
+    test_release.set_source_device_id(kTouchpadId2);
     std::unique_ptr<ui::Event> new_event;
     const ui::MouseEvent* result = RewriteMouseButtonEvent(release, &new_event);
     EXPECT_TRUE(ui::EF_RIGHT_MOUSE_BUTTON & result->flags());
     EXPECT_FALSE(kLeftAndAltFlag & result->flags());
     EXPECT_EQ(ui::EF_RIGHT_MOUSE_BUTTON, result->changed_button_flags());
   }
+
+  // No rewrite for non-touchpad devices.
+  {
+    ui::MouseEvent press(ui::ET_MOUSE_PRESSED, gfx::Point(), gfx::Point(),
+                         ui::EventTimeForNow(), kLeftAndAltFlag,
+                         ui::EF_LEFT_MOUSE_BUTTON);
+    ui::EventTestApi test_press(&press);
+    test_press.set_source_device_id(kMouseId);
+    EXPECT_EQ(ui::ET_MOUSE_PRESSED, press.type());
+    EXPECT_EQ(kLeftAndAltFlag, press.flags());
+    std::unique_ptr<ui::Event> new_event;
+    const ui::MouseEvent* result = RewriteMouseButtonEvent(press, &new_event);
+    EXPECT_TRUE(kLeftAndAltFlag & result->flags());
+    EXPECT_EQ(ui::EF_LEFT_MOUSE_BUTTON, result->changed_button_flags());
+  }
+  {
+    ui::MouseEvent release(ui::ET_MOUSE_RELEASED, gfx::Point(), gfx::Point(),
+                           ui::EventTimeForNow(), kLeftAndAltFlag,
+                           ui::EF_LEFT_MOUSE_BUTTON);
+    ui::EventTestApi test_release(&release);
+    test_release.set_source_device_id(kMouseId);
+    std::unique_ptr<ui::Event> new_event;
+    const ui::MouseEvent* result = RewriteMouseButtonEvent(release, &new_event);
+    EXPECT_TRUE(kLeftAndAltFlag & result->flags());
+    EXPECT_EQ(ui::EF_LEFT_MOUSE_BUTTON, result->changed_button_flags());
+  }
 }
 
 TEST_F(EventRewriterAshTest, StickyKeyEventDispatchImpl) {

ui/chromeos/events/event_rewriter_chromeos.cc

@@ -257,6 +257,17 @@ ui::DomCode RelocateModifier(ui::DomCode code, ui::DomKeyLocation location) {
   return code;
 }
 
+// Returns true if |mouse_event| was generated from a touchpad device.
+bool IsFromTouchpadDevice(const ui::MouseEvent& mouse_event) {
+  for (const ui::InputDevice& touchpad :
+       ui::InputDeviceManager::GetInstance()->GetTouchpadDevices()) {
+    if (touchpad.id == mouse_event.source_device_id())
+      return true;
+  }
+
+  return false;
+}
+
 }  // namespace
 
 EventRewriterChromeOS::EventRewriterChromeOS(
@@ -1171,11 +1182,15 @@ void EventRewriterChromeOS::RewriteLocatedEvent(const ui::Event& event,
 int EventRewriterChromeOS::RewriteModifierClick(
     const ui::MouseEvent& mouse_event,
     int* flags) {
+  // Note that this behavior is limited to mouse events coming from touchpad
+  // devices. https://crbug.com/890648.
+
   // Remap Alt+Button1 to Button3.
   const int kAltLeftButton = (ui::EF_ALT_DOWN | ui::EF_LEFT_MOUSE_BUTTON);
   if (((*flags & kAltLeftButton) == kAltLeftButton) &&
       ((mouse_event.type() == ui::ET_MOUSE_PRESSED) ||
-       pressed_device_ids_.count(mouse_event.source_device_id()))) {
+       pressed_device_ids_.count(mouse_event.source_device_id())) &&
+      IsFromTouchpadDevice(mouse_event)) {
     *flags &= ~kAltLeftButton;
     *flags |= ui::EF_RIGHT_MOUSE_BUTTON;
     if (mouse_event.type() == ui::ET_MOUSE_PRESSED)