Fix RenderTextWin ligature handling.

Currently, RenderTextWin uses logical clusters to determine valid cursor indices.
It only allows the cursor to be places at the first codepoint in a logical cluster.
This prevents cursor locations in the middle of ligatures, like 'ffi' in Meiryo UI.
('ffi' is 1 glyph with 3 non-surrogate code points and 3 bounds rects)

Instead, check if the index is a valid (non-surrogate-tail) code point.
Also, require unique glyph bounds for each code point.
Otherwise, non-surrogate code points that share glyph bounds snag the cursor.
('\x0915\x093f' is 1 glyph with 2 non-surrogate code points and 1 bounds rect)

Add a Meiryo UI ligatures RenderText test (in ui_unittests).
I verified that this fails on Win7 before the RenderTextWin change.

BUG=242831
TEST=Meiryo UI 'ffi' allows cursor positions after each letter, no other cursor placement regressions. Test lots of surrogate pairs, combined glyphs, diacritics, etc.
R=asvitkine@chromium.org

Review URL: https://chromiumcodereview.appspot.com/16358022

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@206097 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 24 insertions, 18 deletions

diff --git a/ui/gfx/render_text_unittest.cc b/ui/gfx/render_text_unittest.cc
index 8780614..151be2a 100644
--- a/ui/gfx/render_text_unittest.cc
+++ b/ui/gfx/render_text_unittest.cc
@@ -614,6 +614,20 @@ TEST_F(RenderTextTest, MoveCursorLeftRight_ComplexScript) {
 }
 #endif
 
+TEST_F(RenderTextTest, MoveCursorLeftRight_MeiryoUILigatures) {
+  scoped_ptr<RenderText> render_text(RenderText::CreateInstance());
+  // Meiryo UI uses single-glyph ligatures for 'ff' and 'ffi', but each letter
+  // (code point) has unique bounds, so mid-glyph cursoring should be possible.
+  render_text->SetFont(Font("Meiryo UI", 12));
+  render_text->SetText(WideToUTF16(L"ff ffi"));
+  EXPECT_EQ(0U, render_text->cursor_position());
+  for (size_t i = 0; i < render_text->text().length(); ++i) {
+    render_text->MoveCursor(CHARACTER_BREAK, CURSOR_RIGHT, false);
+    EXPECT_EQ(i + 1, render_text->cursor_position());
+  }
+  EXPECT_EQ(6U, render_text->cursor_position());
+}
+
 TEST_F(RenderTextTest, GraphemePositions) {
   // LTR 2-character grapheme, LTR abc, LTR 2-character grapheme.
   const base::string16 kText1 =
@@ -1012,12 +1026,12 @@ TEST_F(RenderTextTest, StringSizeBoldWidth) {
   EXPECT_GT(plain_width, 0);
 
   // Apply a bold style and check that the new width is greater.
-  render_text->SetStyle(gfx::BOLD, true);
+  render_text->SetStyle(BOLD, true);
   const int bold_width = render_text->GetStringSize().width();
   EXPECT_GT(bold_width, plain_width);
 
   // Now, apply a plain style over the first word only.
-  render_text->ApplyStyle(gfx::BOLD, false, ui::Range(0, 5));
+  render_text->ApplyStyle(BOLD, false, ui::Range(0, 5));
   const int plain_bold_width = render_text->GetStringSize().width();
   EXPECT_GT(plain_bold_width, plain_width);
   EXPECT_LT(plain_bold_width, bold_width);
@@ -1081,8 +1095,8 @@ TEST_F(RenderTextTest, GetTextOffset) {
   render_text->SetFontList(FontList("Arial, 13px"));
 
   // Set display area's size equal to the font size.
-  const gfx::Size font_size(render_text->GetContentWidth(),
-                            render_text->GetStringSize().height());
+  const Size font_size(render_text->GetContentWidth(),
+                       render_text->GetStringSize().height());
   Rect display_rect(font_size);
   render_text->SetDisplayRect(display_rect);
 
@@ -1133,8 +1147,8 @@ TEST_F(RenderTextTest, GetTextOffsetHorizontalDefaultInRTL) {
   render_text->SetText(ASCIIToUTF16("abcdefg"));
   render_text->SetFontList(FontList("Arial, 13px"));
   const int kEnlargement = 2;
-  const gfx::Size font_size(render_text->GetContentWidth() + kEnlargement,
-                            render_text->GetStringSize().height());
+  const Size font_size(render_text->GetContentWidth() + kEnlargement,
+                       render_text->GetStringSize().height());
   Rect display_rect(font_size);
   render_text->SetDisplayRect(display_rect);
   Vector2d offset = render_text->GetTextOffset();
diff --git a/ui/gfx/render_text_win.cc b/ui/gfx/render_text_win.cc
index 48896c6..0469c0f 100644
--- a/ui/gfx/render_text_win.cc
+++ b/ui/gfx/render_text_win.cc
@@ -409,20 +409,12 @@ size_t RenderTextWin::LayoutIndexToTextIndex(size_t index) const {
 bool RenderTextWin::IsCursorablePosition(size_t position) {
   if (position == 0 || position == text().length())
     return true;
-
   EnsureLayout();
-  const size_t run_index =
-      GetRunContainingCaret(SelectionModel(position, CURSOR_FORWARD));
-  if (run_index >= runs_.size())
-    return false;
 
-  internal::TextRun* run = runs_[run_index];
-  const size_t start = run->range.start();
-  const size_t layout_position = TextIndexToLayoutIndex(position);
-  if (layout_position == start)
-    return true;
-  return run->logical_clusters[layout_position - start] !=
-         run->logical_clusters[layout_position - start - 1];
+  // Check if the index is at a valid code point (not mid-surrgate-pair) with
+  // distinct glyph bounds (not mid-multi-character-grapheme, eg. \x0915\x093f).
+  return ui::IsValidCodePointIndex(text(), position) &&
+         GetGlyphBounds(position) != GetGlyphBounds(position - 1);
 }
 
 void RenderTextWin::ResetLayout() {