Prevent infinite loop about itemizing text in RenderTextHarfBuzz.

ui/gfx/render_text_harfbuzz.cc

 // Copyright 2014 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_harfbuzz.h"
 
 #include <limits>
 
 #include "base/i18n/bidi_line_iterator.h"
 #include "base/i18n/break_iterator.h"
@@ skipping 1147 matching lines @@
     internal::TextRunHarfBuzz* run = new internal::TextRunHarfBuzz;
     run->range.set_start(run_break);
     run->font_style = (style.style(BOLD) ? Font::BOLD : 0) |
                       (style.style(ITALIC) ? Font::ITALIC : 0);
     run->baseline_type = style.baseline();
     run->strike = style.style(STRIKE);
     run->diagonal_strike = style.style(DIAGONAL_STRIKE);
     run->underline = style.style(UNDERLINE);
     int32 script_item_break = 0;
     bidi_iterator.GetLogicalRun(run_break, &script_item_break, &run->level);
+
     // Odd BiDi embedding levels correspond to RTL runs.
     run->is_rtl = (run->level % 2) == 1;
     // Find the length and script of this script run.
     script_item_break = ScriptInterval(text, run_break,
         script_item_break - run_break, &run->script) + run_break;
 
     // Find the next break and advance the iterators as needed.
     run_break = std::min(
         static_cast<size_t>(script_item_break),
         TextIndexToGivenTextIndex(text, style.GetRange().end()));
 
     // Break runs at certain characters that need to be rendered separately to
     // prevent either an unusual character from forcing a fallback font on the
     // entire run, or brackets from being affected by a fallback font.
     // http://crbug.com/278913, http://crbug.com/396776
     if (run_break > run->range.start())
       run_break = FindRunBreakingCharacter(text, run->range.start(), run_break);
 
+    if (run_break <= run->range.start())

[msw, 2015/03/18 16:21:36]

Why does this ever happen? Can we fix the actual defect, presumably with line
1178: TextIndexToGivenTextIndex(text, style.GetRange().end())?

[Jun Mukai, 2015/03/23 23:14:06]

It's still unclear why it happens on the reported environment.
For the new test case I've added, this could happen because |text| is different
from |this->text()|. In the test case, render_text is obscured. Therefore,
|text| is like "**" while this->text() holds 3 code points (1 surrogate pair + 1
character), and TextIndexToGivenTextIndex("**", 2) returns to 1.

This happens because of a confusion; the style ranges are usually specified to
this->text(), but the style's max is capped by text.length(), which is shorter
than this->text().

That said, I can fix this by changing the max of style, uploaded the patchset 3.


However, I don't think this is the case we're seeing in the bug reports, this
would happen only on obscured render text. I can't think of other cases which
run_break cannot proceed, but the crash indicates sometimes it goes into such
case.

+      run_break = std::max(script_item_break, run_break + 1);
+
     DCHECK(IsValidCodePointIndex(text, run_break));
     style.UpdatePosition(DisplayIndexToTextIndex(run_break));
     run->range.set_end(run_break);
 
     run_list_out->add(run);
   }
 
   // Undo the temporarily applied composition underlines and selection colors.
   UndoCompositionAndSelectionStyles();
 
@@ skipping 296 matching lines @@
   DCHECK(!update_layout_run_list_);
   DCHECK(!update_display_run_list_);
   return text_elided() ? display_run_list_.get() : &layout_run_list_;
 }
 
 const internal::TextRunList* RenderTextHarfBuzz::GetRunList() const {
   return const_cast<RenderTextHarfBuzz*>(this)->GetRunList();
 }
 
 }  // namespace gfx