diff --git a/ash/public/cpp/external_arc/message_center/arc_notification_content_view_unittest.cc b/ash/public/cpp/external_arc/message_center/arc_notification_content_view_unittest.cc index 52afa80a5da758..f54485c34b09ec 100644 --- a/ash/public/cpp/external_arc/message_center/arc_notification_content_view_unittest.cc +++ b/ash/public/cpp/external_arc/message_center/arc_notification_content_view_unittest.cc @@ -71,7 +71,7 @@ class MockKeyboardDelegate : public exo::KeyboardDelegate { MOCK_METHOD(void, OnKeyboardEnter, (exo::Surface*, - (const base::flat_map&)), + (const base::flat_map&)), (override)); MOCK_METHOD(void, OnKeyboardLeave, (exo::Surface*), (override)); MOCK_METHOD(uint32_t, diff --git a/components/exo/BUILD.gn b/components/exo/BUILD.gn index d8307dc5b0a43c..b4cc6d6f820b3f 100644 --- a/components/exo/BUILD.gn +++ b/components/exo/BUILD.gn @@ -38,7 +38,6 @@ static_library("exo") { "frame_sink_resource_manager.cc", "frame_sink_resource_manager.h", "input_trace.h", - "key_state.h", "keyboard_delegate.h", "keyboard_device_configuration_delegate.h", "keyboard_observer.h", diff --git a/components/exo/key_state.h b/components/exo/key_state.h deleted file mode 100644 index 1645d888fb3c57..00000000000000 --- a/components/exo/key_state.h +++ /dev/null @@ -1,26 +0,0 @@ -// Copyright 2021 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#ifndef COMPONENTS_EXO_KEY_STATE_H_ -#define COMPONENTS_EXO_KEY_STATE_H_ - -namespace ui { -enum class DomCode; -} - -namespace exo { - -// Represents the current pressed key state. -struct KeyState { - ui::DomCode code; - bool consumed_by_ime; -}; - -inline bool operator==(const KeyState& lhs, const KeyState& rhs) { - return lhs.code == rhs.code && lhs.consumed_by_ime == rhs.consumed_by_ime; -} - -} // namespace exo - -#endif // COMPONENTS_EXO_KEY_STATE_H_ diff --git a/components/exo/keyboard.cc b/components/exo/keyboard.cc index 2461922af7e2b9..f1752cc0b8058a 100644 --- a/components/exo/keyboard.cc +++ b/components/exo/keyboard.cc @@ -294,28 +294,22 @@ void Keyboard::OnKeyEvent(ui::KeyEvent* event) { switch (event->type()) { case ui::ET_KEY_PRESSED: { auto it = pressed_keys_.find(physical_code); - if (it == pressed_keys_.end() && !event->handled() && + if (it == pressed_keys_.end() && !consumed_by_ime && !event->handled() && physical_code != ui::DomCode::NONE) { - for (auto& observer : observer_list_) - observer.OnKeyboardKey(event->time_stamp(), event->code(), true); - - if (!consumed_by_ime) { - // Process key press event if not already handled and not already - // pressed. - uint32_t serial = delegate_->OnKeyboardKey(event->time_stamp(), - event->code(), true); - if (AreKeyboardKeyAcksNeeded()) { - pending_key_acks_.insert( - {serial, - {*event, base::TimeTicks::Now() + - expiration_delay_for_pending_key_acks_}}); - event->SetHandled(); - } + // Process key press event if not already handled and not already + // pressed. + uint32_t serial = + delegate_->OnKeyboardKey(event->time_stamp(), event->code(), true); + if (AreKeyboardKeyAcksNeeded()) { + pending_key_acks_.insert( + {serial, + {*event, base::TimeTicks::Now() + + 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_.emplace(physical_code, - KeyState{event->code(), consumed_by_ime}); + pressed_keys_.insert({physical_code, event->code()}); } else if (it != pressed_keys_.end() && !event->handled()) { // Non-repeate key events for already pressed key can be sent in some // cases (e.g. Holding 'A' key then holding 'B' key then releasing 'A' @@ -330,23 +324,18 @@ void Keyboard::OnKeyEvent(ui::KeyEvent* event) { // Process key release event if currently pressed. auto it = pressed_keys_.find(physical_code); if (it != pressed_keys_.end()) { - for (auto& observer : observer_list_) - observer.OnKeyboardKey(event->time_stamp(), it->second.code, false); - - if (!it->second.consumed_by_ime) { - // We use the code that was generated 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(), - it->second.code, false); - if (AreKeyboardKeyAcksNeeded()) { - pending_key_acks_.insert( - {serial, - {*event, base::TimeTicks::Now() + - expiration_delay_for_pending_key_acks_}}); - event->SetHandled(); - } + // 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(), it->second, false); + if (AreKeyboardKeyAcksNeeded()) { + pending_key_acks_.insert( + {serial, + {*event, base::TimeTicks::Now() + + expiration_delay_for_pending_key_acks_}}); + event->SetHandled(); } pressed_keys_.erase(it); } diff --git a/components/exo/keyboard.h b/components/exo/keyboard.h index 249d2cfacddf62..c86332c18ef81b 100644 --- a/components/exo/keyboard.h +++ b/components/exo/keyboard.h @@ -12,7 +12,6 @@ #include "base/containers/flat_map.h" #include "base/containers/flat_set.h" #include "base/observer_list.h" -#include "components/exo/key_state.h" #include "components/exo/keyboard_observer.h" #include "components/exo/seat_observer.h" #include "components/exo/surface_observer.h" @@ -119,7 +118,7 @@ class Keyboard : public ui::EventHandler, // 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 pressed_keys_; + base::flat_map pressed_keys_; // Key state changes that are expected to be acknowledged. using KeyStateChange = std::pair; diff --git a/components/exo/keyboard_delegate.h b/components/exo/keyboard_delegate.h index a6a1e72c6fdc15..4990b979f0b3eb 100644 --- a/components/exo/keyboard_delegate.h +++ b/components/exo/keyboard_delegate.h @@ -8,7 +8,6 @@ #include "base/containers/flat_map.h" #include "base/strings/string_piece.h" #include "base/time/time.h" -#include "components/exo/key_state.h" namespace ui { enum class DomCode; @@ -30,7 +29,7 @@ class KeyboardDelegate { // Called when keyboard focus enters a new valid target surface. virtual void OnKeyboardEnter( Surface* surface, - const base::flat_map& pressed_keys) = 0; + const base::flat_map& pressed_keys) = 0; // Called when keyboard focus leaves a valid target surface. virtual void OnKeyboardLeave(Surface* surface) = 0; diff --git a/components/exo/keyboard_observer.h b/components/exo/keyboard_observer.h index 32eda8d3736c63..02f0c122c2022c 100644 --- a/components/exo/keyboard_observer.h +++ b/components/exo/keyboard_observer.h @@ -15,14 +15,7 @@ class KeyboardObserver { // Called at the top of the keyboard's destructor, to give observers a change // to remove themselves. - virtual void OnKeyboardDestroying(Keyboard* keyboard) {} - - // Called just before KeyboardDelegate::OnKeyboardKey(). - // KeyboardDelegate::OnKeyboardKey() may not be called, specifically if IME - // consumed the key event, but this is always. - virtual void OnKeyboardKey(base::TimeTicks time_stamp, - ui::DomCode code, - bool pressed) {} + virtual void OnKeyboardDestroying(Keyboard* keyboard) = 0; }; } // namespace exo diff --git a/components/exo/keyboard_unittest.cc b/components/exo/keyboard_unittest.cc index cf6317dfd8c199..c4c62f59c4bfee 100644 --- a/components/exo/keyboard_unittest.cc +++ b/components/exo/keyboard_unittest.cc @@ -57,7 +57,7 @@ class MockKeyboardDelegate : public KeyboardDelegate { MOCK_METHOD(bool, CanAcceptKeyboardEventsForSurface, (Surface*), (const)); MOCK_METHOD(void, OnKeyboardEnter, - (Surface*, (const base::flat_map&))); + (Surface*, (const base::flat_map&))); MOCK_METHOD(void, OnKeyboardLeave, (Surface*)); MOCK_METHOD(uint32_t, OnKeyboardKey, (base::TimeTicks, ui::DomCode, bool)); MOCK_METHOD(void, OnKeyboardModifiers, (const KeyboardModifiers&)); @@ -83,9 +83,7 @@ class MockKeyboardObserver : public KeyboardObserver { // Overridden from KeyboardObserver: MOCK_METHOD(void, OnKeyboardDestroying, (Keyboard*)); - MOCK_METHOD(void, OnKeyboardKey, (base::TimeTicks, ui::DomCode, bool)); }; -using NiceMockKeyboardObserver = ::testing::NiceMock; class TestShellSurface : public ShellSurface { public: @@ -184,12 +182,10 @@ TEST_F(KeyboardTest, OnKeyboardEnter) { .WillOnce(testing::Return(true)); EXPECT_CALL(*delegate_ptr, OnKeyboardModifiers(KeyboardModifiers{ kShiftMask | kNumLockMask, 0, 0, 0})); - EXPECT_CALL( - *delegate_ptr, - OnKeyboardEnter( - surface.get(), - base::flat_map( - {{ui::DomCode::US_A, KeyState{ui::DomCode::US_A, false}}}))); + EXPECT_CALL(*delegate_ptr, + OnKeyboardEnter(surface.get(), + base::flat_map( + {{ui::DomCode::US_A, ui::DomCode::US_A}}))); focus_client->FocusWindow(nullptr); focus_client->FocusWindow(surface->window()); // Surface should maintain keyboard focus when moved to top-level window. @@ -205,9 +201,9 @@ TEST_F(KeyboardTest, OnKeyboardEnter) { .WillOnce(testing::Return(true)); EXPECT_CALL(*delegate_ptr, OnKeyboardModifiers(KeyboardModifiers{ kShiftMask | kNumLockMask, 0, 0, 0})); - EXPECT_CALL( - *delegate_ptr, - OnKeyboardEnter(surface.get(), base::flat_map())); + EXPECT_CALL(*delegate_ptr, + OnKeyboardEnter(surface.get(), + base::flat_map())); focus_client->FocusWindow(surface->window()->GetToplevelWindow()); testing::Mock::VerifyAndClearExpectations(delegate_ptr); } @@ -234,9 +230,9 @@ TEST_F(KeyboardTest, OnKeyboardLeave) { EXPECT_CALL(*delegate_ptr, OnKeyboardModifiers(KeyboardModifiers{kNumLockMask, 0, 0, 0})); - EXPECT_CALL( - *delegate_ptr, - OnKeyboardEnter(surface.get(), base::flat_map())); + EXPECT_CALL(*delegate_ptr, + OnKeyboardEnter(surface.get(), + base::flat_map())); focus_client->FocusWindow(surface->window()); testing::Mock::VerifyAndClearExpectations(delegate_ptr); @@ -246,9 +242,9 @@ TEST_F(KeyboardTest, OnKeyboardLeave) { EXPECT_CALL(*delegate_ptr, OnKeyboardModifiers(KeyboardModifiers{kNumLockMask, 0, 0, 0})); - EXPECT_CALL( - *delegate_ptr, - OnKeyboardEnter(surface.get(), base::flat_map())); + EXPECT_CALL(*delegate_ptr, + OnKeyboardEnter(surface.get(), + base::flat_map())); focus_client->FocusWindow(surface->window()); testing::Mock::VerifyAndClearExpectations(delegate_ptr); @@ -275,79 +271,55 @@ TEST_F(KeyboardTest, OnKeyboardKey) { auto delegate = std::make_unique(); auto* delegate_ptr = delegate.get(); - NiceMockKeyboardObserver observer; Seat seat; Keyboard keyboard(std::move(delegate), &seat); - keyboard.AddObserver(&observer); EXPECT_CALL(*delegate_ptr, CanAcceptKeyboardEventsForSurface(surface.get())) .WillOnce(testing::Return(true)); EXPECT_CALL(*delegate_ptr, OnKeyboardModifiers(KeyboardModifiers{kNumLockMask, 0, 0, 0})); - EXPECT_CALL( - *delegate_ptr, - OnKeyboardEnter(surface.get(), base::flat_map())); + EXPECT_CALL(*delegate_ptr, + OnKeyboardEnter(surface.get(), + base::flat_map())); focus_client->FocusWindow(surface->window()); testing::Mock::VerifyAndClearExpectations(delegate_ptr); ui::test::EventGenerator generator(ash::Shell::GetPrimaryRootWindow()); // This should only generate a press event for KEY_A. - { - testing::InSequence s; - EXPECT_CALL(observer, OnKeyboardKey(testing::_, ui::DomCode::US_A, true)); - EXPECT_CALL(*delegate_ptr, - OnKeyboardKey(testing::_, ui::DomCode::US_A, true)); - } + EXPECT_CALL(*delegate_ptr, + 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); - testing::Mock::VerifyAndClearExpectations(&observer); testing::Mock::VerifyAndClearExpectations(delegate_ptr); // This should not generate another press event for KEY_A. generator.PressKey(ui::VKEY_A, 0); - testing::Mock::VerifyAndClearExpectations(&observer); testing::Mock::VerifyAndClearExpectations(delegate_ptr); // This should only generate a single release event for KEY_A. - { - testing::InSequence s; - EXPECT_CALL(observer, OnKeyboardKey(testing::_, ui::DomCode::US_A, false)); - EXPECT_CALL(*delegate_ptr, - OnKeyboardKey(testing::_, ui::DomCode::US_A, false)); - } + EXPECT_CALL(*delegate_ptr, + OnKeyboardKey(testing::_, ui::DomCode::US_A, false)); generator.ReleaseKey(ui::VKEY_A, 0); - testing::Mock::VerifyAndClearExpectations(&observer); testing::Mock::VerifyAndClearExpectations(delegate_ptr); // Test key event rewriting. In this case, ARROW_DOWN is rewritten to KEY_END // as a result of ALT being pressed. - { - testing::InSequence s; - EXPECT_CALL(observer, OnKeyboardKey(testing::_, ui::DomCode::END, true)); - EXPECT_CALL(*delegate_ptr, - OnKeyboardKey(testing::_, ui::DomCode::END, true)); - } + EXPECT_CALL(*delegate_ptr, OnKeyboardKey(testing::_, ui::DomCode::END, true)); EXPECT_CALL(*delegate_ptr, OnKeyboardModifiers(KeyboardModifiers{ kAltMask | kNumLockMask, 0, 0, 0})); seat.set_physical_code_for_currently_processing_event_for_testing( ui::DomCode::ARROW_DOWN); generator.PressKey(ui::VKEY_END, ui::EF_ALT_DOWN); - testing::Mock::VerifyAndClearExpectations(&observer); testing::Mock::VerifyAndClearExpectations(delegate_ptr); // This should generate a release event for KEY_END as that is the key // associated with the key press. - { - testing::InSequence s; - EXPECT_CALL(observer, OnKeyboardKey(testing::_, ui::DomCode::END, false)); - EXPECT_CALL(*delegate_ptr, - OnKeyboardKey(testing::_, ui::DomCode::END, false)); - } + EXPECT_CALL(*delegate_ptr, + OnKeyboardKey(testing::_, ui::DomCode::END, false)); EXPECT_CALL(*delegate_ptr, OnKeyboardModifiers(KeyboardModifiers{kNumLockMask, 0, 0, 0})); generator.ReleaseKey(ui::VKEY_DOWN, 0); - testing::Mock::VerifyAndClearExpectations(&observer); testing::Mock::VerifyAndClearExpectations(delegate_ptr); // Press accelerator after surface lost focus. @@ -363,24 +335,17 @@ TEST_F(KeyboardTest, OnKeyboardKey) { .WillOnce(testing::Return(true)); EXPECT_CALL(*delegate_ptr, OnKeyboardModifiers(KeyboardModifiers{ kControlMask | kNumLockMask, 0, 0, 0})); - EXPECT_CALL( - *delegate_ptr, - OnKeyboardEnter( - surface.get(), - base::flat_map( - {{ui::DomCode::US_W, KeyState{ui::DomCode::US_W, false}}}))); + EXPECT_CALL(*delegate_ptr, + OnKeyboardEnter(surface.get(), + base::flat_map( + {{ui::DomCode::US_W, ui::DomCode::US_W}}))); focus_client->FocusWindow(surface->window()); testing::Mock::VerifyAndClearExpectations(delegate_ptr); // Releasing accelerator when surface has focus should generate event. - { - testing::InSequence s; - EXPECT_CALL(observer, OnKeyboardKey(testing::_, ui::DomCode::US_W, false)); - EXPECT_CALL(*delegate_ptr, - OnKeyboardKey(testing::_, ui::DomCode::US_W, false)); - } + EXPECT_CALL(*delegate_ptr, + OnKeyboardKey(testing::_, ui::DomCode::US_W, false)); generator.ReleaseKey(ui::VKEY_W, ui::EF_CONTROL_DOWN); - testing::Mock::VerifyAndClearExpectations(&observer); testing::Mock::VerifyAndClearExpectations(delegate_ptr); // Key events should be ignored when the focused window is not an @@ -397,28 +362,20 @@ TEST_F(KeyboardTest, OnKeyboardKey) { EXPECT_FALSE(seat.GetFocusedSurface()); EXPECT_FALSE(keyboard.focused_surface_for_testing()); - EXPECT_CALL(observer, - OnKeyboardKey(testing::_, ui::DomCode::ARROW_LEFT, true)) - .Times(0); EXPECT_CALL(*delegate_ptr, OnKeyboardKey(testing::_, ui::DomCode::ARROW_LEFT, true)) .Times(0); seat.set_physical_code_for_currently_processing_event_for_testing( ui::DomCode::ARROW_LEFT); generator.PressKey(ui::VKEY_LEFT, 0); - testing::Mock::VerifyAndClearExpectations(&observer); testing::Mock::VerifyAndClearExpectations(delegate_ptr); - EXPECT_CALL(observer, - OnKeyboardKey(testing::_, ui::DomCode::ARROW_LEFT, false)) - .Times(0); EXPECT_CALL(*delegate_ptr, OnKeyboardKey(testing::_, ui::DomCode::ARROW_LEFT, false)) .Times(0); generator.ReleaseKey(ui::VKEY_LEFT, 0); // Verify before destroying keyboard to make sure the expected call // is made on the methods above, rather than in the destructor. - testing::Mock::VerifyAndClearExpectations(&observer); testing::Mock::VerifyAndClearExpectations(delegate_ptr); } @@ -437,18 +394,16 @@ TEST_F(KeyboardTest, OnKeyboardKey_NotSendKeyIfConsumedByIme) { auto delegate = std::make_unique(); auto* delegate_ptr = delegate.get(); - NiceMockKeyboardObserver observer; Seat seat; Keyboard keyboard(std::move(delegate), &seat); - keyboard.AddObserver(&observer); EXPECT_CALL(*delegate_ptr, CanAcceptKeyboardEventsForSurface(surface.get())) .WillOnce(testing::Return(true)); EXPECT_CALL(*delegate_ptr, OnKeyboardModifiers(KeyboardModifiers{kNumLockMask, 0, 0, 0})); - EXPECT_CALL( - *delegate_ptr, - OnKeyboardEnter(surface.get(), base::flat_map())); + EXPECT_CALL(*delegate_ptr, + OnKeyboardEnter(surface.get(), + base::flat_map())); focus_client->FocusWindow(surface->window()); testing::Mock::VerifyAndClearExpectations(delegate_ptr); @@ -461,74 +416,49 @@ TEST_F(KeyboardTest, OnKeyboardKey_NotSendKeyIfConsumedByIme) { // If a text field is focused, a pressed key event is not sent to a client // because a key event should be consumed by the IME. - // However, the observer should receive OnKeyboardKey, always. - EXPECT_CALL(observer, OnKeyboardKey(testing::_, ui::DomCode::US_A, true)); EXPECT_CALL(*delegate_ptr, OnKeyboardKey(testing::_, ui::DomCode::US_A, true)) .Times(0); seat.set_physical_code_for_currently_processing_event_for_testing( ui::DomCode::US_A); generator.PressKey(ui::VKEY_A, 0); - testing::Mock::VerifyAndClearExpectations(&observer); testing::Mock::VerifyAndClearExpectations(delegate_ptr); // TODO(yhanada): The below EXPECT_CALL fails because exo::Keyboard currently // sends a key release event for the keys which exo::Keyboard sent a pressed // event for. It might causes a never-ending key repeat in the client. // EXPECT_CALL(delegate, OnKeyboardKey(testing::_, ui::DomCode::US_A, false)); - EXPECT_CALL(observer, OnKeyboardKey(testing::_, ui::DomCode::US_A, false)); generator.ReleaseKey(ui::VKEY_A, 0); - testing::Mock::VerifyAndClearExpectations(&observer); // Any key event should be sent to a client if the focused window is marked as // ImeBlocking. WMHelper::GetInstance()->SetImeBlocked(surface->window()->GetToplevelWindow(), true); - { - testing::InSequence s; - EXPECT_CALL(observer, OnKeyboardKey(testing::_, ui::DomCode::US_B, true)); - EXPECT_CALL(*delegate_ptr, - OnKeyboardKey(testing::_, ui::DomCode::US_B, true)); - } + EXPECT_CALL(*delegate_ptr, + OnKeyboardKey(testing::_, ui::DomCode::US_B, true)); seat.set_physical_code_for_currently_processing_event_for_testing( ui::DomCode::US_B); generator.PressKey(ui::VKEY_B, 0); - testing::Mock::VerifyAndClearExpectations(&observer); testing::Mock::VerifyAndClearExpectations(delegate_ptr); - { - testing::InSequence s; - EXPECT_CALL(observer, OnKeyboardKey(testing::_, ui::DomCode::US_B, false)); - EXPECT_CALL(*delegate_ptr, - OnKeyboardKey(testing::_, ui::DomCode::US_B, false)); - } + EXPECT_CALL(*delegate_ptr, + OnKeyboardKey(testing::_, ui::DomCode::US_B, false)); generator.ReleaseKey(ui::VKEY_B, 0); WMHelper::GetInstance()->SetImeBlocked(surface->window()->GetToplevelWindow(), false); - testing::Mock::VerifyAndClearExpectations(&observer); testing::Mock::VerifyAndClearExpectations(delegate_ptr); // Any key event should be sent to a client if a key event skips IME. surface->window()->SetProperty(aura::client::kSkipImeProcessing, true); - { - testing::InSequence s; - EXPECT_CALL(observer, OnKeyboardKey(testing::_, ui::DomCode::US_C, true)); - EXPECT_CALL(*delegate_ptr, - OnKeyboardKey(testing::_, ui::DomCode::US_C, true)); - } + EXPECT_CALL(*delegate_ptr, + OnKeyboardKey(testing::_, ui::DomCode::US_C, true)); seat.set_physical_code_for_currently_processing_event_for_testing( ui::DomCode::US_C); generator.PressKey(ui::VKEY_C, 0); - testing::Mock::VerifyAndClearExpectations(&observer); testing::Mock::VerifyAndClearExpectations(delegate_ptr); - { - testing::InSequence s; - EXPECT_CALL(observer, OnKeyboardKey(testing::_, ui::DomCode::US_C, false)); - EXPECT_CALL(*delegate_ptr, - OnKeyboardKey(testing::_, ui::DomCode::US_C, false)); - } + EXPECT_CALL(*delegate_ptr, + OnKeyboardKey(testing::_, ui::DomCode::US_C, false)); generator.ReleaseKey(ui::VKEY_C, 0); - testing::Mock::VerifyAndClearExpectations(&observer); testing::Mock::VerifyAndClearExpectations(delegate_ptr); input_method->SetFocusedTextInputClient(nullptr); @@ -632,9 +562,9 @@ TEST_F(KeyboardTest, OnKeyboardModifiers) { .WillOnce(testing::Return(true)); EXPECT_CALL(*delegate_ptr, OnKeyboardModifiers(KeyboardModifiers{kNumLockMask, 0, 0, 0})); - EXPECT_CALL( - *delegate_ptr, - OnKeyboardEnter(surface.get(), base::flat_map())); + EXPECT_CALL(*delegate_ptr, + OnKeyboardEnter(surface.get(), + base::flat_map())); focus_client->FocusWindow(surface->window()); testing::Mock::VerifyAndClearExpectations(delegate_ptr); @@ -973,9 +903,9 @@ TEST_F(KeyboardTest, AckKeyboardKey) { .WillOnce(testing::Return(true)); EXPECT_CALL(*delegate_ptr, OnKeyboardModifiers(KeyboardModifiers{kNumLockMask, 0, 0, 0})); - EXPECT_CALL( - *delegate_ptr, - OnKeyboardEnter(surface.get(), base::flat_map())); + EXPECT_CALL(*delegate_ptr, + OnKeyboardEnter(surface.get(), + base::flat_map())); focus_client->FocusWindow(surface->window()); testing::Mock::VerifyAndClearExpectations(delegate_ptr); @@ -1087,9 +1017,9 @@ TEST_F(KeyboardTest, AckKeyboardKeyMoveFocus) { .WillOnce(testing::Return(true)); EXPECT_CALL(*delegate_ptr, OnKeyboardModifiers(KeyboardModifiers{kNumLockMask, 0, 0, 0})); - EXPECT_CALL( - *delegate_ptr, - OnKeyboardEnter(surface.get(), base::flat_map())); + EXPECT_CALL(*delegate_ptr, + OnKeyboardEnter(surface.get(), + base::flat_map())); focus_client->FocusWindow(surface->window()); testing::Mock::VerifyAndClearExpectations(delegate_ptr); @@ -1138,9 +1068,9 @@ TEST_F(KeyboardTest, AckKeyboardKeyExpired) { .WillOnce(testing::Return(true)); EXPECT_CALL(*delegate_ptr, OnKeyboardModifiers(KeyboardModifiers{kNumLockMask, 0, 0, 0})); - EXPECT_CALL( - *delegate_ptr, - OnKeyboardEnter(surface.get(), base::flat_map())); + EXPECT_CALL(*delegate_ptr, + OnKeyboardEnter(surface.get(), + base::flat_map())); focus_client->FocusWindow(surface->window()); testing::Mock::VerifyAndClearExpectations(delegate_ptr); @@ -1231,9 +1161,9 @@ TEST_F(KeyboardTest, AckKeyboardKeyExpiredWithMovingFocusAccelerator) { .WillOnce(testing::Return(true)); EXPECT_CALL(*delegate_ptr, OnKeyboardModifiers(KeyboardModifiers{kNumLockMask, 0, 0, 0})); - EXPECT_CALL( - *delegate_ptr, - OnKeyboardEnter(surface.get(), base::flat_map())); + EXPECT_CALL(*delegate_ptr, + OnKeyboardEnter(surface.get(), + base::flat_map())); focus_client->FocusWindow(surface->window()); testing::Mock::VerifyAndClearExpectations(delegate_ptr); diff --git a/components/exo/seat.cc b/components/exo/seat.cc index 7054cca7880ed1..3aea96f509a955 100644 --- a/components/exo/seat.cc +++ b/components/exo/seat.cc @@ -321,9 +321,8 @@ void Seat::OnKeyEvent(ui::KeyEvent* event) { if (physical_code_for_currently_processing_event_ != ui::DomCode::NONE) { switch (event->type()) { case ui::ET_KEY_PRESSED: - pressed_keys_.emplace( - physical_code_for_currently_processing_event_, - KeyState{event->code(), /*consumed_by_ime=*/false}); + 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_); diff --git a/components/exo/seat.h b/components/exo/seat.h index 086425e8c9ce63..b9602f8c7b6885 100644 --- a/components/exo/seat.h +++ b/components/exo/seat.h @@ -11,7 +11,6 @@ #include "base/observer_list.h" #include "build/chromeos_buildflags.h" #include "components/exo/data_source_observer.h" -#include "components/exo/key_state.h" #include "ui/aura/client/drag_drop_delegate.h" #include "ui/aura/client/focus_change_observer.h" #include "ui/base/clipboard/clipboard_observer.h" @@ -70,7 +69,7 @@ class Seat : public aura::client::FocusChangeObserver, virtual Surface* GetFocusedSurface(); // Returns currently pressed keys. - const base::flat_map& pressed_keys() const { + const base::flat_map& pressed_keys() const { return pressed_keys_; } @@ -177,7 +176,7 @@ class Seat : public aura::client::FocusChangeObserver, // 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 pressed_keys_; + base::flat_map pressed_keys_; ui::DomCode physical_code_for_currently_processing_event_ = ui::DomCode::NONE; // Data source being used as a clipboard content. diff --git a/components/exo/seat_unittest.cc b/components/exo/seat_unittest.cc index 150c6fd39bc3fc..fa80d56ebe85df 100644 --- a/components/exo/seat_unittest.cc +++ b/components/exo/seat_unittest.cc @@ -585,15 +585,15 @@ TEST_F(SeatTest, PressedKeys) { seat.WillProcessEvent(&press_a); seat.OnKeyEvent(press_a.AsKeyEvent()); seat.DidProcessEvent(&press_a); - base::flat_map pressed_keys; - pressed_keys[ui::CodeFromNative(&press_a)] = KeyState{press_a.code(), false}; + base::flat_map 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)] = KeyState{press_b.code(), false}; + 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, diff --git a/components/exo/wayland/serial_tracker.cc b/components/exo/wayland/serial_tracker.cc index 6430391ab1ab52..7dcd7e1874803c 100644 --- a/components/exo/wayland/serial_tracker.cc +++ b/components/exo/wayland/serial_tracker.cc @@ -85,15 +85,5 @@ void SerialTracker::ResetTouchDownSerial() { touch_down_serial_ = base::nullopt; } -uint32_t SerialTracker::MaybeNextKeySerial() { - if (!key_serial_.has_value()) - key_serial_ = GetNextSerial(OTHER_EVENT); - return key_serial_.value(); -} - -void SerialTracker::ResetKeySerial() { - key_serial_ = base::nullopt; -} - } // namespace wayland } // namespace exo diff --git a/components/exo/wayland/serial_tracker.h b/components/exo/wayland/serial_tracker.h index 7de91b244ba643..6de53f7aed1f31 100644 --- a/components/exo/wayland/serial_tracker.h +++ b/components/exo/wayland/serial_tracker.h @@ -5,10 +5,11 @@ #ifndef COMPONENTS_EXO_WAYLAND_SERIAL_TRACKER_H_ #define COMPONENTS_EXO_WAYLAND_SERIAL_TRACKER_H_ -#include - +#include #include +#include "base/gtest_prod_util.h" +#include "base/macros.h" #include "base/optional.h" struct wl_display; @@ -31,8 +32,6 @@ class SerialTracker { }; explicit SerialTracker(struct wl_display* display); - SerialTracker(const SerialTracker&) = delete; - SerialTracker& operator=(const SerialTracker&) = delete; ~SerialTracker(); // After shutdown, |GetNextSerial| returns 0. @@ -49,19 +48,13 @@ class SerialTracker { // test for it in GetNextSerial. void ResetTouchDownSerial(); - // If there exists a serial for key already, returns it. Or, it creates - // a new serial, and returns it. - uint32_t MaybeNextKeySerial(); - - // Resets the stored key serial, so that next MaybeNextKeySerial() call will - // generate a new serial. - void ResetKeySerial(); - // Get the EventType for a serial number, or nullopt if the serial number was // never sent or is too old. base::Optional GetEventType(uint32_t serial) const; private: + FRIEND_TEST_ALL_PREFIXES(SerialTrackerTest, WrapAroundWholeRange); + struct wl_display* display_; // EventTypes are stored in a circular buffer, because serial numbers are @@ -76,7 +69,8 @@ class SerialTracker { base::Optional pointer_down_serial_; base::Optional touch_down_serial_; - base::Optional key_serial_; + + DISALLOW_COPY_AND_ASSIGN(SerialTracker); }; } // namespace wayland diff --git a/components/exo/wayland/server.cc b/components/exo/wayland/server.cc index 86c3b9c1ab787d..0eb9088a9ce755 100644 --- a/components/exo/wayland/server.cc +++ b/components/exo/wayland/server.cc @@ -197,6 +197,8 @@ Server::Server(Display* display) : display_(display) { wl_global_create(wl_display_.get(), &zcr_keyboard_configuration_v1_interface, zcr_keyboard_configuration_v1_interface.version, display_, bind_keyboard_configuration); + wl_global_create(wl_display_.get(), &zcr_keyboard_extension_v1_interface, 1, + display_, bind_keyboard_extension); wl_global_create(wl_display_.get(), &zcr_notification_shell_v1_interface, 1, display_, bind_notification_shell); wl_global_create(wl_display_.get(), &zcr_remote_shell_v1_interface, @@ -221,11 +223,6 @@ Server::Server(Display* display) : display_(display) { wl_global_create(wl_display_.get(), &zcr_extended_drag_v1_interface, 1, display_, bind_extended_drag); - zcr_keyboard_extension_data_ = - std::make_unique(serial_tracker_.get()); - wl_global_create(wl_display_.get(), &zcr_keyboard_extension_v1_interface, 1, - zcr_keyboard_extension_data_.get(), bind_keyboard_extension); - zwp_text_manager_data_ = std::make_unique( display_->seat()->xkb_tracker(), serial_tracker_.get()); wl_global_create(wl_display_.get(), &zwp_text_input_manager_v1_interface, 1, diff --git a/components/exo/wayland/server.h b/components/exo/wayland/server.h index 62baedd7a3adcf..82a6e554a13229 100644 --- a/components/exo/wayland/server.h +++ b/components/exo/wayland/server.h @@ -27,7 +27,6 @@ namespace wayland { class SerialTracker; struct WaylandDataDeviceManager; class WaylandDisplayOutput; -struct WaylandKeyboardExtension; struct WaylandSeat; struct WaylandTextInputManager; struct WaylandXdgShell; @@ -78,7 +77,6 @@ class Server : public display::DisplayObserver { std::unique_ptr seat_data_; #if BUILDFLAG(IS_CHROMEOS_ASH) - std::unique_ptr zcr_keyboard_extension_data_; std::unique_ptr zwp_text_manager_data_; std::unique_ptr zxdg_shell_data_; std::unique_ptr xdg_shell_data_; diff --git a/components/exo/wayland/wayland_keyboard_delegate.cc b/components/exo/wayland/wayland_keyboard_delegate.cc index 61d1f6e8181974..17ab1ac1d69185 100644 --- a/components/exo/wayland/wayland_keyboard_delegate.cc +++ b/components/exo/wayland/wayland_keyboard_delegate.cc @@ -12,7 +12,6 @@ #include "base/containers/flat_map.h" #include "components/exo/wayland/serial_tracker.h" #include "ui/events/keycodes/dom/dom_code.h" -#include "ui/events/keycodes/dom/keycode_converter.h" #if BUILDFLAG(USE_XKBCOMMON) #include @@ -40,7 +39,7 @@ bool WaylandKeyboardDelegate::CanAcceptKeyboardEventsForSurface( void WaylandKeyboardDelegate::OnKeyboardEnter( Surface* surface, - const base::flat_map& pressed_keys) { + const base::flat_map& pressed_keys) { wl_resource* surface_resource = GetSurfaceResource(surface); DCHECK(surface_resource); wl_array keys; @@ -49,7 +48,7 @@ void WaylandKeyboardDelegate::OnKeyboardEnter( uint32_t* value = static_cast(wl_array_add(&keys, sizeof(uint32_t))); DCHECK(value); - *value = ui::KeycodeConverter::DomCodeToEvdevCode(entry.second.code); + *value = ui::KeycodeConverter::DomCodeToEvdevCode(entry.second); } wl_keyboard_send_enter( keyboard_resource_, @@ -72,8 +71,8 @@ void WaylandKeyboardDelegate::OnKeyboardLeave(Surface* surface) { uint32_t WaylandKeyboardDelegate::OnKeyboardKey(base::TimeTicks time_stamp, ui::DomCode code, bool pressed) { - uint32_t serial = serial_tracker_->MaybeNextKeySerial(); - serial_tracker_->ResetKeySerial(); + uint32_t serial = + serial_tracker_->GetNextSerial(SerialTracker::EventType::OTHER_EVENT); SendTimestamp(time_stamp); wl_keyboard_send_key( keyboard_resource_, serial, TimeTicksToMilliseconds(time_stamp), diff --git a/components/exo/wayland/wayland_keyboard_delegate.h b/components/exo/wayland/wayland_keyboard_delegate.h index a58f4f95e51ca4..d66b290ec68672 100644 --- a/components/exo/wayland/wayland_keyboard_delegate.h +++ b/components/exo/wayland/wayland_keyboard_delegate.h @@ -38,7 +38,7 @@ class WaylandKeyboardDelegate : public WaylandInputDelegate, bool CanAcceptKeyboardEventsForSurface(Surface* surface) const override; void OnKeyboardEnter( Surface* surface, - const base::flat_map& pressed_keys) override; + const base::flat_map& pressed_keys) override; void OnKeyboardLeave(Surface* surface) override; uint32_t OnKeyboardKey(base::TimeTicks time_stamp, ui::DomCode key, diff --git a/components/exo/wayland/zcr_keyboard_extension.cc b/components/exo/wayland/zcr_keyboard_extension.cc index c690eafc54e223..a31f742fc197ca 100644 --- a/components/exo/wayland/zcr_keyboard_extension.cc +++ b/components/exo/wayland/zcr_keyboard_extension.cc @@ -10,10 +10,7 @@ #include "components/exo/keyboard.h" #include "components/exo/keyboard_observer.h" -#include "components/exo/wayland/serial_tracker.h" #include "components/exo/wayland/server_util.h" -#include "ui/events/keycodes/dom/dom_code.h" -#include "ui/events/keycodes/dom/keycode_converter.h" namespace exo { namespace wayland { @@ -25,12 +22,8 @@ namespace { class WaylandExtendedKeyboardImpl : public KeyboardObserver { public: - WaylandExtendedKeyboardImpl(wl_resource* resource, - SerialTracker* serial_tracker, - Keyboard* keyboard) - : resource_(resource), - serial_tracker_(serial_tracker), - keyboard_(keyboard) { + explicit WaylandExtendedKeyboardImpl(Keyboard* keyboard) + : keyboard_(keyboard) { keyboard_->AddObserver(this); keyboard_->SetNeedKeyboardKeyAcks(true); } @@ -50,33 +43,12 @@ class WaylandExtendedKeyboardImpl : public KeyboardObserver { keyboard_ = nullptr; } - void OnKeyboardKey(base::TimeTicks time_stamp, - ui::DomCode code, - bool pressed) override { - if (wl_resource_get_version(resource_) < - ZCR_EXTENDED_KEYBOARD_V1_PEEK_KEY_SINCE_VERSION) { - return; - } - - uint32_t serial = serial_tracker_->MaybeNextKeySerial(); - zcr_extended_keyboard_v1_send_peek_key( - resource_, serial, TimeTicksToMilliseconds(time_stamp), - ui::KeycodeConverter::DomCodeToEvdevCode(code), - pressed ? WL_KEYBOARD_KEY_STATE_PRESSED - : WL_KEYBOARD_KEY_STATE_RELEASED); - wl_client_flush(client()); - } - void AckKeyboardKey(uint32_t serial, bool handled) { if (keyboard_) keyboard_->AckKeyboardKey(serial, handled); } private: - wl_client* client() const { return wl_resource_get_client(resource_); } - - wl_resource* const resource_; - SerialTracker* const serial_tracker_; Keyboard* keyboard_; }; @@ -103,9 +75,6 @@ void keyboard_extension_get_extended_keyboard(wl_client* client, wl_resource* resource, uint32_t id, wl_resource* keyboard_resource) { - WaylandKeyboardExtension* keyboard_extension = - GetUserDataAs(resource); - Keyboard* keyboard = GetUserDataAs(keyboard_resource); if (keyboard->AreKeyboardKeyAcksNeeded()) { wl_resource_post_error( @@ -115,13 +84,11 @@ void keyboard_extension_get_extended_keyboard(wl_client* client, } wl_resource* extended_keyboard_resource = - wl_resource_create(client, &zcr_extended_keyboard_v1_interface, 2, id); + wl_resource_create(client, &zcr_extended_keyboard_v1_interface, 1, id); SetImplementation(extended_keyboard_resource, &extended_keyboard_implementation, - std::make_unique( - extended_keyboard_resource, - keyboard_extension->serial_tracker, keyboard)); + std::make_unique(keyboard)); } const struct zcr_keyboard_extension_v1_interface diff --git a/components/exo/wayland/zcr_keyboard_extension.h b/components/exo/wayland/zcr_keyboard_extension.h index e0d9bfe081fd08..b8679ac10c7e42 100644 --- a/components/exo/wayland/zcr_keyboard_extension.h +++ b/components/exo/wayland/zcr_keyboard_extension.h @@ -11,17 +11,6 @@ struct wl_client; namespace exo { namespace wayland { -class SerialTracker; - -struct WaylandKeyboardExtension { - explicit WaylandKeyboardExtension(SerialTracker* serial_tracker) - : serial_tracker(serial_tracker) {} - WaylandKeyboardExtension(const WaylandKeyboardExtension&) = delete; - WaylandKeyboardExtension& operator=(const WaylandKeyboardExtension&) = delete; - - // Owned by Server, which always outlives zcr_keyboard_extension. - SerialTracker* const serial_tracker; -}; void bind_keyboard_extension(wl_client* client, void* data, diff --git a/third_party/wayland-protocols/unstable/keyboard/keyboard-extension-unstable-v1.xml b/third_party/wayland-protocols/unstable/keyboard/keyboard-extension-unstable-v1.xml index a213272ad593f7..95276789f5aeee 100644 --- a/third_party/wayland-protocols/unstable/keyboard/keyboard-extension-unstable-v1.xml +++ b/third_party/wayland-protocols/unstable/keyboard/keyboard-extension-unstable-v1.xml @@ -56,7 +56,7 @@ - + The zcr_extended_keyboard_v1 interface extends the wl_keyboard interface with requests to notify whether sent key events are handled or not by @@ -79,21 +79,5 @@ - - - - A key event which will be soon delivered to the application via - wl_keyboard::key. If text_input consumes the key event, - wl_keyboard::key is not delivered, but this event is still delivered - to the application. - All arguments are the exactly same one of the following wl_keyboard::key, - including serial. - - - - - - -