Bookkeep the pointer event listeners added to page

Bookkeep the pointer event listeners added to page

Check if the page has pointer event listeners
before firing the pointer events.

BUG=679828
Review-Url: https://codereview.chromium.org/2916893003
Cr-Commit-Position: refs/heads/master@{#477743}
Committed: https://chromium.googlesource.com/chromium/src/+/6026189765a3923e474f2431524199934c6b162d

[eirage, 2017-06-01 15:57:55]

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

[commit-bot: I haz the power, 2017-06-01 15:58:14]

Dry run: CQ is trying da patch.

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

[eirage, 2017-06-01 15:58:47]

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

[dtapuska, 2017-06-01 16:03:01]

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

https://codereview.chromium.org/2916893003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/input/PointerEventManager.cpp:168:
!RuntimeEnabledFeatures::pointerEventEnabled())
We should probably log a bug to remove
RuntimeEnabledFeatures::pointerEventEnabled no? Since it has long shipped?

https://codereview.chromium.org/2916893003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/testing/Internals.cpp (right):

https://codereview.chromium.org/2916893003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/testing/Internals.cpp:1611: unsigned
Internals::pointerEventHandlerCount(Document* document) {
This should be able to be const.

[Navid Zolghadr, 2017-06-01 16:10:41]

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

https://codereview.chromium.org/2916893003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/input/PointerEventManager.cpp:168:
!RuntimeEnabledFeatures::pointerEventEnabled())
On 2017/06/01 16:03:01, dtapuska wrote:
> We should probably log a bug to remove
> RuntimeEnabledFeatures::pointerEventEnabled no? Since it has long shipped?

We have a bug for this:
crbug.com/687976

[Navid Zolghadr, 2017-06-01 16:26:13]

https://codereview.chromium.org/2916893003/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/fast/events/pointerevents/pointer-handler-count.html
(right):

https://codereview.chromium.org/2916893003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/fast/events/pointerevents/pointer-handler-count.html:5:
var listener = function() { }
Can you also add another scenario for testing all possible pointerevent
listeners to make sure we don't miss anyone:

pointerenter
pointerover
pointerleave
pointerout
pointermove
pointerup
pointerdown
pointercancel
gotpointercapture
lostpointercapture

https://codereview.chromium.org/2916893003/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/fast/events/pointerevents/pointer-input-element-change-documents-expected.txt
(right):

https://codereview.chromium.org/2916893003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/fast/events/pointerevents/pointer-input-element-change-documents-expected.txt:1:
This test checks that we correctly update the pointer event handler count when
an Input element with default pointer handlers changes documents.
Is this file needed?

[eirage, 2017-06-01 17:06:06]

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

[commit-bot: I haz the power, 2017-06-01 17:06:43]

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-06-01 17:14:40]

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

[commit-bot: I haz the power, 2017-06-01 17:14:42]

Dry run: Try jobs failed on following builders:
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  android_cronet on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_cron...)
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_headless_rel on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[eirage, 2017-06-01 19:47:49]

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

[commit-bot: I haz the power, 2017-06-01 19:48:38]

Dry run: CQ is trying da patch.

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

[eirage, 2017-06-01 19:59:24]

ptal

https://codereview.chromium.org/2916893003/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/fast/events/pointerevents/pointer-handler-count.html
(right):

https://codereview.chromium.org/2916893003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/fast/events/pointerevents/pointer-handler-count.html:5:
var listener = function() { }
On 2017/06/01 16:26:13, Navid Zolghadr wrote:
> Can you also add another scenario for testing all possible pointerevent
> listeners to make sure we don't miss anyone:
> 
> pointerenter
> pointerover
> pointerleave
> pointerout
> pointermove
> pointerup
> pointerdown
> pointercancel
> gotpointercapture
> lostpointercapture

Done.

https://codereview.chromium.org/2916893003/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/fast/events/pointerevents/pointer-input-element-change-documents-expected.txt
(right):

https://codereview.chromium.org/2916893003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/fast/events/pointerevents/pointer-input-element-change-documents-expected.txt:1:
This test checks that we correctly update the pointer event handler count when
an Input element with default pointer handlers changes documents.
On 2017/06/01 16:26:13, Navid Zolghadr wrote:
> Is this file needed?

This is not needed. I forgot to remove this file.

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

https://codereview.chromium.org/2916893003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/input/PointerEventManager.cpp:168:
!RuntimeEnabledFeatures::pointerEventEnabled())
On 2017/06/01 16:10:41, Navid Zolghadr wrote:
> On 2017/06/01 16:03:01, dtapuska wrote:
> > We should probably log a bug to remove
> > RuntimeEnabledFeatures::pointerEventEnabled no? Since it has long shipped?
> 
> We have a bug for this:
> crbug.com/687976

Acknowledged.

https://codereview.chromium.org/2916893003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/testing/Internals.cpp (right):

https://codereview.chromium.org/2916893003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/testing/Internals.cpp:1611: unsigned
Internals::pointerEventHandlerCount(Document* document) {
On 2017/06/01 16:03:01, dtapuska wrote:
> This should be able to be const.

Done.

[Navid Zolghadr, 2017-06-01 20:31:59]

lgtm

[dtapuska, 2017-06-01 20:34:48]

On 2017/06/01 20:31:59, Navid Zolghadr wrote:
> lgtm

lgtm

[commit-bot: I haz the power, 2017-06-01 21:16:23]

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

[commit-bot: I haz the power, 2017-06-01 21:16:25]

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_...)

[eirage, 2017-06-01 23:20:16]

The CQ bit was checked by eirage@chromium.org

[eirage, 2017-06-01 23:20:17]

The CQ bit was unchecked by eirage@chromium.org

[eirage, 2017-06-02 14:52:38]

The CQ bit was checked by eirage@chromium.org

[eirage, 2017-06-02 14:52:39]

The CQ bit was unchecked by eirage@chromium.org

[dtapuska, 2017-06-05 17:11:49]

The CQ bit was checked by dtapuska@chromium.org

[commit-bot: I haz the power, 2017-06-05 17:12:01]

CQ is trying da patch.

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

[dtapuska, 2017-06-05 17:12:32]

The CQ bit was unchecked by dtapuska@chromium.org

[eirage, 2017-06-05 17:30:59]

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

[commit-bot: I haz the power, 2017-06-05 17:31:37]

Dry run: CQ is trying da patch.

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

[eirage, 2017-06-05 17:35:57]

Patchset #3 (id:40001) has been deleted

[commit-bot: I haz the power, 2017-06-05 17:36:46]

Dry run: Try jobs failed on following builders:
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)

[eirage, 2017-06-05 17:51:01]

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

[commit-bot: I haz the power, 2017-06-05 17:51:24]

Dry run: CQ is trying da patch.

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

[eirage, 2017-06-05 18:01:44]

The CQ bit was unchecked by eirage@chromium.org

[eirage, 2017-06-05 18:01:54]

The CQ bit was checked by eirage@chromium.org

[eirage, 2017-06-05 18:01:54]

The patchset sent to the CQ was uploaded after l-g-t-m from
dtapuska@chromium.org, nzolghadr@chromium.org
Link to the patchset: https://codereview.chromium.org/2916893003/#ps60001
(title: "add pointer event handler in SliderThumbelement")

[commit-bot: I haz the power, 2017-06-05 18:02:14]

CQ is trying da patch.

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

[dtapuska, 2017-06-05 18:31:37]

On 2017/06/05 18:02:14, commit-bot: I haz the power wrote:
> CQ is trying da patch.
> 
> Follow status at:
>
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

still lgtm

[commit-bot: I haz the power, 2017-06-05 19:30:29]

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

[commit-bot: I haz the power, 2017-06-05 19:30:30]

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

[eirage, 2017-06-06 16:01:25]

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

[commit-bot: I haz the power, 2017-06-06 16:01:38]

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-06-06 17:48:00]

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

[commit-bot: I haz the power, 2017-06-06 17:48:02]

Dry run: This issue passed the CQ dry run.

[eirage, 2017-06-06 18:57:52]

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

[commit-bot: I haz the power, 2017-06-06 18:58:24]

Dry run: CQ is trying da patch.

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

[eirage, 2017-06-06 19:00:01]

I uploaded a new patch, ptal

[Navid Zolghadr, 2017-06-06 19:09:21]

https://codereview.chromium.org/2916893003/diff/100001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/inspector/tracing/timeline-misc/timeline-event-dispatch-expected.txt
(left):

https://codereview.chromium.org/2916893003/diff/100001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/inspector/tracing/timeline-misc/timeline-event-dispatch-expected.txt:8:
type : "pointerover"
Dave, I don't recall the purpose of this test particularly. But on a second
thought, should we just add the pointer event listener to the test so we keep
these in the expected file instead or do you think this is alright?

[commit-bot: I haz the power, 2017-06-06 20:43:48]

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

[commit-bot: I haz the power, 2017-06-06 20:43:50]

Dry run: This issue passed the CQ dry run.

[dtapuska, 2017-06-07 15:58:51]

On 2017/06/06 19:09:21, Navid Zolghadr wrote:
>
https://codereview.chromium.org/2916893003/diff/100001/third_party/WebKit/Lay...
> File
>
third_party/WebKit/LayoutTests/inspector/tracing/timeline-misc/timeline-event-dispatch-expected.txt
> (left):
> 
>
https://codereview.chromium.org/2916893003/diff/100001/third_party/WebKit/Lay...
>
third_party/WebKit/LayoutTests/inspector/tracing/timeline-misc/timeline-event-dispatch-expected.txt:8:
> type : "pointerover"
> Dave, I don't recall the purpose of this test particularly. But on a second
> thought, should we just add the pointer event listener to the test so we keep
> these in the expected file instead or do you think this is alright?

Changing the expected results seems fine to me. We have lots of tests already
asserting the ordering of pointer and mouse events.

lgtm

[dtapuska, 2017-06-07 15:58:52]

On 2017/06/06 19:09:21, Navid Zolghadr wrote:
>
https://codereview.chromium.org/2916893003/diff/100001/third_party/WebKit/Lay...
> File
>
third_party/WebKit/LayoutTests/inspector/tracing/timeline-misc/timeline-event-dispatch-expected.txt
> (left):
> 
>
https://codereview.chromium.org/2916893003/diff/100001/third_party/WebKit/Lay...
>
third_party/WebKit/LayoutTests/inspector/tracing/timeline-misc/timeline-event-dispatch-expected.txt:8:
> type : "pointerover"
> Dave, I don't recall the purpose of this test particularly. But on a second
> thought, should we just add the pointer event listener to the test so we keep
> these in the expected file instead or do you think this is alright?

Changing the expected results seems fine to me. We have lots of tests already
asserting the ordering of pointer and mouse events.

lgtm

[eirage, 2017-06-07 15:59:17]

The CQ bit was checked by eirage@chromium.org

[eirage, 2017-06-07 15:59:17]

The patchset sent to the CQ was uploaded after l-g-t-m from
nzolghadr@chromium.org
Link to the patchset: https://codereview.chromium.org/2916893003/#ps100001
(title: "do DidRemoveAllEventHandlers in two loop")

[commit-bot: I haz the power, 2017-06-07 15:59: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, 2017-06-07 16:10:09]

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

[commit-bot: I haz the power, 2017-06-07 16:10:10]

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...)

[eirage, 2017-06-07 16:20:13]

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

[commit-bot: I haz the power, 2017-06-07 16:20:34]

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-06-07 16:30:59]

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

[commit-bot: I haz the power, 2017-06-07 16:31:00]

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

[eirage, 2017-06-07 17:45:10]

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

[commit-bot: I haz the power, 2017-06-07 17:46:29]

Dry run: CQ is trying da patch.

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

[dtapuska, 2017-06-07 19:35:51]

The CQ bit was checked by dtapuska@chromium.org

[dtapuska, 2017-06-07 19:35:51]

The patchset sent to the CQ was uploaded after l-g-t-m from
dtapuska@chromium.org, nzolghadr@chromium.org
Link to the patchset: https://codereview.chromium.org/2916893003/#ps140001
(title: "fix rebase error")

[dtapuska, 2017-06-07 19:35:52]

The CQ bit was unchecked by dtapuska@chromium.org

[dtapuska, 2017-06-07 19:36:05]

The CQ bit was checked by dtapuska@chromium.org

[dtapuska, 2017-06-07 19:36:06]

The patchset sent to the CQ was uploaded after l-g-t-m from
dtapuska@chromium.org, nzolghadr@chromium.org
Link to the patchset: https://codereview.chromium.org/2916893003/#ps140001
(title: "fix rebase error")

[commit-bot: I haz the power, 2017-06-07 19:36:26]

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-06-07 20:14:47]

CQ is committing da patch.

Bot data: {"patchset_id": 140001, "attempt_start_ts": 1496864165988740,
"parent_rev": "0527f72f6822c6aba0370d8245bd9acb6715a2f8", "commit_rev":
"6026189765a3923e474f2431524199934c6b162d"}

[commit-bot: I haz the power, 2017-06-07 20:15:03]

Description was changed from

==========
Bookkeep the pointer event listeners added to page

Check if the page has pointer event listeners
before firing the pointer events.

BUG=679828
==========

to

==========
Bookkeep the pointer event listeners added to page

Check if the page has pointer event listeners
before firing the pointer events.

BUG=679828

Review-Url: https://codereview.chromium.org/2916893003
Cr-Commit-Position: refs/heads/master@{#477743}
Committed:
https://chromium.googlesource.com/chromium/src/+/6026189765a3923e474f24315241...
==========

[commit-bot: I haz the power, 2017-06-07 20:15:05]

Committed patchset #7 (id:140001) as
https://chromium.googlesource.com/chromium/src/+/6026189765a3923e474f24315241...