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

ui/gfx/render_text_unittest.cc

Index: ui/gfx/render_text_unittest.cc
diff --git a/ui/gfx/render_text_unittest.cc b/ui/gfx/render_text_unittest.cc
index 1ceec5171d0afdd4d9c7d87f514ef8c2a22d17a6..6e0b8857cb2bf808a0801b5b334f1ae1e1567bd9 100644
--- a/ui/gfx/render_text_unittest.cc
+++ b/ui/gfx/render_text_unittest.cc
@@ -91,13 +91,12 @@ void RunMoveCursorLeftRightTest(RenderText* render_text,
 #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
@@ -2123,13 +2122,13 @@ TEST_F(RenderTextTest, Multiline_NormalWidth) {
     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;
@@ -2207,7 +2206,7 @@ TEST_F(RenderTextTest, Multiline_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() } },
@@ -2229,8 +2228,11 @@ TEST_F(RenderTextTest, Multiline_Newline) {
 
     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]);
+      }
     }
   }
 }
@@ -2365,6 +2367,53 @@ TEST_F(RenderTextTest, Multiline_WordWrapBehavior) {
   }
 }
 
+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",
@@ -2610,10 +2659,11 @@ TEST_F(RenderTextTest, HarfBuzz_BreakRunsByUnicodeBlocks) {
   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) {