Create MouseEventManager and EventHandlingUtil

The first step of separating mouse code from EventHandler.

There will be follow up CLs to refactor more code and
address the TODOs added in this CL.

Creating MouseEventManager to encapsule mouse related
bookkeeping and dispatching.

Creating abstract BoundaryEventDispatcher to keep the
boundary node elements calculation so PEM and MEM can
make subclasses from it and send their own boundary events.

Creating EventHandlingUtil to share the common functions
used in different EventHandling path instead of putting
them all as static in EventHandler.

BUG=625843
Committed: https://crrev.com/2c6e050f06efedae49f77dd91baa016dbd2e8fe9
Cr-Commit-Position: refs/heads/master@{#416984}

[Navid Zolghadr, 2016-08-19 18:09:59]

nzolghadr@chromium.org changed reviewers:
+ dtapuska@chromium.org, mustaq@chromium.org

[Navid Zolghadr, 2016-08-19 18:09:59]

There is a few more things I wanted to do for this CL.
But I had a quick question regarding a particular change I have in this CL. That
is changing sendBoundaryEvents. Right now this function lives in
PointerEventManager and sends both mouse and pointer events in a separate call.
The boolean parameter decides which path to run.
I was trying to delegate the mouse one to MouseEventManager and also wanted to
share the code between these two classes. So I created a helper class to
generate those events in each class
(EventHandlingUtil::BoundaryEventDispatcher).
Dave, I guess using template functions would change this as much. I still need
to have some behavior particular to each class. Right now I added that behavior
through another hierarchy instead of creating a parent class to
MouseEventManager and PointerEventManager. I thought creating the parent class
just to take care of boundary events might not be as good as what I have here.

Please have a look and let me know if you have any comments or if you don't
agree with what I said.

[mustaq, 2016-08-23 15:20:48]

EventHandlingUtil::BoundaryEventDispatcher looks a good separation of boundary
event specific tasks, thanks.

I think a templated version of the class is better if it lets us avoid creating
a BoundaryEventDispatcher instance on every dispatch. Making PointerEventManager
a subclass of BoundaryEventDispatcher<PointerEvent> should be even better &
cleaner, wdyt?

https://codereview.chromium.org/2255323004/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/input/EventHandlingUtil.cpp (right):

https://codereview.chromium.org/2255323004/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/input/EventHandlingUtil.cpp:112: void
sendBoundaryEvents(
Make it a member function in BoundaryEventDispatcher class?

Consider the same for all the util methods in the anonymous namespace.

https://codereview.chromium.org/2255323004/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/input/PointerEventManager.cpp (right):

https://codereview.chromium.org/2255323004/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/input/PointerEventManager.cpp:172:
PointerEventBoundaryEventDispatcher boundaryEventDispatcher(this, pointerEvent);
Creating a new instance of boundaryEventDispatcher on every dispatch seems a bit
overkill. Move the pointer/mouse event parameter from constructors to dispatcher
methods? The number of parameters is not too many to be a concern here.

May be a templated BoundaryEventDispatcher is better?

[Navid Zolghadr, 2016-08-24 16:07:28]

https://codereview.chromium.org/2255323004/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/input/EventHandlingUtil.cpp (right):

https://codereview.chromium.org/2255323004/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/input/EventHandlingUtil.cpp:112: void
sendBoundaryEvents(
On 2016/08/23 15:20:48, mustaq wrote:
> Make it a member function in BoundaryEventDispatcher class?
> 
> Consider the same for all the util methods in the anonymous namespace.


You mean the methods in anonymous name space in this file? Why would we expose
those in a header file while we don't need them to be? Having them in an
anonymous name space only in this file is less exposed. Isn't it?

https://codereview.chromium.org/2255323004/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/input/PointerEventManager.cpp (right):

https://codereview.chromium.org/2255323004/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/input/PointerEventManager.cpp:172:
PointerEventBoundaryEventDispatcher boundaryEventDispatcher(this, pointerEvent);
On 2016/08/23 15:20:48, mustaq wrote:
> Creating a new instance of boundaryEventDispatcher on every dispatch seems a
bit
> overkill. Move the pointer/mouse event parameter from constructors to
dispatcher
> methods? The number of parameters is not too many to be a concern here.
> 
> May be a templated BoundaryEventDispatcher is better?

I don't know how we should do this with a templated version. Note that if I add
tempate to the BoundaryEventDispatcher that will be a new class. So I cannot
expect any particular class in the function parameter.

I didn't quite understand the comment about how to solve the "creating new
instance every time" problem.

I'm going to make the PointerEventManager and MouseEventManager subclasses of
BoundaryEventDispatcher and hopefully that makes the code cleaner overall. I'll
post a patch for it.

[mustaq, 2016-08-26 19:31:45]

https://codereview.chromium.org/2255323004/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/input/EventHandlingUtil.cpp (right):

https://codereview.chromium.org/2255323004/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/input/EventHandlingUtil.cpp:112: void
sendBoundaryEvents(
On 2016/08/24 16:07:28, Navid Zolghadr wrote:
> On 2016/08/23 15:20:48, mustaq wrote:
> > Make it a member function in BoundaryEventDispatcher class?
> > 
> > Consider the same for all the util methods in the anonymous namespace.
> 
> 
> You mean the methods in anonymous name space in this file? Why would we expose
> those in a header file while we don't need them to be? Having them in an
> anonymous name space only in this file is less exposed. Isn't it?

- The methods in the anonymous namespace have no use outside the
BoundaryEventDispatcher class, so |private| in the class seems better to me.

- All calls to this sendBoundaryEvents() are made right after the dispatcher
object is created. It suggests that these two steps should appear atomic from
the caller's side. Another possibility is to make the dispatcher class private
here, and have two exposed sendBoundaryEvents for ME & PE. Just a food for
thought, I will leave it on your judgement.

[Navid Zolghadr, 2016-08-30 18:24:35]

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

[commit-bot: I haz the power, 2016-08-30 18:25:15]

Dry run: 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, 2016-08-30 18:28:53]

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

[commit-bot: I haz the power, 2016-08-30 18:28:54]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Navid Zolghadr, 2016-08-30 18:36:50]

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

[commit-bot: I haz the power, 2016-08-30 18:37:32]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Navid Zolghadr, 2016-08-30 18:46:26]

ptal.

https://codereview.chromium.org/2255323004/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/input/EventHandlingUtil.cpp (right):

https://codereview.chromium.org/2255323004/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/input/EventHandlingUtil.cpp:112: void
sendBoundaryEvents(
On 2016/08/26 19:31:44, mustaq wrote:
> On 2016/08/24 16:07:28, Navid Zolghadr wrote:
> > On 2016/08/23 15:20:48, mustaq wrote:
> > > Make it a member function in BoundaryEventDispatcher class?
> > > 
> > > Consider the same for all the util methods in the anonymous namespace.
> > 
> > 
> > You mean the methods in anonymous name space in this file? Why would we
expose
> > those in a header file while we don't need them to be? Having them in an
> > anonymous name space only in this file is less exposed. Isn't it?
> 
> - The methods in the anonymous namespace have no use outside the
> BoundaryEventDispatcher class, so |private| in the class seems better to me.
> 

I moved the BoundaryEventDispatcher class to another file so that these methods
are only visible to that file.

> - All calls to this sendBoundaryEvents() are made right after the dispatcher
> object is created. It suggests that these two steps should appear atomic from
> the caller's side. Another possibility is to make the dispatcher class private
> here, and have two exposed sendBoundaryEvents for ME & PE. Just a food for
> thought, I will leave it on your judgement.

The two functions that send Boundary event for ME and PE are essentially
PointerEventManager::sendBoundaryEvents and
MouseEventManager::sendBoundaryEvents and I use these everywhere else in these
files.

https://codereview.chromium.org/2255323004/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/input/PointerEventManager.cpp (right):

https://codereview.chromium.org/2255323004/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/input/PointerEventManager.cpp:172:
PointerEventBoundaryEventDispatcher boundaryEventDispatcher(this, pointerEvent);
On 2016/08/24 16:07:28, Navid Zolghadr wrote:
> On 2016/08/23 15:20:48, mustaq wrote:
> > Creating a new instance of boundaryEventDispatcher on every dispatch seems a
> bit
> > overkill. Move the pointer/mouse event parameter from constructors to
> dispatcher
> > methods? The number of parameters is not too many to be a concern here.
> > 
> > May be a templated BoundaryEventDispatcher is better?
> 
> I don't know how we should do this with a templated version. Note that if I
add
> tempate to the BoundaryEventDispatcher that will be a new class. So I cannot
> expect any particular class in the function parameter.
> 
> I didn't quite understand the comment about how to solve the "creating new
> instance every time" problem.
> 
> I'm going to make the PointerEventManager and MouseEventManager subclasses of
> BoundaryEventDispatcher and hopefully that makes the code cleaner overall.
I'll
> post a patch for it.

I was looking into possibilities of creating this object in the constructor and
just keep the instance in the class. Here is what should happen in that case.
Before call to sendBoundaryEvents I should set some parameters in that object
(i.e. in this case the pointer event that is used to created the boundary events
as it is different every time). I also need to reset those parameters to null at
the end of this function to make sure there is no ref to those parameters by
mistake. So this reminds me of the pattern of RAII which needs this object
creation. I believe creating the object is cleaner but I agree that would cause
a little more memory use use every time.

[commit-bot: I haz the power, 2016-08-30 20:06:48]

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

[commit-bot: I haz the power, 2016-08-30 20:06:49]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[mustaq, 2016-08-30 21:49:23]

Yes, the boundary events class looks better.

I will ask you to check one more thing, details below: I know that the
interaction of mouse-code with the rest of the event-types makes it tricky but
is there a way to move some of the mouse-specific states to ME-manager?

https://codereview.chromium.org/2255323004/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/input/EventHandler.cpp (right):

https://codereview.chromium.org/2255323004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/input/EventHandler.cpp:1066: WebInputEventResult
EventHandler::handleMouseReleaseEvent(const PlatformMouseEvent& mouseEvent)
This the only function that /needs/ the |m_clickCount| state now, right? Can we
move the state plus some of the dispatch code to ME-manager?

https://codereview.chromium.org/2255323004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/input/EventHandler.cpp:1447: WebInputEventResult
EventHandler::dispatchMouseEvent(const AtomicString& eventType, Node*
targetNode, int clickCount, const PlatformMouseEvent& mouseEvent)
The only EventHandler state this function uses is |m_nodeUnderMouse|. Does
moving this state plus this function to ME-Manager make the code cleaner?

https://codereview.chromium.org/2255323004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/input/EventHandler.cpp:1508: WebInputEventResult
EventHandler::handleMouseFocus(const HitTestResult& hitTestResult,
InputDeviceCapabilities* sourceCapabilities)
Can we move it to ME-Manager? This applies only if the last comment above
(moving |m_nodeUnderMouse|) works.

[Navid Zolghadr, 2016-08-31 14:23:48]

There are still quite a bit of code to move out of EventHandler. However, the
very next piece I take out (including those you mentioned) will take out
everything along with itself as they will be used in other functions as well.
So I thought maybe this is a good separation point to land this CL when we are
happy with it and then I move the rest of the code in a later CL.

https://codereview.chromium.org/2255323004/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/input/EventHandler.cpp (right):

https://codereview.chromium.org/2255323004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/input/EventHandler.cpp:1066: WebInputEventResult
EventHandler::handleMouseReleaseEvent(const PlatformMouseEvent& mouseEvent)
On 2016/08/30 21:49:23, mustaq wrote:
> This the only function that /needs/ the |m_clickCount| state now, right? Can
we
> move the state plus some of the dispatch code to ME-manager?

I planned to do these in the next CL if you are okay.

https://codereview.chromium.org/2255323004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/input/EventHandler.cpp:1447: WebInputEventResult
EventHandler::dispatchMouseEvent(const AtomicString& eventType, Node*
targetNode, int clickCount, const PlatformMouseEvent& mouseEvent)
On 2016/08/30 21:49:23, mustaq wrote:
> The only EventHandler state this function uses is |m_nodeUnderMouse|. Does
> moving this state plus this function to ME-Manager make the code cleaner?

I planned to do these in the next CL if you are okay.

https://codereview.chromium.org/2255323004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/input/EventHandler.cpp:1508: WebInputEventResult
EventHandler::handleMouseFocus(const HitTestResult& hitTestResult,
InputDeviceCapabilities* sourceCapabilities)
On 2016/08/30 21:49:23, mustaq wrote:
> Can we move it to ME-Manager? This applies only if the last comment above
> (moving |m_nodeUnderMouse|) works.

I planned to do these in the next CL if you are okay.

[mustaq, 2016-08-31 14:39:37]

LGTM on the incremental plan...
- Add TODOs at least for the mouse states I mentioned.
- Mention in the CL description that this CL minimally separates out the mouse
code, as the first step in your incremental plan.
- s/Creaeting/Creating/ in description if it still applies.

[Navid Zolghadr, 2016-08-31 14:47:48]

Description was changed from

==========
Create MouseEventManager and EventHandlingUtil

Creating MouseEventManager to encapsule mouse related
bookkeeping and dispatching.
Creaeting EventHandlingUtil to share the common functions
used in different EventHandling path instead of putting
them all as static in EventHandler.

BUG=625843
==========

to

==========
The first step of separating mouse code from EventHandler.

There will be follow up CLs to refactor more code and
address the TODOs added in this CL.

Creating MouseEventManager to encapsule mouse related
bookkeeping and dispatching.

Creating abstract BoundaryEventDispatcher to keep the
boundary node elements calculation so PEM and MEM can
make subclasses from is and send their own boundary events.

Creating EventHandlingUtil to share the common functions
used in different EventHandling path instead of putting
them all as static in EventHandler.

BUG=625843
==========

[Navid Zolghadr, 2016-08-31 14:53:33]

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

[commit-bot: I haz the power, 2016-08-31 14:53:49]

Dry run: 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, 2016-08-31 14:56:27]

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

[commit-bot: I haz the power, 2016-08-31 14:56:28]

Dry run: Try jobs failed on following builders:
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Navid Zolghadr, 2016-08-31 14:59:12]

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

[commit-bot: I haz the power, 2016-08-31 14:59:30]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Navid Zolghadr, 2016-08-31 15:19:02]

nzolghadr@chromium.org changed reviewers:
+ bokan@chromium.org

[Navid Zolghadr, 2016-08-31 15:19:03]

bokan@chromium.org: Please review changes in 

Webkit/Source/core/*

[commit-bot: I haz the power, 2016-08-31 16:22:24]

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

[commit-bot: I haz the power, 2016-08-31 16:22:25]

Dry run: This issue passed the CQ dry run.

[bokan, 2016-09-01 13:50:12]

bokan@chromium.org changed reviewers:
+ haraken@chromium.org

[bokan, 2016-09-01 13:50:13]

+haraken@ for Oilpan question.

https://codereview.chromium.org/2255323004/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/html/HTMLLabelElement.cpp (right):

https://codereview.chromium.org/2255323004/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/HTMLLabelElement.cpp:175: if
(isLabelTextSelected && toMouseEvent(evt)->detail() == 1)
Why did this change?

https://codereview.chromium.org/2255323004/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/input/EventHandler.h (right):

https://codereview.chromium.org/2255323004/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/input/EventHandler.h:394: ScrollManager
m_scrollManager;
I just realized, these classes have on-heap members, and we trace these classes
even though they're not held in Member/WeakMember handles nor do they inherit
from GarbageCollected. I suspect this may work but the Oilpan reference seems to
imply that tracing should only happen on garbage collected, on-heap members:
https://chromium.googlesource.com/chromium/src/+/master/third_party/WebKit/So...

+haraken@: is it ok to be tracing non-GarbageCollected, non-handle members like
this?

https://codereview.chromium.org/2255323004/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/input/MouseEventManager.h (right):

https://codereview.chromium.org/2255323004/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/input/MouseEventManager.h:20: // properties of
active pointer events.
Is "pointer events" here related to actual PointerEvents? If not, could you
reword this to avoid potential confusion?

https://codereview.chromium.org/2255323004/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/input/MouseEventManager.h:70: PlatformMouseEvent
m_platformMouseEvent;
Is there a reason we have to copy the PlatformMouseEvent?

https://codereview.chromium.org/2255323004/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/input/PointerEventManager.h (right):

https://codereview.chromium.org/2255323004/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/input/PointerEventManager.h:126: virtual void
dispatchLeave(EventTarget*, EventTarget* relatedTarget, bool checkForListener);
Use `override` instead if `virtual`

[Navid Zolghadr, 2016-09-01 14:30:41]

https://codereview.chromium.org/2255323004/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/html/HTMLLabelElement.cpp (right):

https://codereview.chromium.org/2255323004/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/HTMLLabelElement.cpp:175: if
(isLabelTextSelected && toMouseEvent(evt)->detail() == 1)
On 2016/09/01 13:50:13, bokan wrote:
> Why did this change?

To be able to get rid of clickCount function. Basically we do set the click
count as the "detail" attribute in the event all the time. So that should be
able to replaced here.

https://codereview.chromium.org/2255323004/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/input/MouseEventManager.h (right):

https://codereview.chromium.org/2255323004/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/input/MouseEventManager.h:70: PlatformMouseEvent
m_platformMouseEvent;
On 2016/09/01 13:50:13, bokan wrote:
> Is there a reason we have to copy the PlatformMouseEvent?

I can keep a pointer to that. Do you think that is better?

[bokan, 2016-09-01 16:00:54]

Also, nit in description:

"make subclasses from is and send their own boundary events."
                       ^
                       |------it

https://codereview.chromium.org/2255323004/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/html/HTMLLabelElement.cpp (right):

https://codereview.chromium.org/2255323004/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/HTMLLabelElement.cpp:175: if
(isLabelTextSelected && toMouseEvent(evt)->detail() == 1)
On 2016/09/01 14:30:41, Navid Zolghadr wrote:
> On 2016/09/01 13:50:13, bokan wrote:
> > Why did this change?
> 
> To be able to get rid of clickCount function. Basically we do set the click
> count as the "detail" attribute in the event all the time. So that should be
> able to replaced here.

Acknowledged.

https://codereview.chromium.org/2255323004/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/input/MouseEventManager.h (right):

https://codereview.chromium.org/2255323004/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/input/MouseEventManager.h:70: PlatformMouseEvent
m_platformMouseEvent;
On 2016/09/01 14:30:41, Navid Zolghadr wrote:
> On 2016/09/01 13:50:13, bokan wrote:
> > Is there a reason we have to copy the PlatformMouseEvent?
> 
> I can keep a pointer to that. Do you think that is better?

Yah, lets avoid copying unless we need it.

[Navid Zolghadr, 2016-09-01 17:06:33]

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

[Navid Zolghadr, 2016-09-01 17:07:00]

Description was changed from

==========
The first step of separating mouse code from EventHandler.

There will be follow up CLs to refactor more code and
address the TODOs added in this CL.

Creating MouseEventManager to encapsule mouse related
bookkeeping and dispatching.

Creating abstract BoundaryEventDispatcher to keep the
boundary node elements calculation so PEM and MEM can
make subclasses from is and send their own boundary events.

Creating EventHandlingUtil to share the common functions
used in different EventHandling path instead of putting
them all as static in EventHandler.

BUG=625843
==========

to

==========
The first step of separating mouse code from EventHandler.

There will be follow up CLs to refactor more code and
address the TODOs added in this CL.

Creating MouseEventManager to encapsule mouse related
bookkeeping and dispatching.

Creating abstract BoundaryEventDispatcher to keep the
boundary node elements calculation so PEM and MEM can
make subclasses from it and send their own boundary events.

Creating EventHandlingUtil to share the common functions
used in different EventHandling path instead of putting
them all as static in EventHandler.

BUG=625843
==========

[commit-bot: I haz the power, 2016-09-01 17:07:06]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Navid Zolghadr, 2016-09-01 17:07:28]

ptal

https://codereview.chromium.org/2255323004/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/input/MouseEventManager.h (right):

https://codereview.chromium.org/2255323004/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/input/MouseEventManager.h:20: // properties of
active pointer events.
On 2016/09/01 13:50:13, bokan wrote:
> Is "pointer events" here related to actual PointerEvents? If not, could you
> reword this to avoid potential confusion?

Done.

https://codereview.chromium.org/2255323004/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/input/MouseEventManager.h:70: PlatformMouseEvent
m_platformMouseEvent;
On 2016/09/01 16:00:54, bokan wrote:
> On 2016/09/01 14:30:41, Navid Zolghadr wrote:
> > On 2016/09/01 13:50:13, bokan wrote:
> > > Is there a reason we have to copy the PlatformMouseEvent?
> > 
> > I can keep a pointer to that. Do you think that is better?
> 
> Yah, lets avoid copying unless we need it.

Done.

https://codereview.chromium.org/2255323004/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/input/PointerEventManager.h (right):

https://codereview.chromium.org/2255323004/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/input/PointerEventManager.h:126: virtual void
dispatchLeave(EventTarget*, EventTarget* relatedTarget, bool checkForListener);
On 2016/09/01 13:50:13, bokan wrote:
> Use `override` instead if `virtual`

Done.

[haraken, 2016-09-01 17:12:47]

https://codereview.chromium.org/2255323004/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/input/EventHandler.h (right):

https://codereview.chromium.org/2255323004/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/input/EventHandler.h:394: ScrollManager
m_scrollManager;
On 2016/09/01 13:50:13, bokan wrote:
> I just realized, these classes have on-heap members, and we trace these
classes
> even though they're not held in Member/WeakMember handles nor do they inherit
> from GarbageCollected. I suspect this may work but the Oilpan reference seems
to
> imply that tracing should only happen on garbage collected, on-heap members:
>
https://chromium.googlesource.com/chromium/src/+/master/third_party/WebKit/So...
> 
> +haraken@: is it ok to be tracing non-GarbageCollected, non-handle members
like
> this?

You must trace these objects because they are a-part-of on-heap object (i.e.,
objects that have DISALLOW_NEW) and have members to be traced. They are not
GarbageCollected but a part of a GarbageCollected object (i.e., EventHandler).
So they need to be traced.

[haraken, 2016-09-01 17:21:59]

https://codereview.chromium.org/2255323004/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/input/BoundaryEventDispatcher.cpp (right):

https://codereview.chromium.org/2255323004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/input/BoundaryEventDispatcher.cpp:14: bool
isInDocument(EventTarget* n)

n => target

https://codereview.chromium.org/2255323004/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/input/MouseEventManager.h (right):

https://codereview.chromium.org/2255323004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/input/MouseEventManager.h:69: MouseEventManager*
m_mouseEventManager;

I don't see any reason we want to make MouseEventManager DISALLOW_NEW. I'd
prefer making MouseEventManager GarbageCollected and use a Memeber<> here.

https://codereview.chromium.org/2255323004/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/input/PointerEventManager.h (right):

https://codereview.chromium.org/2255323004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/input/PointerEventManager.h:113: DISALLOW_NEW();

Can we move this to BoundaryEventDispatcher?

Also I think STACK_ALLOCATED would be enough since
PointerEventBoundaryEventDispatcher is always used on stack.

https://codereview.chromium.org/2255323004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/input/PointerEventManager.h:133:
PointerEventManager* m_pointerEventManager;

Ditto. I'd prefer making PointerEventManager GarbageCollected and use a Member<>
here.

[mustaq, 2016-09-01 19:21:27]

Just a nit...

https://codereview.chromium.org/2255323004/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/input/MouseEventManager.h (right):

https://codereview.chromium.org/2255323004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/input/MouseEventManager.h:20: // position and
related states mouse and its buttons.
Missed the class doc before: "...positions and states of mouse."

[Navid Zolghadr, 2016-09-01 19:31:26]

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

[Navid Zolghadr, 2016-09-01 19:31:54]

ptal.

https://codereview.chromium.org/2255323004/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/input/BoundaryEventDispatcher.cpp (right):

https://codereview.chromium.org/2255323004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/input/BoundaryEventDispatcher.cpp:14: bool
isInDocument(EventTarget* n)
On 2016/09/01 17:21:58, haraken wrote:
> 
> n => target

Done.

https://codereview.chromium.org/2255323004/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/input/MouseEventManager.h (right):

https://codereview.chromium.org/2255323004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/input/MouseEventManager.h:20: // position and
related states mouse and its buttons.
On 2016/09/01 19:21:27, mustaq wrote:
> Missed the class doc before: "...positions and states of mouse."

Done.

https://codereview.chromium.org/2255323004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/input/MouseEventManager.h:69: MouseEventManager*
m_mouseEventManager;
On 2016/09/01 17:21:58, haraken wrote:
> 
> I don't see any reason we want to make MouseEventManager DISALLOW_NEW. I'd
> prefer making MouseEventManager GarbageCollected and use a Memeber<> here.

Alright. I made all the *Manager classes GarbageCollected to be consistent.

https://codereview.chromium.org/2255323004/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/input/PointerEventManager.h (right):

https://codereview.chromium.org/2255323004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/input/PointerEventManager.h:113: DISALLOW_NEW();
On 2016/09/01 17:21:58, haraken wrote:
> 
> Can we move this to BoundaryEventDispatcher?
> 
> Also I think STACK_ALLOCATED would be enough since
> PointerEventBoundaryEventDispatcher is always used on stack.
> 

Done.

https://codereview.chromium.org/2255323004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/input/PointerEventManager.h:133:
PointerEventManager* m_pointerEventManager;
On 2016/09/01 17:21:59, haraken wrote:
> 
> Ditto. I'd prefer making PointerEventManager GarbageCollected and use a
Member<>
> here.

Done.

[commit-bot: I haz the power, 2016-09-01 19:32:13]

Dry run: 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, 2016-09-01 21:01:47]

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

[commit-bot: I haz the power, 2016-09-01 21:01:48]

Dry run: This issue passed the CQ dry run.

[haraken, 2016-09-01 23:31:16]

In terms of Oilpan, LGTM.

https://codereview.chromium.org/2255323004/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/input/GestureManager.h (right):

https://codereview.chromium.org/2255323004/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/input/GestureManager.h:23: class CORE_EXPORT
GestureManager final : public GarbageCollectedFinalized<GestureManager> {

Maybe you can drop 'Finalized' and remove the destructor.

https://codereview.chromium.org/2255323004/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/input/KeyboardEventManager.h (right):

https://codereview.chromium.org/2255323004/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/input/KeyboardEventManager.h:29: class
CORE_EXPORT KeyboardEventManager final : public
GarbageCollectedFinalized<KeyboardEventManager> {

Ditto. Drop 'Finalized'.

https://codereview.chromium.org/2255323004/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/input/MouseEventManager.h (right):

https://codereview.chromium.org/2255323004/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/input/MouseEventManager.h:21: class CORE_EXPORT
MouseEventManager final : public GarbageCollectedFinalized<MouseEventManager> {

Ditto. Drop 'Finalized'.

https://codereview.chromium.org/2255323004/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/input/PointerEventManager.h (right):

https://codereview.chromium.org/2255323004/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/input/PointerEventManager.h:114:
DEFINE_INLINE_TRACE()

We don't need to add TRACE to STACK_ALLOCATED objects.

Sorry it was not clear in the documentation. We should improve the gc plugin to
auto-detect this...

[dtapuska, 2016-09-02 03:26:27]

publishing my drafts I had been sitting on.. I know it is from a few revisions
ago

https://codereview.chromium.org/2255323004/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/html/HTMLLabelElement.cpp (right):

https://codereview.chromium.org/2255323004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/HTMLLabelElement.cpp:175: if
(isLabelTextSelected && toMouseEvent(evt)->detail() == 1)
I'd prefer if we added a method to MouseEvent that was the clickCount instead of
expecting someone to know that detail() == clickCount.

https://codereview.chromium.org/2255323004/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/input/BoundaryEventDispatcher.cpp (right):

https://codereview.chromium.org/2255323004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/input/BoundaryEventDispatcher.cpp:103: if
(exitedAncestors[j]->hasCapturingEventListeners(getLeaveEvent()))
I don't think you need to call the getLeaveEvent() for every possible node in
the path do you?

Can't think loop exit early?

https://codereview.chromium.org/2255323004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/input/BoundaryEventDispatcher.cpp:118: for
(size_t i = 0; i < enteredAncestors.size(); i++) {
Same as above.

https://codereview.chromium.org/2255323004/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/input/EventHandler.cpp (right):

https://codereview.chromium.org/2255323004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/input/EventHandler.cpp:226:
visitor->trace(m_mouseEventManager);
The trace should probably happen the order of the fields listed in the class for
clarity.

[Navid Zolghadr, 2016-09-02 16:24:13]

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

[commit-bot: I haz the power, 2016-09-02 16:24:31]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Navid Zolghadr, 2016-09-02 16:26:46]

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

[commit-bot: I haz the power, 2016-09-02 16:27:04]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Navid Zolghadr, 2016-09-02 16:36:46]

ptal.

https://codereview.chromium.org/2255323004/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/html/HTMLLabelElement.cpp (right):

https://codereview.chromium.org/2255323004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/HTMLLabelElement.cpp:175: if
(isLabelTextSelected && toMouseEvent(evt)->detail() == 1)
On 2016/09/02 03:26:27, dtapuska wrote:
> I'd prefer if we added a method to MouseEvent that was the clickCount instead
of
> expecting someone to know that detail() == clickCount.

Done.

https://codereview.chromium.org/2255323004/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/input/BoundaryEventDispatcher.cpp (right):

https://codereview.chromium.org/2255323004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/input/BoundaryEventDispatcher.cpp:103: if
(exitedAncestors[j]->hasCapturingEventListeners(getLeaveEvent()))
On 2016/09/02 03:26:27, dtapuska wrote:
> I don't think you need to call the getLeaveEvent() for every possible node in
> the path do you?
> 
> Can't think loop exit early?

Yup. That is certainly possible. Didn't pay attention when I was copy pasting
here. Done.

https://codereview.chromium.org/2255323004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/input/BoundaryEventDispatcher.cpp:118: for
(size_t i = 0; i < enteredAncestors.size(); i++) {
On 2016/09/02 03:26:27, dtapuska wrote:
> Same as above.

Done.

https://codereview.chromium.org/2255323004/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/input/EventHandler.cpp (right):

https://codereview.chromium.org/2255323004/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/input/EventHandler.cpp:226:
visitor->trace(m_mouseEventManager);
On 2016/09/02 03:26:27, dtapuska wrote:
> The trace should probably happen the order of the fields listed in the class
for
> clarity.

Done.

https://codereview.chromium.org/2255323004/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/input/GestureManager.h (right):

https://codereview.chromium.org/2255323004/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/input/GestureManager.h:23: class CORE_EXPORT
GestureManager final : public GarbageCollectedFinalized<GestureManager> {
On 2016/09/01 23:31:15, haraken wrote:
> 
> Maybe you can drop 'Finalized' and remove the destructor.

Done.

https://codereview.chromium.org/2255323004/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/input/KeyboardEventManager.h (right):

https://codereview.chromium.org/2255323004/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/input/KeyboardEventManager.h:29: class
CORE_EXPORT KeyboardEventManager final : public
GarbageCollectedFinalized<KeyboardEventManager> {
On 2016/09/01 23:31:15, haraken wrote:
> 
> Ditto. Drop 'Finalized'.

Done.

https://codereview.chromium.org/2255323004/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/input/MouseEventManager.h (right):

https://codereview.chromium.org/2255323004/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/input/MouseEventManager.h:21: class CORE_EXPORT
MouseEventManager final : public GarbageCollectedFinalized<MouseEventManager> {
On 2016/09/01 23:31:15, haraken wrote:
> 
> Ditto. Drop 'Finalized'.

Done.

https://codereview.chromium.org/2255323004/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/input/PointerEventManager.h (right):

https://codereview.chromium.org/2255323004/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/input/PointerEventManager.h:114:
DEFINE_INLINE_TRACE()
On 2016/09/01 23:31:15, haraken wrote:
> 
> We don't need to add TRACE to STACK_ALLOCATED objects.
> 
> Sorry it was not clear in the documentation. We should improve the gc plugin
to
> auto-detect this...
> 
> 

Done.

[commit-bot: I haz the power, 2016-09-02 19:55:02]

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

[commit-bot: I haz the power, 2016-09-02 19:55:04]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[Navid Zolghadr, 2016-09-07 14:37:25]

On 2016/09/02 16:36:46, Navid Zolghadr wrote:
> ptal.
> 
>
https://codereview.chromium.org/2255323004/diff/40001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/html/HTMLLabelElement.cpp (right):
> 
>
https://codereview.chromium.org/2255323004/diff/40001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/html/HTMLLabelElement.cpp:175: if
> (isLabelTextSelected && toMouseEvent(evt)->detail() == 1)
> On 2016/09/02 03:26:27, dtapuska wrote:
> > I'd prefer if we added a method to MouseEvent that was the clickCount
instead
> of
> > expecting someone to know that detail() == clickCount.
> 
> Done.
> 
>
https://codereview.chromium.org/2255323004/diff/40001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/input/BoundaryEventDispatcher.cpp (right):
> 
>
https://codereview.chromium.org/2255323004/diff/40001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/input/BoundaryEventDispatcher.cpp:103: if
> (exitedAncestors[j]->hasCapturingEventListeners(getLeaveEvent()))
> On 2016/09/02 03:26:27, dtapuska wrote:
> > I don't think you need to call the getLeaveEvent() for every possible node
in
> > the path do you?
> > 
> > Can't think loop exit early?
> 
> Yup. That is certainly possible. Didn't pay attention when I was copy pasting
> here. Done.
> 
>
https://codereview.chromium.org/2255323004/diff/40001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/input/BoundaryEventDispatcher.cpp:118: for
> (size_t i = 0; i < enteredAncestors.size(); i++) {
> On 2016/09/02 03:26:27, dtapuska wrote:
> > Same as above.
> 
> Done.
> 
>
https://codereview.chromium.org/2255323004/diff/40001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/input/EventHandler.cpp (right):
> 
>
https://codereview.chromium.org/2255323004/diff/40001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/input/EventHandler.cpp:226:
> visitor->trace(m_mouseEventManager);
> On 2016/09/02 03:26:27, dtapuska wrote:
> > The trace should probably happen the order of the fields listed in the class
> for
> > clarity.
> 
> Done.
> 
>
https://codereview.chromium.org/2255323004/diff/120001/third_party/WebKit/Sou...
> File third_party/WebKit/Source/core/input/GestureManager.h (right):
> 
>
https://codereview.chromium.org/2255323004/diff/120001/third_party/WebKit/Sou...
> third_party/WebKit/Source/core/input/GestureManager.h:23: class CORE_EXPORT
> GestureManager final : public GarbageCollectedFinalized<GestureManager> {
> On 2016/09/01 23:31:15, haraken wrote:
> > 
> > Maybe you can drop 'Finalized' and remove the destructor.
> 
> Done.
> 
>
https://codereview.chromium.org/2255323004/diff/120001/third_party/WebKit/Sou...
> File third_party/WebKit/Source/core/input/KeyboardEventManager.h (right):
> 
>
https://codereview.chromium.org/2255323004/diff/120001/third_party/WebKit/Sou...
> third_party/WebKit/Source/core/input/KeyboardEventManager.h:29: class
> CORE_EXPORT KeyboardEventManager final : public
> GarbageCollectedFinalized<KeyboardEventManager> {
> On 2016/09/01 23:31:15, haraken wrote:
> > 
> > Ditto. Drop 'Finalized'.
> 
> Done.
> 
>
https://codereview.chromium.org/2255323004/diff/120001/third_party/WebKit/Sou...
> File third_party/WebKit/Source/core/input/MouseEventManager.h (right):
> 
>
https://codereview.chromium.org/2255323004/diff/120001/third_party/WebKit/Sou...
> third_party/WebKit/Source/core/input/MouseEventManager.h:21: class CORE_EXPORT
> MouseEventManager final : public GarbageCollectedFinalized<MouseEventManager>
{
> On 2016/09/01 23:31:15, haraken wrote:
> > 
> > Ditto. Drop 'Finalized'.
> 
> Done.
> 
>
https://codereview.chromium.org/2255323004/diff/120001/third_party/WebKit/Sou...
> File third_party/WebKit/Source/core/input/PointerEventManager.h (right):
> 
>
https://codereview.chromium.org/2255323004/diff/120001/third_party/WebKit/Sou...
> third_party/WebKit/Source/core/input/PointerEventManager.h:114:
> DEFINE_INLINE_TRACE()
> On 2016/09/01 23:31:15, haraken wrote:
> > 
> > We don't need to add TRACE to STACK_ALLOCATED objects.
> > 
> > Sorry it was not clear in the documentation. We should improve the gc plugin
> to
> > auto-detect this...
> > 
> > 
> 
> Done.

ping

[bokan, 2016-09-07 14:52:39]

lgtm

[Navid Zolghadr, 2016-09-07 15:08:39]

The CQ bit was checked by nzolghadr@chromium.org

[Navid Zolghadr, 2016-09-07 15:08:40]

The patchset sent to the CQ was uploaded after l-g-t-m from mustaq@chromium.org,
haraken@chromium.org
Link to the patchset: https://codereview.chromium.org/2255323004/#ps160001
(title: "Rebasing and comments")

[commit-bot: I haz the power, 2016-09-07 15:09:12]

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, 2016-09-07 15:10:56]

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

[commit-bot: I haz the power, 2016-09-07 15:10:58]

Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[Navid Zolghadr, 2016-09-07 16:08:00]

The CQ bit was checked by nzolghadr@chromium.org

[Navid Zolghadr, 2016-09-07 16:08:01]

The patchset sent to the CQ was uploaded after l-g-t-m from
haraken@chromium.org, mustaq@chromium.org, bokan@chromium.org
Link to the patchset: https://codereview.chromium.org/2255323004/#ps180001
(title: "Rebasing")

[commit-bot: I haz the power, 2016-09-07 16:08:27]

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, 2016-09-07 17:41:10]

Description was changed from

==========
The first step of separating mouse code from EventHandler.

There will be follow up CLs to refactor more code and
address the TODOs added in this CL.

Creating MouseEventManager to encapsule mouse related
bookkeeping and dispatching.

Creating abstract BoundaryEventDispatcher to keep the
boundary node elements calculation so PEM and MEM can
make subclasses from it and send their own boundary events.

Creating EventHandlingUtil to share the common functions
used in different EventHandling path instead of putting
them all as static in EventHandler.

BUG=625843
==========

to

==========
The first step of separating mouse code from EventHandler.

There will be follow up CLs to refactor more code and
address the TODOs added in this CL.

Creating MouseEventManager to encapsule mouse related
bookkeeping and dispatching.

Creating abstract BoundaryEventDispatcher to keep the
boundary node elements calculation so PEM and MEM can
make subclasses from it and send their own boundary events.

Creating EventHandlingUtil to share the common functions
used in different EventHandling path instead of putting
them all as static in EventHandler.

BUG=625843
==========

[commit-bot: I haz the power, 2016-09-07 17:41:12]

Committed patchset #10 (id:180001)

[commit-bot: I haz the power, 2016-09-07 17:43:21]

Description was changed from

==========
The first step of separating mouse code from EventHandler.

There will be follow up CLs to refactor more code and
address the TODOs added in this CL.

Creating MouseEventManager to encapsule mouse related
bookkeeping and dispatching.

Creating abstract BoundaryEventDispatcher to keep the
boundary node elements calculation so PEM and MEM can
make subclasses from it and send their own boundary events.

Creating EventHandlingUtil to share the common functions
used in different EventHandling path instead of putting
them all as static in EventHandler.

BUG=625843
==========

to

==========
The first step of separating mouse code from EventHandler.

There will be follow up CLs to refactor more code and
address the TODOs added in this CL.

Creating MouseEventManager to encapsule mouse related
bookkeeping and dispatching.

Creating abstract BoundaryEventDispatcher to keep the
boundary node elements calculation so PEM and MEM can
make subclasses from it and send their own boundary events.

Creating EventHandlingUtil to share the common functions
used in different EventHandling path instead of putting
them all as static in EventHandler.

BUG=625843

Committed: https://crrev.com/2c6e050f06efedae49f77dd91baa016dbd2e8fe9
Cr-Commit-Position: refs/heads/master@{#416984}
==========

[commit-bot: I haz the power, 2016-09-07 17:43:23]

Patchset 10 (id:??) landed as
https://crrev.com/2c6e050f06efedae49f77dd91baa016dbd2e8fe9
Cr-Commit-Position: refs/heads/master@{#416984}