Commit fe3d571e authored by Aleks Totic's avatar Aleks Totic Committed by Commit Bot

[LayoutNG] Fix stale style on a fragment after paint invalidation

This makes ~50 additional tests to pass, and 3 to fail.

Paint invalidation can cause styles on a NGPhysicalFragment to
get out of sync with LayoutObject styles.

There were two ways to fix this, I found a) to be cleaner for now.

a) Make PhysicalFragments fetch style from LayoutObject
Tricky part here is that :first-line and overflow:ellipsis get
special styles.

b) Rebuild PhysicalFragment tree with a shallow clonewithstyle
I was unable to find a clean way to trigger tree rebuild here.
Sample experiment to convert paint invalidations into
layout invalidations triggered 100s DCHECK(!NeedsLayout())
Possible fix would be to trap all places that call NeedsLayout()
with intent to relayout, and replace them with NeedsLayout() ||
NeedsNGLayout()

This does not fix all invalidation bugs. DisplayItems outside of
LayoutObject tree do not get invalidated correctly.

fast/css/first-letter-hover.html
fast/history/visited-link-hover-emphasis-color.html

Bug: 819372
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_layout_ng;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: Ia11f8288b8f45423762c870a43615707d6a0cc93
Reviewed-on: https://chromium-review.googlesource.com/956552
Commit-Queue: Aleks Totic <atotic@chromium.org>
Reviewed-by: default avatarMorten Stenshorne <mstensho@chromium.org>
Reviewed-by: default avatarEmil A Eklund <eae@chromium.org>
Reviewed-by: default avatarKoji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542664}
parent 9d2f802a
......@@ -307,7 +307,6 @@ crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/positioning/position-re
### virtual/layout_ng/fast/block/basic
crbug.com/635619 [ Mac ] virtual/layout_ng/fast/block/basic/011.html [ Failure ]
crbug.com/635619 virtual/layout_ng/fast/block/basic/018.html [ Failure ]
crbug.com/635619 virtual/layout_ng/fast/block/basic/quirk-percent-height-grandchild.html [ Failure ]
crbug.com/635619 virtual/layout_ng/fast/block/basic/quirk-percent-height-table-cell.html [ Failure ]
crbug.com/635619 [ Mac ] virtual/layout_ng/fast/block/basic/text-indent-rtl.html [ Failure ]
crbug.com/635619 [ Mac ] virtual/layout_ng/fast/block/basic/truncation-rtl.html [ Failure ]
......@@ -469,7 +468,6 @@ crbug.com/714962 virtual/layout_ng/fast/writing-mode/logical-height-after-clear.
crbug.com/714962 [ Mac ] virtual/layout_ng/fast/writing-mode/margins.html [ Failure ]
crbug.com/714962 [ Mac ] virtual/layout_ng/fast/writing-mode/orthogonal-inline-block.html [ Failure ]
crbug.com/714962 virtual/layout_ng/fast/writing-mode/orthogonal-writing-modes-available-width-absolute-crash.html [ Failure ]
crbug.com/714962 virtual/layout_ng/fast/writing-mode/percentage-height-orthogonal-writing-modes-quirks.html [ Failure ]
crbug.com/714962 virtual/layout_ng/fast/writing-mode/percentage-height-orthogonal-writing-modes.html [ Failure ]
crbug.com/714962 virtual/layout_ng/fast/writing-mode/percentage-margins-absolute-replaced.html [ Failure ]
crbug.com/714962 virtual/layout_ng/fast/writing-mode/percentage-margins-absolute.html [ Failure ]
......
......@@ -54,7 +54,8 @@ NGInlineItem::NGInlineItem(NGInlineItemType type,
bidi_level_(UBIDI_LTR),
shape_options_(kPreContext | kPostContext),
is_empty_item_(false),
should_create_box_fragment_(false) {
should_create_box_fragment_(false),
style_variant_(static_cast<unsigned>(NGStyleVariant::kStandard)) {
DCHECK_GE(end, start);
ComputeBoxProperties();
}
......
......@@ -6,6 +6,7 @@
#define NGInlineItem_h
#include "core/CoreExport.h"
#include "core/layout/ng/ng_style_variant.h"
#include "platform/LayoutUnit.h"
#include "platform/fonts/FontFallbackPriority.h"
#include "platform/fonts/SimpleFontData.h"
......@@ -93,6 +94,13 @@ class CORE_EXPORT NGInlineItem {
bool HasStartEdge() const;
bool HasEndEdge() const;
void SetStyleVariant(NGStyleVariant style_variant) {
style_variant_ = static_cast<unsigned>(style_variant);
}
NGStyleVariant StyleVariant() const {
return static_cast<NGStyleVariant>(style_variant_);
}
static void Split(Vector<NGInlineItem>&, unsigned index, unsigned offset);
static unsigned SetBidiLevel(Vector<NGInlineItem>&,
unsigned index,
......@@ -119,7 +127,7 @@ class CORE_EXPORT NGInlineItem {
unsigned shape_options_ : 2;
unsigned is_empty_item_ : 1;
unsigned should_create_box_fragment_ : 1;
unsigned style_variant_ : 2;
friend class NGInlineNode;
};
......
......@@ -412,6 +412,7 @@ void NGInlineNode::ShapeTextForFirstLineIfNeeded(NGInlineNodeData* data) {
if (item.style_) {
DCHECK(item.layout_object_);
item.style_ = item.layout_object_->FirstLineStyle();
item.SetStyleVariant(NGStyleVariant::kFirstLine);
}
}
......
......@@ -468,7 +468,7 @@ void NGLineBreaker::AppendHyphen(const NGInlineItem& item,
shaper.Shape(&style.GetFont(), direction);
NGTextFragmentBuilder builder(node_, constraint_space_.GetWritingMode());
builder.SetText(item.GetLayoutObject(), hyphen_string, &style,
std::move(hyphen_result));
/* is_ellipsis_style */ false, std::move(hyphen_result));
SetLineEndFragment(builder.ToTextFragment(), line_info);
}
......@@ -968,7 +968,8 @@ void NGLineBreaker::TruncateOverflowingText(NGLineInfo* line_info) {
// This is stored seprately from other results so that it can be appended
// after bidi reorder.
NGTextFragmentBuilder builder(node_, constraint_space_.GetWritingMode());
builder.SetText(layout_object, ellipsis, style, std::move(shape_result));
builder.SetText(layout_object, ellipsis, style, true /* is_ellipsis_style */,
std::move(shape_result));
SetLineEndFragment(builder.ToTextFragment(), line_info);
}
......
......@@ -73,6 +73,14 @@ NGPhysicalOffsetRect NGPhysicalTextFragment::SelfVisualRect() const {
return {};
}
scoped_refptr<NGPhysicalFragment> NGPhysicalTextFragment::CloneWithoutOffset()
const {
return base::AdoptRef(new NGPhysicalTextFragment(
layout_object_, Style(), static_cast<NGStyleVariant>(style_variant_),
TextType(), text_, start_offset_, end_offset_, size_, LineOrientation(),
EndEffect(), shape_result_));
}
bool NGPhysicalTextFragment::IsAnonymousText() const {
// TODO(xiaochengh): Introduce and set a flag for anonymous text.
const LayoutObject* layout_object = GetLayoutObject();
......
......@@ -52,6 +52,7 @@ class CORE_EXPORT NGPhysicalTextFragment final : public NGPhysicalFragment {
NGPhysicalTextFragment(LayoutObject* layout_object,
const ComputedStyle& style,
NGStyleVariant style_variant,
NGTextType text_type,
const String& text,
unsigned start_offset,
......@@ -62,6 +63,7 @@ class CORE_EXPORT NGPhysicalTextFragment final : public NGPhysicalFragment {
scoped_refptr<const ShapeResult> shape_result)
: NGPhysicalFragment(layout_object,
style,
style_variant,
size,
kFragmentText,
text_type),
......@@ -111,11 +113,7 @@ class CORE_EXPORT NGPhysicalTextFragment final : public NGPhysicalFragment {
return static_cast<NGTextEndEffect>(end_effect_);
}
scoped_refptr<NGPhysicalFragment> CloneWithoutOffset() const {
return base::AdoptRef(new NGPhysicalTextFragment(
layout_object_, Style(), TextType(), text_, start_offset_, end_offset_,
size_, LineOrientation(), EndEffect(), shape_result_));
}
scoped_refptr<NGPhysicalFragment> CloneWithoutOffset() const;
NGTextFragmentPaintInfo PaintInfo() const {
return NGTextFragmentPaintInfo{text_, StartOffset(), EndOffset(),
......
......@@ -47,7 +47,7 @@ void NGTextFragmentBuilder::SetItem(
item_index_ = item_result->item_index;
start_offset_ = item_result->start_offset;
end_offset_ = item_result->end_offset;
SetStyle(item_result->item->Style());
SetStyle(item_result->item->Style(), item_result->item->StyleVariant());
size_ = {item_result->inline_size, line_height};
end_effect_ = item_result->text_end_effect;
shape_result_ = std::move(item_result->shape_result);
......@@ -58,6 +58,7 @@ void NGTextFragmentBuilder::SetText(
LayoutObject* layout_object,
const String& text,
scoped_refptr<const ComputedStyle> style,
bool is_ellipsis_style,
scoped_refptr<const ShapeResult> shape_result) {
DCHECK(layout_object);
DCHECK(style);
......@@ -68,7 +69,8 @@ void NGTextFragmentBuilder::SetText(
item_index_ = std::numeric_limits<unsigned>::max();
start_offset_ = shape_result->StartIndexForResult();
end_offset_ = shape_result->EndIndexForResult();
SetStyle(style);
SetStyle(style, is_ellipsis_style ? NGStyleVariant::kEllipsis
: NGStyleVariant::kStandard);
FontBaseline baseline_type = style->IsHorizontalWritingMode()
? kAlphabeticBaseline
: kIdeographicBaseline;
......@@ -82,8 +84,8 @@ void NGTextFragmentBuilder::SetText(
scoped_refptr<NGPhysicalTextFragment> NGTextFragmentBuilder::ToTextFragment() {
scoped_refptr<NGPhysicalTextFragment> fragment =
base::AdoptRef(new NGPhysicalTextFragment(
layout_object_, Style(), text_type_, text_, start_offset_,
end_offset_, size_.ConvertToPhysical(GetWritingMode()),
layout_object_, Style(), style_variant_, text_type_, text_,
start_offset_, end_offset_, size_.ConvertToPhysical(GetWritingMode()),
ToLineOrientation(GetWritingMode()), end_effect_,
std::move(shape_result_)));
return fragment;
......
......@@ -31,6 +31,7 @@ class CORE_EXPORT NGTextFragmentBuilder final : public NGBaseFragmentBuilder {
void SetText(LayoutObject*,
const String& text,
scoped_refptr<const ComputedStyle>,
bool is_ellipsis_style,
scoped_refptr<const ShapeResult>);
// Creates the fragment. Can only be called once.
......
......@@ -23,9 +23,11 @@ NGBaseFragmentBuilder::NGBaseFragmentBuilder(WritingMode writing_mode,
NGBaseFragmentBuilder::~NGBaseFragmentBuilder() = default;
NGBaseFragmentBuilder& NGBaseFragmentBuilder::SetStyle(
scoped_refptr<const ComputedStyle> style) {
scoped_refptr<const ComputedStyle> style,
NGStyleVariant style_variant) {
DCHECK(style);
style_ = std::move(style);
style_variant_ = style_variant;
return *this;
}
......
......@@ -7,6 +7,7 @@
#include "base/memory/scoped_refptr.h"
#include "core/CoreExport.h"
#include "core/layout/ng/ng_style_variant.h"
#include "core/style/ComputedStyle.h"
#include "platform/text/TextDirection.h"
#include "platform/text/WritingMode.h"
......@@ -23,7 +24,8 @@ class CORE_EXPORT NGBaseFragmentBuilder {
DCHECK(style_);
return *style_;
}
NGBaseFragmentBuilder& SetStyle(scoped_refptr<const ComputedStyle>);
NGBaseFragmentBuilder& SetStyle(scoped_refptr<const ComputedStyle>,
NGStyleVariant);
WritingMode GetWritingMode() const { return writing_mode_; }
TextDirection Direction() const { return direction_; }
......@@ -38,6 +40,9 @@ class CORE_EXPORT NGBaseFragmentBuilder {
scoped_refptr<const ComputedStyle> style_;
WritingMode writing_mode_;
TextDirection direction_;
protected:
NGStyleVariant style_variant_;
};
} // namespace blink
......
......@@ -63,6 +63,7 @@ NGPhysicalContainerFragment::NGPhysicalContainerFragment(
scoped_refptr<NGBreakToken> break_token)
: NGPhysicalFragment(layout_object,
style,
NGStyleVariant::kStandard,
size,
type,
sub_type,
......
......@@ -5,6 +5,7 @@
#include "core/layout/ng/ng_physical_fragment.h"
#include "core/layout/LayoutBlock.h"
#include "core/layout/LayoutObjectInlines.h"
#include "core/layout/ng/geometry/ng_border_edges.h"
#include "core/layout/ng/geometry/ng_box_strut.h"
#include "core/layout/ng/inline/ng_physical_line_box_fragment.h"
......@@ -182,6 +183,7 @@ void NGPhysicalFragmentTraits::Destruct(const NGPhysicalFragment* fragment) {
NGPhysicalFragment::NGPhysicalFragment(LayoutObject* layout_object,
const ComputedStyle& style,
NGStyleVariant style_variant,
NGPhysicalSize size,
NGFragmentType type,
unsigned sub_type,
......@@ -193,7 +195,8 @@ NGPhysicalFragment::NGPhysicalFragment(LayoutObject* layout_object,
type_(type),
sub_type_(sub_type),
is_old_layout_root_(false),
is_placed_(false) {}
is_placed_(false),
style_variant_((unsigned)style_variant) {}
// Keep the implementation of the destructor here, to avoid dependencies on
// ComputedStyle in the header file.
......@@ -218,6 +221,16 @@ void NGPhysicalFragment::Destroy() const {
const ComputedStyle& NGPhysicalFragment::Style() const {
DCHECK(style_);
if (!GetLayoutObject())
return *style_;
switch ((NGStyleVariant)style_variant_) {
case NGStyleVariant::kStandard:
return *GetLayoutObject()->Style();
case NGStyleVariant::kFirstLine:
return *GetLayoutObject()->FirstLineStyle();
case NGStyleVariant::kEllipsis:
return *style_;
}
return *style_;
}
......
......@@ -11,6 +11,7 @@
#include "core/layout/ng/geometry/ng_physical_offset.h"
#include "core/layout/ng/geometry/ng_physical_size.h"
#include "core/layout/ng/ng_break_token.h"
#include "core/layout/ng/ng_style_variant.h"
#include "platform/geometry/LayoutRect.h"
namespace blink {
......@@ -200,6 +201,7 @@ class CORE_EXPORT NGPhysicalFragment
protected:
NGPhysicalFragment(LayoutObject* layout_object,
const ComputedStyle& style,
NGStyleVariant,
NGPhysicalSize size,
NGFragmentType type,
unsigned sub_type,
......@@ -216,6 +218,7 @@ class CORE_EXPORT NGPhysicalFragment
unsigned is_old_layout_root_ : 1;
unsigned is_placed_ : 1;
unsigned border_edge_ : 4; // NGBorderEdges::Physical
unsigned style_variant_ : 2; // NGStyleVariant
private:
friend struct NGPhysicalFragmentTraits;
......
// Copyright 2018 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef ng_style_variant_h
#define ng_style_variant_h
namespace blink {
// LayoutObject can have multiple style variations.
enum class NGStyleVariant { kStandard, kFirstLine, kEllipsis };
} // namespace blink
#endif // ng_style_variant_h
......@@ -85,7 +85,7 @@ void NGBoxFragmentPainter::Paint(const PaintInfo& paint_info,
adjustment.AdjustedPaintOffset());
return;
}
DCHECK_EQ(layout_object->Style(), &PhysicalFragment().Style());
AdjustPaintOffsetScope adjustment(box_fragment_, paint_info, paint_offset);
PaintWithAdjustedOffset(adjustment.MutablePaintInfo(),
adjustment.AdjustedPaintOffset());
......
......@@ -64,11 +64,11 @@ children with outlines.
// Collect and group outline rects.
for (auto& paint_descendant :
NGPaintFragmentTraversal::DescendantsOf(paint_fragment_)) {
const ComputedStyle& descendant_style = paint_descendant.fragment->Style();
if (!paint_descendant.fragment->PhysicalFragment().IsBox() ||
paint_descendant.fragment->PhysicalFragment().IsAtomicInline())
continue;
const ComputedStyle& descendant_style = paint_descendant.fragment->Style();
if (!descendant_style.HasOutline() ||
descendant_style.Visibility() != EVisibility::kVisible)
continue;
......
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