Fix properties of got/lostpointercapture

Fix properties of got/lostpointercapture

Make the attributes of got/lostpointercapture the same
as the event that caused them to be fired. In the case
of the delayed processing that will be the very next
pointerevent. In the case of implicit release that
will be pointerup/cancel right before got/lostpointercapture
event.

BUG=632766
Committed: https://crrev.com/3bd09006149797436511e02f4196ce54a91cd72e
Cr-Commit-Position: refs/heads/master@{#410409}

[Navid Zolghadr, 2016-08-04 16:15:06]

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

[commit-bot: I haz the power, 2016-08-04 16:15:18]

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-04 16:19:24]

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

[commit-bot: I haz the power, 2016-08-04 17:32:53]

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

[commit-bot: I haz the power, 2016-08-04 17:32:54]

Dry run: This issue passed the CQ dry run.

[mustaq, 2016-08-04 21:43:05]

https://codereview.chromium.org/2199263005/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/fast/events/pointerevents/mouse-pointer-capture.html
(right):

https://codereview.chromium.org/2199263005/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/fast/events/pointerevents/mouse-pointer-capture.html:80:
return false;
Can't it be simpler? Why not return false for all events except
got/lostpointercapture?

[Navid Zolghadr, 2016-08-05 14:26:08]

https://codereview.chromium.org/2199263005/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/fast/events/pointerevents/mouse-pointer-capture.html
(right):

https://codereview.chromium.org/2199263005/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/fast/events/pointerevents/mouse-pointer-capture.html:80:
return false;
On 2016/08/04 21:43:05, mustaq wrote:
> Can't it be simpler? Why not return false for all events except
> got/lostpointercapture?

I cannot imagine a way to make it simpler. Note that we have two events in this
function. One is the previous event and one is the current event. Sometime we
need to do the check when the current event is got/lostpointercapture and
sometime we need to do that when the previous event is got/lostpointercapture. I
tried to capture the three scenarios in three different booleans to make it more
clear.  Do you know a better way of making it simpler?

[mustaq, 2016-08-05 15:51:22]

https://codereview.chromium.org/2199263005/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/fast/events/pointerevents/mouse-pointer-capture.html
(right):

https://codereview.chromium.org/2199263005/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/fast/events/pointerevents/mouse-pointer-capture.html:80:
return false;
On 2016/08/05 14:26:08, Navid Zolghadr wrote:
> On 2016/08/04 21:43:05, mustaq wrote:
> > Can't it be simpler? Why not return false for all events except
> > got/lostpointercapture?
> 
> I cannot imagine a way to make it simpler. Note that we have two events in
this
> function. One is the previous event and one is the current event. Sometime we
> need to do the check when the current event is got/lostpointercapture and
> sometime we need to do that when the previous event is got/lostpointercapture.
I
> tried to capture the three scenarios in three different booleans to make it
more
> clear.  Do you know a better way of making it simpler?

Does the following miss any cases?

For any current PE, check the attributes if the last PE is got/lost, otherwise
do nothing. (In Line 127 below, set "lastPointerEvent = event" unconditionally
for any PE.)

https://codereview.chromium.org/2199263005/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/events/PointerEventFactory.cpp (right):

https://codereview.chromium.org/2199263005/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/events/PointerEventFactory.cpp:213: PointerEvent*
PointerEventFactory::createSecondaryPointerEvent(
This is now a generic PE creator method, so "secondary" doesn't make sense. What
about "createFrom()"?

Please update the description in .h accordingly.

https://codereview.chromium.org/2199263005/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/events/PointerEventFactory.cpp:244: PointerEvent*
PointerEventFactory::createPointerCaptureEvent(
Let's drop createPointerCaptureEvent & createPointerBoundaryEvent to clean the
interface. The DCHECKs themselves don't add any value IMO.

[Navid Zolghadr, 2016-08-05 17:06:35]

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

[commit-bot: I haz the power, 2016-08-05 17:07:08]

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-05 17:08:04]

ptal.

https://codereview.chromium.org/2199263005/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/fast/events/pointerevents/mouse-pointer-capture.html
(right):

https://codereview.chromium.org/2199263005/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/fast/events/pointerevents/mouse-pointer-capture.html:80:
return false;
On 2016/08/05 15:51:22, mustaq wrote:
> On 2016/08/05 14:26:08, Navid Zolghadr wrote:
> > On 2016/08/04 21:43:05, mustaq wrote:
> > > Can't it be simpler? Why not return false for all events except
> > > got/lostpointercapture?
> > 
> > I cannot imagine a way to make it simpler. Note that we have two events in
> this
> > function. One is the previous event and one is the current event. Sometime
we
> > need to do the check when the current event is got/lostpointercapture and
> > sometime we need to do that when the previous event is
got/lostpointercapture.
> I
> > tried to capture the three scenarios in three different booleans to make it
> more
> > clear.  Do you know a better way of making it simpler?
> 
> Does the following miss any cases?
> 
> For any current PE, check the attributes if the last PE is got/lost, otherwise
> do nothing. (In Line 127 below, set "lastPointerEvent = event" unconditionally
> for any PE.)

I removed one of the cases as we discussed offline.

https://codereview.chromium.org/2199263005/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/events/PointerEventFactory.cpp (right):

https://codereview.chromium.org/2199263005/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/events/PointerEventFactory.cpp:213: PointerEvent*
PointerEventFactory::createSecondaryPointerEvent(
On 2016/08/05 15:51:22, mustaq wrote:
> This is now a generic PE creator method, so "secondary" doesn't make sense.
What
> about "createFrom()"?
> 
> Please update the description in .h accordingly.

Done.

https://codereview.chromium.org/2199263005/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/events/PointerEventFactory.cpp:244: PointerEvent*
PointerEventFactory::createPointerCaptureEvent(
On 2016/08/05 15:51:22, mustaq wrote:
> Let's drop createPointerCaptureEvent & createPointerBoundaryEvent to clean the
> interface. The DCHECKs themselves don't add any value IMO.
> 

I would say let's keep the APIs different as one needs a relatedTarget and the
other should not have a relatedTarget. So defaulting that to null doesn't make
it as clear as this possibly.

[mustaq, 2016-08-05 17:29:17]

lgtm

https://codereview.chromium.org/2199263005/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/events/PointerEventFactory.cpp (right):

https://codereview.chromium.org/2199263005/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/events/PointerEventFactory.cpp:244: PointerEvent*
PointerEventFactory::createPointerCaptureEvent(
On 2016/08/05 17:08:03, Navid Zolghadr wrote:
> On 2016/08/05 15:51:22, mustaq wrote:
> > Let's drop createPointerCaptureEvent & createPointerBoundaryEvent to clean
the
> > interface. The DCHECKs themselves don't add any value IMO.
> > 
> 
> I would say let's keep the APIs different as one needs a relatedTarget and the
> other should not have a relatedTarget. So defaulting that to null doesn't make
> it as clear as this possibly.

Acknowledged.

[Navid Zolghadr, 2016-08-05 17:31:30]

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

[Navid Zolghadr, 2016-08-05 17:31:30]

bokan@chromium.org: Please review changes in 

third_party/WebKit/Source/core/*

[commit-bot: I haz the power, 2016-08-05 18:18:42]

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

[commit-bot: I haz the power, 2016-08-05 18:18:43]

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

[bokan, 2016-08-08 15:48:51]

Source/core lgtm

[Navid Zolghadr, 2016-08-08 17:31:59]

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

[commit-bot: I haz the power, 2016-08-08 17:32:26]

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-08 18:57:42]

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

[commit-bot: I haz the power, 2016-08-08 18:57:44]

Dry run: This issue passed the CQ dry run.

[Navid Zolghadr, 2016-08-08 19:30:30]

The CQ bit was checked by nzolghadr@chromium.org

[Navid Zolghadr, 2016-08-08 19:30:31]

The patchset sent to the CQ was uploaded after l-g-t-m from bokan@chromium.org,
mustaq@chromium.org
Link to the patchset: https://codereview.chromium.org/2199263005/#ps40001
(title: "Update the test output")

[commit-bot: I haz the power, 2016-08-08 19:31:09]

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-08 20:04:56]

Committed patchset #3 (id:40001)

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

Description was changed from

==========
Fix properties of got/lostpointercapture

Make the attributes of got/lostpointercapture the same
as the event that caused them to be fired. In the case
of the delayed processing that will be the very next
pointerevent. In the case of implicit release that
will be pointerup/cancel right before got/lostpointercapture
event.

BUG=632766
==========

to

==========
Fix properties of got/lostpointercapture

Make the attributes of got/lostpointercapture the same
as the event that caused them to be fired. In the case
of the delayed processing that will be the very next
pointerevent. In the case of implicit release that
will be pointerup/cancel right before got/lostpointercapture
event.

BUG=632766

Committed: https://crrev.com/3bd09006149797436511e02f4196ce54a91cd72e
Cr-Commit-Position: refs/heads/master@{#410409}
==========

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

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