Stop combining text runs which are connected by 'COMMON' blocks.

ui/gfx/render_text_unittest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "ui/gfx/render_text.h"
 
 #include <algorithm>
 
 #include "base/format_macros.h"
 #include "base/i18n/break_iterator.h"
@@ skipping 73 matching lines @@
   }
   // Check that cursoring is clamped at the line edge.
   EXPECT_EQ(expected.back(), render_text->selection_model());
   // Check that it is the line edge.
   render_text->MoveCursor(LINE_BREAK, direction, false);
   EXPECT_EQ(expected.back(), render_text->selection_model());
 }
 #endif  // !defined(OS_MACOSX)
 
 // Test utility for Multiline_Newline test case. Empty |expected_range| means
-// the blank line which has no segments. Otherwise |segments| should contain
-// exactly one line segment whose range equals to |expected_range|.
+// the blank line which has no segments. Otherwise |segment|'s range should
+// equal to |expected_range|.
 void VerifyLineSegments(const Range& expected_range,
-                        const std::vector<internal::LineSegment>& segments) {
-  EXPECT_EQ(expected_range.is_empty() ? 0ul : 1ul, segments.size());
+                        const internal::LineSegment& segment) {
   if (!expected_range.is_empty())
-    EXPECT_EQ(expected_range, segments[0].char_range);
+    EXPECT_EQ(expected_range, segment.char_range);
 }
 
 // The class which records the drawing operations so that the test case can
 // verify where exactly the glyphs are drawn.
 class TestSkiaTextRenderer : public internal::SkiaTextRenderer {
  public:
   struct TextLog {
     TextLog() : glyph_count(0u) {}
     PointF origin;
     size_t glyph_count;
@@ skipping 2005 matching lines @@
 }
 
 // Ensure strings wrap onto multiple lines for a normal available width.
 TEST_F(RenderTextTest, Multiline_NormalWidth) {
   const struct {
     const wchar_t* const text;
     const Range first_line_char_range;
     const Range second_line_char_range;
     bool is_ltr;
   } kTestStrings[] = {
-    { L"abc defg hijkl", Range(0, 9), Range(9, 14), true },
+    { L"abcdefg hijkl", Range(0, 8), Range(8, 13), true },

[Jun Mukai, 2015/05/06 17:38:52]

Why concatenating them?
What was wrong in the test case?

[xdai1, 2015/05/06 20:23:44]

See line 2153: ASSERT_EQ(1U, render_text.lines_[0].segments.size()); 
Before special handling of white space, they all belong to a single run. But now
"abc defg" are split to two runs: "abc ", "defg", thus will fail the ASSERT in
line 2153.

[Jun Mukai, 2015/05/06 20:47:55]

Please do not do this. Do not update test scenario unless it is wrong. If your
code needs the assumption of the test, update the test data and its structure,
not the test case itself.

     { L"qwertyzxcvbn", Range(0, 10), Range(10, 12), true },
     { L"\x0627\x0644\x0644\x063A\x0629 "
       L"\x0627\x0644\x0639\x0631\x0628\x064A\x0629",
       Range(0, 6), Range(6, 13), false },
-    { L"\x062A\x0641\x0627\x062D\x05EA\x05E4\x05D5\x05D6\x05D9"
-      L"\x05DA\x05DB\x05DD", Range(0, 4), Range(4, 12), false }
+    { L"\x062A\x0641\x0627\x062D \x05EA\x05E4\x05D5\x05D6\x05D9"
+      L"\x05DA\x05DB\x05DD", Range(0, 5), Range(5, 13), false }
   };
 
   RenderTextHarfBuzz render_text;
   // Specify the fixed width for characters to suppress the possible variations
   // of linebreak results.
   render_text.set_glyph_width_for_test(5);
   render_text.SetDisplayRect(Rect(50, 1000));
   render_text.SetMultiline(true);
   render_text.SetWordWrapBehavior(WRAP_LONG_WORDS);
   render_text.SetHorizontalAlignment(ALIGN_TO_HEAD);
@@ skipping 57 matching lines @@
 }
 
 TEST_F(RenderTextTest, Multiline_Newline) {
   const struct {
     const wchar_t* const text;
     const size_t lines_count;
     // Ranges of the characters on each line preceding the newline.
     const Range line_char_ranges[3];
   } kTestStrings[] = {
     {L"abc\ndef", 2ul, { Range(0, 3), Range(4, 7), Range::InvalidRange() } },
-    {L"a \n b ", 2ul, { Range(0, 2), Range(3, 6), Range::InvalidRange() } },
+    {L"a \n b ", 2ul, { Range(0, 2), Range(3, 4), Range(4, 6) } },

[Jun Mukai, 2015/05/06 17:38:52]

Consider updating the data structure a bit. The struct just assumes a single run
per a line, but now it's not, right?
By the way how does that come?  In this test case, the run splitting must not
change, it's still "a ", "\n", " b ".  Am I missing something?

[xdai1, 2015/05/06 20:23:44]

No, the run split does change to: "a ", "\n", " ", "b ". That's why the second
line contains two runs: " " and "b ".

I changed the test to merge all the segments that belongs to one line to a
single range, and then compare it with the expected line range.

     {L"ab\n", 2ul, { Range(0, 2), Range(), Range::InvalidRange() } },
     {L"a\n\nb", 3ul, { Range(0, 1), Range(), Range(3, 4) } },
     {L"\nab", 2ul, { Range(), Range(1, 3), Range::InvalidRange() } },
     {L"\n", 2ul, { Range(), Range(), Range::InvalidRange() } },
   };
 
   RenderTextHarfBuzz render_text;
   render_text.SetDisplayRect(Rect(200, 1000));
   render_text.SetMultiline(true);
   Canvas canvas;
 
   for (size_t i = 0; i < arraysize(kTestStrings); ++i) {
     SCOPED_TRACE(base::StringPrintf("kTestStrings[%" PRIuS "]", i));
     render_text.SetText(WideToUTF16(kTestStrings[i].text));
     render_text.Draw(&canvas);
     EXPECT_EQ(kTestStrings[i].lines_count, render_text.lines_.size());
     if (kTestStrings[i].lines_count != render_text.lines_.size())
       continue;
 
     for (size_t j = 0; j < kTestStrings[i].lines_count; ++j) {
       SCOPED_TRACE(base::StringPrintf("Line %" PRIuS "", j));
-      VerifyLineSegments(kTestStrings[i].line_char_ranges[j],
-                         render_text.lines_[j].segments);
+      // There might be multiple segments in one line.
+      for (size_t k = 0; k < render_text.lines_[j].segments.size(); ++k) {
+        VerifyLineSegments(kTestStrings[i].line_char_ranges[j + k],
+                           render_text.lines_[j].segments[k]);
+      }
     }
   }
 }
 
 // Make sure that multiline mode ignores elide behavior.
 TEST_F(RenderTextTest, Multiline_IgnoreElide) {
   const wchar_t kTestString[] =
       L"very very very long string xxxxxxxxxxxxxxxxxxxxxxxxxx";
   const wchar_t kEllipsis[] = L"\x2026";
 
@@ skipping 114 matching lines @@
     for (size_t j = 0; j < render_text.lines().size(); ++j) {
       SCOPED_TRACE(base::StringPrintf("%" PRIuS "-th line", j));
       EXPECT_EQ(kTestScenarios[i].char_ranges[j],
                 render_text.lines()[j].segments[0].char_range);
       EXPECT_EQ(kTestScenarios[i].char_ranges[j].length() * kGlyphSize,
                 render_text.lines()[j].size.width());
     }
   }
 }
 
+TEST_F(RenderTextTest, Multiline_LineBreakerBehavior) {
+  const int kGlyphSize = 5;
+  const struct {
+    const wchar_t* const text;
+    const WordWrapBehavior behavior;
+    const size_t num_lines;
+    const Range char_ranges[3];
+  } kTestScenarios[] = {
+      { L"That's good. aaa", IGNORE_LONG_WORDS, 3u,
+        {Range(0, 7), Range(7, 13), Range(13, 16) } },
+      { L"That's \"good\". aaa", IGNORE_LONG_WORDS, 3u,
+        {Range(0, 7), Range(7, 15), Range(15, 18) } },
+      // Test for full-width space.
+      { L"That's\x3000good.\x3000yyy", IGNORE_LONG_WORDS, 3u,
+        {Range(0, 7), Range(7, 13), Range(13, 16) } },
+      { L"abcde'f g", TRUNCATE_LONG_WORDS, 2u,
+        {Range(0, 4), Range(8, 9), Range::InvalidRange() } }
+  };
+
+  RenderTextHarfBuzz render_text;
+  render_text.SetMultiline(true);
+  render_text.set_glyph_width_for_test(kGlyphSize);
+  render_text.SetDisplayRect(Rect(0, 0, kGlyphSize * 4, 0));
+
+  Canvas canvas;
+
+  for (size_t i = 0; i < arraysize(kTestScenarios); ++i) {
+    SCOPED_TRACE(base::StringPrintf("kTestStrings[%" PRIuS "]", i));
+    render_text.SetText(WideToUTF16(kTestScenarios[i].text));
+    render_text.SetWordWrapBehavior(kTestScenarios[i].behavior);
+    render_text.Draw(&canvas);
+
+    ASSERT_EQ(kTestScenarios[i].num_lines, render_text.lines().size());
+    for (size_t j = 0; j < render_text.lines().size(); ++j) {
+      SCOPED_TRACE(base::StringPrintf("%" PRIuS "-th line", j));
+      // Merges all the segments ranges in the same line.
+      size_t segment_size = render_text.lines()[j].segments.size();
+      Range line_range(
+          render_text.lines()[j].segments[0].char_range.start(),
+          render_text.lines()[j].segments[segment_size - 1].char_range.end());
+      EXPECT_EQ(kTestScenarios[i].char_ranges[j], line_range);
+      EXPECT_EQ(kTestScenarios[i].char_ranges[j].length() * kGlyphSize,
+                render_text.lines()[j].size.width());
+    }
+  }
+}
+
 TEST_F(RenderTextTest, NewlineWithoutMultilineFlag) {
   const wchar_t* kTestStrings[] = {
     L"abc\ndef", L"a \n b ", L"ab\n", L"a\n\nb", L"\nab", L"\n",
   };
 
   RenderTextHarfBuzz render_text;
   render_text.SetDisplayRect(Rect(200, 1000));
   Canvas canvas;
 
   for (size_t i = 0; i < arraysize(kTestStrings); ++i) {
@@ skipping 225 matching lines @@
   render_text.EnsureLayout();
   internal::TextRunList* run_list = render_text.GetRunList();
   ASSERT_EQ(3U, run_list->size());
   EXPECT_EQ(Range(0, 1), run_list->runs()[0]->range);
   EXPECT_EQ(Range(1, 2), run_list->runs()[1]->range);
   EXPECT_EQ(Range(2, 3), run_list->runs()[2]->range);
 
   render_text.SetText(WideToUTF16(L"x \x25B6 y"));
   render_text.EnsureLayout();
   run_list = render_text.GetRunList();
-  ASSERT_EQ(3U, run_list->size());
+  ASSERT_EQ(4U, run_list->size());
   EXPECT_EQ(Range(0, 2), run_list->runs()[0]->range);
   EXPECT_EQ(Range(2, 3), run_list->runs()[1]->range);
-  EXPECT_EQ(Range(3, 5), run_list->runs()[2]->range);
+  EXPECT_EQ(Range(3, 4), run_list->runs()[2]->range);
+  EXPECT_EQ(Range(4, 5), run_list->runs()[3]->range);
 }
 
 TEST_F(RenderTextTest, HarfBuzz_BreakRunsByEmoji) {
   RenderTextHarfBuzz render_text;
 
   // \xF0\x9F\x98\x81 (U+1F601) is smile icon emoji. \xE2\x9C\xA8 (U+2728) is
   // a sparkle icon. Both can be drawn with color emoji fonts, so runs should be
   // separated. See crbug.com/448909
   render_text.SetText(UTF8ToUTF16("x\xF0\x9F\x98\x81y\xE2\x9C\xA8"));
   render_text.EnsureLayout();
@@ skipping 316 matching lines @@
   string_size.set_width(string_size.width() / 2);
   render_text.SetDisplayRect(gfx::Rect(string_size));
   render_text.EnsureLayout();
   CFIndex glyph_count = CTLineGetGlyphCount(render_text.line_);
   EXPECT_GT(text.size(), static_cast<size_t>(glyph_count));
   EXPECT_NE(0, glyph_count);
 }
 #endif
 
 }  // namespace gfx