Add EventTargeterDelegate for EventTargeter in mus-ws.

Add EventTargeterDelegate for EventTargeter in mus-ws.

Right now EventTargeter talks to EventDispatcherDelegate directly but it
should not have any knowledge of EventDispatcherDelegate. Adding an
EventTargeterDelegate to route calls to EventDispatcher and then
EventDispatcherDelegate.

BUG=none, related to https://codereview.chromium.org/2905333002/
TEST=covered by tests

Review-Url: https://codereview.chromium.org/2911293002
Cr-Commit-Position: refs/heads/master@{#476483}
Committed: https://chromium.googlesource.com/chromium/src/+/6c0fedf2096b222b201b0073a34a179ffca66ba6

[riajiang, 2017-05-30 18:24:24]

The CQ bit was checked by riajiang@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-30 18:24:50]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[riajiang, 2017-05-30 18:26:35]

riajiang@chromium.org changed reviewers:
+ sky@chromium.org

[riajiang, 2017-05-30 18:26:36]

Hi, PTAL, thanks!

[commit-bot: I haz the power, 2017-05-30 20:30:31]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-30 20:30:32]

Dry run: This issue passed the CQ dry run.

[sky, 2017-05-30 22:50:29]

Let me know if you still think it's worth going down this line given my
most recent suggestion for how to incorporate async handling.

On Tue, May 30, 2017 at 11:26 AM, <riajiang@chromium.org> wrote:

> Reviewers: sky
> CL: https://codereview.chromium.org/2911293002/
>
> Message:
> Hi, PTAL, thanks!
>
> Description:
> Add EventTargeterDelegate for EventTargeter in mus-ws.
>
> Right now EventTargeter talks to EventDispatcherDelegate directly but it
> should not have any knowledge of EventDispatcherDelegate. Adding an
> EventTargeterDelegate to route calls to EventDispatcher and then
> EventDispatcherDelegate.
>
> BUG=none, related to https://codereview.chromium.org/2905333002/
> TEST=covered by tests
>
> Affected files (+56, -10 lines):
> M services/ui/ws/BUILD.gn
> M services/ui/ws/event_dispatcher.h
> M services/ui/ws/event_dispatcher.cc
> M services/ui/ws/event_targeter.h
> M services/ui/ws/event_targeter.cc
> A services/ui/ws/event_targeter_delegate.h
>
>
> Index: services/ui/ws/BUILD.gn
> diff --git a/services/ui/ws/BUILD.gn b/services/ui/ws/BUILD.gn
> index 0f93d30b297a033a53eb2e03d156ba02848a60bc..
> 46e503c56c8c22494b2b8fdca63604ea59f9ea60 100644
> --- a/services/ui/ws/BUILD.gn
> +++ b/services/ui/ws/BUILD.gn
> @@ -48,6 +48,7 @@ static_library("lib") {
> "event_matcher.h",
> "event_targeter.cc",
> "event_targeter.h",
> + "event_targeter_delegate.h",
> "focus_controller.cc",
> "focus_controller.h",
> "focus_controller_delegate.h",
> Index: services/ui/ws/event_dispatcher.cc
> diff --git a/services/ui/ws/event_dispatcher.cc b/services/ui/ws/event_
> dispatcher.cc
> index c6e6897508347096389172210d6d3e9b91594479..
> 56342211ce61b49ae3213181842baedea51d6cfa 100644
> --- a/services/ui/ws/event_dispatcher.cc
> +++ b/services/ui/ws/event_dispatcher.cc
> @@ -47,8 +47,7 @@ EventDispatcher::EventDispatcher(EventDispatcherDelegate*
> delegate)
> capture_window_client_id_(kInvalidClientId),
> modal_window_controller_(this),
> event_targeter_(
> - base::MakeUnique<EventTargeter>(delegate_,
> - &modal_window_controller_)),
> + base::MakeUnique<EventTargeter>(this, &modal_window_controller_)),
> mouse_button_down_(false),
> mouse_cursor_source_window_(nullptr),
> mouse_cursor_in_non_client_area_(false) {}
> @@ -317,6 +316,12 @@ void EventDispatcher::ProcessEvent(const ui::Event&
> event,
> return;
> }
>
> +ServerWindow* EventDispatcher::GetRootWindowContaining(
> + gfx::Point* location_in_display,
> + int64_t* display_id) {
> + return delegate_->GetRootWindowContaining(location_in_display,
> display_id);
> +}
> +
> void EventDispatcher::SetMouseCursorSourceWindow(ServerWindow* window) {
> if (mouse_cursor_source_window_ == window)
> return;
> Index: services/ui/ws/event_dispatcher.h
> diff --git a/services/ui/ws/event_dispatcher.h b/services/ui/ws/event_
> dispatcher.h
> index 193786bd6ee5d119c42684623986a3ce9c612e94..
> 1da07968bb4ba5868c01f88531e4509debdce0fd 100644
> --- a/services/ui/ws/event_dispatcher.h
> +++ b/services/ui/ws/event_dispatcher.h
> @@ -17,6 +17,7 @@
> #include "services/ui/public/interfaces/window_manager.mojom.h"
> #include "services/ui/ws/drag_cursor_updater.h"
> #include "services/ui/ws/event_targeter.h"
> +#include "services/ui/ws/event_targeter_delegate.h"
> #include "services/ui/ws/modal_window_controller.h"
> #include "services/ui/ws/server_window_observer.h"
> #include "ui/gfx/geometry/rect_f.h"
> @@ -41,7 +42,9 @@ class EventDispatcherTestApi;
> }
>
> // Handles dispatching events to the right location as well as updating
> focus.
> -class EventDispatcher : public ServerWindowObserver, public
> DragCursorUpdater {
> +class EventDispatcher : public ServerWindowObserver,
> + public DragCursorUpdater,
> + public EventTargeterDelegate {
> public:
> enum class AcceleratorMatchPhase {
> // Both pre and post should be considered.
> @@ -148,6 +151,10 @@ class EventDispatcher : public ServerWindowObserver,
> public DragCursorUpdater {
> const int64_t display_id,
> AcceleratorMatchPhase match_phase);
>
> + // EventTargeterDelegate:
> + ServerWindow* GetRootWindowContaining(gfx::Point* location_in_display,
> + int64_t* display_id) override;
> +
> private:
> friend class test::EventDispatcherTestApi;
>
> Index: services/ui/ws/event_targeter.cc
> diff --git a/services/ui/ws/event_targeter.cc b/services/ui/ws/event_
> targeter.cc
> index 9329b4b536b4726e948574e44ecb88e1df6513b3..
> 7e1f3a4fc4c25374f83a066473075a3336698ac8 100644
> --- a/services/ui/ws/event_targeter.cc
> +++ b/services/ui/ws/event_targeter.cc
> @@ -4,7 +4,7 @@
>
> #include "services/ui/ws/event_targeter.h"
>
> -#include "services/ui/ws/event_dispatcher_delegate.h"
> +#include "services/ui/ws/event_targeter_delegate.h"
> #include "services/ui/ws/modal_window_controller.h"
> #include "services/ui/ws/window_finder.h"
> #include "ui/events/event.h"
> @@ -13,9 +13,9 @@
> namespace ui {
> namespace ws {
>
> -EventTargeter::EventTargeter(EventDispatcherDelegate*
> event_dispatcher_delegate,
> +EventTargeter::EventTargeter(EventTargeterDelegate*
> event_targeter_delegate,
> ModalWindowController* modal_window_controller)
> - : event_dispatcher_delegate_(event_dispatcher_delegate),
> + : event_targeter_delegate_(event_targeter_delegate),
> modal_window_controller_(modal_window_controller) {}
>
> EventTargeter::~EventTargeter() {}
> @@ -41,7 +41,7 @@ DeepestWindow EventTargeter::
> FindDeepestVisibleWindowForEvents(
> gfx::Point* location,
> int64_t* display_id) {
> ServerWindow* root =
> - event_dispatcher_delegate_->GetRootWindowContaining(location,
> display_id);
> + event_targeter_delegate_->GetRootWindowContaining(location, display_id);
> return root ? ui::ws::FindDeepestVisibleWindowForEvents(root, *location)
> : DeepestWindow();
> }
> Index: services/ui/ws/event_targeter.h
> diff --git a/services/ui/ws/event_targeter.h b/services/ui/ws/event_
> targeter.h
> index a59aba2b5b3d3e6c5b5ceb26167a3de0109e2421..
> 91dfaf39626e5cf1e36594e1a3e47278c0cc5921 100644
> --- a/services/ui/ws/event_targeter.h
> +++ b/services/ui/ws/event_targeter.h
> @@ -18,7 +18,7 @@ class LocatedEvent;
>
> namespace ws {
> struct DeepestWindow;
> -class EventDispatcherDelegate;
> +class EventTargeterDelegate;
> class ModalWindowController;
> class ServerWindow;
>
> @@ -46,7 +46,7 @@ struct PointerTarget {
> // Finds the PointerTarget for an event or the DeepestWindow for a
> location.
> class EventTargeter {
> public:
> - EventTargeter(EventDispatcherDelegate* event_dispatcher_delegate,
> + EventTargeter(EventTargeterDelegate* event_targeter_delegate,
> ModalWindowController* modal_window_controller);
> ~EventTargeter();
>
> @@ -62,7 +62,7 @@ class EventTargeter {
> int64_t* display_id);
>
> private:
> - EventDispatcherDelegate* event_dispatcher_delegate_;
> + EventTargeterDelegate* event_targeter_delegate_;
> ModalWindowController* modal_window_controller_;
>
> DISALLOW_COPY_AND_ASSIGN(EventTargeter);
> Index: services/ui/ws/event_targeter_delegate.h
> diff --git a/services/ui/ws/event_targeter_delegate.h
> b/services/ui/ws/event_targeter_delegate.h
> new file mode 100644
> index 0000000000000000000000000000000000000000..
> 2e46860090028b3f5a8e6ee0645e716bc256406a
> --- /dev/null
> +++ b/services/ui/ws/event_targeter_delegate.h
> @@ -0,0 +1,33 @@
> +// Copyright 2017 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 SERVICES_UI_WS_EVENT_TARGETER_DELEGATE_H_
> +#define SERVICES_UI_WS_EVENT_TARGETER_DELEGATE_H_
> +
> +#include <stdint.h>
> +
> +namespace gfx {
> +class Point;
> +}
> +
> +namespace ui {
> +namespace ws {
> +class ServerWindow;
> +
> +// Used by EventTargeter to talk to WindowManagerState.
> +class EventTargeterDelegate {
> + public:
> + // Calls EventDispatcherDelegate::GetRootWindowContaining, see
> + // event_dispatcher_delegate.h for details.
> + virtual ServerWindow* GetRootWindowContaining(gfx::Point*
> location_in_display,
> + int64_t* display_id) = 0;
> +
> + protected:
> + virtual ~EventTargeterDelegate() {}
> +};
> +
> +} // namespace ws
> +} // namespace ui
> +
> +#endif // SERVICES_UI_WS_EVENT_TARGETER_DELEGATE_H_
>
>
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[riajiang, 2017-06-01 18:26:17]

ping :) since EventTargeter will continue to handle async and need
to call WindowManagerState::GetRootWindowContaining

On 2017/05/30 22:50:29, sky wrote:
> Let me know if you still think it's worth going down this line given my
> most recent suggestion for how to incorporate async handling.
> 
> On Tue, May 30, 2017 at 11:26 AM, <mailto:riajiang@chromium.org> wrote:
> 
> > Reviewers: sky
> > CL: https://codereview.chromium.org/2911293002/
> >
> > Message:
> > Hi, PTAL, thanks!
> >
> > Description:
> > Add EventTargeterDelegate for EventTargeter in mus-ws.
> >
> > Right now EventTargeter talks to EventDispatcherDelegate directly but it
> > should not have any knowledge of EventDispatcherDelegate. Adding an
> > EventTargeterDelegate to route calls to EventDispatcher and then
> > EventDispatcherDelegate.
> >
> > BUG=none, related to https://codereview.chromium.org/2905333002/
> > TEST=covered by tests
> >
> > Affected files (+56, -10 lines):
> > M services/ui/ws/BUILD.gn
> > M services/ui/ws/event_dispatcher.h
> > M services/ui/ws/event_dispatcher.cc
> > M services/ui/ws/event_targeter.h
> > M services/ui/ws/event_targeter.cc
> > A services/ui/ws/event_targeter_delegate.h
> >
> >
> > Index: services/ui/ws/BUILD.gn
> > diff --git a/services/ui/ws/BUILD.gn b/services/ui/ws/BUILD.gn
> > index 0f93d30b297a033a53eb2e03d156ba02848a60bc..
> > 46e503c56c8c22494b2b8fdca63604ea59f9ea60 100644
> > --- a/services/ui/ws/BUILD.gn
> > +++ b/services/ui/ws/BUILD.gn
> > @@ -48,6 +48,7 @@ static_library("lib") {
> > "event_matcher.h",
> > "event_targeter.cc",
> > "event_targeter.h",
> > + "event_targeter_delegate.h",
> > "focus_controller.cc",
> > "focus_controller.h",
> > "focus_controller_delegate.h",
> > Index: services/ui/ws/event_dispatcher.cc
> > diff --git a/services/ui/ws/event_dispatcher.cc b/services/ui/ws/event_
> > dispatcher.cc
> > index c6e6897508347096389172210d6d3e9b91594479..
> > 56342211ce61b49ae3213181842baedea51d6cfa 100644
> > --- a/services/ui/ws/event_dispatcher.cc
> > +++ b/services/ui/ws/event_dispatcher.cc
> > @@ -47,8 +47,7 @@ EventDispatcher::EventDispatcher(EventDispatcherDelegate*
> > delegate)
> > capture_window_client_id_(kInvalidClientId),
> > modal_window_controller_(this),
> > event_targeter_(
> > - base::MakeUnique<EventTargeter>(delegate_,
> > - &modal_window_controller_)),
> > + base::MakeUnique<EventTargeter>(this, &modal_window_controller_)),
> > mouse_button_down_(false),
> > mouse_cursor_source_window_(nullptr),
> > mouse_cursor_in_non_client_area_(false) {}
> > @@ -317,6 +316,12 @@ void EventDispatcher::ProcessEvent(const ui::Event&
> > event,
> > return;
> > }
> >
> > +ServerWindow* EventDispatcher::GetRootWindowContaining(
> > + gfx::Point* location_in_display,
> > + int64_t* display_id) {
> > + return delegate_->GetRootWindowContaining(location_in_display,
> > display_id);
> > +}
> > +
> > void EventDispatcher::SetMouseCursorSourceWindow(ServerWindow* window) {
> > if (mouse_cursor_source_window_ == window)
> > return;
> > Index: services/ui/ws/event_dispatcher.h
> > diff --git a/services/ui/ws/event_dispatcher.h b/services/ui/ws/event_
> > dispatcher.h
> > index 193786bd6ee5d119c42684623986a3ce9c612e94..
> > 1da07968bb4ba5868c01f88531e4509debdce0fd 100644
> > --- a/services/ui/ws/event_dispatcher.h
> > +++ b/services/ui/ws/event_dispatcher.h
> > @@ -17,6 +17,7 @@
> > #include "services/ui/public/interfaces/window_manager.mojom.h"
> > #include "services/ui/ws/drag_cursor_updater.h"
> > #include "services/ui/ws/event_targeter.h"
> > +#include "services/ui/ws/event_targeter_delegate.h"
> > #include "services/ui/ws/modal_window_controller.h"
> > #include "services/ui/ws/server_window_observer.h"
> > #include "ui/gfx/geometry/rect_f.h"
> > @@ -41,7 +42,9 @@ class EventDispatcherTestApi;
> > }
> >
> > // Handles dispatching events to the right location as well as updating
> > focus.
> > -class EventDispatcher : public ServerWindowObserver, public
> > DragCursorUpdater {
> > +class EventDispatcher : public ServerWindowObserver,
> > + public DragCursorUpdater,
> > + public EventTargeterDelegate {
> > public:
> > enum class AcceleratorMatchPhase {
> > // Both pre and post should be considered.
> > @@ -148,6 +151,10 @@ class EventDispatcher : public ServerWindowObserver,
> > public DragCursorUpdater {
> > const int64_t display_id,
> > AcceleratorMatchPhase match_phase);
> >
> > + // EventTargeterDelegate:
> > + ServerWindow* GetRootWindowContaining(gfx::Point* location_in_display,
> > + int64_t* display_id) override;
> > +
> > private:
> > friend class test::EventDispatcherTestApi;
> >
> > Index: services/ui/ws/event_targeter.cc
> > diff --git a/services/ui/ws/event_targeter.cc b/services/ui/ws/event_
> > targeter.cc
> > index 9329b4b536b4726e948574e44ecb88e1df6513b3..
> > 7e1f3a4fc4c25374f83a066473075a3336698ac8 100644
> > --- a/services/ui/ws/event_targeter.cc
> > +++ b/services/ui/ws/event_targeter.cc
> > @@ -4,7 +4,7 @@
> >
> > #include "services/ui/ws/event_targeter.h"
> >
> > -#include "services/ui/ws/event_dispatcher_delegate.h"
> > +#include "services/ui/ws/event_targeter_delegate.h"
> > #include "services/ui/ws/modal_window_controller.h"
> > #include "services/ui/ws/window_finder.h"
> > #include "ui/events/event.h"
> > @@ -13,9 +13,9 @@
> > namespace ui {
> > namespace ws {
> >
> > -EventTargeter::EventTargeter(EventDispatcherDelegate*
> > event_dispatcher_delegate,
> > +EventTargeter::EventTargeter(EventTargeterDelegate*
> > event_targeter_delegate,
> > ModalWindowController* modal_window_controller)
> > - : event_dispatcher_delegate_(event_dispatcher_delegate),
> > + : event_targeter_delegate_(event_targeter_delegate),
> > modal_window_controller_(modal_window_controller) {}
> >
> > EventTargeter::~EventTargeter() {}
> > @@ -41,7 +41,7 @@ DeepestWindow EventTargeter::
> > FindDeepestVisibleWindowForEvents(
> > gfx::Point* location,
> > int64_t* display_id) {
> > ServerWindow* root =
> > - event_dispatcher_delegate_->GetRootWindowContaining(location,
> > display_id);
> > + event_targeter_delegate_->GetRootWindowContaining(location, display_id);
> > return root ? ui::ws::FindDeepestVisibleWindowForEvents(root, *location)
> > : DeepestWindow();
> > }
> > Index: services/ui/ws/event_targeter.h
> > diff --git a/services/ui/ws/event_targeter.h b/services/ui/ws/event_
> > targeter.h
> > index a59aba2b5b3d3e6c5b5ceb26167a3de0109e2421..
> > 91dfaf39626e5cf1e36594e1a3e47278c0cc5921 100644
> > --- a/services/ui/ws/event_targeter.h
> > +++ b/services/ui/ws/event_targeter.h
> > @@ -18,7 +18,7 @@ class LocatedEvent;
> >
> > namespace ws {
> > struct DeepestWindow;
> > -class EventDispatcherDelegate;
> > +class EventTargeterDelegate;
> > class ModalWindowController;
> > class ServerWindow;
> >
> > @@ -46,7 +46,7 @@ struct PointerTarget {
> > // Finds the PointerTarget for an event or the DeepestWindow for a
> > location.
> > class EventTargeter {
> > public:
> > - EventTargeter(EventDispatcherDelegate* event_dispatcher_delegate,
> > + EventTargeter(EventTargeterDelegate* event_targeter_delegate,
> > ModalWindowController* modal_window_controller);
> > ~EventTargeter();
> >
> > @@ -62,7 +62,7 @@ class EventTargeter {
> > int64_t* display_id);
> >
> > private:
> > - EventDispatcherDelegate* event_dispatcher_delegate_;
> > + EventTargeterDelegate* event_targeter_delegate_;
> > ModalWindowController* modal_window_controller_;
> >
> > DISALLOW_COPY_AND_ASSIGN(EventTargeter);
> > Index: services/ui/ws/event_targeter_delegate.h
> > diff --git a/services/ui/ws/event_targeter_delegate.h
> > b/services/ui/ws/event_targeter_delegate.h
> > new file mode 100644
> > index 0000000000000000000000000000000000000000..
> > 2e46860090028b3f5a8e6ee0645e716bc256406a
> > --- /dev/null
> > +++ b/services/ui/ws/event_targeter_delegate.h
> > @@ -0,0 +1,33 @@
> > +// Copyright 2017 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 SERVICES_UI_WS_EVENT_TARGETER_DELEGATE_H_
> > +#define SERVICES_UI_WS_EVENT_TARGETER_DELEGATE_H_
> > +
> > +#include <stdint.h>
> > +
> > +namespace gfx {
> > +class Point;
> > +}
> > +
> > +namespace ui {
> > +namespace ws {
> > +class ServerWindow;
> > +
> > +// Used by EventTargeter to talk to WindowManagerState.
> > +class EventTargeterDelegate {
> > + public:
> > + // Calls EventDispatcherDelegate::GetRootWindowContaining, see
> > + // event_dispatcher_delegate.h for details.
> > + virtual ServerWindow* GetRootWindowContaining(gfx::Point*
> > location_in_display,
> > + int64_t* display_id) = 0;
> > +
> > + protected:
> > + virtual ~EventTargeterDelegate() {}
> > +};
> > +
> > +} // namespace ws
> > +} // namespace ui
> > +
> > +#endif // SERVICES_UI_WS_EVENT_TARGETER_DELEGATE_H_
> >
> >
> >
> 
> -- 
> You received this message because you are subscribed to the Google Groups
> "Chromium-reviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

[sky, 2017-06-01 21:14:25]

LGTM

[riajiang, 2017-06-01 21:37:30]

The CQ bit was checked by riajiang@chromium.org

[commit-bot: I haz the power, 2017-06-01 21:38:21]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-01 23:35:36]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1496353050735260, "parent_rev":
"8ea647a2d24bd0cff32ac9c57cd4edba69eff66e", "commit_rev":
"6c0fedf2096b222b201b0073a34a179ffca66ba6"}

[commit-bot: I haz the power, 2017-06-01 23:35:44]

Description was changed from

==========
Add EventTargeterDelegate for EventTargeter in mus-ws.

Right now EventTargeter talks to EventDispatcherDelegate directly but it
should not have any knowledge of EventDispatcherDelegate. Adding an
EventTargeterDelegate to route calls to EventDispatcher and then
EventDispatcherDelegate.

BUG=none, related to https://codereview.chromium.org/2905333002/
TEST=covered by tests
==========

to

==========
Add EventTargeterDelegate for EventTargeter in mus-ws.

Right now EventTargeter talks to EventDispatcherDelegate directly but it
should not have any knowledge of EventDispatcherDelegate. Adding an
EventTargeterDelegate to route calls to EventDispatcher and then
EventDispatcherDelegate.

BUG=none, related to https://codereview.chromium.org/2905333002/
TEST=covered by tests

Review-Url: https://codereview.chromium.org/2911293002
Cr-Commit-Position: refs/heads/master@{#476483}
Committed:
https://chromium.googlesource.com/chromium/src/+/6c0fedf2096b222b201b0073a34a...
==========

[commit-bot: I haz the power, 2017-06-01 23:35:45]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/6c0fedf2096b222b201b0073a34a...