Added counters for pointer boundary events fired.

Added counters for pointer boundary events fired.

Covered both unconditional firing and firing for captured pointer ids.

BUG=640637
TBR=isherman@chromium.org

Committed: https://crrev.com/44f9943d379d6647fa3ef799dc1c6fa427fcd835
Cr-Commit-Position: refs/heads/master@{#414728}

[mustaq, 2016-08-24 19:10:35]

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

[mustaq, 2016-08-24 19:10:36]

ptal

[dtapuska, 2016-08-24 20:20:57]

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

https://codereview.chromium.org/2274253002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/events/EventTarget.cpp:601: } else if
(checkTypeThenUseCount(event, EventTypeNames::pointerover,
UseCounter::PointerOverOutFired)
This is duplicated code; can we parameterize this somehow to avoid the
duplication?

[mustaq, 2016-08-24 20:50:29]

ptal

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

https://codereview.chromium.org/2274253002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/events/EventTarget.cpp:601: } else if
(checkTypeThenUseCount(event, EventTypeNames::pointerover,
UseCounter::PointerOverOutFired)
On 2016/08/24 20:20:57, dtapuska wrote:
> This is duplicated code; can we parameterize this somehow to avoid the
> duplication?

Done.

[dtapuska, 2016-08-24 20:54:57]

lgtm

[mustaq, 2016-08-24 21:08:20]

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

[mustaq, 2016-08-24 21:08:21]

bokan@chromium.org: Please review changes in Source/core

[Navid Zolghadr, 2016-08-24 21:18:02]

https://codereview.chromium.org/2274253002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/events/EventTarget.cpp (right):

https://codereview.chromium.org/2274253002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/events/EventTarget.cpp:604:
UseCounter::count(executingWindow->document(), feature);
This probably won't work as you expect. In the normal case (the way right now
Chrome implements) after setPointerCapture (which makes hasPointerCapture return
true) there might be a boundary event when then pointer capture is processed.
But that is different from the boundary events that we suppress while a pointer
is captured. Do we want to make a distinction there?

[mustaq, 2016-08-24 21:23:36]

Description was changed from

==========
Added counters for pointer boundary events fired.

Covered both unconditional firing and firing for captured pointer ids.

BUG=640637
==========

to

==========
Added counters for pointer boundary events fired.

Covered both unconditional firing and firing for captured pointer ids.

BUG=640637
TBR=isherman@chromium.org
==========

[mustaq, 2016-08-25 15:00:33]

https://codereview.chromium.org/2274253002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/events/EventTarget.cpp (right):

https://codereview.chromium.org/2274253002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/events/EventTarget.cpp:604:
UseCounter::count(executingWindow->document(), feature);
On 2016/08/24 21:18:01, Navid Zolghadr wrote:
> This probably won't work as you expect. In the normal case (the way right now
> Chrome implements) after setPointerCapture (which makes hasPointerCapture
return
> true) there might be a boundary event when then pointer capture is processed.
> But that is different from the boundary events that we suppress while a
pointer
> is captured. Do we want to make a distinction there?

Yes, good catch: this will trigger when set/release capture is called for an
element that is not currently under the pointer. And this is true in different
ways for both our model of PointerEvent (no boundary event while captured) and
the Edge model.

I will add an API "hasProcessedPointerCapture". I will mark it as temporary
because of the confusion it would add to the API surface.

[mustaq, 2016-08-25 15:45:54]

https://codereview.chromium.org/2274253002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/events/EventTarget.cpp (right):

https://codereview.chromium.org/2274253002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/events/EventTarget.cpp:604:
UseCounter::count(executingWindow->document(), feature);
On 2016/08/25 15:00:32, mustaq wrote:
> On 2016/08/24 21:18:01, Navid Zolghadr wrote:
> > This probably won't work as you expect. In the normal case (the way right
now
> > Chrome implements) after setPointerCapture (which makes hasPointerCapture
> return
> > true) there might be a boundary event when then pointer capture is
processed.
> > But that is different from the boundary events that we suppress while a
> pointer
> > is captured. Do we want to make a distinction there?
> 
> Yes, good catch: this will trigger when set/release capture is called for an
> element that is not currently under the pointer. And this is true in different
> ways for both our model of PointerEvent (no boundary event while captured) and
> the Edge model.
> 
> I will add an API "hasProcessedPointerCapture". I will mark it as temporary
> because of the confusion it would add to the API surface.

Done, ptal. I didn't mark the new API as temporary because it could have other
uses.

[Navid Zolghadr, 2016-08-25 15:53:24]

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

https://codereview.chromium.org/2274253002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/input/PointerEventManager.cpp:770: return
m_pointerCaptureTarget.get(pointerId) == target;
This doesn't seem enough either. It has other blind spots. How about this:
m_pointerCaptureTarget.get(pointerId) == target && hasPointerCapture(pointerId,
target)
?

[mustaq, 2016-08-25 17:38:01]

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

https://codereview.chromium.org/2274253002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/input/PointerEventManager.cpp:770: return
m_pointerCaptureTarget.get(pointerId) == target;
On 2016/08/25 15:53:23, Navid Zolghadr wrote:
> This doesn't seem enough either. It has other blind spots. How about this:
> m_pointerCaptureTarget.get(pointerId) == target &&
hasPointerCapture(pointerId,
> target)
> ?

Changed the UseCounter condition instead. I kept my API unchanged because it can
provide more info about the state of capturing. E.g.
- hasPointerCapture XOR hasProcessedCapturing means transient state,
- !hasPointerCapture AND hasProcessedCapturing means "about to lose capture"
state etc.

[Navid Zolghadr, 2016-08-25 18:04:29]

lgtm

[bokan, 2016-08-25 18:05:04]

rs lgtm

[mustaq, 2016-08-25 18:08:55]

The CQ bit was checked by mustaq@chromium.org

[mustaq, 2016-08-25 18:08:55]

The patchset sent to the CQ was uploaded after l-g-t-m from
dtapuska@chromium.org
Link to the patchset: https://codereview.chromium.org/2274253002/#ps60001
(title: "Added hasProcessedPendingPointerCapture API.")

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

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

[mustaq, 2016-08-25 18:10:10]

The CQ bit was unchecked by mustaq@chromium.org

[mustaq, 2016-08-25 18:15:36]

The CQ bit was checked by mustaq@chromium.org

[mustaq, 2016-08-25 18:15:37]

The patchset sent to the CQ was uploaded after l-g-t-m from bokan@chromium.org,
dtapuska@chromium.org, nzolghadr@chromium.org
Link to the patchset: https://codereview.chromium.org/2274253002/#ps60002
(title: "Rebased.")

[commit-bot: I haz the power, 2016-08-25 18:16:32]

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-25 18:19:39]

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

[commit-bot: I haz the power, 2016-08-25 18:19:40]

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...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[mustaq, 2016-08-25 19:37:55]

The CQ bit was checked by mustaq@chromium.org

[mustaq, 2016-08-25 19:37:56]

The patchset sent to the CQ was uploaded after l-g-t-m from bokan@chromium.org,
dtapuska@chromium.org, nzolghadr@chromium.org
Link to the patchset: https://codereview.chromium.org/2274253002/#ps90001
(title: "Rebased.")

[commit-bot: I haz the power, 2016-08-25 19:38:44]

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-25 22:54:46]

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

[commit-bot: I haz the power, 2016-08-25 22:54:47]

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-26 14:37:34]

The CQ bit was checked by mustaq@chromium.org

[commit-bot: I haz the power, 2016-08-26 14:37:54]

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-26 16:25:54]

Description was changed from

==========
Added counters for pointer boundary events fired.

Covered both unconditional firing and firing for captured pointer ids.

BUG=640637
TBR=isherman@chromium.org
==========

to

==========
Added counters for pointer boundary events fired.

Covered both unconditional firing and firing for captured pointer ids.

BUG=640637
TBR=isherman@chromium.org
==========

[commit-bot: I haz the power, 2016-08-26 16:25:56]

Committed patchset #6 (id:90001)

[commit-bot: I haz the power, 2016-08-26 16:27:11]

Description was changed from

==========
Added counters for pointer boundary events fired.

Covered both unconditional firing and firing for captured pointer ids.

BUG=640637
TBR=isherman@chromium.org
==========

to

==========
Added counters for pointer boundary events fired.

Covered both unconditional firing and firing for captured pointer ids.

BUG=640637
TBR=isherman@chromium.org

Committed: https://crrev.com/44f9943d379d6647fa3ef799dc1c6fa427fcd835
Cr-Commit-Position: refs/heads/master@{#414728}
==========

[commit-bot: I haz the power, 2016-08-26 16:27:13]

Patchset 6 (id:??) landed as
https://crrev.com/44f9943d379d6647fa3ef799dc1c6fa427fcd835
Cr-Commit-Position: refs/heads/master@{#414728}