Fix the wrong non-element node handling in EventHanlder and MouseEventManager

Fix the wrong non-element node handling in EventHanlder and MouseEventManager

{EventHandler,MouseEventManager} is storing a {clicked,tapped} node as Node*,
instead of Element*, that would be a bad practice. Actually, that has been the
cause of several wrong behaviors in Blink, such as:

1. Blink does NOT fire a click event when a clicked text node has been removed
   in mouseup.  This is wrong because a click event should be fired on an
   element node.  See [1] for details. This bug was just tentatively fixed at
   another CL [2].

2. Blink DOES fire a touch event on a non-element node.  This is wrong because
   TouchEvent specification says all kinds of touch events' event target types
  are limited to Document and Element [3].

To prevent these wrong behaviors, this CL does:
- Have EventHandlingUtil::ParentElementIfNeeded and use it everywhere so that
  we surely adjust a node to the appropriate element, at which an event should be
  fired.
- Replaces the usage of Node* to Element* as much as possible so that it rejects
  the wrong code at type level check.

- [1]: topmost event target;
  https://www.w3.org/TR/uievents/#topmost-event-target
- [2] https://codereview.chromium.org/2812613004
- [3]: Trusted proximal event target types are: Document and Element;
  https://w3c.github.io/touch-events/#list-of-touchevent-types

BUG=708394, 710425
Review-Url: https://codereview.chromium.org/2807123002
Cr-Commit-Position: refs/heads/master@{#464344}
Committed: https://chromium.googlesource.com/chromium/src/+/85e1871d673fb6c2e4bdb34dde02e5bb1ea896c6

[hayato, 2017-04-10 11:01:06]

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

[commit-bot: I haz the power, 2017-04-10 11:01:27]

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

[hayato, 2017-04-10 12:08:36]

try

[hayato, 2017-04-10 12:08:37]

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

[commit-bot: I haz the power, 2017-04-10 12:08:56]

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

[hayato, 2017-04-10 12:11:56]

Description was changed from

==========
# Enter a description of the change.
wip

BUG=
==========

to

==========
Fire a click event even when a clicked text node has been removed in mouseup

BUG=708394
==========

[commit-bot: I haz the power, 2017-04-10 13:28:54]

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

[commit-bot: I haz the power, 2017-04-10 13:28:55]

Dry run: This issue passed the CQ dry run.

[hayato, 2017-04-11 03:51:56]

Description was changed from

==========
Fire a click event even when a clicked text node has been removed in mouseup

BUG=708394
==========

to

==========
Fire a click event even when a clicked text node has been removed in mouseup

When a text node is clicked, the following could happen:

1. A user presses a mouse button.
2. UA did a hittest. The result's innerNode() is a text node.
3. MouseEventManager#click_node_ is set to the parent element of the clicked
   text node
4. Dispatch a mouse press event (or something)
5. A user releases a mouse button.
6. UA did another hittest because UA would dispatch a click event at the common
   ancestor of:
   - A location where mouse pressed; MouseEventManager#click_node_ is used in
this case.
   - and A location where mouse released; The 2nd hit test's result is used in
this case.
7. A user removed a text node, which is the result of 2nd hit test, in mouseup
   event listener.
8. Since there is no common ancestor of MouseEventManager#click_node_ and the
   2nd hit test's result, a click event is not dispatched.

This CL addressed this use case.

BUG=708394
==========

[hayato, 2017-04-11 03:52:40]

Description was changed from

==========
Fire a click event even when a clicked text node has been removed in mouseup

When a text node is clicked, the following could happen:

1. A user presses a mouse button.
2. UA did a hittest. The result's innerNode() is a text node.
3. MouseEventManager#click_node_ is set to the parent element of the clicked
   text node
4. Dispatch a mouse press event (or something)
5. A user releases a mouse button.
6. UA did another hittest because UA would dispatch a click event at the common
   ancestor of:
   - A location where mouse pressed; MouseEventManager#click_node_ is used in
this case.
   - and A location where mouse released; The 2nd hit test's result is used in
this case.
7. A user removed a text node, which is the result of 2nd hit test, in mouseup
   event listener.
8. Since there is no common ancestor of MouseEventManager#click_node_ and the
   2nd hit test's result, a click event is not dispatched.

This CL addressed this use case.

BUG=708394
==========

to

==========
Fire a click event even when a clicked text node has been removed in mouseup

When a text node is clicked, the following could happen:

1. A user presses a mouse button.
2. UA did a hittest. The result's innerNode() is a text node.
3. MouseEventManager#click_node_ is set to the parent element of the clicked
   text node
4. Dispatch a mouse press event (or something)
5. A user releases a mouse button.
6. UA did another hittest because UA would dispatch a click event at the common
   ancestor of:
   - the location where mouse pressed; MouseEventManager#click_node_ is used in
this case.
   - and the location where mouse released; The 2nd hit test's result is used in
this case.
7. A user removed a text node, which is the result of 2nd hit test, in mouseup
   event listener.
8. Since there is no common ancestor of MouseEventManager#click_node_ and the
   2nd hit test's result, a click event is not dispatched.

This CL addressed this use case.

BUG=708394
==========

[hayato, 2017-04-11 03:53:32]

hayato@chromium.org changed reviewers:
+ kochi@chromium.org, tkent@chromium.org

[hayato, 2017-04-11 03:53:32]

PTAL

[tkent, 2017-04-11 04:08:50]

Does Patch Set 1 work if an inner Text node is removed on mousedown?

[tkent, 2017-04-11 04:16:14]

https://codereview.chromium.org/2807123002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/input/MouseEventManager.cpp (right):

https://codereview.chromium.org/2807123002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/input/MouseEventManager.cpp:263: if
((mev.InnerNode()->IsTextNode() && click_node_->isConnected() &&
I think this code has the following unexpected behavior.

1. Open fast/events/remove-text-node-in-mouseup.html
2. Mousedown on a Text child of the first <p> element
3. Move mouse cursor onto the second <div> element
4. Mouseup 

In this case, click_node_ is <p>, mev.InnerNode() is the removed Text node, and
this code will dispatch 'click' on <p>.

[tkent, 2017-04-11 04:24:16]

https://codereview.chromium.org/2807123002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/input/MouseEventManager.cpp (right):

https://codereview.chromium.org/2807123002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/input/MouseEventManager.cpp:263: if
((mev.InnerNode()->IsTextNode() && click_node_->isConnected() &&
On 2017/04/11 at 04:16:14, tkent wrote:
> I think this code has the following unexpected behavior.
> 
> 1. Open fast/events/remove-text-node-in-mouseup.html
> 2. Mousedown on a Text child of the first <p> element
> 3. Move mouse cursor onto the second <div> element
> 4. Mouseup 
> 
> In this case, click_node_ is <p>, mev.InnerNode() is the removed Text node,
and this code will dispatch 'click' on <p>.

Maybe a logic would be:

Node* mouseup_node = mev.InnerNode()->isTextNode() ?
mev.InnerNode()->parentNode() : mev.InnerNode();
if (click_node_ == mouseup_node) {
  ...

[kochi, 2017-04-11 04:25:00]

https://codereview.chromium.org/2807123002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/input/MouseEventManager.cpp (right):

https://codereview.chromium.org/2807123002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/input/MouseEventManager.cpp:264:
!mev.InnerNode()->isConnected()) ||
(I am not sure and just checking) - Isn't click_node_->isConnected()
already guaranteed somewhere above?  (I vaguely assumed that
click_node_->isConnected() is implied in |should_dispatch_click_event|,
but it seems not.)
If click_node_ is not connected, still click event should be dispatched
to it?

For readability, I prefer the following order:
 "mev.InnerNode()->IsTextNode() && !mev.InnerNode()->isConnected() &&
  click_node_->isConnected()"
but it's up to you.

https://codereview.chromium.org/2807123002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/input/MouseEventManager.cpp:265: click_node_ ==
mev.InnerNode()) {
I'd guess "click_node_ == mev.InnerNode()" is the more common case,
so in that case the condition may be better checked first?

[hayato, 2017-04-11 07:22:58]

try

[hayato, 2017-04-11 07:22:59]

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

[commit-bot: I haz the power, 2017-04-11 07:23:18]

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, 2017-04-11 08:29:25]

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

[commit-bot: I haz the power, 2017-04-11 08:29:26]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[hayato, 2017-04-11 09:30:45]

rebase tests, try

[hayato, 2017-04-11 09:30:47]

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

[commit-bot: I haz the power, 2017-04-11 09:31:09]

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

[dtapuska, 2017-04-11 10:56:58]

Description was changed from

==========
Fire a click event even when a clicked text node has been removed in mouseup

When a text node is clicked, the following could happen:

1. A user presses a mouse button.
2. UA did a hittest. The result's innerNode() is a text node.
3. MouseEventManager#click_node_ is set to the parent element of the clicked
   text node
4. Dispatch a mouse press event (or something)
5. A user releases a mouse button.
6. UA did another hittest because UA would dispatch a click event at the common
   ancestor of:
   - the location where mouse pressed; MouseEventManager#click_node_ is used in
this case.
   - and the location where mouse released; The 2nd hit test's result is used in
this case.
7. A user removed a text node, which is the result of 2nd hit test, in mouseup
   event listener.
8. Since there is no common ancestor of MouseEventManager#click_node_ and the
   2nd hit test's result, a click event is not dispatched.

This CL addressed this use case.

BUG=708394
==========

to

==========
Fire a click event even when a clicked text node has been removed in mouseup

When a text node is clicked, the following could happen:

1. A user presses a mouse button.
2. UA did a hittest. The result's innerNode() is a text node.
3. MouseEventManager#click_node_ is set to the parent element of the clicked
   text node
4. Dispatch a mouse press event (or something)
5. A user releases a mouse button.
6. UA did another hittest because UA would dispatch a click event at the common
   ancestor of:
   - the location where mouse pressed; MouseEventManager#click_node_ is used in
this case.
   - and the location where mouse released; The 2nd hit test's result is used in
this case.
7. A user removed a text node, which is the result of 2nd hit test, in mouseup
   event listener.
8. Since there is no common ancestor of MouseEventManager#click_node_ and the
   2nd hit test's result, a click event is not dispatched.

This CL addressed this use case.

BUG=708394
==========

[dtapuska, 2017-04-11 10:56:58]

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

[dtapuska, 2017-04-11 10:58:21]

On 2017/04/11 09:30:45, hayato wrote:
> rebase tests, try

Navid should review this from a specification perspective/interop. This doesn't
have web platform tests so that is concerning.

[commit-bot: I haz the power, 2017-04-11 11:13:38]

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

[commit-bot: I haz the power, 2017-04-11 11:13:39]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[hayato, 2017-04-11 11:53:14]

clean up

[hayato, 2017-04-11 11:53:15]

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

[commit-bot: I haz the power, 2017-04-11 11:53:25]

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

[hayato, 2017-04-11 11:55:37]

Description was changed from

==========
Fire a click event even when a clicked text node has been removed in mouseup

When a text node is clicked, the following could happen:

1. A user presses a mouse button.
2. UA did a hittest. The result's innerNode() is a text node.
3. MouseEventManager#click_node_ is set to the parent element of the clicked
   text node
4. Dispatch a mouse press event (or something)
5. A user releases a mouse button.
6. UA did another hittest because UA would dispatch a click event at the common
   ancestor of:
   - the location where mouse pressed; MouseEventManager#click_node_ is used in
this case.
   - and the location where mouse released; The 2nd hit test's result is used in
this case.
7. A user removed a text node, which is the result of 2nd hit test, in mouseup
   event listener.
8. Since there is no common ancestor of MouseEventManager#click_node_ and the
   2nd hit test's result, a click event is not dispatched.

This CL addressed this use case.

BUG=708394
==========

to

==========
Fixing the wrong TextNode handling in EventHanlder and MouseEventManager

EventHandler and MouseEventManager are storing a clicked node (or a touched
node) as Node*, instead of Element*,
That would be a bad practice. Actually, that would be the cause of several wrong
behaviors in Blink, such as:

(A) Blink does NOT fire a click event when a clicked text node has been removed
in mouseup.
  This is wrong because a click event should be fired on an element node.
  See [1] for details.

(B) Blink DOES fire a touch event on a non-element node.
  This is wrong because TouchEvent specification says all kinds of touch events'
event target types are only Document and Element.

For reference, (A) would happen in the following scenario:

1. User presses a mouse button
2. UA did a hittest. The result's innerNode() is a text node.
3. MouseEventManager#click_node_ is set to the parent element of the clicked
    text node
4. Dispatch mouse press events (or anything)
5. User releases a mouse button
6. UA did another hittest because UA would dispatch a click event at the commo
   n ancestor of:
   - A location of mouse pressed; MouseEventManager#click_node_ is used in this
case.
   - and A location of mouse released; The 2nd hit test's result is used in this
case.
7. Users removed a text node, which is the result of 2nd hit test.
8. Since there is no common ancestor of MouseEventManager#click_node_ and the
    2nd hit test's result, a click event is not dispatched.

To prevent these wrong behaviors, this CL does:

- Replaces the usage of |Node*| to |Element*| as much as possible so that it
rejects the wrong code at type level.
- To prevent a false-negative event firing, like in the mentioned case, preserve
the parent *element*, if necessary, of a clicked node
   before dispatching MouseRelease event so that a following click event should
be fired correctly even when a text node has been removed in mouseup.

- [1]: topmost event target;
https://www.w3.org/TR/uievents/#topmost-event-target
- [2]: Trusted proximal event target types are: Document and Element; 
https://w3c.github.io/touch-events/#list-of-touchevent-types

This CL mainly aims to fix bug 708394. I think there are other places, such as
other events, which should be fixed, which can be done in another CL.

BUG=708394
==========

[hayato, 2017-04-11 12:02:20]

I have completely rewritten the CL so that it could fix the root cause, as an
experiment. I am not going to land this CL as is.

Now, Touch Events is also fixed; Touch event is no longer fired on a text node.
However, I am not sure that I can land this patch as is because android's
ContextualSearchManagerTest started to fail, which seems to be the only
regression of the patch #4.

https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...

[hayato, 2017-04-11 12:04:37]

hayato@chromium.org changed reviewers:
+ donnd@chromium.org

[hayato, 2017-04-11 12:04:38]

Adding donnd@ as a reviewer because the latest patch set has a regression on
Android's ContextualSearchManagerTest.

https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...

donnd@, could you have a chance to take a look? Is this a true regression?

[hayato, 2017-04-11 12:06:44]

Description was changed from

==========
Fixing the wrong TextNode handling in EventHanlder and MouseEventManager

EventHandler and MouseEventManager are storing a clicked node (or a touched
node) as Node*, instead of Element*,
That would be a bad practice. Actually, that would be the cause of several wrong
behaviors in Blink, such as:

(A) Blink does NOT fire a click event when a clicked text node has been removed
in mouseup.
  This is wrong because a click event should be fired on an element node.
  See [1] for details.

(B) Blink DOES fire a touch event on a non-element node.
  This is wrong because TouchEvent specification says all kinds of touch events'
event target types are only Document and Element.

For reference, (A) would happen in the following scenario:

1. User presses a mouse button
2. UA did a hittest. The result's innerNode() is a text node.
3. MouseEventManager#click_node_ is set to the parent element of the clicked
    text node
4. Dispatch mouse press events (or anything)
5. User releases a mouse button
6. UA did another hittest because UA would dispatch a click event at the commo
   n ancestor of:
   - A location of mouse pressed; MouseEventManager#click_node_ is used in this
case.
   - and A location of mouse released; The 2nd hit test's result is used in this
case.
7. Users removed a text node, which is the result of 2nd hit test.
8. Since there is no common ancestor of MouseEventManager#click_node_ and the
    2nd hit test's result, a click event is not dispatched.

To prevent these wrong behaviors, this CL does:

- Replaces the usage of |Node*| to |Element*| as much as possible so that it
rejects the wrong code at type level.
- To prevent a false-negative event firing, like in the mentioned case, preserve
the parent *element*, if necessary, of a clicked node
   before dispatching MouseRelease event so that a following click event should
be fired correctly even when a text node has been removed in mouseup.

- [1]: topmost event target;
https://www.w3.org/TR/uievents/#topmost-event-target
- [2]: Trusted proximal event target types are: Document and Element; 
https://w3c.github.io/touch-events/#list-of-touchevent-types

This CL mainly aims to fix bug 708394. I think there are other places, such as
other events, which should be fixed, which can be done in another CL.

BUG=708394
==========

to

==========
Fix the wrong TextNode handling in EventHanlder and MouseEventManager

EventHandler and MouseEventManager are storing a clicked node (or a touched
node) as Node*, instead of Element*,
That would be a bad practice. Actually, that would be the cause of several wrong
behaviors in Blink, such as:

(A) Blink does NOT fire a click event when a clicked text node has been removed
in mouseup.
  This is wrong because a click event should be fired on an element node.
  See [1] for details.

(B) Blink DOES fire a touch event on a non-element node.
  This is wrong because TouchEvent specification says all kinds of touch events'
event target types are only Document and Element.

For reference, (A) would happen in the following scenario:

1. User presses a mouse button
2. UA did a hittest. The result's innerNode() is a text node.
3. MouseEventManager#click_node_ is set to the parent element of the clicked
    text node
4. Dispatch mouse press events (or anything)
5. User releases a mouse button
6. UA did another hittest because UA would dispatch a click event at the commo
   n ancestor of:
   - A location of mouse pressed; MouseEventManager#click_node_ is used in this
case.
   - and A location of mouse released; The 2nd hit test's result is used in this
case.
7. Users removed a text node, which is the result of 2nd hit test.
8. Since there is no common ancestor of MouseEventManager#click_node_ and the
    2nd hit test's result, a click event is not dispatched.

To prevent these wrong behaviors, this CL does:

- Replaces the usage of |Node*| to |Element*| as much as possible so that it
rejects the wrong code at type level.
- To prevent a false-negative event firing, like in the mentioned case, preserve
the parent *element*, if necessary, of a clicked node
   before dispatching MouseRelease event so that a following click event should
be fired correctly even when a text node has been removed in mouseup.

- [1]: topmost event target;
https://www.w3.org/TR/uievents/#topmost-event-target
- [2]: Trusted proximal event target types are: Document and Element; 
https://w3c.github.io/touch-events/#list-of-touchevent-types

This CL mainly aims to fix bug 708394. I think there are other places, such as
other events, which should be fixed, which can be done in another CL.

BUG=708394
==========

[hayato, 2017-04-11 12:41:52]

Description was changed from

==========
Fix the wrong TextNode handling in EventHanlder and MouseEventManager

EventHandler and MouseEventManager are storing a clicked node (or a touched
node) as Node*, instead of Element*,
That would be a bad practice. Actually, that would be the cause of several wrong
behaviors in Blink, such as:

(A) Blink does NOT fire a click event when a clicked text node has been removed
in mouseup.
  This is wrong because a click event should be fired on an element node.
  See [1] for details.

(B) Blink DOES fire a touch event on a non-element node.
  This is wrong because TouchEvent specification says all kinds of touch events'
event target types are only Document and Element.

For reference, (A) would happen in the following scenario:

1. User presses a mouse button
2. UA did a hittest. The result's innerNode() is a text node.
3. MouseEventManager#click_node_ is set to the parent element of the clicked
    text node
4. Dispatch mouse press events (or anything)
5. User releases a mouse button
6. UA did another hittest because UA would dispatch a click event at the commo
   n ancestor of:
   - A location of mouse pressed; MouseEventManager#click_node_ is used in this
case.
   - and A location of mouse released; The 2nd hit test's result is used in this
case.
7. Users removed a text node, which is the result of 2nd hit test.
8. Since there is no common ancestor of MouseEventManager#click_node_ and the
    2nd hit test's result, a click event is not dispatched.

To prevent these wrong behaviors, this CL does:

- Replaces the usage of |Node*| to |Element*| as much as possible so that it
rejects the wrong code at type level.
- To prevent a false-negative event firing, like in the mentioned case, preserve
the parent *element*, if necessary, of a clicked node
   before dispatching MouseRelease event so that a following click event should
be fired correctly even when a text node has been removed in mouseup.

- [1]: topmost event target;
https://www.w3.org/TR/uievents/#topmost-event-target
- [2]: Trusted proximal event target types are: Document and Element; 
https://w3c.github.io/touch-events/#list-of-touchevent-types

This CL mainly aims to fix bug 708394. I think there are other places, such as
other events, which should be fixed, which can be done in another CL.

BUG=708394
==========

to

==========
Fix the wrong TextNode handling in EventHanlder and MouseEventManager

EventHandler and MouseEventManager are storing a clicked node (or a touched
node) as Node*, instead of Element*,
That would be a bad practice. Actually, that would be the cause of several wrong
behaviors in Blink, such as:

(A) Blink does NOT fire a click event when a clicked text node has been removed
in mouseup.
  This is wrong because a click event should be fired on an element node.
  See [1] for details.

(B) Blink DOES fire a touch event on a non-element node.
  This is wrong because TouchEvent specification says all kinds of touch events'
event target types are only Document and Element.

For reference, (A) would happen in the following scenario:

1. User presses a mouse button
2. UA did a hittest. The result's innerNode() is a text node.
3. MouseEventManager#click_node_ is set to the parent element of the clicked
    text node
4. Dispatch mouse press events (or anything)
5. User releases a mouse button
6. UA did another hittest because UA would dispatch a click event at the commo
   n ancestor of:
   - A location of mouse pressed; MouseEventManager#click_node_ is used in this
case.
   - and A location of mouse released; The 2nd hit test's result is used in this
case.
7. Users removed a text node, which is the result of 2nd hit test.
8. Since there is no common ancestor of MouseEventManager#click_node_ and the
    2nd hit test's result, a click event is not dispatched.

To prevent these wrong behaviors, this CL does:

- Replaces the usage of |Node*| to |Element*| as much as possible so that it
rejects the wrong code at type level.
- To prevent a false-negative event firing, like in the mentioned case, preserve
the parent *element*, if necessary, of a clicked node
   before dispatching MouseRelease event so that a following click event should
be fired correctly even when a text node has been removed in mouseup.

- [1]: topmost event target;
https://www.w3.org/TR/uievents/#topmost-event-target
- [2]: Trusted proximal event target types are: Document and Element; 
https://w3c.github.io/touch-events/#list-of-touchevent-types

This CL mainly aims to fix bug 708394. I think there are other places, such as
other events, which should be fixed, which can be done in another CL.

BUG=708394,710425
==========

[commit-bot: I haz the power, 2017-04-11 13:44:44]

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

[commit-bot: I haz the power, 2017-04-11 13:44:45]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Donn Denman, 2017-04-11 18:29:23]

I took a quick look at a few test failures and my guess is that Contextual
Search would be completely broken by this change.  I'll try to take a closer
look at this later today, but my initial impression is that CS probably doesn't
get the signal that the event happened but was not handled so it can handle it
through ShowUnhandledTapUIIfNeeded.

[hayato, 2017-04-12 02:25:07]

wip

[hayato, 2017-04-12 02:25:07]

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

[commit-bot: I haz the power, 2017-04-12 02:25:38]

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

[hayato, 2017-04-12 03:33:55]

try after merging https://codereview.chromium.org/2809333002

[hayato, 2017-04-12 03:33:56]

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

[commit-bot: I haz the power, 2017-04-12 03:34:24]

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, 2017-04-12 03:40:39]

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

[commit-bot: I haz the power, 2017-04-12 03:40:40]

Dry run: Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[hayato, 2017-04-12 04:06:42]

According to the comment in
https://bugs.chromium.org/p/chromium/issues/detail?id=708394#c18, I am thinking
that we should have a tentative fix only for bug 708394.
See https://codereview.chromium.org/2812613004

[hayato, 2017-04-12 05:04:26]

Pass the original tapped node to ShadowUnhnadledTapUIIfNeeded

[hayato, 2017-04-12 05:04:26]

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

[commit-bot: I haz the power, 2017-04-12 05:04:44]

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, 2017-04-12 06:34:49]

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

[commit-bot: I haz the power, 2017-04-12 06:34:50]

Dry run: This issue passed the CQ dry run.

[hayato, 2017-04-12 07:05:07]

Description was changed from

==========
Fix the wrong TextNode handling in EventHanlder and MouseEventManager

EventHandler and MouseEventManager are storing a clicked node (or a touched
node) as Node*, instead of Element*,
That would be a bad practice. Actually, that would be the cause of several wrong
behaviors in Blink, such as:

(A) Blink does NOT fire a click event when a clicked text node has been removed
in mouseup.
  This is wrong because a click event should be fired on an element node.
  See [1] for details.

(B) Blink DOES fire a touch event on a non-element node.
  This is wrong because TouchEvent specification says all kinds of touch events'
event target types are only Document and Element.

For reference, (A) would happen in the following scenario:

1. User presses a mouse button
2. UA did a hittest. The result's innerNode() is a text node.
3. MouseEventManager#click_node_ is set to the parent element of the clicked
    text node
4. Dispatch mouse press events (or anything)
5. User releases a mouse button
6. UA did another hittest because UA would dispatch a click event at the commo
   n ancestor of:
   - A location of mouse pressed; MouseEventManager#click_node_ is used in this
case.
   - and A location of mouse released; The 2nd hit test's result is used in this
case.
7. Users removed a text node, which is the result of 2nd hit test.
8. Since there is no common ancestor of MouseEventManager#click_node_ and the
    2nd hit test's result, a click event is not dispatched.

To prevent these wrong behaviors, this CL does:

- Replaces the usage of |Node*| to |Element*| as much as possible so that it
rejects the wrong code at type level.
- To prevent a false-negative event firing, like in the mentioned case, preserve
the parent *element*, if necessary, of a clicked node
   before dispatching MouseRelease event so that a following click event should
be fired correctly even when a text node has been removed in mouseup.

- [1]: topmost event target;
https://www.w3.org/TR/uievents/#topmost-event-target
- [2]: Trusted proximal event target types are: Document and Element; 
https://w3c.github.io/touch-events/#list-of-touchevent-types

This CL mainly aims to fix bug 708394. I think there are other places, such as
other events, which should be fixed, which can be done in another CL.

BUG=708394,710425
==========

to

==========
Fix the wrong non-element node handling in EventHanlder and MouseEventManager

EventHandler and MouseEventManager are storing a clicked node (or a touched
node) as Node*, instead of Element*,
That would be a bad practice. Actually, that would be the cause of several wrong
behaviors in Blink, such as:

(A) Blink does NOT fire a click event when a clicked text node has been removed
in mouseup.
  This is wrong because a click event should be fired on an element node.
  See [1] for details.

(B) Blink DOES fire a touch event on a non-element node.
  This is wrong because TouchEvent specification says all kinds of touch events'
event target types are only Document and Element.

For reference, (A) would happen in the following scenario:

1. User presses a mouse button
2. UA did a hittest. The result's innerNode() is a text node.
3. MouseEventManager#click_node_ is set to the parent element of the clicked
    text node
4. Dispatch mouse press events (or anything)
5. User releases a mouse button
6. UA did another hittest because UA would dispatch a click event at the commo
   n ancestor of:
   - A location of mouse pressed; MouseEventManager#click_node_ is used in this
case.
   - and A location of mouse released; The 2nd hit test's result is used in this
case.
7. Users removed a text node, which is the result of 2nd hit test.
8. Since there is no common ancestor of MouseEventManager#click_node_ and the
    2nd hit test's result, a click event is not dispatched.

To prevent these wrong behaviors, this CL does:

- Replaces the usage of |Node*| to |Element*| as much as possible so that it
rejects the wrong code at type level.
- To prevent a false-negative event firing, like in the mentioned case, preserve
the parent *element*, if necessary, of a clicked node
   before dispatching MouseRelease event so that a following click event should
be fired correctly even when a text node has been removed in mouseup.

- [1]: topmost event target;
https://www.w3.org/TR/uievents/#topmost-event-target
- [2]: Trusted proximal event target types are: Document and Element; 
https://w3c.github.io/touch-events/#list-of-touchevent-types

This CL mainly aims to fix bug 708394. I think there are other places, such as
other events, which should be fixed, which can be done in another CL.

BUG=708394,710425
==========

[hayato, 2017-04-12 09:08:36]

The result looks all green. Let me clean up the CL and the description tomorrow.
I'll ping you once it is ready.

Note that I have already landed https://codereview.chromium.org/2812613004 as a
separated CL, so this CL is no longer urgent.

[Donn Denman, 2017-04-12 17:12:19]

LGTM % nit below.

https://codereview.chromium.org/2807123002/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/input/GestureManager.cpp (right):

https://codereview.chromium.org/2807123002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/input/GestureManager.cpp:293: if (event_result ==
WebInputEventResult::kNotHandled && tapped_element &&
Nit: I wonder if it would be better to test the tapped_node here instead of the
tapped_element since that's what we're going to pass to the client.  I suppose
it's not possible for one to be null without the other right now, but seems
better and cleaner the other way.

[hayato, 2017-04-13 04:47:58]

Description was changed from

==========
Fix the wrong non-element node handling in EventHanlder and MouseEventManager

EventHandler and MouseEventManager are storing a clicked node (or a touched
node) as Node*, instead of Element*,
That would be a bad practice. Actually, that would be the cause of several wrong
behaviors in Blink, such as:

(A) Blink does NOT fire a click event when a clicked text node has been removed
in mouseup.
  This is wrong because a click event should be fired on an element node.
  See [1] for details.

(B) Blink DOES fire a touch event on a non-element node.
  This is wrong because TouchEvent specification says all kinds of touch events'
event target types are only Document and Element.

For reference, (A) would happen in the following scenario:

1. User presses a mouse button
2. UA did a hittest. The result's innerNode() is a text node.
3. MouseEventManager#click_node_ is set to the parent element of the clicked
    text node
4. Dispatch mouse press events (or anything)
5. User releases a mouse button
6. UA did another hittest because UA would dispatch a click event at the commo
   n ancestor of:
   - A location of mouse pressed; MouseEventManager#click_node_ is used in this
case.
   - and A location of mouse released; The 2nd hit test's result is used in this
case.
7. Users removed a text node, which is the result of 2nd hit test.
8. Since there is no common ancestor of MouseEventManager#click_node_ and the
    2nd hit test's result, a click event is not dispatched.

To prevent these wrong behaviors, this CL does:

- Replaces the usage of |Node*| to |Element*| as much as possible so that it
rejects the wrong code at type level.
- To prevent a false-negative event firing, like in the mentioned case, preserve
the parent *element*, if necessary, of a clicked node
   before dispatching MouseRelease event so that a following click event should
be fired correctly even when a text node has been removed in mouseup.

- [1]: topmost event target;
https://www.w3.org/TR/uievents/#topmost-event-target
- [2]: Trusted proximal event target types are: Document and Element; 
https://w3c.github.io/touch-events/#list-of-touchevent-types

This CL mainly aims to fix bug 708394. I think there are other places, such as
other events, which should be fixed, which can be done in another CL.

BUG=708394,710425
==========

to

==========
Fix the wrong non-element node handling in EventHanlder and MouseEventManager

EventHandler/MouseEventManager is storing a clicked node/tapped node as Node*,
instead of Element*, that would be a bad practice. Actually, that has been the
cause of several wrong behaviors in Blink, such as:

1. Blink does NOT fire a click event when a clicked text node has been removed
   in mouseup.  This is wrong because a click event should be fired on an
   element node.  See [1] for details. This bug was jsut tentatively fixed at
   another CL [2].

2. Blink DOES fire a touch event on a non-element node.  This is wrong because
   TouchEvent specification says all kinds of touch events' event target types
  are limited to Document and Element [3].

To prevent these wrong behaviors, this CL replaces the usage of Node* to
Element* as much as possible so that it rejects the wrong code at type level
check.

- [1]: topmost event target;
  https://www.w3.org/TR/uievents/#topmost-event-target
- [2] https://codereview.chromium.org/2812613004
- [3]: Trusted proximal event target types are: Document and Element;
  https://w3c.github.io/touch-events/#list-of-touchevent-types

BUG=708394,710425
==========

[hayato, 2017-04-13 04:48:31]

Description was changed from

==========
Fix the wrong non-element node handling in EventHanlder and MouseEventManager

EventHandler/MouseEventManager is storing a clicked node/tapped node as Node*,
instead of Element*, that would be a bad practice. Actually, that has been the
cause of several wrong behaviors in Blink, such as:

1. Blink does NOT fire a click event when a clicked text node has been removed
   in mouseup.  This is wrong because a click event should be fired on an
   element node.  See [1] for details. This bug was jsut tentatively fixed at
   another CL [2].

2. Blink DOES fire a touch event on a non-element node.  This is wrong because
   TouchEvent specification says all kinds of touch events' event target types
  are limited to Document and Element [3].

To prevent these wrong behaviors, this CL replaces the usage of Node* to
Element* as much as possible so that it rejects the wrong code at type level
check.

- [1]: topmost event target;
  https://www.w3.org/TR/uievents/#topmost-event-target
- [2] https://codereview.chromium.org/2812613004
- [3]: Trusted proximal event target types are: Document and Element;
  https://w3c.github.io/touch-events/#list-of-touchevent-types

BUG=708394,710425
==========

to

==========
Fix the wrong non-element node handling in EventHanlder and MouseEventManager

{EventHandler,MouseEventManager} is storing a {clicked,tapped} node as Node*,
instead of Element*, that would be a bad practice. Actually, that has been the
cause of several wrong behaviors in Blink, such as:

1. Blink does NOT fire a click event when a clicked text node has been removed
   in mouseup.  This is wrong because a click event should be fired on an
   element node.  See [1] for details. This bug was jsut tentatively fixed at
   another CL [2].

2. Blink DOES fire a touch event on a non-element node.  This is wrong because
   TouchEvent specification says all kinds of touch events' event target types
  are limited to Document and Element [3].

To prevent these wrong behaviors, this CL replaces the usage of Node* to
Element* as much as possible so that it rejects the wrong code at type level
check.

- [1]: topmost event target;
  https://www.w3.org/TR/uievents/#topmost-event-target
- [2] https://codereview.chromium.org/2812613004
- [3]: Trusted proximal event target types are: Document and Element;
  https://w3c.github.io/touch-events/#list-of-touchevent-types

BUG=708394,710425
==========

[hayato, 2017-04-13 04:48:53]

Description was changed from

==========
Fix the wrong non-element node handling in EventHanlder and MouseEventManager

{EventHandler,MouseEventManager} is storing a {clicked,tapped} node as Node*,
instead of Element*, that would be a bad practice. Actually, that has been the
cause of several wrong behaviors in Blink, such as:

1. Blink does NOT fire a click event when a clicked text node has been removed
   in mouseup.  This is wrong because a click event should be fired on an
   element node.  See [1] for details. This bug was jsut tentatively fixed at
   another CL [2].

2. Blink DOES fire a touch event on a non-element node.  This is wrong because
   TouchEvent specification says all kinds of touch events' event target types
  are limited to Document and Element [3].

To prevent these wrong behaviors, this CL replaces the usage of Node* to
Element* as much as possible so that it rejects the wrong code at type level
check.

- [1]: topmost event target;
  https://www.w3.org/TR/uievents/#topmost-event-target
- [2] https://codereview.chromium.org/2812613004
- [3]: Trusted proximal event target types are: Document and Element;
  https://w3c.github.io/touch-events/#list-of-touchevent-types

BUG=708394,710425
==========

to

==========
Fix the wrong non-element node handling in EventHanlder and MouseEventManager

{EventHandler,MouseEventManager} is storing a {clicked,tapped} node as Node*,
instead of Element*, that would be a bad practice. Actually, that has been the
cause of several wrong behaviors in Blink, such as:

1. Blink does NOT fire a click event when a clicked text node has been removed
   in mouseup.  This is wrong because a click event should be fired on an
   element node.  See [1] for details. This bug was just tentatively fixed at
   another CL [2].

2. Blink DOES fire a touch event on a non-element node.  This is wrong because
   TouchEvent specification says all kinds of touch events' event target types
  are limited to Document and Element [3].

To prevent these wrong behaviors, this CL replaces the usage of Node* to
Element* as much as possible so that it rejects the wrong code at type level
check.

- [1]: topmost event target;
  https://www.w3.org/TR/uievents/#topmost-event-target
- [2] https://codereview.chromium.org/2812613004
- [3]: Trusted proximal event target types are: Document and Element;
  https://w3c.github.io/touch-events/#list-of-touchevent-types

BUG=708394,710425
==========

[hayato, 2017-04-13 04:55:13]

Description was changed from

==========
Fix the wrong non-element node handling in EventHanlder and MouseEventManager

{EventHandler,MouseEventManager} is storing a {clicked,tapped} node as Node*,
instead of Element*, that would be a bad practice. Actually, that has been the
cause of several wrong behaviors in Blink, such as:

1. Blink does NOT fire a click event when a clicked text node has been removed
   in mouseup.  This is wrong because a click event should be fired on an
   element node.  See [1] for details. This bug was just tentatively fixed at
   another CL [2].

2. Blink DOES fire a touch event on a non-element node.  This is wrong because
   TouchEvent specification says all kinds of touch events' event target types
  are limited to Document and Element [3].

To prevent these wrong behaviors, this CL replaces the usage of Node* to
Element* as much as possible so that it rejects the wrong code at type level
check.

- [1]: topmost event target;
  https://www.w3.org/TR/uievents/#topmost-event-target
- [2] https://codereview.chromium.org/2812613004
- [3]: Trusted proximal event target types are: Document and Element;
  https://w3c.github.io/touch-events/#list-of-touchevent-types

BUG=708394,710425
==========

to

==========
Fix the wrong non-element node handling in EventHanlder and MouseEventManager

{EventHandler,MouseEventManager} is storing a {clicked,tapped} node as Node*,
instead of Element*, that would be a bad practice. Actually, that has been the
cause of several wrong behaviors in Blink, such as:

1. Blink does NOT fire a click event when a clicked text node has been removed
   in mouseup.  This is wrong because a click event should be fired on an
   element node.  See [1] for details. This bug was just tentatively fixed at
   another CL [2].

2. Blink DOES fire a touch event on a non-element node.  This is wrong because
   TouchEvent specification says all kinds of touch events' event target types
  are limited to Document and Element [3].

To prevent these wrong behaviors, this CL does:
- Have EventHandlingUtil::ParentElementIfNeeded and use it everywhere so that
  we surely adjust a node to the appropriate element, where an event should be
  fired.
- Replaces the usage of Node* to Element* as much as possible so that it rejects
  the wrong code at type level check.

- [1]: topmost event target;
  https://www.w3.org/TR/uievents/#topmost-event-target
- [2] https://codereview.chromium.org/2812613004
- [3]: Trusted proximal event target types are: Document and Element;
  https://w3c.github.io/touch-events/#list-of-touchevent-types

BUG=708394,710425
==========

[hayato, 2017-04-13 04:55:50]

Description was changed from

==========
Fix the wrong non-element node handling in EventHanlder and MouseEventManager

{EventHandler,MouseEventManager} is storing a {clicked,tapped} node as Node*,
instead of Element*, that would be a bad practice. Actually, that has been the
cause of several wrong behaviors in Blink, such as:

1. Blink does NOT fire a click event when a clicked text node has been removed
   in mouseup.  This is wrong because a click event should be fired on an
   element node.  See [1] for details. This bug was just tentatively fixed at
   another CL [2].

2. Blink DOES fire a touch event on a non-element node.  This is wrong because
   TouchEvent specification says all kinds of touch events' event target types
  are limited to Document and Element [3].

To prevent these wrong behaviors, this CL does:
- Have EventHandlingUtil::ParentElementIfNeeded and use it everywhere so that
  we surely adjust a node to the appropriate element, where an event should be
  fired.
- Replaces the usage of Node* to Element* as much as possible so that it rejects
  the wrong code at type level check.

- [1]: topmost event target;
  https://www.w3.org/TR/uievents/#topmost-event-target
- [2] https://codereview.chromium.org/2812613004
- [3]: Trusted proximal event target types are: Document and Element;
  https://w3c.github.io/touch-events/#list-of-touchevent-types

BUG=708394,710425
==========

to

==========
Fix the wrong non-element node handling in EventHanlder and MouseEventManager

{EventHandler,MouseEventManager} is storing a {clicked,tapped} node as Node*,
instead of Element*, that would be a bad practice. Actually, that has been the
cause of several wrong behaviors in Blink, such as:

1. Blink does NOT fire a click event when a clicked text node has been removed
   in mouseup.  This is wrong because a click event should be fired on an
   element node.  See [1] for details. This bug was just tentatively fixed at
   another CL [2].

2. Blink DOES fire a touch event on a non-element node.  This is wrong because
   TouchEvent specification says all kinds of touch events' event target types
  are limited to Document and Element [3].

To prevent these wrong behaviors, this CL does:
- Have EventHandlingUtil::ParentElementIfNeeded and use it everywhere so that
  we surely adjust a node to the appropriate element, at which an event should
be
  fired.
- Replaces the usage of Node* to Element* as much as possible so that it rejects
  the wrong code at type level check.

- [1]: topmost event target;
  https://www.w3.org/TR/uievents/#topmost-event-target
- [2] https://codereview.chromium.org/2812613004
- [3]: Trusted proximal event target types are: Document and Element;
  https://w3c.github.io/touch-events/#list-of-touchevent-types

BUG=708394,710425
==========

[hayato, 2017-04-13 05:01:03]

Addressed

[hayato, 2017-04-13 05:01:05]

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

[commit-bot: I haz the power, 2017-04-13 05:01:31]

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

[hayato, 2017-04-13 05:01:40]

https://codereview.chromium.org/2807123002/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/input/GestureManager.cpp (right):

https://codereview.chromium.org/2807123002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/input/GestureManager.cpp:293: if (event_result ==
WebInputEventResult::kNotHandled && tapped_element &&
Nice catch. tapped_node should be better.
Done.

[hayato, 2017-04-13 05:03:06]

The CQ bit was checked by hayato@chromium.org

[hayato, 2017-04-13 05:03:07]

The patchset sent to the CQ was uploaded after l-g-t-m from donnd@chromium.org
Link to the patchset: https://codereview.chromium.org/2807123002/#ps160001
(title: "Addressed")

[commit-bot: I haz the power, 2017-04-13 05:03:58]

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-04-13 06:51:16]

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

[commit-bot: I haz the power, 2017-04-13 06:51:17]

Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[hayato, 2017-04-13 07:08:50]

The CQ bit was checked by hayato@chromium.org

[commit-bot: I haz the power, 2017-04-13 07:09:14]

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-04-13 08:26:25]

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

[commit-bot: I haz the power, 2017-04-13 08:26:27]

Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[hayato, 2017-04-13 08:29:23]

The CQ bit was checked by hayato@chromium.org

[commit-bot: I haz the power, 2017-04-13 08:29:50]

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-04-13 08:53:18]

CQ is committing da patch.

Bot data: {"patchset_id": 160001, "attempt_start_ts": 1492072163282880,
"parent_rev": "3b8cd5612815432d74825a69c4431ea4968e7050", "commit_rev":
"85e1871d673fb6c2e4bdb34dde02e5bb1ea896c6"}

[commit-bot: I haz the power, 2017-04-13 08:54:12]

Description was changed from

==========
Fix the wrong non-element node handling in EventHanlder and MouseEventManager

{EventHandler,MouseEventManager} is storing a {clicked,tapped} node as Node*,
instead of Element*, that would be a bad practice. Actually, that has been the
cause of several wrong behaviors in Blink, such as:

1. Blink does NOT fire a click event when a clicked text node has been removed
   in mouseup.  This is wrong because a click event should be fired on an
   element node.  See [1] for details. This bug was just tentatively fixed at
   another CL [2].

2. Blink DOES fire a touch event on a non-element node.  This is wrong because
   TouchEvent specification says all kinds of touch events' event target types
  are limited to Document and Element [3].

To prevent these wrong behaviors, this CL does:
- Have EventHandlingUtil::ParentElementIfNeeded and use it everywhere so that
  we surely adjust a node to the appropriate element, at which an event should
be
  fired.
- Replaces the usage of Node* to Element* as much as possible so that it rejects
  the wrong code at type level check.

- [1]: topmost event target;
  https://www.w3.org/TR/uievents/#topmost-event-target
- [2] https://codereview.chromium.org/2812613004
- [3]: Trusted proximal event target types are: Document and Element;
  https://w3c.github.io/touch-events/#list-of-touchevent-types

BUG=708394,710425
==========

to

==========
Fix the wrong non-element node handling in EventHanlder and MouseEventManager

{EventHandler,MouseEventManager} is storing a {clicked,tapped} node as Node*,
instead of Element*, that would be a bad practice. Actually, that has been the
cause of several wrong behaviors in Blink, such as:

1. Blink does NOT fire a click event when a clicked text node has been removed
   in mouseup.  This is wrong because a click event should be fired on an
   element node.  See [1] for details. This bug was just tentatively fixed at
   another CL [2].

2. Blink DOES fire a touch event on a non-element node.  This is wrong because
   TouchEvent specification says all kinds of touch events' event target types
  are limited to Document and Element [3].

To prevent these wrong behaviors, this CL does:
- Have EventHandlingUtil::ParentElementIfNeeded and use it everywhere so that
  we surely adjust a node to the appropriate element, at which an event should
be
  fired.
- Replaces the usage of Node* to Element* as much as possible so that it rejects
  the wrong code at type level check.

- [1]: topmost event target;
  https://www.w3.org/TR/uievents/#topmost-event-target
- [2] https://codereview.chromium.org/2812613004
- [3]: Trusted proximal event target types are: Document and Element;
  https://w3c.github.io/touch-events/#list-of-touchevent-types

BUG=708394,710425

Review-Url: https://codereview.chromium.org/2807123002
Cr-Commit-Position: refs/heads/master@{#464344}
Committed:
https://chromium.googlesource.com/chromium/src/+/85e1871d673fb6c2e4bdb34dde02...
==========

[commit-bot: I haz the power, 2017-04-13 08:54:13]

Committed patchset #9 (id:160001) as
https://chromium.googlesource.com/chromium/src/+/85e1871d673fb6c2e4bdb34dde02...

[dtapuska, 2017-04-19 01:56:06]

dtapuska@chromium.org changed reviewers:
+ dtapuska@chromium.org

[dtapuska, 2017-04-19 01:56:07]

https://codereview.chromium.org/2807123002/diff/160001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/input/EventHandlingUtil.h (right):

https://codereview.chromium.org/2807123002/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/input/EventHandlingUtil.h:34: Element*
ParentElementIfNeeded(Node*);
Ia this method really needed? Could HitTestResult::innerElement not be called
instead of adding this additional code?

[hayato, 2017-04-19 02:26:35]

https://codereview.chromium.org/2807123002/diff/160001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/input/EventHandlingUtil.h (right):

https://codereview.chromium.org/2807123002/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/input/EventHandlingUtil.h:34: Element*
ParentElementIfNeeded(Node*);
Ah, I didn't notice innerElement() at all because the old code around here
didn't use it. :(

HitTestResult::innerElement() (after some refinements) would be better.