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

ui/gfx/render_text_harfbuzz.cc

Index: ui/gfx/render_text_harfbuzz.cc
diff --git a/ui/gfx/render_text_harfbuzz.cc b/ui/gfx/render_text_harfbuzz.cc
index 32a9e554f60b47127cef9d426aef71a9f410adfa..2442daf466f324e8acb37e6db3bd15f0b4094033 100644
--- a/ui/gfx/render_text_harfbuzz.cc
+++ b/ui/gfx/render_text_harfbuzz.cc
@@ -90,15 +90,12 @@ size_t FindRunBreakingCharacter(const base::string16& text,
   return run_break;
 }
 
-// If the given scripts match, returns the one that isn't USCRIPT_COMMON or
-// USCRIPT_INHERITED, i.e. the more specific one. Otherwise returns
-// USCRIPT_INVALID_CODE.
+// If the given scripts match, returns the one that isn't USCRIPT_INHERITED,
+// i.e. the more specific one. Otherwise returns USCRIPT_INVALID_CODE.
 UScriptCode ScriptIntersect(UScriptCode first, UScriptCode second) {
-  if (first == second ||
-      (second > USCRIPT_INVALID_CODE && second <= USCRIPT_INHERITED)) {
+  if (first == second || second == USCRIPT_INHERITED)
     return first;
-  }
-  if (first > USCRIPT_INVALID_CODE && first <= USCRIPT_INHERITED)
+  if (first == USCRIPT_INHERITED)
     return second;
   return USCRIPT_INVALID_CODE;
 }
@@ -167,6 +164,12 @@ int ScriptInterval(const base::string16& text,
 
   while (char_iterator.Advance()) {
     ScriptSetIntersect(char_iterator.get(), scripts, &scripts_size);
+    // Special handling for white space. White space should be merged into the

[msw, 2015/04/23 00:00:38]

nit: "Special handling to merge white space into the previous run."

[xdai1, 2015/04/23 22:59:42]

Done.

+    // previous run.
+    if (u_isUWhiteSpace(char_iterator.get())) {

[msw, 2015/04/23 00:00:38]

Just "continue;" instead of faking out |scripts| and |scripts_size|?

[xdai1, 2015/04/23 22:59:42]

Actually this is depending on how we want to break the text into runs. For
example, suppose we have text "abc defg". If we use "continue;" here, then we'll
get two runs: "abc " and "defg", but if we fake out |scripts| and
|scripts_size|, then we'll get one single run: "abc defg", which is same with
the behavior before this change. I think these two approaches are both fine. If
we decide to use the former one (continue;), we'll need to modify some
corresponding tests. What do you think? 

+      scripts[0] = *script;
+      scripts_size = 1U;
+    }
     if (scripts_size == 0U)
       return char_iterator.array_pos();
     *script = scripts[0];
@@ -318,7 +321,8 @@ class HarfBuzzLineBreaker {
     BreakList<size_t>::const_iterator word = words_->GetBreak(start_char);
     BreakList<size_t>::const_iterator next_word = word + 1;
     // Width from |std::max(word->first, start_char)| to the current character.
-    SkScalar word_width = 0;
+    SkScalar word_width =

[msw, 2015/04/23 00:00:38]

Is this necessary for the run breaking change? Can you split this out?

[xdai1, 2015/04/23 22:59:42]

Yes, it has to be modified as described in the CL description.

+        start_char == 0 ? 0 : GetGlyphWidth(word->first, start_char - 1);
     *width = 0;
 
     Range char_range;
@@ -355,7 +359,8 @@ class HarfBuzzLineBreaker {
 
       // TODO(mukai): implement ELIDE_LONG_WORDS.
       if (*width > available_width) {
-        if (line_x_ != 0 || word_width < *width) {
+        if ((line_x_ != 0 && word_width <= *width) ||
+            (line_x_ == 0 && word_width < *width)) {
           // Roll back one word.
           *width -= word_width;
           *next_char = std::max(word->first, start_char);
@@ -470,6 +475,65 @@ class HarfBuzzLineBreaker {
     line_x_ += SkFloatToScalar(width);
   }
 
+  // Returns the index for the run that contains text_[pos] using binary search.
+  size_t GetRunIndexAt(size_t pos) const {
+    pos = (pos < text_.size()) ? pos : text_.size() - 1;
+    size_t low = 0, high = run_list_.size() - 1;
+    while (low <= high) {
+      size_t mid = low + (high - low) / 2;
+      if (run_list_.runs()[mid]->range.start() <= pos &&
+          run_list_.runs()[mid]->range.end() > pos)
+        return mid;
+      else if (run_list_.runs()[mid]->range.start() > pos)
+        high = mid - 1;
+      else
+        low = mid + 1;
+    }
+    return 0;
+  }
+
+  // Returns the glyph width for |text_| between [start_pos, end_pos].
+  SkScalar GetGlyphWidth(size_t start_pos, size_t end_pos) const {
+    end_pos = (end_pos < text_.size()) ? end_pos : text_.size() - 1;
+    if (start_pos > end_pos)
+      return 0;
+
+    // Finding the runs that containing |start_char| and |end_char|.
+    size_t start_run_idx = GetRunIndexAt(start_pos);
+    size_t end_run_idx = GetRunIndexAt(end_pos);
+    const internal::TextRunHarfBuzz* start_run =
+        run_list_.runs()[start_run_idx];
+    const internal::TextRunHarfBuzz* end_run = run_list_.runs()[end_run_idx];
+
+    SkScalar width = 0;
+    for (size_t i = start_run_idx; i <= end_run_idx; ++i) {
+      width += run_list_.runs()[i]->width;
+    }
+
+    Range char_range;
+    Range glyph_range;
+    for (size_t pos = start_run->range.start(); pos < start_pos; ++pos) {
+      start_run->GetClusterAt(pos, &char_range, &glyph_range);
+      SkScalar char_width =
+          ((glyph_range.end() >= start_run->glyph_count)
+               ? SkFloatToScalar(start_run->width)
+               : start_run->positions[glyph_range.end()].x()) -
+          start_run->positions[glyph_range.start()].x();
+      width -= char_width;
+    }
+
+    for (size_t pos = end_pos + 1; pos < end_run->range.end(); ++pos) {
+      end_run->GetClusterAt(pos, &char_range, &glyph_range);
+      SkScalar char_width = ((glyph_range.end() >= end_run->glyph_count)
+                                 ? SkFloatToScalar(end_run->width)
+                                 : end_run->positions[glyph_range.end()].x()) -
+                            end_run->positions[glyph_range.start()].x();
+      width -= char_width;
+    }
+
+    return width;
+  }
+
   const SkScalar max_width_;
   const int min_baseline_;
   const float min_height_;