Commit 65585821 authored by Olga Gerchikov's avatar Olga Gerchikov Committed by Commit Bot

Optimize kWheelEventRegions checks

CL [1] introduced performance regressions in number of benchmarks.
The regression is caused by the fact of querying the value of
kWheelEventRegions flag.

This CL optimizes the querying. Most of the regressions partially or
fully reverted.

[1]https://chromium-review.googlesource.com/c/chromium/src/+/2419718

Bug: 1140321
Change-Id: Ib0cd7cfbafa0603579459e36853f993d2133a969
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2490327Reviewed-by: default avatarvmpstr <vmpstr@chromium.org>
Reviewed-by: default avatarSam Fortiner <samfort@microsoft.com>
Reviewed-by: default avatarStephen Chenney <schenney@chromium.org>
Commit-Queue: Olga Gerchikov <gerchiko@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#823253}
parent c99d3a88
......@@ -1321,21 +1321,20 @@ void Layer::PushPropertiesTo(LayerImpl* layer) {
layer->set_may_contain_video(may_contain_video_);
layer->SetNonFastScrollableRegion(inputs_.non_fast_scrollable_region);
layer->SetTouchActionRegion(inputs_.touch_action_region);
if (!base::FeatureList::IsEnabled(::features::kWheelEventRegions)) {
// TODO(sunxd): Pass the correct region for wheel event handlers, see
// https://crbug.com/841364.
EventListenerProperties mouse_wheel_props =
layer_tree_host()->event_listener_properties(
EventListenerClass::kMouseWheel);
if (mouse_wheel_props == EventListenerProperties::kBlocking ||
mouse_wheel_props == EventListenerProperties::kBlockingAndPassive) {
layer->SetWheelEventHandlerRegion(Region(gfx::Rect(bounds())));
} else {
layer->SetWheelEventHandlerRegion(Region());
}
} else {
// TODO(https://crbug.com/841364): This block is optimized to avoid checks
// for kWheelEventRegions. It will be simplified once kWheelEventRegions
// feature flag is removed.
EventListenerProperties mouse_wheel_props =
layer_tree_host()->event_listener_properties(
EventListenerClass::kMouseWheel);
if ((mouse_wheel_props == EventListenerProperties::kBlocking ||
mouse_wheel_props == EventListenerProperties::kBlockingAndPassive) &&
!base::FeatureList::IsEnabled(::features::kWheelEventRegions))
layer->SetWheelEventHandlerRegion(Region(gfx::Rect(bounds())));
else
layer->SetWheelEventHandlerRegion(inputs_.wheel_event_region);
}
layer->SetContentsOpaque(inputs_.contents_opaque);
layer->SetContentsOpaqueForText(inputs_.contents_opaque_for_text);
layer->SetShouldCheckBackfaceVisibility(should_check_backface_visibility_);
......
......@@ -1208,28 +1208,26 @@ void LayerTreeHost::SetEventListenerProperties(
if (event_listener_properties_[index] == properties)
return;
if (!base::FeatureList::IsEnabled(::features::kWheelEventRegions)) {
// If the mouse wheel event listener is blocking, then every layer in the
// layer tree sets a wheel event handler region to be its entire bounds,
// otherwise it sets it to empty.
//
// 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 ||
properties == EventListenerProperties::kBlockingAndPassive;
EventListenerProperties old_properties =
event_listener_properties_[index];
bool old_property_is_blocking =
old_properties == EventListenerProperties::kBlocking ||
old_properties == EventListenerProperties::kBlockingAndPassive;
if (old_property_is_blocking != new_property_is_blocking) {
for (auto* layer : *this)
layer->SetNeedsPushProperties();
}
// If the mouse wheel event listener is blocking, then every layer in the
// layer tree sets a wheel event handler region to be its entire bounds,
// otherwise it sets it to empty.
//
// 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 &&
!base::FeatureList::IsEnabled(::features::kWheelEventRegions)) {
bool new_property_is_blocking =
properties == EventListenerProperties::kBlocking ||
properties == EventListenerProperties::kBlockingAndPassive;
EventListenerProperties old_properties = event_listener_properties_[index];
bool old_property_is_blocking =
old_properties == EventListenerProperties::kBlocking ||
old_properties == EventListenerProperties::kBlockingAndPassive;
if (old_property_is_blocking != new_property_is_blocking) {
for (auto* layer : *this)
layer->SetNeedsPushProperties();
}
}
......
......@@ -3012,7 +3012,14 @@ class CORE_EXPORT LayoutObject : public ImageResourceObserver,
// PrePaint tree walk to update blocking wheel event handler state.
void MarkBlockingWheelEventHandlerChanged();
bool BlockingWheelEventHandlerChanged() const {
return bitfields_.BlockingWheelEventHandlerChanged();
// TODO(https://crbug.com/841364): This block is optimized to avoid costly
// checks for kWheelEventRegions. It will be simplified once
// kWheelEventRegions feature flag is removed.
if (!bitfields_.BlockingWheelEventHandlerChanged())
return false;
return base::FeatureList::IsEnabled(::features::kWheelEventRegions)
? bitfields_.BlockingWheelEventHandlerChanged()
: false;
}
bool DescendantBlockingWheelEventHandlerChanged() const {
return bitfields_.DescendantBlockingWheelEventHandlerChanged();
......@@ -3734,8 +3741,7 @@ class CORE_EXPORT LayoutObject : public ImageResourceObserver,
effective_allowed_touch_action_changed_(true),
descendant_effective_allowed_touch_action_changed_(false),
inside_blocking_wheel_event_handler_(false),
blocking_wheel_event_handler_changed_(
base::FeatureList::IsEnabled(::features::kWheelEventRegions)),
blocking_wheel_event_handler_changed_(true),
descendant_blocking_wheel_event_handler_changed_(false),
is_effective_root_scroller_(false),
is_global_root_scroller_(false),
......
......@@ -282,7 +282,8 @@ void PrePaintTreeWalk::Walk(LocalFrameView& frame_view) {
}
#endif
Walk(*view, /* iterator */ nullptr);
Walk(*view, /* iterator */ nullptr,
base::FeatureList::IsEnabled(::features::kWheelEventRegions));
#if DCHECK_IS_ON()
view->AssertSubtreeClearedPaintInvalidationFlags();
#endif
......@@ -556,7 +557,8 @@ void PrePaintTreeWalk::UpdatePaintInvalidationContainer(
void PrePaintTreeWalk::WalkInternal(const LayoutObject& object,
const NGFragmentChildIterator* iterator,
PrePaintTreeWalkContext& context) {
PrePaintTreeWalkContext& context,
bool is_wheel_event_regions_enabled) {
PaintInvalidatorContext& paint_invalidator_context =
context.paint_invalidator_context;
......@@ -598,7 +600,7 @@ void PrePaintTreeWalk::WalkInternal(const LayoutObject& object,
// depends on the effective allowed touch action and blocking wheel event
// handlers.
UpdateEffectiveAllowedTouchAction(object, context);
if (base::FeatureList::IsEnabled(::features::kWheelEventRegions))
if (is_wheel_event_regions_enabled)
UpdateBlockingWheelEventHandler(object, context);
if (paint_invalidator_.InvalidatePaint(
......@@ -676,14 +678,16 @@ LocalFrameView* FindWebViewPluginContentFrameView(
}
void PrePaintTreeWalk::WalkNGChildren(const LayoutObject* parent,
NGFragmentChildIterator* iterator) {
NGFragmentChildIterator* iterator,
bool is_wheel_event_regions_enabled) {
for (; !iterator->IsAtEnd(); iterator->Advance()) {
const LayoutObject* object = (*iterator)->GetLayoutObject();
if (const auto* fragment_item = (*iterator)->FragmentItem()) {
// Line boxes are not interesting. They have no paint effects. Descend
// directly into children.
if (fragment_item->Type() == NGFragmentItem::kLine) {
WalkChildren(/* parent */ nullptr, iterator);
WalkChildren(/* parent */ nullptr, iterator,
is_wheel_event_regions_enabled);
continue;
}
} else if (!object) {
......@@ -710,12 +714,13 @@ void PrePaintTreeWalk::WalkNGChildren(const LayoutObject* parent,
context.fixed_position = *containing_block_context;
}
}
WalkChildren(/* parent */ nullptr, iterator);
WalkChildren(/* parent */ nullptr, iterator,
is_wheel_event_regions_enabled);
if (containing_block_context)
containing_block_context->paint_offset -= offset;
continue;
}
Walk(*object, iterator);
Walk(*object, iterator, is_wheel_event_regions_enabled);
}
const LayoutBlockFlow* parent_block = DynamicTo<LayoutBlockFlow>(parent);
......@@ -731,7 +736,8 @@ void PrePaintTreeWalk::WalkNGChildren(const LayoutObject* parent,
}
}
void PrePaintTreeWalk::WalkLegacyChildren(const LayoutObject& object) {
void PrePaintTreeWalk::WalkLegacyChildren(const LayoutObject& object,
bool is_wheel_event_regions_enabled) {
if (const LayoutBox* layout_box = ToLayoutBoxOrNull(&object)) {
if (layout_box->CanTraversePhysicalFragments()) {
// Enter NG child fragment traversal. We'll stay in this mode for all
......@@ -744,7 +750,7 @@ void PrePaintTreeWalk::WalkLegacyChildren(const LayoutObject& object) {
To<NGPhysicalBoxFragment>(*layout_box->GetPhysicalFragment(0));
DCHECK(!fragment.BreakToken());
NGFragmentChildIterator child_iterator(fragment);
WalkNGChildren(&object, &child_iterator);
WalkNGChildren(&object, &child_iterator, is_wheel_event_regions_enabled);
return;
}
}
......@@ -757,7 +763,7 @@ void PrePaintTreeWalk::WalkLegacyChildren(const LayoutObject& object) {
// way).
if (const LayoutBox* legend =
LayoutFieldset::FindInFlowLegend(To<LayoutBlock>(object)))
Walk(*legend, /* iterator */ nullptr);
Walk(*legend, /* iterator */ nullptr, is_wheel_event_regions_enabled);
}
for (const LayoutObject* child = object.SlowFirstChild(); child;
......@@ -785,7 +791,7 @@ void PrePaintTreeWalk::WalkLegacyChildren(const LayoutObject& object) {
if (UNLIKELY(child->IsRenderedLegend()))
continue;
Walk(*child, /* iterator */ nullptr);
Walk(*child, /* iterator */ nullptr, is_wheel_event_regions_enabled);
}
if (!RuntimeEnabledFeatures::LayoutNGFragmentTraversalEnabled())
......@@ -829,17 +835,18 @@ void PrePaintTreeWalk::WalkLegacyChildren(const LayoutObject& object) {
!ObjectRequiresTreeBuilderContext(*box))
continue;
DCHECK_EQ(box->Container(), &object);
Walk(*box, /* iterator */ nullptr);
Walk(*box, /* iterator */ nullptr, is_wheel_event_regions_enabled);
}
}
void PrePaintTreeWalk::WalkChildren(const LayoutObject* object,
const NGFragmentChildIterator* iterator) {
const NGFragmentChildIterator* iterator,
bool is_wheel_event_regions_enabled) {
DCHECK(iterator || object);
if (!iterator) {
// We're not doing LayoutNG fragment traversal of this object.
WalkLegacyChildren(*object);
WalkLegacyChildren(*object, is_wheel_event_regions_enabled);
return;
}
......@@ -853,17 +860,18 @@ void PrePaintTreeWalk::WalkChildren(const LayoutObject* object,
(object->IsBox() &&
ToLayoutBox(object)->GetNGPaginationBreakability() ==
LayoutBox::kForbidBreaks));
WalkLegacyChildren(*object);
WalkLegacyChildren(*object, is_wheel_event_regions_enabled);
return;
}
// Traverse child NG fragments.
NGFragmentChildIterator child_iterator(iterator->Descend());
WalkNGChildren(object, &child_iterator);
WalkNGChildren(object, &child_iterator, is_wheel_event_regions_enabled);
}
void PrePaintTreeWalk::Walk(const LayoutObject& object,
const NGFragmentChildIterator* iterator) {
const NGFragmentChildIterator* iterator,
bool is_wheel_event_regions_enabled) {
const NGPhysicalBoxFragment* physical_fragment = nullptr;
bool is_last_fragment = true;
if (iterator) {
......@@ -918,7 +926,7 @@ void PrePaintTreeWalk::Walk(const LayoutObject& object,
context().tree_builder_context->clip_changed = false;
}
WalkInternal(object, iterator, context());
WalkInternal(object, iterator, context(), is_wheel_event_regions_enabled);
bool child_walk_blocked = object.ChildPrePaintBlockedByDisplayLock();
// If we need a subtree walk due to context flags, we need to store that
......@@ -938,7 +946,7 @@ void PrePaintTreeWalk::Walk(const LayoutObject& object,
}
if (!child_walk_blocked) {
WalkChildren(&object, iterator);
WalkChildren(&object, iterator, is_wheel_event_regions_enabled);
if (object.IsLayoutEmbeddedContent()) {
const LayoutEmbeddedContent& layout_embedded_content =
......
......@@ -122,13 +122,24 @@ class CORE_EXPORT PrePaintTreeWalk {
// very big stack frames. Splitting the heavy lifting to a separate function
// makes sure the stack frame is freed prior to making a recursive call.
// See https://crbug.com/781301 .
// TODO(https://crbug.com/841364): Remove is_wheel_event_regions_enabled
// argument once kWheelEventRegions feature flag is removed.
NOINLINE void WalkInternal(const LayoutObject&,
const NGFragmentChildIterator*,
PrePaintTreeWalkContext&);
void WalkNGChildren(const LayoutObject* parent, NGFragmentChildIterator*);
void WalkLegacyChildren(const LayoutObject&);
void WalkChildren(const LayoutObject*, const NGFragmentChildIterator*);
void Walk(const LayoutObject&, const NGFragmentChildIterator*);
PrePaintTreeWalkContext&,
bool is_wheel_event_regions_enabled);
void WalkNGChildren(const LayoutObject* parent,
NGFragmentChildIterator*,
bool is_wheel_event_regions_enabled);
void WalkLegacyChildren(const LayoutObject&,
bool is_wheel_event_regions_enabled);
void WalkChildren(const LayoutObject*,
const NGFragmentChildIterator*,
bool is_wheel_event_regions_enabled);
void Walk(const LayoutObject&,
const NGFragmentChildIterator*,
bool is_wheel_event_regions_enabled);
bool NeedsTreeBuilderContextUpdate(const LocalFrameView&,
const PrePaintTreeWalkContext&);
......
......@@ -947,7 +947,11 @@ static void UpdateTouchActionWheelEventHandlerAndNonFastScrollableRegions(
auto chunk_state = chunk.properties.GetPropertyTreeState().Unalias();
UpdateTouchActionRegion(*chunk.hit_test_data, layer_state, chunk_state,
layer_offset, touch_action_region);
if (base::FeatureList::IsEnabled(::features::kWheelEventRegions)) {
// TODO(https://crbug.com/841364): Checking for empty rect here is to avoid
// costly checks for kWheelEventRegions. This "if" condition will be gone
// once kWheelEventRegions feature flag is removed.
if (!chunk.hit_test_data->wheel_event_rects.IsEmpty() &&
base::FeatureList::IsEnabled(::features::kWheelEventRegions)) {
UpdateWheelEventRegion(*chunk.hit_test_data, layer_state, chunk_state,
layer_offset, wheel_event_region);
}
......@@ -956,8 +960,13 @@ static void UpdateTouchActionWheelEventHandlerAndNonFastScrollableRegions(
property_tree_manager, non_fast_scrollable_region);
}
layer.SetTouchActionRegion(std::move(touch_action_region));
if (base::FeatureList::IsEnabled(::features::kWheelEventRegions))
// TODO(https://crbug.com/841364): Fist condition in the "if" statement below
// is to avoid costly checks for kWheelEventRegions. This "if" condition will
// be gone once kWheelEventRegions feature flag is removed.
if (wheel_event_region != layer.wheel_event_region() &&
base::FeatureList::IsEnabled(::features::kWheelEventRegions))
layer.SetWheelEventRegion(std::move(wheel_event_region));
layer.SetNonFastScrollableRegion(std::move(non_fast_scrollable_region));
}
......
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