Commit 582e65aa authored by Koji Ishii's avatar Koji Ishii Committed by Commit Bot

[LayoutNG] Ensure ShapingLineBreaker moves offset forward

After the fix for crbug.com/915567, there are reports of
cases where ShapingLineBreaker does not move forward.

While I could not find a reproducible case for this, there
is a case it may occur from the code review. This patch fixes
the case, along with adding CHECK to ShapingLineBreaker.

This is a speculative fix.

Bug: 928178
Change-Id: I51cf733fdad9abf2feee529bfaf90a7e1bdbb09d
Reviewed-on: https://chromium-review.googlesource.com/c/1453777
Commit-Queue: Emil A Eklund <eae@chromium.org>
Reviewed-by: default avatarEmil A Eklund <eae@chromium.org>
Cr-Commit-Position: refs/heads/master@{#629151}
parent 0829f9e2
......@@ -255,7 +255,8 @@ scoped_refptr<const ShapeResultView> ShapingLineBreaker::ShapeLine(
return ShapeToEnd(start, first_safe, range_start, range_end);
}
}
DCHECK_GT(break_opportunity.offset, start);
// It is critical to move forward, or callers may end up in an infinite loop.
CHECK_GT(break_opportunity.offset, start);
// If the start offset is not at a safe-to-break boundary the content between
// the start and the next safe-to-break boundary needs to be reshaped and the
......@@ -281,50 +282,58 @@ scoped_refptr<const ShapeResultView> ShapingLineBreaker::ShapeLine(
scoped_refptr<const ShapeResult> line_end_result;
unsigned last_safe = break_opportunity.offset;
if (break_opportunity.offset > start) {
// If the previous valid break opportunity is not at a safe-to-break
// boundary reshape between the safe-to-break offset and the valid break
// offset. If the resulting width exceeds the available space the
// preceding boundary is tried until the available space is sufficient.
while (true) {
DCHECK_LE(first_safe, break_opportunity.offset);
last_safe = std::max(
result_->CachedPreviousSafeToBreakOffset(break_opportunity.offset),
first_safe);
DCHECK_LE(last_safe, break_opportunity.offset);
DCHECK_GE(last_safe, first_safe);
if (last_safe == break_opportunity.offset)
break;
DCHECK_LE(break_opportunity.offset, range_end);
if (is_overflow) {
line_end_result = Shape(last_safe, break_opportunity.offset);
break;
}
LayoutUnit safe_position = SnapStart(
result_->CachedPositionForOffset(last_safe - range_start), direction);
// If the previous valid break opportunity is not at a safe-to-break
// boundary reshape between the safe-to-break offset and the valid break
// offset. If the resulting width exceeds the available space the
// preceding boundary is tried until the available space is sufficient.
while (true) {
DCHECK_LE(first_safe, break_opportunity.offset);
last_safe = std::max(
result_->CachedPreviousSafeToBreakOffset(break_opportunity.offset),
first_safe);
DCHECK_LE(last_safe, break_opportunity.offset);
DCHECK_GE(last_safe, first_safe);
if (last_safe == break_opportunity.offset)
break;
DCHECK_LE(break_opportunity.offset, range_end);
if (is_overflow) {
line_end_result = Shape(last_safe, break_opportunity.offset);
if (line_end_result->SnappedWidth() <=
FlipRtl(end_position - safe_position, direction))
break;
// Doesn't fit after the reshape. Try the previous break opportunity.
line_end_result = nullptr;
break_opportunity =
PreviousBreakOpportunity(break_opportunity.offset - 1, start);
if (break_opportunity.offset > start)
continue;
// No suitable break opportunity, not exceeding the available space,
// found. Any break opportunities beyond candidate_break won't fit
// either because the ShapeResult has the full context.
// This line will overflow, but there are multiple choices to break,
// because none can fit. The one after candidate_break is better for
// ligatures, but the one before is better for kernings.
break_opportunity = PreviousBreakOpportunity(candidate_break, start);
// Loop once more to compute last_safe for the new break opportunity.
is_overflow = true;
break;
}
LayoutUnit safe_position = SnapStart(
result_->CachedPositionForOffset(last_safe - range_start), direction);
line_end_result = Shape(last_safe, break_opportunity.offset);
if (line_end_result->SnappedWidth() <=
FlipRtl(end_position - safe_position, direction))
break;
// Doesn't fit after the reshape. Try the previous break opportunity.
line_end_result = nullptr;
break_opportunity =
PreviousBreakOpportunity(break_opportunity.offset - 1, start);
if (break_opportunity.offset > start)
continue;
// No suitable break opportunity, not exceeding the available space,
// found. Any break opportunities beyond candidate_break won't fit
// either because the ShapeResult has the full context.
// This line will overflow, but there are multiple choices to break,
// because none can fit. The one after candidate_break is better for
// ligatures, but the one before is better for kernings.
break_opportunity = PreviousBreakOpportunity(candidate_break, start);
if (break_opportunity.offset <= start) {
break_opportunity = NextBreakOpportunity(
std::max(candidate_break, start + 1), start, range_end);
if (break_opportunity.offset >= range_end) {
result_out->break_offset = range_end;
return ShapeToEnd(start, first_safe, range_start, range_end);
}
}
// Loop once more to compute last_safe for the new break opportunity.
is_overflow = true;
}
// It is critical to move forward, or callers may end up in an infinite loop.
CHECK_GT(break_opportunity.offset, start);
DCHECK_GE(break_opportunity.offset, last_safe);
DCHECK_EQ(break_opportunity.offset - start,
(line_start_result ? line_start_result->NumCharacters() : 0) +
......@@ -344,7 +353,6 @@ scoped_refptr<const ShapeResultView> ShapingLineBreaker::ShapeLine(
segments[count++] = {line_end_result.get(), last_safe, max_length};
auto line_result = ShapeResultView::Create(&segments[0], count);
DCHECK_GT(break_opportunity.offset, start);
DCHECK_LE(break_opportunity.offset, range_end);
DCHECK_EQ(break_opportunity.offset - start, line_result->NumCharacters());
......
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