Commit 9cd0ef1c authored by Ian Kilpatrick's avatar Ian Kilpatrick Committed by Commit Bot

[FlexNG] Walk over anonymous objects within ScopedPaintTimingDetectorBlockPaintHook

This fixes timeouts that were occurring when FlexNG was enabled:
https://test-results.appspot.com/data/layout_results/linux-rel/292466/webkit_layout_tests%20%28with%20patch%29/layout-test-results/results.html
(*/text-with-display-style.html)

These tests timeout if they don't receive all the expected performance
entries, causing the failure (the test never completes, waiting for the
performance entries).

The specific failure for FlexNG was occurring as we didn't have a
ScopedPaintTimingDetectorBlockPaintHook for the flexbox on the stack,
only one for the anonymous block-flow. This meant that the performance
entry was never recorded for the flexbox element.

Previously the ScopedPaintTimingDetectorBlockPaintHook would get
emplaced in two different places -
 1) Within BlockPainter
 2) Within the inline painter

This changes the EmplaceIfNeeded call to walk up the layout object
tree to the first non-anonymous object which has a DOM node associated
with it.

This walk up the layout object tree should be relatively cheap. Most of
the time it will be only walking up zero or one objects.

Additionally with the layout object walk we don't need to EmplaceIfNeeded
on nested <div>s, only when we actually encounter some text to be
painted.

Bug: 845235
Change-Id: I80e18539f70d2e1df56d3d85dd928dc10a35e28e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2032428
Commit-Queue: Ian Kilpatrick <ikilpatrick@chromium.org>
Reviewed-by: default avatarXianzhu Wang <wangxianzhu@chromium.org>
Reviewed-by: default avatarSteve Kobes <skobes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#739111}
parent f938d8e5
...@@ -210,19 +210,11 @@ void BlockPainter::PaintObject(const PaintInfo& paint_info, ...@@ -210,19 +210,11 @@ void BlockPainter::PaintObject(const PaintInfo& paint_info,
if (layout_block_.IsTruncated()) if (layout_block_.IsTruncated())
return; return;
ScopedPaintTimingDetectorBlockPaintHook
scoped_paint_timing_detector_block_paint_hook;
if (paint_info.phase == PaintPhase::kForeground) {
scoped_paint_timing_detector_block_paint_hook.EmplaceIfNeeded(
layout_block_,
paint_info.context.GetPaintController().CurrentPaintChunkProperties());
}
// If we're *printing or creating a paint preview of* the foreground, paint // If we're *printing or creating a paint preview of* the foreground, paint
// the URL. // the URL.
if (paint_phase == PaintPhase::kForeground && if (paint_phase == PaintPhase::kForeground &&
paint_info.ShouldAddUrlMetadata()) { paint_info.ShouldAddUrlMetadata())
ObjectPainter(layout_block_).AddURLRectIfNeeded(paint_info, paint_offset); ObjectPainter(layout_block_).AddURLRectIfNeeded(paint_info, paint_offset);
}
// If we're painting our background (either 1. kBlockBackground - background // If we're painting our background (either 1. kBlockBackground - background
// of the current object and non-self-painting descendants, or 2. // of the current object and non-self-painting descendants, or 2.
......
...@@ -1153,15 +1153,14 @@ void NGBoxFragmentPainter::PaintLineBoxChildren( ...@@ -1153,15 +1153,14 @@ void NGBoxFragmentPainter::PaintLineBoxChildren(
ScopedPaintTimingDetectorBlockPaintHook ScopedPaintTimingDetectorBlockPaintHook
scoped_paint_timing_detector_block_paint_hook; scoped_paint_timing_detector_block_paint_hook;
const auto& layout_block = To<LayoutBlock>(*layout_object);
if (paint_info.phase == PaintPhase::kForeground) { if (paint_info.phase == PaintPhase::kForeground) {
scoped_paint_timing_detector_block_paint_hook.EmplaceIfNeeded( scoped_paint_timing_detector_block_paint_hook.EmplaceIfNeeded(
layout_block, To<LayoutBoxModelObject>(*layout_object),
paint_info.context.GetPaintController().CurrentPaintChunkProperties()); paint_info.context.GetPaintController().CurrentPaintChunkProperties());
} }
if (paint_info.phase == PaintPhase::kForcedColorsModeBackplate && if (paint_info.phase == PaintPhase::kForcedColorsModeBackplate &&
layout_block.GetDocument().InForcedColorsMode()) { layout_object->GetDocument().InForcedColorsMode()) {
PaintBackplate(children, paint_info, paint_offset); PaintBackplate(children, paint_info, paint_offset);
return; return;
} }
......
...@@ -335,21 +335,29 @@ ScopedPaintTimingDetectorBlockPaintHook* ...@@ -335,21 +335,29 @@ ScopedPaintTimingDetectorBlockPaintHook*
ScopedPaintTimingDetectorBlockPaintHook::top_ = nullptr; ScopedPaintTimingDetectorBlockPaintHook::top_ = nullptr;
void ScopedPaintTimingDetectorBlockPaintHook::EmplaceIfNeeded( void ScopedPaintTimingDetectorBlockPaintHook::EmplaceIfNeeded(
const LayoutBoxModelObject& aggregator, const LayoutBoxModelObject& layout_object,
const PropertyTreeState& property_tree_state) { const PropertyTreeState& property_tree_state) {
// |reset_top_| is unset when |aggregator| is anonymous so that each // Find our first non-anonymous parent.
// aggregation corresponds to an element. See crbug.com/988593. When set, const LayoutObject* object = &layout_object;
// |top_| becomes |this|, and |top_| is restored to the previous value when while (object && !object->GetNode())
// the ScopedPaintTimingDetectorBlockPaintHook goes out of scope. object = object->Parent();
if (!aggregator.GetNode())
if (!object || !object->IsBoxModelObject())
return; return;
const LayoutBoxModelObject& aggregator = To<LayoutBoxModelObject>(*object);
DCHECK(aggregator.GetNode());
// When set, |top_| becomes |this|, and |top_| is restored to the previous
// value when the ScopedPaintTimingDetectorBlockPaintHook goes out of scope.
reset_top_.emplace(&top_, this); reset_top_.emplace(&top_, this);
TextPaintTimingDetector* detector = aggregator.GetFrameView() TextPaintTimingDetector* detector = aggregator.GetFrameView()
->GetPaintTimingDetector() ->GetPaintTimingDetector()
.GetTextPaintTimingDetector(); .GetTextPaintTimingDetector();
// Only set |data_| if we need to walk the object. // Only set |data_| if we need to walk the object.
if (detector && detector->ShouldWalkObject(aggregator)) if (detector->ShouldWalkObject(aggregator))
data_.emplace(aggregator, property_tree_state, detector); data_.emplace(aggregator, property_tree_state, detector);
} }
......
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