Commit c98d55f1 authored by David Reveman's avatar David Reveman Committed by Commit Bot

exo: Fix incorrect repeat of keyboard events.

We should not be generating keyboard press events when a key is
already pressed.

Fix this issue by restoring the "pressed keys" set in the
Keyboard class. Seat is still tracking pressed keys in order
to ensure that accelerator key presses are not lost.

Bug: 820641
Test: exo_unittests --gtest_filter=KeyboardTest.OnKeyboardKey
Change-Id: I56c7110361c0894f3fd8d882c45c1d2bfe3d634f
Reviewed-on: https://chromium-review.googlesource.com/967727
Commit-Queue: David Reveman <reveman@chromium.org>
Reviewed-by: default avatarDaniele Castagna <dcastagna@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543949}
parent 7064311c
...@@ -241,28 +241,32 @@ void Keyboard::OnKeyEvent(ui::KeyEvent* event) { ...@@ -241,28 +241,32 @@ void Keyboard::OnKeyEvent(ui::KeyEvent* event) {
switch (event->type()) { switch (event->type()) {
case ui::ET_KEY_PRESSED: case ui::ET_KEY_PRESSED:
if (focus_ && !consumed_by_ime && !event->handled()) { if (pressed_keys_.insert(event->code()).second) {
uint32_t serial = if (focus_ && !consumed_by_ime && !event->handled()) {
delegate_->OnKeyboardKey(event->time_stamp(), event->code(), true); uint32_t serial = delegate_->OnKeyboardKey(event->time_stamp(),
if (are_keyboard_key_acks_needed_) { event->code(), true);
pending_key_acks_.insert( if (are_keyboard_key_acks_needed_) {
{serial, pending_key_acks_.insert(
{*event, base::TimeTicks::Now() + {serial,
expiration_delay_for_pending_key_acks_}}); {*event, base::TimeTicks::Now() +
event->SetHandled(); expiration_delay_for_pending_key_acks_}});
event->SetHandled();
}
} }
} }
break; break;
case ui::ET_KEY_RELEASED: case ui::ET_KEY_RELEASED:
if (focus_ && !consumed_by_ime && !event->handled()) { if (pressed_keys_.erase(event->code())) {
uint32_t serial = if (focus_ && !consumed_by_ime && !event->handled()) {
delegate_->OnKeyboardKey(event->time_stamp(), event->code(), false); uint32_t serial = delegate_->OnKeyboardKey(event->time_stamp(),
if (are_keyboard_key_acks_needed_) { event->code(), false);
pending_key_acks_.insert( if (are_keyboard_key_acks_needed_) {
{serial, pending_key_acks_.insert(
{*event, base::TimeTicks::Now() + {serial,
expiration_delay_for_pending_key_acks_}}); {*event, base::TimeTicks::Now() +
event->SetHandled(); expiration_delay_for_pending_key_acks_}});
event->SetHandled();
}
} }
} }
break; break;
...@@ -336,8 +340,9 @@ void Keyboard::SetFocus(Surface* surface) { ...@@ -336,8 +340,9 @@ void Keyboard::SetFocus(Surface* surface) {
} }
if (surface) { if (surface) {
modifier_flags_ = seat_->modifier_flags() & kModifierMask; modifier_flags_ = seat_->modifier_flags() & kModifierMask;
pressed_keys_ = seat_->pressed_keys();
delegate_->OnKeyboardModifiers(modifier_flags_); delegate_->OnKeyboardModifiers(modifier_flags_);
delegate_->OnKeyboardEnter(surface, seat_->pressed_keys()); delegate_->OnKeyboardEnter(surface, pressed_keys_);
focus_ = surface; focus_ = surface;
focus_->AddSurfaceObserver(this); focus_->AddSurfaceObserver(this);
} }
......
...@@ -9,6 +9,7 @@ ...@@ -9,6 +9,7 @@
#include "ash/wm/tablet_mode/tablet_mode_observer.h" #include "ash/wm/tablet_mode/tablet_mode_observer.h"
#include "base/containers/flat_map.h" #include "base/containers/flat_map.h"
#include "base/containers/flat_set.h"
#include "base/macros.h" #include "base/macros.h"
#include "base/observer_list.h" #include "base/observer_list.h"
#include "components/exo/keyboard_observer.h" #include "components/exo/keyboard_observer.h"
...@@ -19,6 +20,7 @@ ...@@ -19,6 +20,7 @@
#include "ui/events/event_handler.h" #include "ui/events/event_handler.h"
namespace ui { namespace ui {
enum class DomCode;
class KeyEvent; class KeyEvent;
} }
...@@ -109,6 +111,9 @@ class Keyboard : public ui::EventHandler, ...@@ -109,6 +111,9 @@ class Keyboard : public ui::EventHandler,
// The current focus surface for the keyboard. // The current focus surface for the keyboard.
Surface* focus_ = nullptr; Surface* focus_ = nullptr;
// Set of currently pressed keys.
base::flat_set<ui::DomCode> pressed_keys_;
// Current set of modifier flags. // Current set of modifier flags.
int modifier_flags_ = 0; int modifier_flags_ = 0;
......
...@@ -174,6 +174,9 @@ TEST_F(KeyboardTest, OnKeyboardKey) { ...@@ -174,6 +174,9 @@ TEST_F(KeyboardTest, OnKeyboardKey) {
EXPECT_CALL(delegate, OnKeyboardKey(testing::_, ui::DomCode::US_A, true)); EXPECT_CALL(delegate, OnKeyboardKey(testing::_, ui::DomCode::US_A, true));
generator.PressKey(ui::VKEY_A, 0); generator.PressKey(ui::VKEY_A, 0);
// This should not generate another press event for KEY_A.
generator.PressKey(ui::VKEY_A, 0);
// This should only generate a release event for KEY_A. // This should only generate a release event for KEY_A.
EXPECT_CALL(delegate, OnKeyboardKey(testing::_, ui::DomCode::US_A, false)); EXPECT_CALL(delegate, OnKeyboardKey(testing::_, ui::DomCode::US_A, false));
generator.ReleaseKey(ui::VKEY_A, 0); generator.ReleaseKey(ui::VKEY_A, 0);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment