Skip to content

GitLab

T
tangled
Commit 53eab5a4 authored by Morten Stenshorne's avatar Morten Stenshorne Committed by Commit Bot
Browse files
Options

Avoid re-using lines with floats adjacent to dirty lines. 

If a line (RootInlineBox) is marked dirty, also treat adjacent following
lines with floats on them as dirty. We cannot safely skip laying them
out, because of bugs in the legacy line layout engine.

We ended up in a scenario where just one line got marked dirty, because
we removed some text there. A float that was associated with the second
line got associated with the first (dirty) line during re-layout. We'd
skip layout of the next line because it wasn't dirty, and we found it
safe to stop laying out after the first line and re-use the remaining
lines from the previous layout pass. Suddenly the float was associated
with two lines.

In addition to definitely fixing the fuzzer crash in bug 724830, it is
also speculative fix for bug 969325 (which I've been unable to
reproduce, but both the test and the crash seem very similar).

Bug: 724830, 969325
Change-Id: I0bbceeac1e19588c58206ed075c21ea19347109f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1664910Reviewed-by: default avatarMorten Stenshorne <mstensho@chromium.org>
Reviewed-by: default avatarEmil A Eklund <eae@chromium.org>
Commit-Queue: Emil A Eklund <eae@chromium.org>
Cr-Commit-Position: refs/heads/master@{#670216}
parent 3c64dba2
Hide whitespace changes
Inline Side-by-side
Showing with 44 additions and 3 deletions
third_party/blink/renderer/core/layout/layout_block_flow_line.cc
View file @ 53eab5a4
......@@ -2196,15 +2196,25 @@ void LayoutBlockFlow::DetermineEndPosition(LineLayoutState& layout_state,
BidiStatus& clean_line_bidi_status) {
DCHECK(!layout_state.EndLine());
RootInlineBox* last = nullptr;
bool previous_was_clean = false;
for (RootInlineBox* curr = start_line->NextRootBox(); curr;
curr = curr->NextRootBox()) {
if (!curr->IsDirty() && LineBoxHasBRWithClearance(curr))
return;
if (curr->IsDirty())
// A line is considered clean when it's not marked dirty, AND it either
// doesn't contain floats, or follows another clean line. The legacy line
// layout engine has issues with handling floats at line boundaries,
// potentially resulting in a float belonging to two different lines,
// causing all kinds of misery.
if (curr->IsDirty() || (curr->FloatsPtr() && !previous_was_clean)) {
last = nullptr;
else if (!last)
last = curr;
previous_was_clean = false;
} else {
if (!last)
last = curr;
previous_was_clean = true;
}
}
if (!last)
......
third_party/blink/web_tests/fast/inline/remove-text-near-float-and-line-boundary-crash.html 0 → 100644
View file @ 53eab5a4
<!DOCTYPE html>
<style>
.c0 { float:right; width:100px; height:150px; }
.c15 { font-size:20px; font-family:Ahem; }
.c17 { height:10px; padding-right:100%; }
.c19 { float:left; width:100px; height:100px; }
</style>
<!-- Add something editable, to disable LayoutNG, to trigger the bug. -->
<div contenteditable></div>
<div id="container" style="width:400px;">
<div></div>
<span class="c15">888888888<span id="removeme"> 88</span>
8888888888</span><div class="c0"></div><span class="c17"></span>
<div class="c19"></div>
</div>
<script src="../../resources/testharness.js"></script>
<script src="../../resources/testharnessreport.js"></script>
<script>
test(() => {
container.offsetTop;
// Remove some text without affecting where lines break.
removeme.style.display = "none";
container.offsetTop;
// Relayout the container without changing line widths.
container.style.paddingBottom = "1px";
}, "No crash");
</script>
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