Merge 3809: [LayoutNG] Fix float + nbsp to create a break opportunity
Floats should create break opportunities, and
|NGLineBreaker::Rewind()| assumes this. However, |nbsp|
after floats suppressed the break opportunity, which caused
NGLineBreaker to loop infinitely under certain conditions.
This patch fixes floats to allow break after them.
Note the new behavior is the same as Gecko.
(cherry picked from commit 7b1e6a920eb103abedf45c95f5d86dda52d8713c)
Bug: 972421
Change-Id: Ie8c1d7927c5c282078a20f90941398149ea702a3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1653171
Commit-Queue: Koji Ishii <kojii@chromium.org>
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#668681}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1660076
Reviewed-by: Koji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/branch-heads/3809@{#362}
Cr-Branched-From: d82dec1a818f378c464ba307ddd9c92133eac355-refs/heads/master@{#665002}
diff --git a/third_party/blink/renderer/core/layout/ng/inline/ng_line_breaker.cc b/third_party/blink/renderer/core/layout/ng/inline/ng_line_breaker.cc
index 3bbf7919..8dba43f5 100644
--- a/third_party/blink/renderer/core/layout/ng/inline/ng_line_breaker.cc
+++ b/third_party/blink/renderer/core/layout/ng/inline/ng_line_breaker.cc
@@ -1225,7 +1225,7 @@
// fragmentainer break. In that case the floats associated with this line will
// already have been processed.
NGInlineItemResult* item_result = AddItem(item, line_info);
- ComputeCanBreakAfter(item_result, auto_wrap_, break_iterator_);
+ item_result->can_break_after = auto_wrap_;
MoveToNextOf(item);
// If we are currently computing our min/max-content size simply append to
@@ -1539,6 +1539,8 @@
// most cases where our support for rewinding positioned floats is not great
// yet (see below.)
while (item_results[new_end].item->Type() == NGInlineItem::kFloating) {
+ // We assume floats can break after, or this may cause an infinite loop.
+ DCHECK(item_results[new_end].can_break_after);
++new_end;
if (new_end == item_results.size()) {
position_ = line_info->ComputeWidth();
@@ -1551,6 +1553,8 @@
for (unsigned i = item_results.size(); i > new_end;) {
NGInlineItemResult& rewind = item_results[--i];
if (rewind.positioned_float) {
+ // We assume floats can break after, or this may cause an infinite loop.
+ DCHECK(rewind.can_break_after);
// TODO(kojii): We do not have mechanism to remove once positioned floats
// yet, and that rewinding them may lay it out twice. For now, prohibit
// rewinding positioned floats. This may results in incorrect layout, but
diff --git a/third_party/blink/web_tests/external/wpt/css/css-text/word-break/word-break-break-word-crash-001.html b/third_party/blink/web_tests/external/wpt/css/css-text/word-break/word-break-break-word-crash-001.html
new file mode 100644
index 0000000..894a6f3a
--- /dev/null
+++ b/third_party/blink/web_tests/external/wpt/css/css-text/word-break/word-break-break-word-crash-001.html
@@ -0,0 +1,39 @@
+<!DOCTYPE html>
+<title>Test float + nbsp + break-word does not freeze</title>
+<link rel="help" href="https://bugs.chromium.org/p/chromium/issues/detail?id=972421">
+<link rel="author" title="Koji Ishii" href="mailto:kojii@chromium.org">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<style>
+div {
+ font-size: 10px;
+ width: 10ch;
+ border: 2px solid blue;
+}
+float {
+ float: left;
+}
+.nowrap > div {
+ white-space: nowrap;
+}
+.break-word {
+ word-break: break-word;
+}
+</style>
+<body>
+ <section>
+ <div>123456<float></float>654321</div>
+ <div>123456<float></float> 654321</div>
+ </section>
+ <section class="nowrap">
+ <div>123456<float></float>654321</div>
+ <div>123456<float></float> 654321</div>
+ </section>
+ <section class="break-word">
+ <div>123456<float></float>654321</div>
+ <div>123456<float></float> 654321</div>
+ </section>
+<script>test(() => {
+ document.body.offsetTop; // layout should not freeze.
+});</script>
+</body>