Commit 23e30de5 authored by Jeffrey Kardatzke's avatar Jeffrey Kardatzke Committed by Commit Bot

exo: Fix stuck keys with wayland clients

Remove a key from the pressed keys map in the DidProcessEvent call as
in addition to the existing removal in OnKeyEvent. When IME is
active, then sometimes DidProcessEvent gets invoked before OnKeyEvent,
and when that happens we end up with a stuck key in the pressed keys
map.

Bug: 840155
Test: exo_unitests --gtest_filter=SeatTest.PressedKeys
Change-Id: I54bbaaeb5656b4ece8cd10f8b5f7065ddad33813
Reviewed-on: https://chromium-review.googlesource.com/c/1373010
Commit-Queue: Jeffrey Kardatzke <jkardatzke@google.com>
Reviewed-by: default avatarDaniele Castagna <dcastagna@chromium.org>
Reviewed-by: default avatarDavid Reveman <reveman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#616084}
parent e4accf84
...@@ -125,9 +125,17 @@ void Seat::WillProcessEvent(const ui::PlatformEvent& event) { ...@@ -125,9 +125,17 @@ void Seat::WillProcessEvent(const ui::PlatformEvent& event) {
void Seat::DidProcessEvent(const ui::PlatformEvent& event) { void Seat::DidProcessEvent(const ui::PlatformEvent& event) {
switch (ui::EventTypeFromNative(event)) { switch (ui::EventTypeFromNative(event)) {
case ui::ET_KEY_PRESSED: case ui::ET_KEY_PRESSED:
case ui::ET_KEY_RELEASED:
physical_code_for_currently_processing_event_ = ui::DomCode::NONE; physical_code_for_currently_processing_event_ = ui::DomCode::NONE;
break; break;
case ui::ET_KEY_RELEASED:
// Remove this from the pressed key map because when IME is active we can
// end up getting the DidProcessEvent call before we get the OnKeyEvent
// callback and then the key will end up being stuck pressed.
if (physical_code_for_currently_processing_event_ != ui::DomCode::NONE) {
pressed_keys_.erase(physical_code_for_currently_processing_event_);
physical_code_for_currently_processing_event_ = ui::DomCode::NONE;
}
break;
default: default:
break; break;
} }
......
...@@ -15,6 +15,8 @@ ...@@ -15,6 +15,8 @@
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
#include "ui/base/clipboard/clipboard.h" #include "ui/base/clipboard/clipboard.h"
#include "ui/base/clipboard/scoped_clipboard_writer.h" #include "ui/base/clipboard/scoped_clipboard_writer.h"
#include "ui/events/event.h"
#include "ui/events/event_utils.h"
namespace exo { namespace exo {
namespace { namespace {
...@@ -248,5 +250,43 @@ TEST_F(SeatTest, SetSelection_NullSource) { ...@@ -248,5 +250,43 @@ TEST_F(SeatTest, SetSelection_NullSource) {
EXPECT_EQ(clipboard, ""); EXPECT_EQ(clipboard, "");
} }
TEST_F(SeatTest, PressedKeys) {
Seat seat;
ui::KeyEvent press_a(ui::ET_KEY_PRESSED, ui::VKEY_A, ui::DomCode::US_A, 0);
ui::KeyEvent release_a(ui::ET_KEY_RELEASED, ui::VKEY_A, ui::DomCode::US_A, 0);
ui::KeyEvent press_b(ui::ET_KEY_PRESSED, ui::VKEY_B, ui::DomCode::US_B, 0);
ui::KeyEvent release_b(ui::ET_KEY_RELEASED, ui::VKEY_B, ui::DomCode::US_B, 0);
// Press A, it should be in the map.
seat.WillProcessEvent(&press_a);
seat.OnKeyEvent(press_a.AsKeyEvent());
seat.DidProcessEvent(&press_a);
base::flat_map<ui::DomCode, ui::DomCode> pressed_keys;
pressed_keys[ui::CodeFromNative(&press_a)] = press_a.code();
EXPECT_EQ(pressed_keys, seat.pressed_keys());
// Press B, then A & B should be in the map.
seat.WillProcessEvent(&press_b);
seat.OnKeyEvent(press_b.AsKeyEvent());
seat.DidProcessEvent(&press_b);
pressed_keys[ui::CodeFromNative(&press_b)] = press_b.code();
EXPECT_EQ(pressed_keys, seat.pressed_keys());
// Release A, with the normal order where DidProcessEvent is after OnKeyEvent,
// only B should be in the map.
seat.WillProcessEvent(&release_a);
seat.OnKeyEvent(release_a.AsKeyEvent());
seat.DidProcessEvent(&release_a);
pressed_keys.erase(ui::CodeFromNative(&press_a));
EXPECT_EQ(pressed_keys, seat.pressed_keys());
// Release B, do it out of order so DidProcessEvent is before OnKeyEvent, the
// map should then be empty.
seat.WillProcessEvent(&release_b);
seat.DidProcessEvent(&release_b);
seat.OnKeyEvent(release_b.AsKeyEvent());
EXPECT_TRUE(seat.pressed_keys().empty());
}
} // namespace } // namespace
} // namespace exo } // namespace exo
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