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 5607a3ad465211a73eaf9800ffda1ab872be205e..0adc521619e49b2d12496974b88270e549bca429 100644
--- a/ui/gfx/render_text_unittest.cc
+++ b/ui/gfx/render_text_unittest.cc
@@ -90,16 +90,6 @@ 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|.
-void VerifyLineSegments(const Range& expected_range,
-                        const std::vector<internal::LineSegment>& segments) {
-  EXPECT_EQ(expected_range.is_empty() ? 0ul : 1ul, segments.size());
-  if (!expected_range.is_empty())
-    EXPECT_EQ(expected_range, segments[0].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 {
@@ -2128,8 +2118,8 @@ TEST_F(RenderTextTest, Multiline_NormalWidth) {
     { 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;
@@ -2229,8 +2219,15 @@ 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. Merge all the segments

[msw, 2015/05/27 17:38:00]

Are there actually multiple segments in any of these test cases? (they only
appear to contain 'a'-'f', '\n', and ' ', so I'm not sure why your change would
increase the runs/segments here)

[msw, 2015/05/27 17:38:00]

nit: "Merge all the segments ranges in the same line."

[xdai1, 2015/06/01 16:51:16]

Yes, for test case "a \n b", there are four runs here: "a ", "\n", " " and "b".
The reason is we always merge white space into the previous run and then we
special handle "\n" to form a single run. And that's why we have two segments in
the second line: " " and "b".

If there is no special character, for example, "a bc" is a single run.

[xdai1, 2015/06/01 16:51:16]

Done.

+      // range.
+      size_t segment_size = render_text.lines()[j].segments.size();

[msw, 2015/05/27 17:38:00]

nit: const

[xdai1, 2015/06/01 16:51:16]

Done.

+      Range line_range;
+      if (segment_size > 0)
+        line_range = Range(
+            render_text.lines()[j].segments[0].char_range.start(),
+            render_text.lines()[j].segments[segment_size - 1].char_range.end());

[msw, 2015/05/27 17:38:00]

Would we need to union all the ranges for RTL? Probably not worth fixing now.

[xdai1, 2015/06/01 16:51:16]

I don't think so. This test function doesn't test RTL related features.

+      EXPECT_EQ(kTestStrings[i].line_char_ranges[j], line_range);
     }
   }
 }
@@ -2365,6 +2362,72 @@ 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;

[msw, 2015/05/27 17:38:00]

nit: Why not just assume 3 in the code; we assume 3 char_ranges.

[xdai1, 2015/06/01 16:51:16]

Done.

+    const Range char_ranges[3];
+  } kTestScenarios[] = {
+      { L"a single run", IGNORE_LONG_WORDS, 3u,
+        {Range(0, 2), Range(2, 9), Range(9, 12) } },
+      // 3 words: "That's ", ""good". ", "aaa" and 7 runs: "That", "'", "s ",
+      // """, "good", "". ", "aaa". They all mixed together.
+      { L"That's \"good\". aaa", IGNORE_LONG_WORDS, 3u,
+        {Range(0, 7), Range(7, 15), Range(15, 18) } },
+      // Test "\"" should be put into a new line correctly.
+      { L"a \"good\" one.", IGNORE_LONG_WORDS, 3u,
+        {Range(0, 2), Range(2, 9), Range(9, 13) } },
+      // Test for full-width space.
+      { L"That's\x3000good.\x3000yyy", IGNORE_LONG_WORDS, 3u,
+        {Range(0, 7), Range(7, 13), Range(13, 16) } },
+      { L"a single run", TRUNCATE_LONG_WORDS, 3u,
+        {Range(0, 2), Range(2, 6), Range(9, 12) } },
+      { L"That's \"good\". aaa", TRUNCATE_LONG_WORDS, 3u,
+        {Range(0, 4), Range(7, 11), Range(15, 18) } },
+      { L"That's good. aaa", TRUNCATE_LONG_WORDS, 3u,
+        {Range(0, 4), Range(7, 11), Range(13, 16) } },
+      { L"a \"good\" one.", TRUNCATE_LONG_WORDS, 3u,
+        {Range(0, 2), Range(2, 6), Range(9, 13) } },
+      { L"asingleword", WRAP_LONG_WORDS, 3u,
+        {Range(0, 4), Range(4, 8), Range(8, 11) } },
+      { L"That's good", WRAP_LONG_WORDS, 3u,
+        {Range(0, 4), Range(4, 7), Range(7, 11) } },
+      { L"That's \"g\".", WRAP_LONG_WORDS, 3u,
+        {Range(0, 4), Range(4, 7), Range(7, 11) } },
+  };
+
+  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.

[msw, 2015/05/27 17:38:00]

nit: "Merge"

[xdai1, 2015/06/01 16:51:16]

Done.

+      size_t segment_size = render_text.lines()[j].segments.size();
+      Range line_range;
+      if (segment_size > 0)
+        line_range = 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 +2673,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) {