Move have_wheel_event_handlers to WebLayerTreeView.

Move have_wheel_event_handlers to WebLayerTreeView.

Since having wheel event handlers was based on the document in general
there was no need for the hit testing code for determining the layer in
cc. So it makes more sense to have these fields on the layer tree view.

BUG=489802
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/ee0b6981de62e9a234d031eab793da8d7752197b
Cr-Commit-Position: refs/heads/master@{#372346}

[dtapuska, 2016-01-27 16:56:06]

Description was changed from

==========
Move have_wheel_event_handlers to WebLayerTreeView.

Since having wheel event handlers was based on the document in general
there was no need for the hit testing code for determining the layer in
cc. So it makes more sense to have these fields on the layer tree view.

BUG=489802
==========

to

==========
Move have_wheel_event_handlers to WebLayerTreeView.

Since having wheel event handlers was based on the document in general
there was no need for the hit testing code for determining the layer in
cc. So it makes more sense to have these fields on the layer tree view.

BUG=489802
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[dtapuska, 2016-01-27 16:57:04]

Description was changed from

==========
Move have_wheel_event_handlers to WebLayerTreeView.

Since having wheel event handlers was based on the document in general
there was no need for the hit testing code for determining the layer in
cc. So it makes more sense to have these fields on the layer tree view.

BUG=489802
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
Move have_wheel_event_handlers to WebLayerTreeView.

Since having wheel event handlers was based on the document in general
there was no need for the hit testing code for determining the layer in
cc. So it makes more sense to have these fields on the layer tree view.

BUG=489802
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[dtapuska, 2016-01-27 16:57:04]

dtapuska@chromium.org changed reviewers:
+ aelias@chromium.org, rbyers@chromium.org

[dtapuska, 2016-01-27 16:57:37]

PTAL

[aelias_OOO_until_Jul13, 2016-01-27 23:55:46]

OK, looks pretty good.  The only awkward part of this that I didn't realize is
the profiling tools which still expect to get a rect for this.  I guess we can
keep supplying them a single rect covering the whole document.  At least, now
you're aware of the profiling use cases -- please make sure that passive event
listeners don't show up in those rects, because we don't need to alert
developers about the perf impact of those.

https://codereview.chromium.org/1639363002/diff/40001/cc/blink/web_layer_impl.cc
File cc/blink/web_layer_impl.cc (right):

https://codereview.chromium.org/1639363002/diff/40001/cc/blink/web_layer_impl...
cc/blink/web_layer_impl.cc:330: bool WebLayerImpl::haveWheelEventHandlers()
const {
Looks like this is only used by InspectorLayerTreeAgent.  It should probably
check if the layer is the root layer to avoid spammy duplication in the profiler
view.

https://codereview.chromium.org/1639363002/diff/40001/cc/debug/debug_rect_his...
File cc/debug/debug_rect_history.cc (right):

https://codereview.chromium.org/1639363002/diff/40001/cc/debug/debug_rect_his...
cc/debug/debug_rect_history.cc:184: if
(!layer->layer_tree_impl()->have_wheel_event_handlers())
I guess we also want to check if the layer is the root scrolling layer for the
purposes of this kind of profiling rect (I believe for chrome://tracing)

https://codereview.chromium.org/1639363002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/page/scrolling/ScrollingCoordinator.cpp
(right):

https://codereview.chromium.org/1639363002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/page/scrolling/ScrollingCoordinator.cpp:730:
m_webLayerTreeView = &layerTreeView;
I don't think we should make ScrollingCoordinator hold a pointer to the
layerTreeView.  ScrollingCoordinator::updateHaveWheelEventHandlers() has exactly
one caller, EventHandlerRegistry::notifyHasHandlersChanged().  So please delete
the ScrollingCoordinator methods, and instead change that site to use a path
similar to the one next to it for touch events.

[Rick Byers, 2016-01-28 00:18:32]

I can't help but feel like this is a step backwards - we do really need this
tracking to be more granular.  But since the new granular tracking is unlikely
to look anything like the existing per-layer tracking, I agree it's better to
just make the code reflect reality for now.

LGTM (modulo Alex's feedback of course).

https://codereview.chromium.org/1639363002/diff/40001/cc/trees/layer_tree_hos...
File cc/trees/layer_tree_host_common.cc (left):

https://codereview.chromium.org/1639363002/diff/40001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host_common.cc:1133:
recursive_data->num_layer_or_descendants_with_input_handler++;
nit: rename this from "input_handlers" to "touch_handlers" to be correct now.

[dtapuska, 2016-01-28 16:23:06]

PTAL

https://codereview.chromium.org/1639363002/diff/40001/cc/blink/web_layer_impl.cc
File cc/blink/web_layer_impl.cc (right):

https://codereview.chromium.org/1639363002/diff/40001/cc/blink/web_layer_impl...
cc/blink/web_layer_impl.cc:330: bool WebLayerImpl::haveWheelEventHandlers()
const {
On 2016/01/27 23:55:46, aelias wrote:
> Looks like this is only used by InspectorLayerTreeAgent.  It should probably
> check if the layer is the root layer to avoid spammy duplication in the
profiler
> view.

I've removed this method for clarity.

https://codereview.chromium.org/1639363002/diff/40001/cc/debug/debug_rect_his...
File cc/debug/debug_rect_history.cc (right):

https://codereview.chromium.org/1639363002/diff/40001/cc/debug/debug_rect_his...
cc/debug/debug_rect_history.cc:184: if
(!layer->layer_tree_impl()->have_wheel_event_handlers())
On 2016/01/27 23:55:46, aelias wrote:
> I guess we also want to check if the layer is the root scrolling layer for the
> purposes of this kind of profiling rect (I believe for chrome://tracing)

SaveWheelEventHandlerRects is only called on the root rect. This is enabled via
the Renderer Setttings in the inspector; it shows a rect for where there are
wheel handlers and it should be the entire page.

https://codereview.chromium.org/1639363002/diff/40001/cc/trees/layer_tree_hos...
File cc/trees/layer_tree_host_common.cc (left):

https://codereview.chromium.org/1639363002/diff/40001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host_common.cc:1133:
recursive_data->num_layer_or_descendants_with_input_handler++;
On 2016/01/28 00:18:31, Rick Byers wrote:
> nit: rename this from "input_handlers" to "touch_handlers" to be correct now.

Done.

https://codereview.chromium.org/1639363002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/page/scrolling/ScrollingCoordinator.cpp
(right):

https://codereview.chromium.org/1639363002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/page/scrolling/ScrollingCoordinator.cpp:730:
m_webLayerTreeView = &layerTreeView;
On 2016/01/27 23:55:46, aelias wrote:
> I don't think we should make ScrollingCoordinator hold a pointer to the
> layerTreeView.  ScrollingCoordinator::updateHaveWheelEventHandlers() has
exactly
> one caller, EventHandlerRegistry::notifyHasHandlersChanged().  So please
delete
> the ScrollingCoordinator methods, and instead change that site to use a path
> similar to the one next to it for touch events.

Done.

[aelias_OOO_until_Jul13, 2016-01-28 21:16:45]

lgtm, thanks!

[dtapuska, 2016-01-28 21:54:43]

The CQ bit was checked by dtapuska@chromium.org

[dtapuska, 2016-01-28 21:54:44]

The patchset sent to the CQ was uploaded after l-g-t-m from rbyers@chromium.org,
aelias@chromium.org
Link to the patchset: https://codereview.chromium.org/1639363002/#ps120001
(title: "Rebase (resolve scroll blocks removal) conflicts")

[commit-bot: I haz the power, 2016-01-28 21:55:20]

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

[commit-bot: I haz the power, 2016-01-28 22:08:50]

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

[commit-bot: I haz the power, 2016-01-28 22:08:51]

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

[dtapuska, 2016-01-28 22:11:16]

dtapuska@chromium.org changed reviewers:
+ creis@chromium.org

[dtapuska, 2016-01-28 22:11:36]

+creis for content/*

[Charlie Reis, 2016-01-28 23:44:01]

Rubber stamp LGTM for content/, since I don't know this code very well.

https://codereview.chromium.org/1639363002/diff/120001/content/test/web_layer...
File content/test/web_layer_tree_view_impl_for_testing.h (right):

https://codereview.chromium.org/1639363002/diff/120001/content/test/web_layer...
content/test/web_layer_tree_view_impl_for_testing.h:70: 
nit: No blank line here, since it's still part of the WebLayerTreeView block of
overrides.

[dtapuska, 2016-01-29 14:03:31]

The CQ bit was checked by dtapuska@chromium.org

[dtapuska, 2016-01-29 14:03:32]

The patchset sent to the CQ was uploaded after l-g-t-m from aelias@chromium.org,
creis@chromium.org, rbyers@chromium.org
Link to the patchset: https://codereview.chromium.org/1639363002/#ps140001
(title: "Remove blank line")

[commit-bot: I haz the power, 2016-01-29 14:03:43]

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

[commit-bot: I haz the power, 2016-01-29 15:14:55]

Description was changed from

==========
Move have_wheel_event_handlers to WebLayerTreeView.

Since having wheel event handlers was based on the document in general
there was no need for the hit testing code for determining the layer in
cc. So it makes more sense to have these fields on the layer tree view.

BUG=489802
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
Move have_wheel_event_handlers to WebLayerTreeView.

Since having wheel event handlers was based on the document in general
there was no need for the hit testing code for determining the layer in
cc. So it makes more sense to have these fields on the layer tree view.

BUG=489802
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[commit-bot: I haz the power, 2016-01-29 15:14:57]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2016-01-29 15:15:42]

Description was changed from

==========
Move have_wheel_event_handlers to WebLayerTreeView.

Since having wheel event handlers was based on the document in general
there was no need for the hit testing code for determining the layer in
cc. So it makes more sense to have these fields on the layer tree view.

BUG=489802
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
Move have_wheel_event_handlers to WebLayerTreeView.

Since having wheel event handlers was based on the document in general
there was no need for the hit testing code for determining the layer in
cc. So it makes more sense to have these fields on the layer tree view.

BUG=489802
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/ee0b6981de62e9a234d031eab793da8d7752197b
Cr-Commit-Position: refs/heads/master@{#372346}
==========

[commit-bot: I haz the power, 2016-01-29 15:15:43]

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

[dtapuska, 2016-01-29 20:20:16]

Patchset #9 (id:160001) has been deleted