exo: Fix key event rewrite support.

Use a map to track pressed keys. The key for the map is the physical
code that was pressed and the value is a potentially rewritten code.

This allows us to generate appropriate release events when physical
keys are released, while still allowing the event rewriting logic
to work as expected.

Note: Exo clients will always see a release for the same code as
the code that was generated when the physical key was pressed. This
is necessary in order to not break client side keyboard state
tracking and might not match the events that are generated internally.
That is because event rewrite logic is usually not rewriting both the
press and the release events, often just the press event.

Bug: 847500
Test: exo_unittests --gtest_filter=KeyboardTest.OnKeyboardKey
Tbr: hidehiko@chromium.org
Change-Id: I23240304bd377e3855971edd079ade1b537a74f9
Reviewed-on: https://chromium-review.googlesource.com/1112967Reviewed-by: Daniele Castagna <dcastagna@chromium.org>
Commit-Queue: David Reveman <reveman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#570074}

ash/system/message_center/arc/arc_notification_content_view_unittest.cc

@@ -63,7 +63,8 @@ class MockKeyboardDelegate : public exo::KeyboardDelegate {
   MOCK_METHOD1(OnKeyboardDestroying, void(exo::Keyboard*));
   MOCK_CONST_METHOD1(CanAcceptKeyboardEventsForSurface, bool(exo::Surface*));
   MOCK_METHOD2(OnKeyboardEnter,
-               void(exo::Surface*, const base::flat_set<ui::DomCode>&));
+               void(exo::Surface*,
+                    const base::flat_map<ui::DomCode, ui::DomCode>&));
   MOCK_METHOD1(OnKeyboardLeave, void(exo::Surface*));
   MOCK_METHOD3(OnKeyboardKey, uint32_t(base::TimeTicks, ui::DomCode, bool));
   MOCK_METHOD1(OnKeyboardModifiers, void(int));
@@ -529,6 +530,8 @@ TEST_F(ArcNotificationContentViewTest, AcceptInputTextWithActivate) {
 
   ui::test::EventGenerator generator(Shell::GetPrimaryRootWindow());
   EXPECT_CALL(delegate, OnKeyboardKey(testing::_, ui::DomCode::US_A, true));
+  seat.set_physical_code_for_currently_processing_event_for_testing(
+      ui::DomCode::US_A);
   generator.PressKey(ui::VKEY_A, 0);
 
   keyboard.reset();
@@ -553,6 +556,8 @@ TEST_F(ArcNotificationContentViewTest, NotAcceptInputTextWithoutActivate) {
   ui::test::EventGenerator generator(Shell::GetPrimaryRootWindow());
   EXPECT_CALL(delegate, OnKeyboardKey(testing::_, testing::_, testing::_))
       .Times(0);
+  seat.set_physical_code_for_currently_processing_event_for_testing(
+      ui::DomCode::US_A);
   generator.PressKey(ui::VKEY_A, 0);
 
   keyboard.reset();

components/exo/keyboard.cc

@@ -241,11 +241,13 @@ void Keyboard::OnKeyEvent(ui::KeyEvent* event) {
   }
 
   switch (event->type()) {
-    case ui::ET_KEY_PRESSED:
-      // Process key press event if not already handled and not
-      // already pressed.
-      if (!consumed_by_ime && !event->handled() &&
-          pressed_keys_.insert(event->code()).second) {
+    case ui::ET_KEY_PRESSED: {
+      // Process key press event if not already handled and not already pressed.
+      auto it = pressed_keys_.find(
+          seat_->physical_code_for_currently_processing_event());
+      if (it == pressed_keys_.end() && !consumed_by_ime && !event->handled() &&
+          seat_->physical_code_for_currently_processing_event() !=
+              ui::DomCode::NONE) {
         uint32_t serial =
             delegate_->OnKeyboardKey(event->time_stamp(), event->code(), true);
         if (are_keyboard_key_acks_needed_) {
@@ -255,13 +257,24 @@ void Keyboard::OnKeyEvent(ui::KeyEvent* event) {
                             expiration_delay_for_pending_key_acks_}});
           event->SetHandled();
         }
+        // Keep track of both the physical code and potentially re-written
+        // code that this event generated.
+        pressed_keys_.insert(
+            {seat_->physical_code_for_currently_processing_event(),
+             event->code()});
       }
-      break;
-    case ui::ET_KEY_RELEASED:
+    } break;
+    case ui::ET_KEY_RELEASED: {
       // Process key release event if currently pressed.
-      if (pressed_keys_.erase(event->code())) {
+      auto it = pressed_keys_.find(
+          seat_->physical_code_for_currently_processing_event());
+      if (it != pressed_keys_.end()) {
+        // We use the code that was generate when the physical key was
+        // pressed rather than the current event code. This allows events
+        // to be re-written before dispatch, while still allowing the
+        // client to track the state of the physical keyboard.
         uint32_t serial =
-            delegate_->OnKeyboardKey(event->time_stamp(), event->code(), false);
+            delegate_->OnKeyboardKey(event->time_stamp(), it->second, false);
         if (are_keyboard_key_acks_needed_) {
           pending_key_acks_.insert(
               {serial,
@@ -269,8 +282,9 @@ void Keyboard::OnKeyEvent(ui::KeyEvent* event) {
                             expiration_delay_for_pending_key_acks_}});
           event->SetHandled();
         }
+        pressed_keys_.erase(it);
       }
-      break;
+    } break;
     default:
       NOTREACHED();
       break;

components/exo/keyboard.h

@@ -111,8 +111,10 @@ class Keyboard : public ui::EventHandler,
   // The current focus surface for the keyboard.
   Surface* focus_ = nullptr;
 
-  // Set of currently pressed keys.
-  base::flat_set<ui::DomCode> pressed_keys_;
+  // Set of currently pressed keys. First value is a platform code and second
+  // value is the code that was delivered to client. See Seat.h for more
+  // details.
+  base::flat_map<ui::DomCode, ui::DomCode> pressed_keys_;
 
   // Current set of modifier flags.
   int modifier_flags_ = 0;

components/exo/keyboard_delegate.h

@@ -25,7 +25,7 @@ class KeyboardDelegate {
   // Called when keyboard focus enters a new valid target surface.
   virtual void OnKeyboardEnter(
       Surface* surface,
-      const base::flat_set<ui::DomCode>& pressed_keys) = 0;
+      const base::flat_map<ui::DomCode, ui::DomCode>& pressed_keys) = 0;
 
   // Called when keyboard focus leaves a valid target surface.
   virtual void OnKeyboardLeave(Surface* surface) = 0;

components/exo/seat.cc

@@ -15,6 +15,8 @@
 #include "ui/aura/client/focus_client.h"
 #include "ui/base/clipboard/clipboard_monitor.h"
 #include "ui/base/clipboard/scoped_clipboard_writer.h"
+#include "ui/events/event_utils.h"
+#include "ui/events/platform/platform_event_source.h"
 
 namespace exo {
 namespace {
@@ -40,6 +42,10 @@ Seat::Seat() : changing_clipboard_data_to_selection_source_(false) {
   // Prepend handler as it's critical that we see all events.
   WMHelper::GetInstance()->PrependPreTargetHandler(this);
   ui::ClipboardMonitor::GetInstance()->AddObserver(this);
+  // TODO(reveman): Need to handle the mus case where PlatformEventSource is
+  // null. https://crbug.com/856230
+  if (ui::PlatformEventSource::GetInstance())
+    ui::PlatformEventSource::GetInstance()->AddPlatformEventObserver(this);
 }
 
 Seat::~Seat() {
@@ -48,6 +54,8 @@ Seat::~Seat() {
       ->RemoveObserver(this);
   WMHelper::GetInstance()->RemovePreTargetHandler(this);
   ui::ClipboardMonitor::GetInstance()->RemoveObserver(this);
+  if (ui::PlatformEventSource::GetInstance())
+    ui::PlatformEventSource::GetInstance()->RemovePlatformEventObserver(this);
 }
 
 void Seat::AddObserver(SeatObserver* observer) {
@@ -104,23 +112,51 @@ void Seat::OnWindowFocused(aura::Window* gained_focus,
 }
 
 ////////////////////////////////////////////////////////////////////////////////
-// ui::EventHandler overrides:
+// ui::PlatformEventObserver overrides:
 
-void Seat::OnKeyEvent(ui::KeyEvent* event) {
-  // Ignore synthetic key repeat events.
-  if (event->is_repeat())
-    return;
-  switch (event->type()) {
+void Seat::WillProcessEvent(const ui::PlatformEvent& event) {
+  switch (ui::EventTypeFromNative(event)) {
     case ui::ET_KEY_PRESSED:
-      pressed_keys_.insert(event->code());
+    case ui::ET_KEY_RELEASED:
+      physical_code_for_currently_processing_event_ = ui::CodeFromNative(event);
       break;
+    default:
+      break;
+  }
+}
+
+void Seat::DidProcessEvent(const ui::PlatformEvent& event) {
+  switch (ui::EventTypeFromNative(event)) {
+    case ui::ET_KEY_PRESSED:
     case ui::ET_KEY_RELEASED:
-      pressed_keys_.erase(event->code());
+      physical_code_for_currently_processing_event_ = ui::DomCode::NONE;
       break;
     default:
-      NOTREACHED();
       break;
   }
+}
+
+////////////////////////////////////////////////////////////////////////////////
+// ui::EventHandler overrides:
+
+void Seat::OnKeyEvent(ui::KeyEvent* event) {
+  // Ignore synthetic key repeat events.
+  if (event->is_repeat())
+    return;
+  if (physical_code_for_currently_processing_event_ != ui::DomCode::NONE) {
+    switch (event->type()) {
+      case ui::ET_KEY_PRESSED:
+        pressed_keys_.insert(
+            {physical_code_for_currently_processing_event_, event->code()});
+        break;
+      case ui::ET_KEY_RELEASED:
+        pressed_keys_.erase(physical_code_for_currently_processing_event_);
+        break;
+      default:
+        NOTREACHED();
+        break;
+    }
+  }
   modifier_flags_ = event->flags();
 }
 

components/exo/seat.h

@@ -5,7 +5,7 @@
 #ifndef COMPONENTS_EXO_SEAT_H_
 #define COMPONENTS_EXO_SEAT_H_
 
-#include "base/containers/flat_set.h"
+#include "base/containers/flat_map.h"
 #include "base/memory/weak_ptr.h"
 #include "base/observer_list.h"
 #include "components/exo/data_source_observer.h"
@@ -13,6 +13,8 @@
 #include "ui/aura/client/focus_change_observer.h"
 #include "ui/base/clipboard/clipboard_observer.h"
 #include "ui/events/event_handler.h"
+#include "ui/events/keycodes/dom/dom_codes.h"
+#include "ui/events/platform/platform_event_observer.h"
 
 namespace ui {
 enum class DomCode;
@@ -27,6 +29,7 @@ class Surface;
 // Seat object represent a group of input devices such as keyboard, pointer and
 // touch devices and keeps track of input focus.
 class Seat : public aura::client::FocusChangeObserver,
+             public ui::PlatformEventObserver,
              public ui::EventHandler,
              public ui::ClipboardObserver,
              public DataSourceObserver {
@@ -42,13 +45,18 @@ class Seat : public aura::client::FocusChangeObserver,
   virtual Surface* GetFocusedSurface();
 
   // Returns currently pressed keys.
-  const base::flat_set<ui::DomCode>& pressed_keys() const {
+  const base::flat_map<ui::DomCode, ui::DomCode>& pressed_keys() const {
     return pressed_keys_;
   }
 
   // Returns current set of modifier flags.
   int modifier_flags() const { return modifier_flags_; }
 
+  // Returns physical code for the currently processing event.
+  ui::DomCode physical_code_for_currently_processing_event() const {
+    return physical_code_for_currently_processing_event_;
+  }
+
   // Sets clipboard data from |source|.
   void SetSelection(DataSource* source);
 
@@ -56,6 +64,10 @@ class Seat : public aura::client::FocusChangeObserver,
   void OnWindowFocused(aura::Window* gained_focus,
                        aura::Window* lost_focus) override;
 
+  // Overridden from ui::PlatformEventObserver:
+  void WillProcessEvent(const ui::PlatformEvent& event) override;
+  void DidProcessEvent(const ui::PlatformEvent& event) override;
+
   // Overridden from ui::EventHandler:
   void OnKeyEvent(ui::KeyEvent* event) override;
 
@@ -65,6 +77,12 @@ class Seat : public aura::client::FocusChangeObserver,
   // Overridden from DataSourceObserver:
   void OnDataSourceDestroying(DataSource* source) override;
 
+  void set_physical_code_for_currently_processing_event_for_testing(
+      ui::DomCode physical_code_for_currently_processing_event) {
+    physical_code_for_currently_processing_event_ =
+        physical_code_for_currently_processing_event;
+  }
+
  private:
   // Called when data is read from FD passed from a client.
   // |data| is read data. |source| is source of the data, or nullptr if
@@ -72,7 +90,11 @@ class Seat : public aura::client::FocusChangeObserver,
   void OnDataRead(const std::vector<uint8_t>& data);
 
   base::ObserverList<SeatObserver> observers_;
-  base::flat_set<ui::DomCode> pressed_keys_;
+  // The platform code is the key in this map as it represents the physical
+  // key that was pressed. The value is a potentially rewritten code that the
+  // physical key press generated.
+  base::flat_map<ui::DomCode, ui::DomCode> pressed_keys_;
+  ui::DomCode physical_code_for_currently_processing_event_ = ui::DomCode::NONE;
   int modifier_flags_ = 0;
 
   // Data source being used as a clipboard content.

components/exo/wayland/server.cc

@@ -3767,16 +3767,16 @@ class WaylandKeyboardDelegate : public WaylandInputDelegate,
   }
   void OnKeyboardEnter(
       Surface* surface,
-      const base::flat_set<ui::DomCode>& pressed_keys) override {
+      const base::flat_map<ui::DomCode, ui::DomCode>& pressed_keys) override {
     wl_resource* surface_resource = GetSurfaceResource(surface);
     DCHECK(surface_resource);
     wl_array keys;
     wl_array_init(&keys);
-    for (auto key : pressed_keys) {
+    for (const auto& entry : pressed_keys) {
       uint32_t* value =
           static_cast<uint32_t*>(wl_array_add(&keys, sizeof(uint32_t)));
       DCHECK(value);
-      *value = DomCodeToKey(key);
+      *value = DomCodeToKey(entry.second);
     }
     wl_keyboard_send_enter(keyboard_resource_, next_serial(), surface_resource,
                            &keys);