Fix Views inline autocomplete with multi-char graphemes.

ui/gfx/render_text.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 <climits>
 
 #include "base/i18n/break_iterator.h"
@@ skipping 483 matching lines @@
   }
 }
 
 void RenderText::SetCursorPosition(size_t position) {
   MoveCursorTo(position, false);
 }
 
 void RenderText::MoveCursor(BreakType break_type,
                             VisualCursorDirection direction,
                             bool select) {
-  SelectionModel position(cursor_position(), selection_model_.caret_affinity());
+  SelectionModel cursor(cursor_position(), selection_model_.caret_affinity());
   // Cancelling a selection moves to the edge of the selection.
   if (break_type != LINE_BREAK && !selection().is_empty() && !select) {
     SelectionModel selection_start = GetSelectionModelForSelectionStart();
     int start_x = GetCursorBounds(selection_start, true).x();
-    int cursor_x = GetCursorBounds(position, true).x();
+    int cursor_x = GetCursorBounds(cursor, true).x();
     // Use the selection start if it is left (when |direction| is CURSOR_LEFT)
     // or right (when |direction| is CURSOR_RIGHT) of the selection end.
     if (direction == CURSOR_RIGHT ? start_x > cursor_x : start_x < cursor_x)
-      position = selection_start;
-    // For word breaks, use the nearest word boundary in the appropriate
-    // |direction|.
+      cursor = selection_start;
+    // Use the nearest word boundary in the proper |direction| for word breaks.
     if (break_type == WORD_BREAK)
-      position = GetAdjacentSelectionModel(position, break_type, direction);
+      cursor = GetAdjacentSelectionModel(cursor, break_type, direction);
+    // Use an adjacent selection model if the cursor is not at a valid position.
+    if (!IsValidCursorIndex(cursor.caret_pos()))
+      cursor = GetAdjacentSelectionModel(cursor, CHARACTER_BREAK, direction);
   } else {
-    position = GetAdjacentSelectionModel(position, break_type, direction);
+    cursor = GetAdjacentSelectionModel(cursor, break_type, direction);
   }
   if (select)
-    position.set_selection_start(selection().start());
-  MoveCursorTo(position);
+    cursor.set_selection_start(selection().start());
+  MoveCursorTo(cursor);
 }
 
 bool RenderText::MoveCursorTo(const SelectionModel& model) {
   // Enforce valid selection model components.
   size_t text_length = text().length();
   Range range(std::min(model.selection().start(), text_length),
               std::min(model.caret_pos(), text_length));
-  // The current model only supports caret positions at valid character indices.
-  if (!IsCursorablePosition(range.start()) ||
-      !IsCursorablePosition(range.end()))
+  // The current model only supports caret positions at valid cursor indices.
+  if (!IsValidCursorIndex(range.start()) || !IsValidCursorIndex(range.end()))
     return false;
   SelectionModel sel(range, model.caret_affinity());
   bool changed = sel != selection_model_;
   SetSelectionModel(sel);
   return changed;
 }
 
 bool RenderText::MoveCursorTo(const Point& point, bool select) {
   SelectionModel position = FindCursorPosition(point);
   if (select)
     position.set_selection_start(selection().start());
   return MoveCursorTo(position);
 }
 
 bool RenderText::SelectRange(const Range& range) {
   Range sel(std::min(range.start(), text().length()),
             std::min(range.end(), text().length()));
-  if (!IsCursorablePosition(sel.start()) || !IsCursorablePosition(sel.end()))
+  // Allow selection bounds at valid indicies amid multi-character graphemes.
+  if (!IsValidLogicalIndex(sel.start()) || !IsValidLogicalIndex(sel.end()))
     return false;
   LogicalCursorDirection affinity =
       (sel.is_reversed() || sel.is_empty()) ? CURSOR_FORWARD : CURSOR_BACKWARD;
   SetSelectionModel(SelectionModel(sel, affinity));
   return true;
 }
 
 bool RenderText::IsPointInSelection(const Point& point) {
   if (selection().is_empty())
     return false;
@@ skipping 198 matching lines @@
   if (clip_to_display_rect())
     canvas->Restore();
 }
 
 void RenderText::DrawCursor(Canvas* canvas, const SelectionModel& position) {
   // Paint cursor. Replace cursor is drawn as rectangle for now.
   // TODO(msw): Draw a better cursor with a better indication of association.
   canvas->FillRect(GetCursorBounds(position, true), cursor_color_);
 }
 
+bool RenderText::IsValidLogicalIndex(size_t index) {
+  // Check that the index is at a valid code point (not mid-surrgate-pair) and
+  // that it's not truncated from the layout text (its glyph may be shown).
+  return ((index == 0 || index == text().length()) ||
+          (IsValidCodePointIndex(text(), index) &&
+           index < GetLayoutText().length()));

[Alexei Svitkine (slow), 2014/04/30 19:53:24]

Hmm, I don't understand this logic anymore.

I know that these checks were done at the other functions already.... but...

It seems incorrect to me to check against both text().length() and
GetLayotText().length().

It should be clear whether the caller is expected to pass in a text() index or a
GetLayoutText() index. And depending on that, we should only use one of text()
or GetLayoutText() to validate, not both.

But perhaps I'm missing something. It's quite a while that I've worked on this
and don't remember all the subtleties. Either way, we should make code comments
clarify what type of indexes are being passed in and then once we have this
clarification, we can reason from there.

[msw, 2014/05/01 20:40:04]

I've modified the check to be simpler, clearer, and more correct. Sorry for the
confusion; |index| is a logical index into text(), but we also verify here that
it does not exceed the truncation length of the layout text. The
truncate_length_ is only used on Windows, where it's set to
kMaxUniscribeTextLength (10000). This hard limit only comes into play in
extraordinary circumstances and may not be needed once we switch to HarfBuzz.

I also added a text() length check to avoid IsValidCodePointIndex crashes (this
callpath accounts for ~24 of ~220 lifetime crash reports). LMK if you think that
new check is too aggressive.

+}
+
 Rect RenderText::GetCursorBounds(const SelectionModel& caret,
                                  bool insert_mode) {
   // TODO(ckocagil): Support multiline. This function should return the height
   //                 of the line the cursor is on. |GetStringSize()| now returns
   //                 the multiline size, eliminate its use here.
 
   EnsureLayout();
-
   size_t caret_pos = caret.caret_pos();
-  DCHECK(IsCursorablePosition(caret_pos));
+  DCHECK(IsValidLogicalIndex(caret_pos));
   // In overtype mode, ignore the affinity and always indicate that we will
   // overtype the next character.
   LogicalCursorDirection caret_affinity =
       insert_mode ? caret.caret_affinity() : CURSOR_FORWARD;
   int x = 0, width = 1;
   Size size = GetStringSize();
   if (caret_pos == (caret_affinity == CURSOR_BACKWARD ? 0 : text().length())) {
     // The caret is attached to the boundary. Always return a 1-dip width caret,
     // since there is nothing to overtype.
     if ((GetTextDirection() == base::i18n::RIGHT_TO_LEFT) == (caret_pos == 0))
@@ skipping 20 matching lines @@
 size_t RenderText::IndexOfAdjacentGrapheme(size_t index,
                                            LogicalCursorDirection direction) {
   if (index > text().length())
     return text().length();
 
   EnsureLayout();
 
   if (direction == CURSOR_FORWARD) {
     while (index < text().length()) {
       index++;
-      if (IsCursorablePosition(index))
+      if (IsValidCursorIndex(index))
         return index;
     }
     return text().length();
   }
 
   while (index > 0) {
     index--;
-    if (IsCursorablePosition(index))
+    if (IsValidCursorIndex(index))
       return index;
   }
   return 0;
 }
 
 SelectionModel RenderText::GetSelectionModelForSelectionStart() {
   const Range& sel = selection();
   if (sel.is_empty())
     return selection_model_;
   return SelectionModel(sel.start(),
@@ skipping 262 matching lines @@
                                     size_t caret_pos,
                                     LogicalCursorDirection caret_affinity) {
   // NB: exploits unsigned wraparound (WG14/N1124 section 6.2.5 paragraph 9).
   size_t adjacent = (caret_affinity == CURSOR_BACKWARD) ?
       caret_pos - 1 : caret_pos + 1;
   return range.Contains(Range(caret_pos, adjacent));
 }
 
 void RenderText::MoveCursorTo(size_t position, bool select) {
   size_t cursor = std::min(position, text().length());
-  if (IsCursorablePosition(cursor))
+  if (IsValidCursorIndex(cursor))
     SetSelectionModel(SelectionModel(
         Range(select ? selection().start() : cursor, cursor),
         (cursor == 0) ? CURSOR_FORWARD : CURSOR_BACKWARD));
 }
 
 void RenderText::UpdateLayoutText() {
   layout_text_.clear();
   line_breaks_.SetMax(0);
 
   if (obscured_) {
@@ skipping 179 matching lines @@
   cursor_bounds_ += delta_offset;
 }
 
 void RenderText::DrawSelection(Canvas* canvas) {
   const std::vector<Rect> sel = GetSubstringBounds(selection());
   for (std::vector<Rect>::const_iterator i = sel.begin(); i < sel.end(); ++i)
     canvas->FillRect(*i, selection_background_focused_color_);
 }
 
 }  // namespace gfx