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

[LayoutNG] Fix crash for very long line with `break-word`

This patch fixes NGLineBreaker crash when handling a very
long line.

Because line breaker determines overflow by the position
being larger than the available width, it cannot stop if the
available width is |LayoutUnit::Max()|. This patch clamps the
available width by |LayoutUnit::NearlyMax()| to prevent that.

The `word-wrap: break-word` is not necessary to cause the
failure, but it makes things worse by setting |ShapeResult|
to |nullptr| when overflow occurs for the performance reason.

This patch also changes |BreakText| to return whether the
overflow occurred or not, rather than callers to determine it
from the position. Doing so helps clearer communication with
the callers.

Bug: 961987
Change-Id: I17eb758aca8b9b6d3e2f328e5b1b49acb83c0a5e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1613138
Commit-Queue: Koji Ishii <kojii@chromium.org>
Reviewed-by: default avatarEmil A Eklund <eae@chromium.org>
Cr-Commit-Position: refs/heads/master@{#660871}
parent c5e775d5
...@@ -157,6 +157,14 @@ inline void ClearNeedsLayout(const NGInlineItem& item) { ...@@ -157,6 +157,14 @@ inline void ClearNeedsLayout(const NGInlineItem& item) {
} // namespace } // namespace
LayoutUnit NGLineBreaker::ComputeAvailableWidth() const {
LayoutUnit available_width = line_opportunity_.AvailableInlineSize();
// Available width must be smaller than |LayoutUnit::Max()| so that the
// position can be larger.
available_width = std::min(available_width, LayoutUnit::NearlyMax());
return available_width;
}
NGLineBreaker::NGLineBreaker(NGInlineNode node, NGLineBreaker::NGLineBreaker(NGInlineNode node,
NGLineBreakerMode mode, NGLineBreakerMode mode,
const NGConstraintSpace& space, const NGConstraintSpace& space,
...@@ -184,6 +192,7 @@ NGLineBreaker::NGLineBreaker(NGInlineNode node, ...@@ -184,6 +192,7 @@ NGLineBreaker::NGLineBreaker(NGInlineNode node,
leading_floats_(leading_floats), leading_floats_(leading_floats),
handled_leading_floats_index_(handled_leading_floats_index), handled_leading_floats_index_(handled_leading_floats_index),
base_direction_(node_.BaseDirection()) { base_direction_(node_.BaseDirection()) {
available_width_ = ComputeAvailableWidth();
break_iterator_.SetBreakSpace(BreakSpaceType::kBeforeSpaceRun); break_iterator_.SetBreakSpace(BreakSpaceType::kBeforeSpaceRun);
if (break_token) { if (break_token) {
...@@ -235,20 +244,23 @@ void NGLineBreaker::SetMaxSizeCache(MaxSizeCache* max_size_cache) { ...@@ -235,20 +244,23 @@ void NGLineBreaker::SetMaxSizeCache(MaxSizeCache* max_size_cache) {
max_size_cache_ = max_size_cache; max_size_cache_ = max_size_cache;
} }
void NGLineBreaker::SetLineEndFragment( LayoutUnit NGLineBreaker::SetLineEndFragment(
scoped_refptr<const NGPhysicalTextFragment> fragment, scoped_refptr<const NGPhysicalTextFragment> fragment,
NGLineInfo* line_info) { NGLineInfo* line_info) {
LayoutUnit inline_size;
bool is_horizontal = bool is_horizontal =
IsHorizontalWritingMode(constraint_space_.GetWritingMode()); IsHorizontalWritingMode(constraint_space_.GetWritingMode());
if (line_info->LineEndFragment()) { if (line_info->LineEndFragment()) {
const PhysicalSize& size = line_info->LineEndFragment()->Size(); const PhysicalSize& size = line_info->LineEndFragment()->Size();
position_ -= is_horizontal ? size.width : size.height; inline_size = is_horizontal ? -size.width : -size.height;
} }
if (fragment) { if (fragment) {
const PhysicalSize& size = fragment->Size(); const PhysicalSize& size = fragment->Size();
position_ += is_horizontal ? size.width : size.height; inline_size = is_horizontal ? size.width : size.height;
} }
line_info->SetLineEndFragment(std::move(fragment)); line_info->SetLineEndFragment(std::move(fragment));
position_ += inline_size;
return inline_size;
} }
// Compute the base direction for bidi algorithm for this line. // Compute the base direction for bidi algorithm for this line.
...@@ -519,17 +531,18 @@ void NGLineBreaker::HandleText(const NGInlineItem& item, ...@@ -519,17 +531,18 @@ void NGLineBreaker::HandleText(const NGInlineItem& item,
// Try to break inside of this text item. // Try to break inside of this text item.
LayoutUnit available_width = AvailableWidthToFit(); LayoutUnit available_width = AvailableWidthToFit();
BreakText(item_result, item, shape_result, available_width - position_, BreakResult break_result =
line_info); BreakText(item_result, item, shape_result, available_width - position_,
line_info);
LayoutUnit next_position = position_ + item_result->inline_size; DCHECK(item_result->shape_result ||
bool is_overflow = next_position > available_width; (break_result == kOverflow && break_anywhere_if_overflow_ &&
DCHECK(is_overflow || item_result->shape_result); !override_break_anywhere_));
position_ = next_position; position_ += item_result->inline_size;
item_result->may_break_inside = !is_overflow; DCHECK_EQ(break_result == kSuccess, position_ <= available_width);
item_result->may_break_inside = break_result == kSuccess;
MoveToNextOf(*item_result); MoveToNextOf(*item_result);
if (!is_overflow || if (break_result == kSuccess ||
(state_ == LineBreakState::kTrailing && item_result->shape_result)) { (state_ == LineBreakState::kTrailing && item_result->shape_result)) {
if (item_result->end_offset < item.EndOffset()) { if (item_result->end_offset < item.EndOffset()) {
// The break point found, and text follows. Break here, after trailing // The break point found, and text follows. Break here, after trailing
...@@ -542,6 +555,7 @@ void NGLineBreaker::HandleText(const NGInlineItem& item, ...@@ -542,6 +555,7 @@ void NGLineBreaker::HandleText(const NGInlineItem& item,
return; return;
} }
DCHECK_EQ(break_result, kOverflow);
return HandleOverflow(line_info); return HandleOverflow(line_info);
} }
...@@ -570,11 +584,12 @@ void NGLineBreaker::HandleText(const NGInlineItem& item, ...@@ -570,11 +584,12 @@ void NGLineBreaker::HandleText(const NGInlineItem& item,
MoveToNextOf(item); MoveToNextOf(item);
} }
void NGLineBreaker::BreakText(NGInlineItemResult* item_result, NGLineBreaker::BreakResult NGLineBreaker::BreakText(
const NGInlineItem& item, NGInlineItemResult* item_result,
const ShapeResult& item_shape_result, const NGInlineItem& item,
LayoutUnit available_width, const ShapeResult& item_shape_result,
NGLineInfo* line_info) { LayoutUnit available_width,
NGLineInfo* line_info) {
DCHECK(item.Type() == NGInlineItem::kText || DCHECK(item.Type() == NGInlineItem::kText ||
(item.Type() == NGInlineItem::kControl && (item.Type() == NGInlineItem::kControl &&
Text()[item.StartOffset()] == kTabulationCharacter)); Text()[item.StartOffset()] == kTabulationCharacter));
...@@ -600,7 +615,6 @@ void NGLineBreaker::BreakText(NGInlineItemResult* item_result, ...@@ -600,7 +615,6 @@ void NGLineBreaker::BreakText(NGInlineItemResult* item_result,
shape_callback, &shape_callback_context); shape_callback, &shape_callback_context);
if (!enable_soft_hyphen_) if (!enable_soft_hyphen_)
breaker.DisableSoftHyphen(); breaker.DisableSoftHyphen();
available_width = std::max(LayoutUnit(0), available_width);
// Use kStartShouldBeSafe if at the beginning of a line. // Use kStartShouldBeSafe if at the beginning of a line.
unsigned options = ShapingLineBreaker::kDefaultOptions; unsigned options = ShapingLineBreaker::kDefaultOptions;
...@@ -621,7 +635,8 @@ void NGLineBreaker::BreakText(NGInlineItemResult* item_result, ...@@ -621,7 +635,8 @@ void NGLineBreaker::BreakText(NGInlineItemResult* item_result,
ShapingLineBreaker::Result result; ShapingLineBreaker::Result result;
scoped_refptr<const ShapeResultView> shape_result = breaker.ShapeLine( scoped_refptr<const ShapeResultView> shape_result = breaker.ShapeLine(
item_result->start_offset, available_width, options, &result); item_result->start_offset, available_width.ClampNegativeToZero(), options,
&result);
// If this item overflows and 'break-word' is set, this line will be // If this item overflows and 'break-word' is set, this line will be
// rewinded. Making this item long enough to overflow is enough. // rewinded. Making this item long enough to overflow is enough.
...@@ -629,13 +644,15 @@ void NGLineBreaker::BreakText(NGInlineItemResult* item_result, ...@@ -629,13 +644,15 @@ void NGLineBreaker::BreakText(NGInlineItemResult* item_result,
DCHECK(options & ShapingLineBreaker::kNoResultIfOverflow); DCHECK(options & ShapingLineBreaker::kNoResultIfOverflow);
item_result->inline_size = available_width + 1; item_result->inline_size = available_width + 1;
item_result->end_offset = item.EndOffset(); item_result->end_offset = item.EndOffset();
return; return kOverflow;
} }
DCHECK_EQ(shape_result->NumCharacters(), DCHECK_EQ(shape_result->NumCharacters(),
result.break_offset - item_result->start_offset); result.break_offset - item_result->start_offset);
LayoutUnit inline_size = shape_result->SnappedWidth().ClampNegativeToZero();
item_result->inline_size = inline_size;
if (result.is_hyphenated) { if (result.is_hyphenated) {
SetLineEndFragment( inline_size += SetLineEndFragment(
CreateHyphenFragment(node_, constraint_space_.GetWritingMode(), item), CreateHyphenFragment(node_, constraint_space_.GetWritingMode(), item),
line_info); line_info);
...@@ -673,6 +690,7 @@ void NGLineBreaker::BreakText(NGInlineItemResult* item_result, ...@@ -673,6 +690,7 @@ void NGLineBreaker::BreakText(NGInlineItemResult* item_result,
break_iterator_.IsBreakable(item_result->end_offset); break_iterator_.IsBreakable(item_result->end_offset);
trailing_whitespace_ = WhitespaceState::kUnknown; trailing_whitespace_ = WhitespaceState::kUnknown;
} }
return inline_size <= available_width ? kSuccess : kOverflow;
} }
// This function handles text item for min-content. The specialized logic is // This function handles text item for min-content. The specialized logic is
...@@ -1288,6 +1306,7 @@ void NGLineBreaker::HandleFloat(const NGInlineItem& item, ...@@ -1288,6 +1306,7 @@ void NGLineBreaker::HandleFloat(const NGInlineItem& item,
line_opportunity_ = opportunity.ComputeLineLayoutOpportunity( line_opportunity_ = opportunity.ComputeLineLayoutOpportunity(
constraint_space_, line_opportunity_.line_block_size, LayoutUnit()); constraint_space_, line_opportunity_.line_block_size, LayoutUnit());
available_width_ = ComputeAvailableWidth();
DCHECK_GE(AvailableWidth(), LayoutUnit()); DCHECK_GE(AvailableWidth(), LayoutUnit());
} }
...@@ -1441,14 +1460,18 @@ void NGLineBreaker::HandleOverflow(NGLineInfo* line_info) { ...@@ -1441,14 +1460,18 @@ void NGLineBreaker::HandleOverflow(NGLineInfo* line_info) {
LayoutUnit item_available_width = LayoutUnit item_available_width =
std::min(-width_to_rewind, item_result->inline_size - 1); std::min(-width_to_rewind, item_result->inline_size - 1);
SetCurrentStyle(*item.Style()); SetCurrentStyle(*item.Style());
BreakText(item_result, item, *item.TextShapeResult(), BreakResult break_result =
item_available_width, line_info); BreakText(item_result, item, *item.TextShapeResult(),
item_available_width, line_info);
#if DCHECK_IS_ON() #if DCHECK_IS_ON()
item_result->CheckConsistency(true); item_result->CheckConsistency(true);
#endif #endif
// If BreakText() changed this item small enough to fit, break here. // If BreakText() changed this item small enough to fit, break here.
if (item_result->inline_size <= item_available_width) { DCHECK_EQ(break_result == kSuccess,
DCHECK(item_result->end_offset < item.EndOffset()); item_result->inline_size <= item_available_width);
if (break_result == kSuccess) {
DCHECK_LE(item_result->inline_size, item_available_width);
DCHECK_LT(item_result->end_offset, item.EndOffset());
DCHECK(item_result->can_break_after); DCHECK(item_result->can_break_after);
DCHECK_LE(i + 1, item_results->size()); DCHECK_LE(i + 1, item_results->size());
if (i + 1 == item_results->size()) { if (i + 1 == item_results->size()) {
......
...@@ -98,8 +98,8 @@ class CORE_EXPORT NGLineBreaker { ...@@ -98,8 +98,8 @@ class CORE_EXPORT NGLineBreaker {
unsigned end_offset, unsigned end_offset,
NGLineInfo*); NGLineInfo*);
NGInlineItemResult* AddItem(const NGInlineItem&, NGLineInfo*); NGInlineItemResult* AddItem(const NGInlineItem&, NGLineInfo*);
void SetLineEndFragment(scoped_refptr<const NGPhysicalTextFragment>, LayoutUnit SetLineEndFragment(scoped_refptr<const NGPhysicalTextFragment>,
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, Vector<LayoutObject*>* out_floats_for_min_max,
...@@ -126,11 +126,12 @@ class CORE_EXPORT NGLineBreaker { ...@@ -126,11 +126,12 @@ class CORE_EXPORT NGLineBreaker {
HandleText(item, *item.TextShapeResult(), line_info); HandleText(item, *item.TextShapeResult(), line_info);
} }
void HandleText(const NGInlineItem& item, const ShapeResult&, NGLineInfo*); void HandleText(const NGInlineItem& item, const ShapeResult&, NGLineInfo*);
void BreakText(NGInlineItemResult*, enum BreakResult { kSuccess, kOverflow };
const NGInlineItem&, BreakResult BreakText(NGInlineItemResult*,
const ShapeResult&, const NGInlineItem&,
LayoutUnit available_width, const ShapeResult&,
NGLineInfo*); LayoutUnit available_width,
NGLineInfo*);
bool HandleTextForFastMinContent(NGInlineItemResult*, bool HandleTextForFastMinContent(NGInlineItemResult*,
const NGInlineItem&, const NGInlineItem&,
const ShapeResult&, const ShapeResult&,
...@@ -179,11 +180,13 @@ class CORE_EXPORT NGLineBreaker { ...@@ -179,11 +180,13 @@ class CORE_EXPORT NGLineBreaker {
void ComputeBaseDirection(); void ComputeBaseDirection();
LayoutUnit AvailableWidth() const { LayoutUnit AvailableWidth() const {
return line_opportunity_.AvailableInlineSize(); DCHECK_EQ(available_width_, ComputeAvailableWidth());
return available_width_;
} }
LayoutUnit AvailableWidthToFit() const { LayoutUnit AvailableWidthToFit() const {
return AvailableWidth().AddEpsilon(); return AvailableWidth().AddEpsilon();
} }
LayoutUnit ComputeAvailableWidth() const;
// Represents the current offset of the input. // Represents the current offset of the input.
LineBreakState state_; LineBreakState state_;
...@@ -197,6 +200,7 @@ class CORE_EXPORT NGLineBreaker { ...@@ -197,6 +200,7 @@ class CORE_EXPORT NGLineBreaker {
// The current position from inline_start. Unlike NGInlineLayoutAlgorithm // The current position from inline_start. Unlike NGInlineLayoutAlgorithm
// that computes position in visual order, this position in logical order. // that computes position in visual order, this position in logical order.
LayoutUnit position_; LayoutUnit position_;
LayoutUnit available_width_;
NGLineLayoutOpportunity line_opportunity_; NGLineLayoutOpportunity line_opportunity_;
NGInlineNode node_; NGInlineNode node_;
......
<!DOCTYPE html>
<title>CSS Text Test: very long line with `overflow-wrap: break-word` should not crash</title>
<link rel="help" href="https://crbug.com/961987">
<link rel="author" title="Koji Ishii" href="kojii@chromium.org">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<style>
html, body {
margin: 0;
}
body {
overflow-wrap: break-word;
width: 2147483648px;
}
div {
/* Double the width in case CSS parser clamps the body width. */
width: 200%;
}
span {
border-left: 2147483648px solid;
}
</style>
<body>
<div><span>x</span></div>
<script>
test(() => { document.body.offsetTop; });
</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