Release mouse pointer capture when it starts drag

Release mouse pointer capture when it starts drag

This CL releases mouse pointer capture if there
was one in place when drag operation starts.
Note that this doesn't do anything for touch
drag operation as that needs to be solved
when we have a better coordination of gesture
and touch events.

BUG=706106, 720201
Review-Url: https://codereview.chromium.org/2973963003
Cr-Commit-Position: refs/heads/master@{#490010}
Committed: https://chromium.googlesource.com/chromium/src/+/656f9b939150e9a40553757cea46d385bcb423d9

[Navid Zolghadr, 2017-07-07 18:35:04]

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

[commit-bot: I haz the power, 2017-07-07 18:35:21]

Dry run: CQ is trying da patch.

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

[Navid Zolghadr, 2017-07-07 19:09:47]

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

[commit-bot: I haz the power, 2017-07-07 21:02:11]

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

[commit-bot: I haz the power, 2017-07-07 21:02:11]

Dry run: This issue passed the CQ dry run.

[mustaq, 2017-07-10 16:17:31]

LGTM with nits.

https://codereview.chromium.org/2973963003/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/external/wpt/pointerevents/html/pointerevent_drag_interaction-manual.html
(right):

https://codereview.chromium.org/2973963003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/external/wpt/pointerevents/html/pointerevent_drag_interaction-manual.html:47:
assert_equals(received_events.join(','),
"pointerdown@target0,dragstart@target0,pointercancel@target0", "Pointercancel
should be fired with the expected order when drag operation starts.");
Nit: I think joining with ', ' (with a space) makes both the code and the HTML
test outcome a bit more readable.

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

https://codereview.chromium.org/2973963003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/input/PointerEventManager.cpp:496: if
(web_pointer_event.GetType() == WebInputEvent::Type::kPointerCancel) {
Nit: DCHECK for kPointerCancel instead, until we have a full-fledged EH here?

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

https://codereview.chromium.org/2973963003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/input/PointerEventManager.h:34: // cause firing
DOM pointerevents, mouseevent, and touch events accordingly.
Say it's a WIP, link to the bug (we have one, right?).

[Navid Zolghadr, 2017-07-10 19:12:25]

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

[commit-bot: I haz the power, 2017-07-10 19:12:58]

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-07-10 22:11:43]

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

[commit-bot: I haz the power, 2017-07-10 22:11:44]

Dry run: This issue passed the CQ dry run.

[Navid Zolghadr, 2017-07-11 15:41:06]

https://codereview.chromium.org/2973963003/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/external/wpt/pointerevents/html/pointerevent_drag_interaction-manual.html
(right):

https://codereview.chromium.org/2973963003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/external/wpt/pointerevents/html/pointerevent_drag_interaction-manual.html:47:
assert_equals(received_events.join(','),
"pointerdown@target0,dragstart@target0,pointercancel@target0", "Pointercancel
should be fired with the expected order when drag operation starts.");
On 2017/07/10 16:17:30, mustaq wrote:
> Nit: I think joining with ', ' (with a space) makes both the code and the HTML
> test outcome a bit more readable.

Done.

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

https://codereview.chromium.org/2973963003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/input/PointerEventManager.cpp:496: if
(web_pointer_event.GetType() == WebInputEvent::Type::kPointerCancel) {
On 2017/07/10 16:17:30, mustaq wrote:
> Nit: DCHECK for kPointerCancel instead, until we have a full-fledged EH here?

Done.

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

https://codereview.chromium.org/2973963003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/input/PointerEventManager.h:34: // cause firing
DOM pointerevents, mouseevent, and touch events accordingly.
On 2017/07/10 16:17:30, mustaq wrote:
> Say it's a WIP, link to the bug (we have one, right?).

Done.

[dtapuska, 2017-07-11 16:00:52]

https://codereview.chromium.org/2973963003/diff/20001/third_party/WebKit/publ...
File third_party/WebKit/public/platform/WebPointerEvent.h (right):

https://codereview.chromium.org/2973963003/diff/20001/third_party/WebKit/publ...
third_party/WebKit/public/platform/WebPointerEvent.h:30: BLINK_PLATFORM_EXPORT
WebPointerEvent(const WebMouseEvent&, bool is_cancel);
Can we get rid of the is_cancel? It is really odd.

What if this was WebPointerEvent(Type type, const WebMouseEvent&);

And then we have a static method that returns the WebMouseEvent type ->
WebPointerEvent type..

[Navid Zolghadr, 2017-07-12 16:08:20]

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

[commit-bot: I haz the power, 2017-07-12 16:08:39]

Dry run: CQ is trying da patch.

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

[Navid Zolghadr, 2017-07-12 18:36:45]

https://codereview.chromium.org/2973963003/diff/20001/third_party/WebKit/publ...
File third_party/WebKit/public/platform/WebPointerEvent.h (right):

https://codereview.chromium.org/2973963003/diff/20001/third_party/WebKit/publ...
third_party/WebKit/public/platform/WebPointerEvent.h:30: BLINK_PLATFORM_EXPORT
WebPointerEvent(const WebMouseEvent&, bool is_cancel);
On 2017/07/11 16:00:52, dtapuska wrote:
> Can we get rid of the is_cancel? It is really odd.
> 
> What if this was WebPointerEvent(Type type, const WebMouseEvent&);
> 
> And then we have a static method that returns the WebMouseEvent type ->
> WebPointerEvent type..

Done.

[commit-bot: I haz the power, 2017-07-12 19:52:57]

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

[commit-bot: I haz the power, 2017-07-12 19:52:58]

Dry run: This issue passed the CQ dry run.

[Navid Zolghadr, 2017-07-13 16:21:23]

ptal

[dtapuska, 2017-07-13 19:20:21]

On 2017/07/13 16:21:23, Navid Zolghadr wrote:
> ptal

lgtm

[dtapuska, 2017-07-13 19:21:36]

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

https://codereview.chromium.org/2973963003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/input/PointerEventManager.cpp:498:
DCHECK(web_pointer_event.GetType() == WebInputEvent::Type::kPointerCancel);
DCHECK_EQ is preferred

[Navid Zolghadr, 2017-07-26 19:56:17]

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

[Navid Zolghadr, 2017-07-26 19:56:18]

rbyers@chromium.org please take a look at 

* third_party/WebKit/public/platform/WebPointerEvent.h

[Rick Byers, 2017-07-26 23:50:06]

public/platform LGTM

[Navid Zolghadr, 2017-07-27 15:35:50]

The CQ bit was checked by nzolghadr@chromium.org

[Navid Zolghadr, 2017-07-27 15:35:50]

The patchset sent to the CQ was uploaded after l-g-t-m from mustaq@chromium.org
Link to the patchset: https://codereview.chromium.org/2973963003/#ps40001
(title: "Remove is_cancel from parameters")

[commit-bot: I haz the power, 2017-07-27 15:35:56]

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-07-27 17:27:08]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1501169750221320,
"parent_rev": "4e8cac2a96b0553324b2860479084fe84dd0528e", "commit_rev":
"656f9b939150e9a40553757cea46d385bcb423d9"}

[commit-bot: I haz the power, 2017-07-27 17:27:24]

Description was changed from

==========
Release mouse pointer capture when it starts drag

This CL releases mouse pointer capture if there
was one in place when drag operation starts.
Note that this doesn't do anything for touch
drag operation as that needs to be solved
when we have a better coordination of gesture
and touch events.

BUG=706106, 720201
==========

to

==========
Release mouse pointer capture when it starts drag

This CL releases mouse pointer capture if there
was one in place when drag operation starts.
Note that this doesn't do anything for touch
drag operation as that needs to be solved
when we have a better coordination of gesture
and touch events.

BUG=706106, 720201

Review-Url: https://codereview.chromium.org/2973963003
Cr-Commit-Position: refs/heads/master@{#490010}
Committed:
https://chromium.googlesource.com/chromium/src/+/656f9b939150e9a40553757cea46...
==========

[commit-bot: I haz the power, 2017-07-27 17:27:26]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/656f9b939150e9a40553757cea46...