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

Fix HarfBuzzShaperTest::SafeToBreakMissingRun

This test was added when we found Mac produces rare
ShapeResult, by constructing the same run/glyph data
artificially.

During tweaking the tests, a character was added to runs,
but forgot to update the NumCharacters(), which made
Start/EndIndexForResult() incorrect.

This patch fixes that, and also add CheckConsistency() and
two new EXPECT() to ensure the aritificially built
ShapeResult is good.

Bug: 636993
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
Change-Id: Iea0189f370f9e3d4546f33582f6ad363929511cc
Reviewed-on: https://chromium-review.googlesource.com/1100599Reviewed-by: default avatarEmil A Eklund <eae@chromium.org>
Commit-Queue: Emil A Eklund <eae@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567332}
parent cb482510
...@@ -1234,11 +1234,17 @@ TEST_F(HarfBuzzShaperTest, DISABLED_SafeToBreakArabicCommonLigatures) { ...@@ -1234,11 +1234,17 @@ TEST_F(HarfBuzzShaperTest, DISABLED_SafeToBreakArabicCommonLigatures) {
// RTL on Mac may not have runs for all characters. crbug.com/774034 // RTL on Mac may not have runs for all characters. crbug.com/774034
TEST_P(ShapeParameterTest, SafeToBreakMissingRun) { TEST_P(ShapeParameterTest, SafeToBreakMissingRun) {
TextDirection direction = GetParam(); TextDirection direction = GetParam();
scoped_refptr<ShapeResult> result = ShapeResult::Create(&font, 7, direction); scoped_refptr<ShapeResult> result = ShapeResult::Create(&font, 8, direction);
result->InsertRunForTesting(2, 1, direction, {0}); result->InsertRunForTesting(2, 1, direction, {0});
result->InsertRunForTesting(3, 3, direction, {0, 1}); result->InsertRunForTesting(3, 3, direction, {0, 1});
// The character index 7 is missing. // The character index 6 and 7 is missing.
result->InsertRunForTesting(8, 2, direction, {0}); result->InsertRunForTesting(8, 2, direction, {0});
#if DCHECK_IS_ON()
result->CheckConsistency();
#endif
EXPECT_EQ(2u, result->StartIndexForResult());
EXPECT_EQ(10u, result->EndIndexForResult());
EXPECT_EQ(2u, result->NextSafeToBreakOffset(2)); EXPECT_EQ(2u, result->NextSafeToBreakOffset(2));
EXPECT_EQ(3u, result->NextSafeToBreakOffset(3)); EXPECT_EQ(3u, result->NextSafeToBreakOffset(3));
......
...@@ -821,6 +821,7 @@ ShapeResult::RunInfo* ShapeResult::InsertRunForTesting( ...@@ -821,6 +821,7 @@ ShapeResult::RunInfo* ShapeResult::InsertRunForTesting(
// RTL runs have glyphs in the descending order of character_index. // RTL runs have glyphs in the descending order of character_index.
if (Rtl()) if (Rtl())
run->glyph_data_.Reverse(); run->glyph_data_.Reverse();
num_glyphs_ += run->NumGlyphs();
RunInfo* run_ptr = run.get(); RunInfo* run_ptr = run.get();
InsertRun(std::move(run)); InsertRun(std::move(run));
return run_ptr; return run_ptr;
...@@ -1013,24 +1014,28 @@ void ShapeResult::CheckConsistency() const { ...@@ -1013,24 +1014,28 @@ void ShapeResult::CheckConsistency() const {
return; return;
} }
unsigned index = StartIndexForResult(); const unsigned start_index = StartIndexForResult();
unsigned index = start_index;
unsigned num_glyphs = 0; unsigned num_glyphs = 0;
if (!Rtl()) { if (!Rtl()) {
for (const auto& run : runs_) { for (const auto& run : runs_) {
DCHECK_EQ(index, run->start_index_); // Characters maybe missing, but must be in increasing order.
index += run->num_characters_; DCHECK_GE(run->start_index_, index);
index = run->start_index_ + run->num_characters_;
num_glyphs += run->glyph_data_.size(); num_glyphs += run->glyph_data_.size();
} }
} else { } else {
// RTL on Mac may not have runs for the all characters. crbug.com/774034 // RTL on Mac may not have runs for the all characters. crbug.com/774034
index = runs_.back()->start_index_; index = runs_.back()->start_index_;
for (const auto& run : base::Reversed(runs_)) { for (const auto& run : base::Reversed(runs_)) {
DCHECK_EQ(index, run->start_index_); DCHECK_GE(run->start_index_, index);
index += run->num_characters_; index = run->start_index_ + run->num_characters_;
num_glyphs += run->glyph_data_.size(); num_glyphs += run->glyph_data_.size();
} }
} }
DCHECK_EQ(index, EndIndexForResult()); const unsigned end_index = EndIndexForResult();
DCHECK_LE(index, end_index);
DCHECK_EQ(end_index - start_index, num_characters_);
DCHECK_EQ(num_glyphs, num_glyphs_); DCHECK_EQ(num_glyphs, num_glyphs_);
} }
#endif #endif
......
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