Support `box-decoration-break: clone` in NGLineBreaker

In LayoutNG, when `box-decoration-break: clone` is set,
|NGInlineBoxState| clones the inline box fragments for every
line, but it had no effects in line breaking and measuring.

This patch implements the logic in |NGLineBreaker| so that
the property is taken account for line breaking, line width
(and therefore `text-align`), min-max sizing, etc.

|NGInlineNode::ComputeMinMaxSizes| started to hit DCHECK with
the |NGLineBreaker| change, because it computes `max-content`
from `min-content`. When this property clones box decorations
to every line, the logic does not work. This patch disables
the fast codepath, instead run a separate layout path when
this property is effective. This fixed an old crbug.com/612641.

This property is used ~0.7%. The regression crbug.com/1006599
got 10 stars.

Bug: 1006599, 612641
Change-Id: I6425fb5e7f8dc951d882eb90d1c081abd127468e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2454349Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Commit-Queue: Koji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#815477}

third_party/blink/renderer/core/layout/ng/inline/ng_inline_break_token.h

@@ -18,7 +18,8 @@ class CORE_EXPORT NGInlineBreakToken final : public NGBreakToken {
   enum NGInlineBreakTokenFlags {
     kDefault = 0,
     kIsForcedBreak = 1 << 0,
-    kUseFirstLineStyle = 1 << 1
+    kUseFirstLineStyle = 1 << 1,
+    kHasClonedBoxDecorations = 1 << 2,
     // When adding values, ensure |flags_| has enough storage.
   };
 
@@ -69,6 +70,13 @@ class CORE_EXPORT NGInlineBreakToken final : public NGBreakToken {
     return flags_ & kIsForcedBreak;
   }
 
+  // True if the current position has open tags that has `box-decoration-break:
+  // clone`. They should be cloned to the start of the next line.
+  bool HasClonedBoxDecorations() const {
+    DCHECK(!IsFinished());
+    return flags_ & kHasClonedBoxDecorations;
+  }
+
   using PassKey = util::PassKey<NGInlineBreakToken>;
   NGInlineBreakToken(PassKey,
                      NGInlineNode node,

third_party/blink/renderer/core/layout/ng/inline/ng_inline_item.cc

@@ -183,6 +183,17 @@ unsigned NGInlineItem::SetBidiLevel(Vector<NGInlineItem>& items,
   return index + 1;
 }
 
+void NGInlineItemsData::GetOpenTagItems(wtf_size_t size,
+                                        OpenTagItems* open_items) const {
+  DCHECK_LE(size, items.size());
+  for (const NGInlineItem& item : base::make_span(items.data(), size)) {
+    if (item.Type() == NGInlineItem::kOpenTag)
+      open_items->push_back(&item);
+    else if (item.Type() == NGInlineItem::kCloseTag)
+      open_items->pop_back();
+  }
+}
+
 String NGInlineItem::ToString() const {
   return String::Format("NGInlineItem. Type: '%s'. LayoutObject: '%s'",
                         NGInlineItemTypeToString(Type()),

third_party/blink/renderer/core/layout/ng/inline/ng_inline_item.h

@@ -306,6 +306,10 @@ struct CORE_EXPORT NGInlineItemsData {
   void AssertEndOffset(unsigned index, unsigned offset) const {
     items[index].AssertEndOffset(offset);
   }
+
+  // Get a list of |kOpenTag| that are open at |size|.
+  using OpenTagItems = Vector<const NGInlineItem*, 16>;
+  void GetOpenTagItems(wtf_size_t size, OpenTagItems* open_items) const;
 };
 
 }  // namespace blink

third_party/blink/renderer/core/layout/ng/inline/ng_inline_layout_algorithm.cc

@@ -154,15 +154,8 @@ void NGInlineLayoutAlgorithm::RebuildBoxStates(
     const NGInlineBreakToken* break_token,
     NGInlineLayoutStateStack* box_states) const {
   // Compute which tags are not closed at the beginning of this line.
-  const Vector<NGInlineItem>& items = line_info.ItemsData().items;
-  Vector<const NGInlineItem*, 16> open_items;
-  for (unsigned i = 0; i < break_token->ItemIndex(); i++) {
-    const NGInlineItem& item = items[i];
-    if (item.Type() == NGInlineItem::kOpenTag)
-      open_items.push_back(&item);
-    else if (item.Type() == NGInlineItem::kCloseTag)
-      open_items.pop_back();
-  }
+  NGInlineItemsData::OpenTagItems open_items;
+  line_info.ItemsData().GetOpenTagItems(break_token->ItemIndex(), &open_items);
 
   // Create box states for tags that are not closed yet.
   NGLogicalLineItems line_box;

third_party/blink/renderer/core/layout/ng/inline/ng_inline_node.cc

@@ -1727,6 +1727,7 @@ static LayoutUnit ComputeContentSize(
     }
   };
   FloatsMaxSize floats_max_size(input);
+  bool can_compute_max_size_from_min_size = true;
   MaxSizeFromMinSize max_size_from_min_size(items_data, *max_size_cache,
                                             &floats_max_size);
 
@@ -1737,16 +1738,6 @@ static LayoutUnit ComputeContentSize(
       break;
 
     LayoutUnit inline_size = line_info.Width();
-#if DCHECK_IS_ON()
-    // Text measurement is done using floats which may introduce small rounding
-    // errors for near-saturated values.
-    // See http://crbug.com/1112560
-    if (!LayoutUnit(line_info.ComputeWidthInFloat()).MightBeSaturated()) {
-      DCHECK_EQ(inline_size.Round(),
-                line_info.ComputeWidth().ClampNegativeToZero().Round());
-    }
-#endif
-
     for (const NGInlineItemResult& item_result : line_info.Results()) {
       DCHECK(item_result.item);
       const NGInlineItem& item = *item_result.item;
@@ -1782,13 +1773,21 @@ static LayoutUnit ComputeContentSize(
 
     if (mode == NGLineBreakerMode::kMinContent) {
       result = std::max(result, inline_size);
-      max_size_from_min_size.ComputeFromMinSize(line_info);
+      can_compute_max_size_from_min_size =
+          can_compute_max_size_from_min_size &&
+          // `box-decoration-break: clone` clones box decorations to each
+          // fragment (line) that we cannot compute max-content from
+          // min-content.
+          !line_breaker.HasClonedBoxDecorations();
+      if (can_compute_max_size_from_min_size)
+        max_size_from_min_size.ComputeFromMinSize(line_info);
     } else {
       result = floats_max_size.ComputeMaxSizeForLine(inline_size, result);
     }
   } while (!line_breaker.IsFinished());
 
-  if (mode == NGLineBreakerMode::kMinContent) {
+  if (mode == NGLineBreakerMode::kMinContent &&
+      can_compute_max_size_from_min_size) {
     *max_size_out = max_size_from_min_size.Finish(items_data.items.end());
     // Check the max size matches to the value computed from 2 pass.
 #if DCHECK_IS_ON()
@@ -1823,8 +1822,13 @@ MinMaxSizesResult NGInlineNode::ComputeMinMaxSizes(
   sizes.min_size = ComputeContentSize(
       *this, container_writing_mode, input, NGLineBreakerMode::kMinContent,
       &max_size_cache, &max_size, &depends_on_percentage_block_size);
-  DCHECK(max_size.has_value());
-  sizes.max_size = *max_size;
+  if (max_size) {
+    sizes.max_size = *max_size;
+  } else {
+    sizes.max_size = ComputeContentSize(*this, container_writing_mode, input,
+                                        NGLineBreakerMode::kMaxContent,
+                                        &max_size_cache, nullptr, nullptr);
+  }
 
   // Negative text-indent can make min > max. Ensure min is the minimum size.
   sizes.min_size = std::min(sizes.min_size, sizes.max_size);

third_party/blink/renderer/core/layout/ng/inline/ng_line_breaker.cc

@@ -158,6 +158,10 @@ inline void NGLineBreaker::ClearNeedsLayout(const NGInlineItem& item) {
 
 LayoutUnit NGLineBreaker::ComputeAvailableWidth() const {
   LayoutUnit available_width = line_opportunity_.AvailableInlineSize();
+  // Make sure it's at least the initial size, which is usually 0 but not so
+  // when `box-decoration-break: clone`.
+  available_width =
+      std::max(available_width, cloned_box_decorations_initial_size_);
   // Available width must be smaller than |LayoutUnit::Max()| so that the
   // position can be larger.
   available_width = std::min(available_width, LayoutUnit::NearlyMax());
@@ -284,6 +288,36 @@ void NGLineBreaker::ComputeBaseDirection() {
           : StringView(text, offset_, end_offset - offset_));
 }
 
+void NGLineBreaker::RecalcClonedBoxDecorations() {
+  cloned_box_decorations_count_ = 0u;
+  cloned_box_decorations_initial_size_ = LayoutUnit();
+  cloned_box_decorations_end_size_ = LayoutUnit();
+  has_cloned_box_decorations_ = false;
+
+  // Compute which tags are not closed at |item_index_|.
+  NGInlineItemsData::OpenTagItems open_items;
+  items_data_.GetOpenTagItems(item_index_, &open_items);
+
+  for (const NGInlineItem* item : open_items) {
+    if (item->Style()->BoxDecorationBreak() == EBoxDecorationBreak::kClone) {
+      has_cloned_box_decorations_ = true;
+      ++cloned_box_decorations_count_;
+      NGInlineItemResult item_result;
+      ComputeOpenTagResult(*item, constraint_space_, &item_result);
+      cloned_box_decorations_initial_size_ += item_result.inline_size;
+      cloned_box_decorations_end_size_ += item_result.margins.inline_end +
+                                          item_result.borders.inline_end +
+                                          item_result.padding.inline_end;
+    }
+  }
+  // Advance |position_| by the initial size so that the tab position can
+  // accommodate cloned box decorations.
+  position_ += cloned_box_decorations_initial_size_;
+  // |cloned_box_decorations_initial_size_| may affect available width.
+  available_width_ = ComputeAvailableWidth();
+  DCHECK_GE(available_width_, cloned_box_decorations_initial_size_);
+}
+
 // Initialize internal states for the next line.
 void NGLineBreaker::PrepareNextLine(NGLineInfo* line_info) {
   // NGLineInfo is not supposed to be re-used because it's not much gain and to
@@ -328,6 +362,11 @@ void NGLineBreaker::PrepareNextLine(NGLineInfo* line_info) {
   // regardless of 'text-indent'.
   position_ = line_info->TextIndent();
 
+  has_cloned_box_decorations_ = false;
+  if (UNLIKELY((break_token_ && break_token_->HasClonedBoxDecorations()) ||
+               cloned_box_decorations_count_))
+    RecalcClonedBoxDecorations();
+
   ResetRewindLoopDetector();
 }
 
@@ -466,16 +505,8 @@ void NGLineBreaker::ComputeLineLocation(NGLineInfo* line_info) const {
   // Negative margins can make the position negative, but the inline size is
   // always positive or 0.
   LayoutUnit available_width = AvailableWidth();
-
-#if DCHECK_IS_ON()
-  // Text measurement is done using floats which may introduce small rounding
-  // errors for near-saturated values.
-  // See http://crbug.com/1098795
-  if (!LayoutUnit(line_info->ComputeWidthInFloat()).MightBeSaturated())
-    DCHECK_EQ(position_.Round(), line_info->ComputeWidth().Round());
-#endif
-
-  line_info->SetWidth(available_width, position_);
+  line_info->SetWidth(available_width,
+                      position_ + cloned_box_decorations_end_size_);
   line_info->SetBfcOffset(
       {line_opportunity_.line_left_offset, line_opportunity_.bfc_block_offset});
   if (mode_ == NGLineBreakerMode::kContent)
@@ -1678,6 +1709,8 @@ void NGLineBreaker::HandleOpenTag(const NGInlineItem& item,
   DCHECK_EQ(item.Type(), NGInlineItem::kOpenTag);
 
   NGInlineItemResult* item_result = AddItem(item, line_info);
+  DCHECK(item.Style());
+  const ComputedStyle& style = *item.Style();
   if (ComputeOpenTagResult(item, constraint_space_, item_result)) {
     // Negative margins on open tags may bring the position back. Update
     // |state_| if that happens.
@@ -1698,11 +1731,17 @@ void NGLineBreaker::HandleOpenTag(const NGInlineItem& item,
     // Force to create a box, because such inline boxes affect line heights.
     if (!item_result->should_create_line_box && !item.IsEmptyItem())
       item_result->should_create_line_box = true;
+
+    if (UNLIKELY(style.BoxDecorationBreak() == EBoxDecorationBreak::kClone)) {
+      has_cloned_box_decorations_ = true;
+      ++cloned_box_decorations_count_;
+      cloned_box_decorations_end_size_ += item_result->margins.inline_end +
+                                          item_result->borders.inline_end +
+                                          item_result->padding.inline_end;
+    }
   }
 
   bool was_auto_wrap = auto_wrap_;
-  DCHECK(item.Style());
-  const ComputedStyle& style = *item.Style();
   SetCurrentStyle(style);
   MoveToNextOf(item);
 
@@ -1720,12 +1759,20 @@ void NGLineBreaker::HandleCloseTag(const NGInlineItem& item,
 
   item_result->has_edge = item.HasEndEdge();
   if (item_result->has_edge) {
-    item_result->inline_size =
-        ComputeInlineEndSize(constraint_space_, item.Style());
+    DCHECK(item.Style());
+    const ComputedStyle& style = *item.Style();
+    item_result->inline_size = ComputeInlineEndSize(constraint_space_, &style);
     position_ += item_result->inline_size;
 
     if (!item_result->should_create_line_box && !item.IsEmptyItem())
       item_result->should_create_line_box = true;
+
+    if (UNLIKELY(style.BoxDecorationBreak() == EBoxDecorationBreak::kClone)) {
+      DCHECK_GT(cloned_box_decorations_count_, 0u);
+      --cloned_box_decorations_count_;
+      DCHECK_GE(cloned_box_decorations_end_size_, item_result->inline_size);
+      cloned_box_decorations_end_size_ -= item_result->inline_size;
+    }
   }
   DCHECK(item.GetLayoutObject() && item.GetLayoutObject()->Parent());
   bool was_auto_wrap = auto_wrap_;
@@ -2082,6 +2129,9 @@ void NGLineBreaker::Rewind(unsigned new_end, NGLineInfo* line_info) {
 
   trailing_collapsible_space_.reset();
   position_ = line_info->ComputeWidth();
+
+  if (UNLIKELY(has_cloned_box_decorations_))
+    RecalcClonedBoxDecorations();
 }
 
 // Returns the style to use for |item_result_index|. Normally when handling
@@ -2217,9 +2267,13 @@ scoped_refptr<NGInlineBreakToken> NGLineBreaker::CreateBreakToken(
     return NGInlineBreakToken::Create(node_);
   return NGInlineBreakToken::Create(
       node_, current_style_.get(), item_index_, offset_,
-      ((is_after_forced_break_ ? NGInlineBreakToken::kIsForcedBreak : 0) |
-       (line_info.UseFirstLineStyle() ? NGInlineBreakToken::kUseFirstLineStyle
-                                      : 0)));
+      (is_after_forced_break_ ? NGInlineBreakToken::kIsForcedBreak : 0) |
+          (line_info.UseFirstLineStyle()
+               ? NGInlineBreakToken::kUseFirstLineStyle
+               : 0) |
+          (cloned_box_decorations_count_
+               ? NGInlineBreakToken::kHasClonedBoxDecorations
+               : 0));
 }
 
 void NGLineBreaker::PropagateBreakToken(

third_party/blink/renderer/core/layout/ng/inline/ng_line_breaker.h

@@ -44,6 +44,10 @@ class CORE_EXPORT NGLineBreaker {
 
   const NGInlineItemsData& ItemsData() const { return items_data_; }
 
+  // True if the last line has `box-decoration-break: clone`, which affected the
+  // size.
+  bool HasClonedBoxDecorations() const { return has_cloned_box_decorations_; }
+
   // Compute the next line break point and produces NGInlineItemResults for
   // the line.
   inline void NextLine(NGLineInfo* line_info) {
@@ -199,6 +203,7 @@ class CORE_EXPORT NGLineBreaker {
   void MoveToNextOf(const NGInlineItemResult&);
 
   void ComputeBaseDirection();
+  void RecalcClonedBoxDecorations();
 
   LayoutUnit AvailableWidth() const {
     DCHECK_EQ(available_width_, ComputeAvailableWidth());
@@ -315,6 +320,12 @@ class CORE_EXPORT NGLineBreaker {
 
   Vector<scoped_refptr<const NGBlockBreakToken>> propagated_break_tokens_;
 
+  // Fields for `box-decoration-break: clone`.
+  unsigned cloned_box_decorations_count_ = 0;
+  LayoutUnit cloned_box_decorations_initial_size_;
+  LayoutUnit cloned_box_decorations_end_size_;
+  bool has_cloned_box_decorations_ = false;
+
 #if DCHECK_IS_ON()
   // These fields are to detect rewind-loop.
   unsigned last_rewind_from_item_index_ = 0;

third_party/blink/renderer/core/layout/ng/ng_break_token.h

@@ -97,7 +97,7 @@ class CORE_EXPORT NGBreakToken : public RefCounted<NGBreakToken> {
   // The following bitfields are only to be used by NGInlineBreakToken (it's
   // defined here to save memory, since that class has no bitfields).
 
-  unsigned flags_ : 2;  // NGInlineBreakTokenFlags
+  unsigned flags_ : 3;  // NGInlineBreakTokenFlags
 
   // The following bitfields are only to be used by NGBlockBreakToken (it's
   // defined here to save memory, since that class has no bitfields).

third_party/blink/web_tests/FlagExpectations/disable-layout-ng

@@ -363,6 +363,9 @@ crbug.com/591099 external/wpt/css/vendor-imports/mozilla/mozilla-central-reftest
 ### external/wpt/quirks/
 crbug.com/591099 external/wpt/quirks/line-height-trailing-collapsable-whitespace.html [ Failure ]
 
+### fast/box-decoration-break/
+crbug.com/612641 fast/box-decoration-break/box-decoration-break-min-max.html [ Failure ]
+
 ### fast/css/
 crbug.com/591099 fast/css/absolute-inline-alignment-2.html [ Failure ]
 crbug.com/591099 fast/css/text-overflow-ellipsis-button.html [ Failure ]

third_party/blink/web_tests/fast/box-decoration-break/box-decoration-break-align-expected.html

+<!DOCTYPE html>
+<style>
+.border {
+  border-left: solid 10px orange;
+  border-right: solid 20px purple
+}
+div {
+  border: 1px solid blue;
+  width: 20ch;
+}
+.right {
+  text-align: right;
+}
+.center {
+  text-align: center;
+}
+</style>
+<body>
+  <div class="right">
+    <span class="border">Line1</span><br>
+    <span class="border">Line2</span><br>
+    <span class="border">Line3</span><br>
+  </div>
+  <div class="center">
+    <span class="border">Line1</span><br>
+    <span class="border">Line2</span><br>
+    <span class="border">Line3</span><br>
+  </div>
+</body>

third_party/blink/web_tests/fast/box-decoration-break/box-decoration-break-align.html

+<!DOCTYPE html>
+<style>
+.clone {
+  -webkit-box-decoration-break: clone;
+  box-decoration-break: clone;
+}
+.border {
+  border-left: solid 10px orange;
+  border-right: solid 20px purple
+}
+div {
+  width: 20ch;
+  border: 1px solid blue;
+}
+.right {
+  text-align: right;
+}
+.center {
+  text-align: center;
+}
+</style>
+<body>
+  <div class="right">
+    <span class="clone border">
+      Line1<br>
+      Line2<br>
+      Line3
+    </span>
+  </div>
+  <div class="center">
+    <span class="clone border">
+      Line1<br>
+      Line2<br>
+      Line3
+    </span>
+  </div>
+</body>

third_party/blink/web_tests/fast/box-decoration-break/box-decoration-break-min-max-expected.html

+<!DOCTYPE html>
+<style>
+.border {
+  border-left: solid 10px orange;
+  border-right: solid 20px purple
+}
+.padding {
+  padding-left: 10px;
+  padding-right: 20px;
+}
+div {
+  border: 5px solid blue;
+}
+.min {
+  width: min-content;
+}
+.max {
+  width: max-content;
+}
+</style>
+<body>
+  <div class="min">
+    <span class="border">line1</span>
+    <span class="border">line2</span>
+    <span class="border">line3</span>
+  </div>
+  <div class="min">
+    <span class="padding">line1</span>
+    <span class="padding">line2</span>
+    <span class="padding">line3</span>
+  </div>
+  <div class="max">
+    <span class="border">
+      line1 line2 line3
+    </span>
+  </div>
+  <div class="max">
+    <span class="border">line1 line2</span><br>
+    <span class="border">line3</span>
+  </div>
+</body>

third_party/blink/web_tests/fast/box-decoration-break/box-decoration-break-min-max.html

+<!DOCTYPE html>
+<style>
+.clone {
+  -webkit-box-decoration-break: clone;
+  box-decoration-break: clone;
+}
+.border {
+  border-left: solid 10px orange;
+  border-right: solid 20px purple
+}
+.padding {
+  padding-left: 10px;
+  padding-right: 20px;
+}
+div {
+  border: 5px solid blue;
+}
+.min {
+  width: min-content;
+}
+.max {
+  width: max-content;
+}
+</style>
+<body>
+  <div class="min">
+    <span class="clone border">
+      line1 line2 line3
+    </span>
+  </div>
+  <div class="min">
+    <span class="clone padding">
+      line1 line2 line3
+    </span>
+  </div>
+  <div class="max">
+    <span class="clone border">
+      line1 line2 line3
+    </span>
+  </div>
+  <div class="max">
+    <span class="clone border">
+      line1 line2<br>line3
+    </span>
+  </div>
+</body>