Skip to content

GitLab

T
tangled
Commit 82ac5bb7 authored by Morten Stenshorne's avatar Morten Stenshorne Committed by Commit Bot
Browse files
Options

[LayoutNG] Do not examine sibling nodes when walking break tokens. 

When processing incoming break tokens as part of block child layout, do
not call NGLayoutInputNode::NextSibling(), because:

1: We're not going to use the result
2: It would trigger a DCHECK failure in some cases

If we're resuming at a float in an inline formatting context, we'll do
this directly from the block layout algorithm. However, the float may
have inline-level siblings, and those should be handled by the inline
layout algorithm, not the block layout algorithm.
NGBlockNode::NextSibling() (rightly) chokes on inline-level boxes, so
just avoid it. There will always be an inline break token which will
take care of this correctly for us.

The attached test used to DCHECK-fail (but otherwise pass) without this
fix.

Change-Id: I826c30b362f2ae40af12f1b7cf1b0a9e8cd9ccfb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2537675Reviewed-by: default avatarIan Kilpatrick <ikilpatrick@chromium.org>
Reviewed-by: default avatarKoji Ishii <kojii@chromium.org>
Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/master@{#827730}
parent 021d300d
Hide whitespace changes
Inline Side-by-side
Showing with 31 additions and 14 deletions
third_party/blink/renderer/core/layout/ng/ng_block_child_iterator.cc
View file @ 82ac5bb7
...@@ -23,6 +23,7 @@ NGBlockChildIterator::Entry NGBlockChildIterator::NextChild( ...@@ -23,6 +23,7 @@ NGBlockChildIterator::Entry NGBlockChildIterator::NextChild(
previous_inline_break_token); previous_inline_break_token);
} }
bool next_is_from_break_token = false;
if (break_token_) { if (break_token_) {
// If we're resuming layout after a fragmentainer break, we'll first resume // If we're resuming layout after a fragmentainer break, we'll first resume
// the children that fragmented earlier (represented by one break token // the children that fragmented earlier (represented by one break token
...@@ -33,24 +34,31 @@ NGBlockChildIterator::Entry NGBlockChildIterator::NextChild( ...@@ -33,24 +34,31 @@ NGBlockChildIterator::Entry NGBlockChildIterator::NextChild(
child_break_token = child_break_tokens[child_token_idx_++]; child_break_token = child_break_tokens[child_token_idx_++];
break; break;
} }
// If there are no break tokens left to resume, the iterator machinery (see
// further below) will just continue at the next sibling. The last break next_is_from_break_token = child_token_idx_ < child_break_tokens.size();
// token would be the last node that got fragmented. However, there may be
// parallel flows caused by visible overflow, established by descendants of if (child_break_token) {
// our children, and these may go on, fragmentainer after fragmentainer, // If we have a child break token to resume at, that's the source of
// even if we're done with our direct children. When this happens, we need // truth.
// to prevent the machinery from continuing iterating, if we're already done child_ = child_break_token->InputNode();
// with those siblings. } else if (break_token_->HasSeenAllChildren()) {
if (!child_break_token && break_token_->HasSeenAllChildren()) // If there are no break tokens left to resume, the iterator machinery
// (see further below) will by default just continue at the next sibling.
// The last break token would be the last node that previously got
// fragmented. However, there may be parallel flows caused by visible
// overflow, established by descendants of our children, and these may go
// on, fragmentainer after fragmentainer, even if we're done with our
// direct children. When this happens, we need to prevent the machinery
// from continuing iterating, if we're already done with those siblings.
child_ = nullptr; child_ = nullptr;
}
} }
// If we have a child break token to resume at, that's the source of truth.
if (child_break_token)
child_ = child_break_token->InputNode();
NGLayoutInputNode child = child_; NGLayoutInputNode child = child_;
if (child_)
// Unless we're going to grab the next child off a break token, we'll use the
// next sibling of the current child. Prepare it now.
if (child_ && !next_is_from_break_token)
child_ = child_.NextSibling(); child_ = child_.NextSibling();
return Entry(child, child_break_token); return Entry(child, child_break_token);
......
third_party/blink/web_tests/external/wpt/css/css-break/line-pushed-by-float-000.html 0 → 100644
View file @ 82ac5bb7
<!DOCTYPE html>
<link rel="author" title="Morten Stenshorne" href="mailto:mstensho@chromium.org">
<link rel="help" href="https://www.w3.org/TR/css-break-3/#breaking-rules">
<link rel="match" href="../reference/ref-filled-green-100px-square.xht">
<p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
<div style="columns:2; column-fill:auto; column-gap:0; width:100px; height:100px; background:red;">
<div style="float:left; width:100%; height:150px; background:green;"></div>
<div style="display:inline-block; vertical-align:top; width:100%; height:50px; background:green;"></div>
</div>
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