exo: Fix keyboard pressed key tracking.

Use a top priority pre-target handler in Seat class to track
pressed keys. This is needed to ensure that we see all key events.

Also cleanup some of the code in Keyboard class where we send
modifiers. It doesn't make sense to send modifiers in cases
where the key event is a reserved accelerator and won't be
delivered to the client.

Bug: 820641
Test: exo_unittests --gtest_filter=KeyboardTest.OnKeyboardEnter
Change-Id: Ib1169a7c0ad70e8e7973c94edf4b9885fecc7819
Reviewed-on: https://chromium-review.googlesource.com/1001634Reviewed-by: Daniele Castagna <dcastagna@chromium.org>
Commit-Queue: Daniele Castagna <dcastagna@chromium.org>
Cr-Commit-Position: refs/heads/master@{#549364}

components/exo/keyboard.cc

@@ -216,13 +216,6 @@ void Keyboard::AckKeyboardKey(uint32_t serial, bool handled) {
 // ui::EventHandler overrides:
 
 void Keyboard::OnKeyEvent(ui::KeyEvent* event) {
-  int modifier_flags = event->flags() & kModifierMask;
-  if (modifier_flags != modifier_flags_) {
-    modifier_flags_ = modifier_flags;
-    if (focus_)
-      delegate_->OnKeyboardModifiers(modifier_flags_);
-  }
-
   // Process reserved accelerators before sending it to client.
   if (focus_ && ProcessAcceleratorIfReserved(focus_, event)) {
     // Discard a key press event if it's a reserved accelerator and it's
@@ -239,10 +232,16 @@ void Keyboard::OnKeyEvent(ui::KeyEvent* event) {
   // and client) and causes undesired behavior.
   bool consumed_by_ime = focus_ ? ConsumedByIme(focus_, event) : false;
 
-  switch (event->type()) {
-    case ui::ET_KEY_PRESSED:
-      if (pressed_keys_.insert(event->code()).second) {
-        if (focus_ && !consumed_by_ime && !event->handled()) {
+  if (focus_ && !consumed_by_ime && !event->handled()) {
+    int modifier_flags = event->flags() & kModifierMask;
+    if (modifier_flags != modifier_flags_) {
+      modifier_flags_ = modifier_flags;
+      delegate_->OnKeyboardModifiers(modifier_flags_);
+    }
+
+    switch (event->type()) {
+      case ui::ET_KEY_PRESSED:
+        if (pressed_keys_.insert(event->code()).second) {
           uint32_t serial = delegate_->OnKeyboardKey(event->time_stamp(),
                                                      event->code(), true);
           if (are_keyboard_key_acks_needed_) {
@@ -253,11 +252,9 @@ void Keyboard::OnKeyEvent(ui::KeyEvent* event) {
             event->SetHandled();
           }
         }
-      }
-      break;
-    case ui::ET_KEY_RELEASED:
-      if (pressed_keys_.erase(event->code())) {
-        if (focus_ && !consumed_by_ime && !event->handled()) {
+        break;
+      case ui::ET_KEY_RELEASED:
+        if (pressed_keys_.erase(event->code())) {
           uint32_t serial = delegate_->OnKeyboardKey(event->time_stamp(),
                                                      event->code(), false);
           if (are_keyboard_key_acks_needed_) {
@@ -268,11 +265,11 @@ void Keyboard::OnKeyEvent(ui::KeyEvent* event) {
             event->SetHandled();
           }
         }
-      }
-      break;
-    default:
-      NOTREACHED();
-      break;
+        break;
+      default:
+        NOTREACHED();
+        break;
+    }
   }
 
   if (pending_key_acks_.empty())

components/exo/keyboard_unittest.cc

@@ -103,6 +103,18 @@ TEST_F(KeyboardTest, OnKeyboardEnter) {
   // Surface should maintain keyboard focus when moved to top-level window.
   focus_client->FocusWindow(surface->window()->GetToplevelWindow());
 
+  // Release key after surface lost focus.
+  focus_client->FocusWindow(nullptr);
+  generator.ReleaseKey(ui::VKEY_A, ui::EF_SHIFT_DOWN);
+
+  // Key should no longer be pressed when focus returns.
+  EXPECT_CALL(delegate, CanAcceptKeyboardEventsForSurface(surface.get()))
+      .WillOnce(testing::Return(true));
+  EXPECT_CALL(delegate, OnKeyboardModifiers(ui::EF_SHIFT_DOWN));
+  EXPECT_CALL(delegate,
+              OnKeyboardEnter(surface.get(), base::flat_set<ui::DomCode>()));
+  focus_client->FocusWindow(surface->window()->GetToplevelWindow());
+
   keyboard.reset();
 }
 

components/exo/seat.cc

@@ -37,7 +37,8 @@ Surface* GetEffectiveFocus(aura::Window* window) {
 Seat::Seat() : changing_clipboard_data_to_selection_source_(false) {
   aura::client::GetFocusClient(ash::Shell::Get()->GetPrimaryRootWindow())
       ->AddObserver(this);
-  WMHelper::GetInstance()->AddPreTargetHandler(this);
+  // Prepend handler as it's critical that we see all events.
+  WMHelper::GetInstance()->PrependPreTargetHandler(this);
   ui::ClipboardMonitor::GetInstance()->AddObserver(this);
 }
 

components/exo/wm_helper.cc

@@ -179,8 +179,8 @@ void WMHelper::AddPreTargetHandler(ui::EventHandler* handler) {
 }
 
 void WMHelper::PrependPreTargetHandler(ui::EventHandler* handler) {
-  ash::Shell::Get()->AddPreTargetHandler(handler,
-                                         ui::EventTarget::Priority::kSystem);
+  ash::Shell::Get()->AddPreTargetHandler(
+      handler, ui::EventTarget::Priority::kAccessibility);
 }
 
 void WMHelper::RemovePreTargetHandler(ui::EventHandler* handler) {