Commit 6a4a425f authored by David Bokan's avatar David Bokan Committed by Commit Bot

[BlinkGenPropertyTrees] Fix layer commit for wheel handler region

When a wheel event listener is added or removed on a page, we set each
layer as having a WheelEventHandlerRegion equal to the entire bounds of
the layer. This happens in Layer::PushPropertiesTo.

This means that when an event listener is changed, we need to cause all
layers to need a push properties and commit. Prior to this CL, this
happens in LayerTreeHost::SetEventListenerProperties by setting
SetNeedsFullTreeSync and SetSubtreePropertyChanged. However, as noted by
surrounding comments, this looks wrong; SetNeedsFullTreeSync only causes
the tree structure to be synced, properties aren't pushed.
SetSubtreePropertyChanged causes a property sync only on the root layer.

This works prior to BGPT because SetNeedsFullTreeSync causes a property
tree rebuild during which all layers are marked as needing a property
push. See PropertyTreeBuilderContext::BuildPropertyTreesInternal which
calls SetLayerPropertyTreeChangedForChild for all child layers as the
tree is built.

When BGPT is turned on, property trees are built in Blink independent of
the layer tree so this doesn't cause us to SetNeedsPushProperties. This
CL fixes SetEventListenerProperties to set the correct flags to
propagate the listener state to the active tree.

This CL fixes:

PointerLockBrowserTest.PointerLockWheelEventRouting
TouchpadPinchBrowserTest.WheelListenerAllowingPinch/0
TouchpadPinchBrowserTest.WheelListenerAllowingPinch/1

With BGPT turned on.

Bug: 912334,881011
Change-Id: I4446ccfd32de12e618219e5fa15c48f5093bcc98
Reviewed-on: https://chromium-review.googlesource.com/c/1369059Reviewed-by: default avatarPhilip Rogers <pdr@chromium.org>
Commit-Queue: David Bokan <bokan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#615579}
parent 0d5d9e6b
......@@ -1102,18 +1102,9 @@ void LayerTreeHost::SetEventListenerProperties(
// layer tree sets a wheel event handler region to be its entire bounds,
// otherwise it sets it to empty.
//
// Thus when it changes, we might want to request every layer to push
// properties and recompute its wheel event handler region, since the
// computation is done in PushPropertiesTo. However neither
// SetSubtreePropertyChanged() nor SetNeedsFullTreeSync() do this, so
// it is unclear why we call them.
// Also why we don't want to recompute the wheel event handler region for all
// layers when the blocking state goes away is unclear. Also why we mark all
// layers below the root layer as damaged is unclear.
// TODO(bokan): Sort out what should be set and why. https://crbug.com/881011
//
// TODO(sunxd): Remove NeedsFullTreeSync when computing mouse wheel event
// handler region is done.
// Thus when it changes, we want to request every layer to push properties
// and recompute its wheel event handler region, since the computation is
// done in PushPropertiesTo.
if (event_class == EventListenerClass::kMouseWheel) {
bool new_property_is_blocking =
properties == EventListenerProperties::kBlocking ||
......@@ -1123,10 +1114,9 @@ void LayerTreeHost::SetEventListenerProperties(
old_properties == EventListenerProperties::kBlocking ||
old_properties == EventListenerProperties::kBlockingAndPassive;
if (!old_property_is_blocking && new_property_is_blocking) {
if (root_layer())
root_layer()->SetSubtreePropertyChanged();
SetNeedsFullTreeSync();
if (old_property_is_blocking != new_property_is_blocking) {
LayerTreeHostCommon::CallFunctionForEveryLayer(
this, [](Layer* layer) { layer->SetNeedsPushProperties(); });
}
}
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment