Commit f9a83ba3 authored by Koji Ishii's avatar Koji Ishii Committed by Commit Bot

Fix line cache when `-webkit-line-clamp` is specified

|NGBlockLayoutAlgorithm| has two branches, one when
|lines_until_clamp_| becomes |<= 0| by the layout, and another
when it is |== 0|.

To make sure we run all these logic, this patch:
* Updates |lines_until_clamp_| by the number of reused lines.
* Limits reusing only up to 1, so that conditions above go
  normal layout codepath.

Bug: 1140951
Change-Id: I15fe81b4957fe70d23bb88aae700642297bb1550
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2491561Reviewed-by: default avatarIan Kilpatrick <ikilpatrick@chromium.org>
Commit-Queue: Koji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#819867}
parent b9a179d1
...@@ -199,7 +199,8 @@ NGFragmentItemsBuilder::AddPreviousItems( ...@@ -199,7 +199,8 @@ NGFragmentItemsBuilder::AddPreviousItems(
const NGFragmentItems& items, const NGFragmentItems& items,
const PhysicalSize& container_size, const PhysicalSize& container_size,
NGBoxFragmentBuilder* container_builder, NGBoxFragmentBuilder* container_builder,
const NGFragmentItem* end_item) { const NGFragmentItem* end_item,
wtf_size_t max_lines) {
if (end_item) { if (end_item) {
DCHECK(node_); DCHECK(node_);
DCHECK(container_builder); DCHECK(container_builder);
...@@ -235,6 +236,7 @@ NGFragmentItemsBuilder::AddPreviousItems( ...@@ -235,6 +236,7 @@ NGFragmentItemsBuilder::AddPreviousItems(
const NGInlineBreakToken* last_break_token = nullptr; const NGInlineBreakToken* last_break_token = nullptr;
const NGInlineItemsData* items_data = nullptr; const NGInlineItemsData* items_data = nullptr;
LayoutUnit used_block_size; LayoutUnit used_block_size;
wtf_size_t line_count = 0;
for (NGInlineCursor cursor(items); cursor;) { for (NGInlineCursor cursor(items); cursor;) {
DCHECK(cursor.Current().Item()); DCHECK(cursor.Current().Item());
...@@ -286,6 +288,8 @@ NGFragmentItemsBuilder::AddPreviousItems( ...@@ -286,6 +288,8 @@ NGFragmentItemsBuilder::AddPreviousItems(
line_child.Size()), line_child.Size()),
line_child); line_child);
} }
if (++line_count == max_lines)
break;
cursor.MoveToNextSkippingChildren(); cursor.MoveToNextSkippingChildren();
continue; continue;
} }
...@@ -299,7 +303,10 @@ NGFragmentItemsBuilder::AddPreviousItems( ...@@ -299,7 +303,10 @@ NGFragmentItemsBuilder::AddPreviousItems(
if (end_item && last_break_token) { if (end_item && last_break_token) {
DCHECK(!last_break_token->IsFinished()); DCHECK(!last_break_token->IsFinished());
return AddPreviousItemsResult{last_break_token, used_block_size, true}; DCHECK_GT(line_count, 0u);
DCHECK(!max_lines || line_count <= max_lines);
return AddPreviousItemsResult{last_break_token, used_block_size, line_count,
true};
} }
return AddPreviousItemsResult(); return AddPreviousItemsResult();
} }
......
...@@ -89,6 +89,7 @@ class CORE_EXPORT NGFragmentItemsBuilder { ...@@ -89,6 +89,7 @@ class CORE_EXPORT NGFragmentItemsBuilder {
public: public:
const NGInlineBreakToken* inline_break_token = nullptr; const NGInlineBreakToken* inline_break_token = nullptr;
LayoutUnit used_block_size; LayoutUnit used_block_size;
wtf_size_t line_count = 0;
bool succeeded = false; bool succeeded = false;
}; };
...@@ -100,7 +101,8 @@ class CORE_EXPORT NGFragmentItemsBuilder { ...@@ -100,7 +101,8 @@ class CORE_EXPORT NGFragmentItemsBuilder {
const NGFragmentItems& items, const NGFragmentItems& items,
const PhysicalSize& container_size, const PhysicalSize& container_size,
NGBoxFragmentBuilder* container_builder = nullptr, NGBoxFragmentBuilder* container_builder = nullptr,
const NGFragmentItem* end_item = nullptr); const NGFragmentItem* end_item = nullptr,
wtf_size_t max_lines = 0);
struct ItemWithOffset { struct ItemWithOffset {
DISALLOW_NEW(); DISALLOW_NEW();
......
...@@ -992,15 +992,23 @@ bool NGBlockLayoutAlgorithm::TryReuseFragmentsFromCache( ...@@ -992,15 +992,23 @@ bool NGBlockLayoutAlgorithm::TryReuseFragmentsFromCache(
if (!end_item || end_item == &previous_items->front()) if (!end_item || end_item == &previous_items->front())
return false; return false;
wtf_size_t max_lines = 0;
if (lines_until_clamp_) {
// There is an additional logic for the last clamped line. Reuse only up to
// before that to use the same logic.
if (*lines_until_clamp_ <= 1)
return false;
max_lines = *lines_until_clamp_ - 1;
}
const auto& children = container_builder_.Children(); const auto& children = container_builder_.Children();
const wtf_size_t children_before = children.size(); const wtf_size_t children_before = children.size();
NGFragmentItemsBuilder* items_builder = container_builder_.ItemsBuilder(); NGFragmentItemsBuilder* items_builder = container_builder_.ItemsBuilder();
const NGConstraintSpace& space = ConstraintSpace(); const NGConstraintSpace& space = ConstraintSpace();
DCHECK_EQ(items_builder->GetWritingMode(), space.GetWritingMode()); DCHECK_EQ(items_builder->GetWritingDirection(), space.GetWritingDirection());
DCHECK_EQ(items_builder->Direction(), space.Direction()); const auto result =
const auto result = items_builder->AddPreviousItems( items_builder->AddPreviousItems(*previous_items, previous_fragment.Size(),
*previous_items, previous_fragment.Size(), &container_builder_, end_item); &container_builder_, end_item, max_lines);
if (UNLIKELY(!result.succeeded)) { if (UNLIKELY(!result.succeeded)) {
DCHECK_EQ(children.size(), children_before); DCHECK_EQ(children.size(), children_before);
DCHECK(!result.used_block_size); DCHECK(!result.used_block_size);
...@@ -1008,6 +1016,13 @@ bool NGBlockLayoutAlgorithm::TryReuseFragmentsFromCache( ...@@ -1008,6 +1016,13 @@ bool NGBlockLayoutAlgorithm::TryReuseFragmentsFromCache(
return false; return false;
} }
DCHECK_GT(result.line_count, 0u);
DCHECK(!max_lines || result.line_count <= max_lines);
if (lines_until_clamp_) {
DCHECK_GT(*lines_until_clamp_, static_cast<int>(result.line_count));
lines_until_clamp_ = *lines_until_clamp_ - result.line_count;
}
// |AddPreviousItems| may have added more than one lines. Propagate baselines // |AddPreviousItems| may have added more than one lines. Propagate baselines
// from them. // from them.
for (const auto& child : base::make_span(children).subspan(children_before)) { for (const auto& child : base::make_span(children).subspan(children_before)) {
......
<!DOCTYPE html>
<style>
#container {
display: -webkit-box;
-webkit-line-clamp: 2;
-webkit-box-orient: vertical;
font-size: 20px;
width: 10ch;
}
</style>
<body>
<div id="container">
abc def ghi jkl mno pqr stu vwx yz
123 456 789
</div>
</body>
<!DOCTYPE html>
<title>CSS Overflow: appending to a box with -webkit-line-clamp</title>
<link rel="help" href="https://drafts.csswg.org/css-overflow-3/#webkit-line-clamp">
<link rel="match" href="reference/webkit-line-clamp-dynamic-001-ref.html">
<link rel="author" title="Koji Ishii" href="mailto:kojii@chromium.org">
<style>
#container {
display: -webkit-box;
-webkit-line-clamp: 2;
-webkit-box-orient: vertical;
font-size: 20px;
width: 10ch;
}
</style>
<body>
<div id="container">
abc def ghi jkl mno pqr stu vwx yz
</div>
<script>
testAppend();
function testAppend() {
document.body.offsetTop;
let span = document.createElement('span');
span.textContent = '123 456 789';
container.appendChild(span);
}
</script>
</body>
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