Commit 0fbe4abc authored by Hidehiko Abe's avatar Hidehiko Abe Committed by Commit Bot

exo: Simplifies WaylandKeyboardDelegate lifetime management.

It is just the delegate of Keyboard, so instead of using
the observer, this CL just defers the lifetime management
of WaylandKeyboardDelegate to Keyboard.
This allows us to remove the inheritance of KeyboardObserver
from WaylandKeyboardDelegate.

BUG=1123705
TEST=Ran exo_unittests.

Change-Id: I45b35a016edc3c7bd441223f1b25a28ad1debaa3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2416476
Commit-Queue: Hidehiko Abe <hidehiko@chromium.org>
Reviewed-by: default avatarJun Mukai <mukai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#808192}
parent 424e0563
...@@ -589,14 +589,16 @@ TEST_F(ArcNotificationContentViewTest, AcceptInputTextWithActivate) { ...@@ -589,14 +589,16 @@ TEST_F(ArcNotificationContentViewTest, AcceptInputTextWithActivate) {
ActivateArcNotification(); ActivateArcNotification();
EXPECT_EQ(surface()->window(), GetFocusedWindow()); EXPECT_EQ(surface()->window(), GetFocusedWindow());
MockKeyboardDelegate delegate; auto delegate = std::make_unique<MockKeyboardDelegate>();
EXPECT_CALL(delegate, CanAcceptKeyboardEventsForSurface(surface())) auto* delegate_ptr = delegate.get();
EXPECT_CALL(*delegate, CanAcceptKeyboardEventsForSurface(surface()))
.WillOnce(testing::Return(true)); .WillOnce(testing::Return(true));
exo::Seat seat; exo::Seat seat;
auto keyboard = std::make_unique<exo::Keyboard>(&delegate, &seat); auto keyboard = std::make_unique<exo::Keyboard>(std::move(delegate), &seat);
ui::test::EventGenerator generator(Shell::GetPrimaryRootWindow()); ui::test::EventGenerator generator(Shell::GetPrimaryRootWindow());
EXPECT_CALL(delegate, 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( seat.set_physical_code_for_currently_processing_event_for_testing(
ui::DomCode::US_A); ui::DomCode::US_A);
generator.PressKey(ui::VKEY_A, 0); generator.PressKey(ui::VKEY_A, 0);
...@@ -615,13 +617,15 @@ TEST_F(ArcNotificationContentViewTest, NotAcceptInputTextWithoutActivate) { ...@@ -615,13 +617,15 @@ TEST_F(ArcNotificationContentViewTest, NotAcceptInputTextWithoutActivate) {
CreateAndShowNotificationView(notification); CreateAndShowNotificationView(notification);
EXPECT_FALSE(GetFocusedWindow()); EXPECT_FALSE(GetFocusedWindow());
MockKeyboardDelegate delegate; auto delegate = std::make_unique<MockKeyboardDelegate>();
EXPECT_CALL(delegate, CanAcceptKeyboardEventsForSurface(surface())).Times(0); auto* delegate_ptr = delegate.get();
EXPECT_CALL(*delegate_ptr, CanAcceptKeyboardEventsForSurface(surface()))
.Times(0);
exo::Seat seat; exo::Seat seat;
auto keyboard = std::make_unique<exo::Keyboard>(&delegate, &seat); auto keyboard = std::make_unique<exo::Keyboard>(std::move(delegate), &seat);
ui::test::EventGenerator generator(Shell::GetPrimaryRootWindow()); ui::test::EventGenerator generator(Shell::GetPrimaryRootWindow());
EXPECT_CALL(delegate, OnKeyboardKey(testing::_, testing::_, testing::_)) EXPECT_CALL(*delegate_ptr, OnKeyboardKey(testing::_, testing::_, testing::_))
.Times(0); .Times(0);
seat.set_physical_code_for_currently_processing_event_for_testing( seat.set_physical_code_for_currently_processing_event_for_testing(
ui::DomCode::US_A); ui::DomCode::US_A);
......
...@@ -165,8 +165,8 @@ bool IsArcSurface(Surface* surface) { ...@@ -165,8 +165,8 @@ bool IsArcSurface(Surface* surface) {
//////////////////////////////////////////////////////////////////////////////// ////////////////////////////////////////////////////////////////////////////////
// Keyboard, public: // Keyboard, public:
Keyboard::Keyboard(KeyboardDelegate* delegate, Seat* seat) Keyboard::Keyboard(std::unique_ptr<KeyboardDelegate> delegate, Seat* seat)
: delegate_(delegate), : delegate_(std::move(delegate)),
seat_(seat), seat_(seat),
expiration_delay_for_pending_key_acks_(base::TimeDelta::FromMilliseconds( expiration_delay_for_pending_key_acks_(base::TimeDelta::FromMilliseconds(
kExpirationDelayForPendingKeyAcksMs)) { kExpirationDelayForPendingKeyAcksMs)) {
......
...@@ -5,7 +5,7 @@ ...@@ -5,7 +5,7 @@
#ifndef COMPONENTS_EXO_KEYBOARD_H_ #ifndef COMPONENTS_EXO_KEYBOARD_H_
#define COMPONENTS_EXO_KEYBOARD_H_ #define COMPONENTS_EXO_KEYBOARD_H_
#include <vector> #include <memory>
#include "ash/public/cpp/keyboard/keyboard_controller_observer.h" #include "ash/public/cpp/keyboard/keyboard_controller_observer.h"
#include "base/containers/flat_map.h" #include "base/containers/flat_map.h"
...@@ -36,10 +36,10 @@ class Keyboard : public ui::EventHandler, ...@@ -36,10 +36,10 @@ class Keyboard : public ui::EventHandler,
public SeatObserver, public SeatObserver,
public ash::KeyboardControllerObserver { public ash::KeyboardControllerObserver {
public: public:
Keyboard(KeyboardDelegate* delegate, Seat* seat); Keyboard(std::unique_ptr<KeyboardDelegate> delegate, Seat* seat);
~Keyboard() override; ~Keyboard() override;
KeyboardDelegate* delegate() const { return delegate_; } KeyboardDelegate* delegate() const { return delegate_.get(); }
bool HasDeviceConfigurationDelegate() const; bool HasDeviceConfigurationDelegate() const;
void SetDeviceConfigurationDelegate( void SetDeviceConfigurationDelegate(
...@@ -91,7 +91,7 @@ class Keyboard : public ui::EventHandler, ...@@ -91,7 +91,7 @@ class Keyboard : public ui::EventHandler,
// The delegate instance that all events except for events about device // The delegate instance that all events except for events about device
// configuration are dispatched to. // configuration are dispatched to.
KeyboardDelegate* const delegate_; std::unique_ptr<KeyboardDelegate> delegate_;
// Seat that the Keyboard recieves focus events from. // Seat that the Keyboard recieves focus events from.
Seat* const seat_; Seat* const seat_;
......
...@@ -19,6 +19,8 @@ class Surface; ...@@ -19,6 +19,8 @@ class Surface;
// Handles events on keyboards in context-specific ways. // Handles events on keyboards in context-specific ways.
class KeyboardDelegate { class KeyboardDelegate {
public: public:
virtual ~KeyboardDelegate() = default;
// This should return true if |surface| is a valid target for this keyboard. // This should return true if |surface| is a valid target for this keyboard.
// E.g. the surface is owned by the same client as the keyboard. // E.g. the surface is owned by the same client as the keyboard.
virtual bool CanAcceptKeyboardEventsForSurface(Surface* surface) const = 0; virtual bool CanAcceptKeyboardEventsForSurface(Surface* surface) const = 0;
...@@ -46,9 +48,6 @@ class KeyboardDelegate { ...@@ -46,9 +48,6 @@ class KeyboardDelegate {
virtual void OnKeyRepeatSettingsChanged(bool enabled, virtual void OnKeyRepeatSettingsChanged(bool enabled,
base::TimeDelta delay, base::TimeDelta delay,
base::TimeDelta interval) = 0; base::TimeDelta interval) = 0;
protected:
virtual ~KeyboardDelegate() {}
}; };
} // namespace exo } // namespace exo
......
This diff is collapsed.
...@@ -42,10 +42,6 @@ WaylandKeyboardDelegate::~WaylandKeyboardDelegate() { ...@@ -42,10 +42,6 @@ WaylandKeyboardDelegate::~WaylandKeyboardDelegate() {
#endif #endif
} }
void WaylandKeyboardDelegate::OnKeyboardDestroying(Keyboard* keyboard) {
delete this;
}
bool WaylandKeyboardDelegate::CanAcceptKeyboardEventsForSurface( bool WaylandKeyboardDelegate::CanAcceptKeyboardEventsForSurface(
Surface* surface) const { Surface* surface) const {
wl_resource* surface_resource = GetSurfaceResource(surface); wl_resource* surface_resource = GetSurfaceResource(surface);
......
...@@ -38,8 +38,7 @@ class SerialTracker; ...@@ -38,8 +38,7 @@ class SerialTracker;
// Keyboard delegate class that accepts events for surfaces owned by the same // Keyboard delegate class that accepts events for surfaces owned by the same
// client as a keyboard resource. // client as a keyboard resource.
class WaylandKeyboardDelegate : public WaylandInputDelegate, class WaylandKeyboardDelegate : public WaylandInputDelegate,
public KeyboardDelegate, public KeyboardDelegate
public KeyboardObserver
#if defined(OS_CHROMEOS) #if defined(OS_CHROMEOS)
, ,
public ash::ImeControllerImpl::Observer public ash::ImeControllerImpl::Observer
...@@ -52,7 +51,6 @@ class WaylandKeyboardDelegate : public WaylandInputDelegate, ...@@ -52,7 +51,6 @@ class WaylandKeyboardDelegate : public WaylandInputDelegate,
~WaylandKeyboardDelegate() override; ~WaylandKeyboardDelegate() override;
// Overridden from KeyboardDelegate: // Overridden from KeyboardDelegate:
void OnKeyboardDestroying(Keyboard* keyboard) override;
bool CanAcceptKeyboardEventsForSurface(Surface* surface) const override; bool CanAcceptKeyboardEventsForSurface(Surface* surface) const override;
void OnKeyboardEnter( void OnKeyboardEnter(
Surface* surface, Surface* surface,
......
...@@ -86,27 +86,22 @@ void seat_get_pointer(wl_client* client, wl_resource* resource, uint32_t id) { ...@@ -86,27 +86,22 @@ void seat_get_pointer(wl_client* client, wl_resource* resource, uint32_t id) {
} }
void seat_get_keyboard(wl_client* client, wl_resource* resource, uint32_t id) { void seat_get_keyboard(wl_client* client, wl_resource* resource, uint32_t id) {
#if defined(OS_CHROMEOS) #if defined(OS_CHROMEOS) && BUILDFLAG(USE_XKBCOMMON)
#if BUILDFLAG(USE_XKBCOMMON)
auto* data = GetUserDataAs<WaylandSeat>(resource); auto* data = GetUserDataAs<WaylandSeat>(resource);
uint32_t version = wl_resource_get_version(resource); uint32_t version = wl_resource_get_version(resource);
wl_resource* keyboard_resource = wl_resource* keyboard_resource =
wl_resource_create(client, &wl_keyboard_interface, version, id); wl_resource_create(client, &wl_keyboard_interface, version, id);
WaylandKeyboardDelegate* delegate = auto keyboard =
new WaylandKeyboardDelegate(keyboard_resource, data->serial_tracker); std::make_unique<Keyboard>(std::make_unique<WaylandKeyboardDelegate>(
std::unique_ptr<Keyboard> keyboard = keyboard_resource, data->serial_tracker),
std::make_unique<Keyboard>(delegate, data->seat); data->seat);
keyboard->AddObserver(delegate);
SetImplementation(keyboard_resource, &keyboard_implementation, SetImplementation(keyboard_resource, &keyboard_implementation,
std::move(keyboard)); std::move(keyboard));
#else #else
NOTIMPLEMENTED(); NOTIMPLEMENTED();
#endif // BUILDFLAG(USE_XKBCOMMON) #endif // defined(OS_CHROMEOS) && BUILDFLAG(USE_XKBCOMMON)
#else
NOTIMPLEMENTED();
#endif // defined(OS_CHROMEOS)
} }
void seat_get_touch(wl_client* client, wl_resource* resource, uint32_t id) { void seat_get_touch(wl_client* client, wl_resource* resource, uint32_t id) {
......
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