events: Inlcude the event-phase and event-result in Event.

events: Inlcude the event-phase and event-result in Event.

An event-handler can be installed on a target as both a pre-target and a post-target handler.
In such cases, it will be necessary to know the phase of the event-dispatch. So add EventPhase.
Also, from a handler, it may be necessary to know whether the event has been handled by any other
handlers before this. So include EventResult in the event too.

BUG=none.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=155825

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=155957

[Ben Goodger (Google), 2012-09-10 18:07:14]

http://codereview.chromium.org/10916210/diff/7001/ui/base/events/event.h
File ui/base/events/event.h (right):

http://codereview.chromium.org/10916210/diff/7001/ui/base/events/event.h#newc...
ui/base/events/event.h:30: explicit DispatcherApi(Event* event) : event_(event)
{}
should this be private and friend class EventDispatcher?

http://codereview.chromium.org/10916210/diff/7001/ui/base/events/event_consta...
File ui/base/events/event_constants.h (right):

http://codereview.chromium.org/10916210/diff/7001/ui/base/events/event_consta...
ui/base/events/event_constants.h:114: EP_COMPLETE
i'd tend to reset to EP_NONE after dispatch.

if you believe there's a benefit to knowing pre- and post- dispatch, then i'd
change this and EP_NONE to

EP_PREDISPATCH & EP_POSTDISPATCH
or
EP_UNDISPATCHED & EP_DISPATCHED

http://codereview.chromium.org/10916210/diff/7001/ui/base/events/event_dispat...
File ui/base/events/event_dispatcher.h (right):

http://codereview.chromium.org/10916210/diff/7001/ui/base/events/event_dispat...
ui/base/events/event_dispatcher.h:36: Event::DispatcherApi dispatcher(event);
this variable name is short, but I find it a little confusing, since it's not
really a dispatcher.

http://codereview.chromium.org/10916210/diff/7001/ui/base/events/event_dispat...
ui/base/events/event_dispatcher.h:45: dispatcher.set_phase(EP_COMPLETE);
it would be nice to find a way to do this with a scoping object for safety.

http://codereview.chromium.org/10916210/diff/7001/ui/base/events/event_target.cc
File ui/base/events/event_target.cc (right):

http://codereview.chromium.org/10916210/diff/7001/ui/base/events/event_target...
ui/base/events/event_target.cc:40: EventTarget* target = this;
I don't think you meant for this to sneak into this CL.

[sadrul, 2012-09-10 18:35:24]

http://codereview.chromium.org/10916210/diff/7001/ui/base/events/event.h
File ui/base/events/event.h (right):

http://codereview.chromium.org/10916210/diff/7001/ui/base/events/event.h#newc...
ui/base/events/event.h:30: explicit DispatcherApi(Event* event) : event_(event)
{}
On 2012/09/10 18:07:14, Ben Goodger (Google) wrote:
> should this be private and friend class EventDispatcher?

RootWindow needs to also set_target for touch-events, unfortunately. So we
introduced this DispatcherApi so that both ED and RW can use this. I am hoping
the name 'DispatcherApi' would be sufficient for this to not be misused from
other places (like TestApi below)?

http://codereview.chromium.org/10916210/diff/7001/ui/base/events/event_consta...
File ui/base/events/event_constants.h (right):

http://codereview.chromium.org/10916210/diff/7001/ui/base/events/event_consta...
ui/base/events/event_constants.h:114: EP_COMPLETE
On 2012/09/10 18:07:14, Ben Goodger (Google) wrote:
> i'd tend to reset to EP_NONE after dispatch.
> 
> if you believe there's a benefit to knowing pre- and post- dispatch, then i'd
> change this and EP_NONE to
> 
> EP_PREDISPATCH & EP_POSTDISPATCH
> or
> EP_UNDISPATCHED & EP_DISPATCHED

I renamed NONE,COMPLETE to PREDISPATCH/POSTDISPATCH (I don't have a use-case in
mind for separating the two at the moment, but I feel like this might be useful
at some point, and it would probably be easier to merge the two values later if
they are unneeded than to separate them at a later time should the need arise).

http://codereview.chromium.org/10916210/diff/7001/ui/base/events/event_dispat...
File ui/base/events/event_dispatcher.h (right):

http://codereview.chromium.org/10916210/diff/7001/ui/base/events/event_dispat...
ui/base/events/event_dispatcher.h:36: Event::DispatcherApi dispatcher(event);
On 2012/09/10 18:07:14, Ben Goodger (Google) wrote:
> this variable name is short, but I find it a little confusing, since it's not
> really a dispatcher.

Renamed to dispatch_helper

http://codereview.chromium.org/10916210/diff/7001/ui/base/events/event_dispat...
ui/base/events/event_dispatcher.h:45: dispatcher.set_phase(EP_COMPLETE);
On 2012/09/10 18:07:14, Ben Goodger (Google) wrote:
> it would be nice to find a way to do this with a scoping object for safety.

Done.

http://codereview.chromium.org/10916210/diff/7001/ui/base/events/event_target.cc
File ui/base/events/event_target.cc (right):

http://codereview.chromium.org/10916210/diff/7001/ui/base/events/event_target...
ui/base/events/event_target.cc:40: EventTarget* target = this;
On 2012/09/10 18:07:14, Ben Goodger (Google) wrote:
> I don't think you meant for this to sneak into this CL.

Whoops. Removed.

[Ben Goodger (Google), 2012-09-10 19:23:30]

LGTM