Commit 29ef555e authored by Koji Ishii's avatar Koji Ishii Committed by Commit Bot

[LayoutNG] Remove out_floats_for_min_max from NGLineBreaker

This patch removes |out_floats_for_min_max| from
|NGLineBreaker|. The caller instead computes the list of
unpositioned floats from |NGLineInfo::Results()|.

This change is needed for fixes for crbug.com/10118167 and
crbug.com/1001000, but does not fix them. Fixes some cases of
|ComputeContentSize()| DCHECK failures in crbug.com/976304,
but maybe not all.

|out_floats_for_min_max| was originally added in r619070
(crrev.com/c/1365984) to keep track of unpositioned floats.
But today, |NGLineInfo::Results()| has all unpositioned
floats. Furthermore, changing rewind logic for
crbug.com/10118167 caused several tests to fail because we
don't adjust |out_floats_for_min_max| when we rewind. This
patch avoids mismatch of the two lists by unifying them.

blink_perf.layout looks neutral:
https://pinpoint-dot-chromeperf.appspot.com/job/108a2986c20000
Current assumption is that looping items isn't too expensive,
but we could add a flag to |NGLineInfo| if this loop turned
out to be more expensive in some cases.

Bug: 976304, 10118167, 1001000
Change-Id: I4a5cb98f5dc6d331a699991390d7bd97ce51cd34
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1857844
Commit-Queue: Ian Kilpatrick <ikilpatrick@chromium.org>
Reviewed-by: default avatarIan Kilpatrick <ikilpatrick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#705820}
parent b56a2d60
...@@ -1692,13 +1692,9 @@ static LayoutUnit ComputeContentSize( ...@@ -1692,13 +1692,9 @@ static LayoutUnit ComputeContentSize(
MaxSizeFromMinSize max_size_from_min_size(items_data, *max_size_cache, MaxSizeFromMinSize max_size_from_min_size(items_data, *max_size_cache,
&floats_max_size); &floats_max_size);
Vector<LayoutObject*> floats_for_min_max;
do { do {
floats_for_min_max.Shrink(0);
NGLineInfo line_info; NGLineInfo line_info;
line_breaker.NextLine(input.percentage_resolution_block_size, line_breaker.NextLine(input.percentage_resolution_block_size, &line_info);
&floats_for_min_max, &line_info);
if (line_info.Results().IsEmpty()) if (line_info.Results().IsEmpty())
break; break;
...@@ -1708,8 +1704,13 @@ static LayoutUnit ComputeContentSize( ...@@ -1708,8 +1704,13 @@ static LayoutUnit ComputeContentSize(
DCHECK_EQ(inline_size.Round(), DCHECK_EQ(inline_size.Round(),
line_info.ComputeWidth().ClampNegativeToZero().Round()); line_info.ComputeWidth().ClampNegativeToZero().Round());
for (auto* floating_object : floats_for_min_max) { for (const NGInlineItemResult& item_result : line_info.Results()) {
DCHECK(floating_object->IsFloating()); DCHECK(item_result.item);
const NGInlineItem& item = *item_result.item;
if (item.Type() != NGInlineItem::kFloating)
continue;
LayoutObject* floating_object = item.GetLayoutObject();
DCHECK(floating_object && floating_object->IsFloating());
NGBlockNode float_node(ToLayoutBox(floating_object)); NGBlockNode float_node(ToLayoutBox(floating_object));
const ComputedStyle& float_style = float_node.Style(); const ComputedStyle& float_style = float_node.Style();
......
...@@ -323,16 +323,9 @@ void NGLineBreaker::PrepareNextLine(NGLineInfo* line_info) { ...@@ -323,16 +323,9 @@ void NGLineBreaker::PrepareNextLine(NGLineInfo* line_info) {
void NGLineBreaker::NextLine( void NGLineBreaker::NextLine(
LayoutUnit percentage_resolution_block_size_for_min_max, LayoutUnit percentage_resolution_block_size_for_min_max,
Vector<LayoutObject*>* out_floats_for_min_max,
NGLineInfo* line_info) { NGLineInfo* line_info) {
// out_floats_for_min_max is required for min/max and prohibited for regular
// content mode.
DCHECK(mode_ == NGLineBreakerMode::kContent || out_floats_for_min_max);
DCHECK(!(mode_ == NGLineBreakerMode::kContent && out_floats_for_min_max));
PrepareNextLine(line_info); PrepareNextLine(line_info);
BreakLine(percentage_resolution_block_size_for_min_max, BreakLine(percentage_resolution_block_size_for_min_max, line_info);
out_floats_for_min_max, line_info);
RemoveTrailingCollapsibleSpace(line_info); RemoveTrailingCollapsibleSpace(line_info);
const NGInlineItemResults& item_results = line_info->Results(); const NGInlineItemResults& item_results = line_info->Results();
...@@ -365,7 +358,6 @@ void NGLineBreaker::NextLine( ...@@ -365,7 +358,6 @@ void NGLineBreaker::NextLine(
void NGLineBreaker::BreakLine( void NGLineBreaker::BreakLine(
LayoutUnit percentage_resolution_block_size_for_min_max, LayoutUnit percentage_resolution_block_size_for_min_max,
Vector<LayoutObject*>* out_floats_for_min_max,
NGLineInfo* line_info) { NGLineInfo* line_info) {
DCHECK(!line_info->IsLastLine()); DCHECK(!line_info->IsLastLine());
const Vector<NGInlineItem>& items = Items(); const Vector<NGInlineItem>& items = Items();
...@@ -422,7 +414,7 @@ void NGLineBreaker::BreakLine( ...@@ -422,7 +414,7 @@ void NGLineBreaker::BreakLine(
continue; continue;
} }
if (item.Type() == NGInlineItem::kFloating) { if (item.Type() == NGInlineItem::kFloating) {
HandleFloat(item, out_floats_for_min_max, line_info); HandleFloat(item, line_info);
continue; continue;
} }
if (item.Type() == NGInlineItem::kBidiControl) { if (item.Type() == NGInlineItem::kBidiControl) {
...@@ -1358,7 +1350,6 @@ void NGLineBreaker::HandleAtomicInline( ...@@ -1358,7 +1350,6 @@ void NGLineBreaker::HandleAtomicInline(
// allowed to position a float "above" another float which has come before us // allowed to position a float "above" another float which has come before us
// in the document. // in the document.
void NGLineBreaker::HandleFloat(const NGInlineItem& item, void NGLineBreaker::HandleFloat(const NGInlineItem& item,
Vector<LayoutObject*>* out_floats_for_min_max,
NGLineInfo* line_info) { NGLineInfo* line_info) {
// When rewind occurs, an item may be handled multiple times. // When rewind occurs, an item may be handled multiple times.
// Since floats are put into a separate list, avoid handling same floats // Since floats are put into a separate list, avoid handling same floats
...@@ -1373,13 +1364,10 @@ void NGLineBreaker::HandleFloat(const NGInlineItem& item, ...@@ -1373,13 +1364,10 @@ void NGLineBreaker::HandleFloat(const NGInlineItem& item,
item_result->can_break_after = auto_wrap_; item_result->can_break_after = auto_wrap_;
MoveToNextOf(item); MoveToNextOf(item);
// If we are currently computing our min/max-content size simply append to // If we are currently computing our min/max-content size, simply append the
// the unpositioned floats list and abort. // unpositioned floats to |NGLineInfo| and abort.
if (mode_ != NGLineBreakerMode::kContent) { if (mode_ != NGLineBreakerMode::kContent)
DCHECK(out_floats_for_min_max);
out_floats_for_min_max->push_back(item.GetLayoutObject());
return; return;
}
// Make sure we populate the positioned_float inside the |item_result|. // Make sure we populate the positioned_float inside the |item_result|.
if (item_index_ <= handled_leading_floats_index_ && if (item_index_ <= handled_leading_floats_index_ &&
......
...@@ -47,7 +47,7 @@ class CORE_EXPORT NGLineBreaker { ...@@ -47,7 +47,7 @@ class CORE_EXPORT NGLineBreaker {
// Compute the next line break point and produces NGInlineItemResults for // Compute the next line break point and produces NGInlineItemResults for
// the line. // the line.
inline void NextLine(NGLineInfo* line_info) { inline void NextLine(NGLineInfo* line_info) {
NextLine(kIndefiniteSize, nullptr, line_info); NextLine(kIndefiniteSize, line_info);
} }
// During the min/max size calculation we need a special percentage // During the min/max size calculation we need a special percentage
...@@ -56,7 +56,6 @@ class CORE_EXPORT NGLineBreaker { ...@@ -56,7 +56,6 @@ class CORE_EXPORT NGLineBreaker {
// better yet, subclass or templetize the line-breaker for Min/Max computation // better yet, subclass or templetize the line-breaker for Min/Max computation
// if we can do that without incurring a performance penalty // if we can do that without incurring a performance penalty
void NextLine(LayoutUnit percentage_resolution_block_size_for_min_max, void NextLine(LayoutUnit percentage_resolution_block_size_for_min_max,
Vector<LayoutObject*>* out_floats_for_min_max,
NGLineInfo*); NGLineInfo*);
bool IsFinished() const { return item_index_ >= Items().size(); } bool IsFinished() const { return item_index_ >= Items().size(); }
...@@ -104,7 +103,6 @@ class CORE_EXPORT NGLineBreaker { ...@@ -104,7 +103,6 @@ class CORE_EXPORT NGLineBreaker {
NGLineInfo*); NGLineInfo*);
void BreakLine(LayoutUnit percentage_resolution_block_size_for_min_max, void BreakLine(LayoutUnit percentage_resolution_block_size_for_min_max,
Vector<LayoutObject*>* out_floats_for_min_max,
NGLineInfo*); NGLineInfo*);
void PrepareNextLine(NGLineInfo*); void PrepareNextLine(NGLineInfo*);
...@@ -173,7 +171,6 @@ class CORE_EXPORT NGLineBreaker { ...@@ -173,7 +171,6 @@ class CORE_EXPORT NGLineBreaker {
bool IsAtomicInlineBeforeNoBreakSpace( bool IsAtomicInlineBeforeNoBreakSpace(
const NGInlineItemResult& item_result) const; const NGInlineItemResult& item_result) const;
void HandleFloat(const NGInlineItem&, void HandleFloat(const NGInlineItem&,
Vector<LayoutObject*>* out_floats_for_min_max,
NGLineInfo*); NGLineInfo*);
void HandleOpenTag(const NGInlineItem&, NGLineInfo*); void HandleOpenTag(const NGInlineItem&, NGLineInfo*);
......
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