Retarget mouse event if the target is removed

Retarget mouse event if the target is removed

This CL calculates the ancenstor chain for every mouse event
and retarget the mouse event to the first ancestor of the node
that is still in the document if the pointer event handler
removes the target node.

BUG=608003
Committed: https://crrev.com/827f606ae83c257352ab1711fe0eb4b142b8828f
Cr-Commit-Position: refs/heads/master@{#392319}

[Navid Zolghadr, 2016-05-03 16:41:37]

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

[Navid Zolghadr, 2016-05-03 16:41:38]

This is the change to handle removing the node in pointer event handler. I
personally don't have a great feeling about this change as we are calculating
the ancestors on every event. Not sure what else we can do there to address the
issue.

[dtapuska, 2016-05-03 17:07:25]

On 2016/05/03 16:41:38, Navid Zolghadr wrote:
> This is the change to handle removing the node in pointer event handler. I
> personally don't have a great feeling about this change as we are calculating
> the ancestors on every event. Not sure what else we can do there to address
the
> issue.

Are we able to store the dom generation and then just clone the ancestor chain
if the dom generation didn't change?

[Navid Zolghadr, 2016-05-03 17:23:48]

On 2016/05/03 17:07:37, dtapuska wrote:

Can you give me some pointers? Where do we handle that generation?

[dtapuska, 2016-05-03 17:25:36]

On 2016/05/03 17:23:48, Navid Zolghadr wrote:
> On 2016/05/03 17:07:37, dtapuska wrote:
> 
> Can you give me some pointers? Where do we handle that generation?

https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

[dtapuska, 2016-05-03 17:32:00]

On 2016/05/03 17:25:36, dtapuska wrote:
> On 2016/05/03 17:23:48, Navid Zolghadr wrote:
> > On 2016/05/03 17:07:37, dtapuska wrote:
> > 
> > Can you give me some pointers? Where do we handle that generation?
> 
>
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

Or can we re-use the event path on the original dispatch?

https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

[Navid Zolghadr, 2016-05-03 18:46:30]

ptal.

This one does sound better.

[Navid Zolghadr, 2016-05-05 15:22:30]

On 2016/05/03 18:46:30, Navid Zolghadr wrote:
> ptal.
> 
> This one does sound better.

ping.

[mustaq, 2016-05-05 15:31:02]

https://codereview.chromium.org/1949563002/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/virtual/pointerevent/fast/events/pointerevents/mouse-node-remove.html
(right):

https://codereview.chromium.org/1949563002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/virtual/pointerevent/fast/events/pointerevents/mouse-node-remove.html:30:

Nit: empty line.

https://codereview.chromium.org/1949563002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/virtual/pointerevent/fast/events/pointerevents/mouse-node-remove.html:80:
removeNodeEvent = "pointerdown";
Please add a control case when green is not removed.

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

https://codereview.chromium.org/1949563002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/input/PointerEventManager.cpp:454: EventTarget*
mouseTarget = effectiveTarget;
Can we ever have both of these false?
- isInDocument(effectiveTarget)
- pointerEvent->hasEventPath()

Can't we just iterate on pointerEvent->eventPath() instead of relying on
effectiveTarget at all?

[Navid Zolghadr, 2016-05-06 15:47:20]

ptal.

https://codereview.chromium.org/1949563002/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/virtual/pointerevent/fast/events/pointerevents/mouse-node-remove.html
(right):

https://codereview.chromium.org/1949563002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/virtual/pointerevent/fast/events/pointerevents/mouse-node-remove.html:30:

On 2016/05/05 15:31:02, mustaq wrote:
> Nit: empty line.

Done.

https://codereview.chromium.org/1949563002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/virtual/pointerevent/fast/events/pointerevents/mouse-node-remove.html:80:
removeNodeEvent = "pointerdown";
On 2016/05/05 15:31:02, mustaq wrote:
> Please add a control case when green is not removed.

Done.

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

https://codereview.chromium.org/1949563002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/input/PointerEventManager.cpp:454: EventTarget*
mouseTarget = effectiveTarget;
On 2016/05/05 15:31:02, mustaq wrote:
> Can we ever have both of these false?
> - isInDocument(effectiveTarget)
> - pointerEvent->hasEventPath()
> 
> Can't we just iterate on pointerEvent->eventPath() instead of relying on
> effectiveTarget at all?

When pointer event feature is not enabled we are not sending the pointe event
and as the result eventPath is null and we can not always rely on that. I added
a comment to make it more clear.

[mustaq, 2016-05-06 15:54:47]

lgtm

[Navid Zolghadr, 2016-05-06 15:55:52]

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

[Navid Zolghadr, 2016-05-06 15:55:53]

bokan@chromium.org: Please review changes in 

hird_party/WebKit/Source/core/*

[dtapuska, 2016-05-06 16:04:28]

On 2016/05/06 15:55:53, Navid Zolghadr wrote:
> mailto:bokan@chromium.org: Please review changes in 
> 
> hird_party/WebKit/Source/core/*

lgtm

[bokan, 2016-05-06 16:12:25]

lgtm

[Navid Zolghadr, 2016-05-09 12:11:14]

The CQ bit was checked by nzolghadr@chromium.org

[commit-bot: I haz the power, 2016-05-09 12:11:25]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1949563002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1949563002/40001

[commit-bot: I haz the power, 2016-05-09 13:14:45]

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

[commit-bot: I haz the power, 2016-05-09 13:14:46]

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

[Navid Zolghadr, 2016-05-09 14:11:02]

The CQ bit was checked by nzolghadr@chromium.org

[commit-bot: I haz the power, 2016-05-09 14:11:10]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1949563002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1949563002/40001

[commit-bot: I haz the power, 2016-05-09 15:18:06]

Committed patchset #3 (id:40001)

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

Description was changed from

==========
Retarget mouse event if the target is removed

This CL calculates the ancenstor chain for every mouse event
and retarget the mouse event to the first ancestor of the node
that is still in the document if the pointer event handler
removes the target node.

BUG=608003
==========

to

==========
Retarget mouse event if the target is removed

This CL calculates the ancenstor chain for every mouse event
and retarget the mouse event to the first ancestor of the node
that is still in the document if the pointer event handler
removes the target node.

BUG=608003

Committed: https://crrev.com/827f606ae83c257352ab1711fe0eb4b142b8828f
Cr-Commit-Position: refs/heads/master@{#392319}
==========

[commit-bot: I haz the power, 2016-05-09 15:19:13]

Patchset 3 (id:??) landed as
https://crrev.com/827f606ae83c257352ab1711fe0eb4b142b8828f
Cr-Commit-Position: refs/heads/master@{#392319}