Skip to content

Commit

Permalink
Prevent NeedsLayout when recomputing visual overflow for NG
Browse files Browse the repository at this point in the history
When |NeedsRecomputeVisualOverflow|, r736422
crrev.com/c/2025389 added a workaround to set |NeedsLayout|
for legacy objects. But when r740231 crrev.com/c/2046983
changed the condition, it accidentally included LayoutNG
|LayoutInline|.

This patch fixes the condition to exclude it. Note the fix
is limited to FragmentItem as recent ink overflow fix is only
applied to FragmentItem.

Bug: 1128199
Change-Id: I3c909c3ae176b6c58b4f29f91a3f188feff69eaf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2426147
Reviewed-by: Xianzhu Wang <wangxianzhu@chromium.org>
Commit-Queue: Koji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#815727}
GitOrigin-RevId: 2389d47bb8b475c5116ab2b8da9112c6ce3f6365
  • Loading branch information
kojiishi authored and copybara-github committed Oct 9, 2020
1 parent ca50d38 commit c7deeda
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 7 deletions.
4 changes: 3 additions & 1 deletion blink/renderer/core/layout/layout_object.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2326,7 +2326,9 @@ void LayoutObject::SetStyle(scoped_refptr<const ComputedStyle> style,
}

if (diff.NeedsRecomputeVisualOverflow()) {
if (!IsLayoutNGObject() && !IsLayoutBlock() && !NeedsLayout()) {
if (!(IsInLayoutNGInlineFormattingContext() &&
RuntimeEnabledFeatures::LayoutNGFragmentItemEnabled()) &&
!IsLayoutNGObject() && !IsLayoutBlock() && !NeedsLayout()) {
// TODO(crbug.com/1128199): This is still needed because
// RecalcVisualOverflow() does not actually compute the visual overflow
// for inline elements (legacy layout). However in LayoutNG
Expand Down
11 changes: 5 additions & 6 deletions blink/renderer/core/layout/ng/inline/ng_inline_node_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -598,18 +598,16 @@ struct StyleChangeData {
};
unsigned needs_collect_inlines;
base::Optional<bool> is_line_dirty;
bool invalidate_ink_overflow = false;
base::Optional<bool> invalidate_ink_overflow;
} style_change_data[] = {
// Changing color, text-decoration, outline, etc. should not re-run
// |CollectInlines()|.
{"#parent.after { color: red; }", StyleChangeData::kNone, false},
// TODO(crbug.com/1128199): text-decorations, outline, etc. should not
// require layout, only ink overflow, but they currently do.
{"#parent.after { text-decoration-line: underline; }",
StyleChangeData::kNone, true, true},
StyleChangeData::kNone, false, true},
{"#parent { background: orange; }" // Make sure it's not culled.
"#parent.after { outline: auto; }",
StyleChangeData::kNone, true},
StyleChangeData::kNone, false, false},
// Changing fonts should re-run |CollectInlines()|.
{"#parent.after { font-size: 200%; }", StyleChangeData::kAll, true},
// Changing from/to out-of-flow should re-rerun |CollectInlines()|.
Expand Down Expand Up @@ -702,7 +700,8 @@ TEST_P(StyleChangeTest, NeedsCollectInlinesOnStyle) {
for (cursor.MoveTo(*child); cursor;
cursor.MoveToNextForSameLayoutObject()) {
const NGFragmentItem* item = cursor.CurrentItem();
EXPECT_FALSE(item->IsInkOverflowComputed());
EXPECT_EQ(item->IsInkOverflowComputed(),
!*data.invalidate_ink_overflow);
}
}
}
Expand Down

0 comments on commit c7deeda

Please sign in to comment.