Improve RenderTextWin font fallback.

ui/gfx/render_text_win.cc

 // Copyright (c) 2011 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_win.h"
 
 #include <algorithm>
 
 #include "base/logging.h"
 #include "base/stl_util.h"
@@ skipping 287 matching lines @@
   return SelectionModel(cursor, caret, placement);
 }
 
 SelectionModel RenderTextWin::RightEndSelectionModel() {
   if (text().empty())
     return SelectionModel(0, 0, SelectionModel::LEADING);
 
   EnsureLayout();
   size_t cursor = base::i18n::IsRTL() ? 0 : text().length();
   internal::TextRun* run = runs_[visual_to_logical_[runs_.size() - 1]];
-  bool rtl = run->script_analysis.fRTL;
-  size_t caret = rtl ? run->range.start() : run->range.end() - 1;
-  SelectionModel::CaretPlacement placement =
-      rtl ? SelectionModel::LEADING : SelectionModel::TRAILING;
+  size_t caret;
+  SelectionModel::CaretPlacement placement;
+  if (run->script_analysis.fRTL) {
+    caret = run->range.start();
+    placement = SelectionModel::LEADING;
+  } else {
+    caret = IndexOfAdjacentGrapheme(run->range.end(), false);
+    placement = SelectionModel::TRAILING;
+  }
   return SelectionModel(cursor, caret, placement);
 }
 
 void RenderTextWin::GetSubstringBounds(size_t from,
                                        size_t to,
                                        std::vector<Rect>* bounds) {
   DCHECK(!needs_layout_);
   ui::Range range(from, to);
   DCHECK(ui::Range(0, text().length()).Contains(range));
   Point display_offset(GetUpdatedDisplayOffset());
@@ skipping 132 matching lines @@
     size_t byte_length = run->glyph_count * sizeof(WORD);
     canvas_skia->drawPosText(run->glyphs.get(), byte_length, &pos[0], paint);
 
     if (run->strike || run->underline)
       DrawTextRunDecorations(canvas_skia, paint, *run, run_x, y);
   }
 }
 
 size_t RenderTextWin::IndexOfAdjacentGrapheme(size_t index, bool next) {
   EnsureLayout();
+
+  if (text().empty())
+    return 0;
+
+  if (index >= text().length()) {
+    if (next) {
+      return text().length();
+    } else {
+      size_t last = text().length() - 1;
+      index = IndexOfAdjacentGrapheme(last, false);

[xji, 2011/12/01 08:31:58]

is the reason of this special handling that:
run_index returned from GetRunContainingPosition(text.length()) >= runs_.size()
,so that the 'run' will be assigned to NULL and following ops are wrong?

[Alexei Svitkine (slow), 2011/12/01 18:17:24]

Correct. The previous logic was incorrect in this case. By special-casing it, it
also makes the run != NULL code cleaner.

+      if (index == last)
+        return index;
+      next = true;
+    }
+  }
+
   size_t run_index = GetRunContainingPosition(index);
-  internal::TextRun* run = run_index < runs_.size() ? runs_[run_index] : NULL;
-  int start = run ? run->range.start() : 0;
-  int length = run ? run->range.length() : text().length();
+  DCHECK(run_index < runs_.size());
+  internal::TextRun* run = runs_[run_index];
+  int start = run->range.start();
+  int length = run->range.length();
   int ch = index - start;
-  WORD cluster = run ? run->logical_clusters[ch] : 0;
+  WORD cluster = run->logical_clusters[ch];
 
   if (!next) {
     do {
       ch--;
-    } while (ch >= 0 && run && run->logical_clusters[ch] == cluster);
+    } while (ch >= 0 && run->logical_clusters[ch] == cluster);
+
+    // Now, |ch| is at the end of the previous grapheme. Find the start.
+    while (ch > 0 && run->logical_clusters[ch - 1] == run->logical_clusters[ch])
+      ch--;

[xji, 2011/12/01 08:31:58]

this does not seem right.
for example: ABCDE, assume CDE are in same grapheme,
if index is 3, IndexOfAdjacentGrapheme(3, false) should be 2, not 1.

It is the same for Linux.
The name is confusing, I think the purpose of this function is looking for the
previous or next grapheme boundary.

I think we should have test to cover this, or maybe the test is disabled in Win?
If there is not test coverage, we should definitely add one. 

And could you please also clarify this in the comments in render_text.h?

[Alexei Svitkine (slow), 2011/12/01 18:17:24]

I see - this wasn't clear to me before. I've adjusted the code to fit this
behaviour and fixed the header comments to be clear about this.
I've now added unit tests for this, as promised. I also had to fix the case of
param |index| >= text().length() on Linux to return text().length().
Done.

   } else {
-    while (ch < length && run && run->logical_clusters[ch] == cluster)
+    while (ch < length && run->logical_clusters[ch] == cluster)
       ch++;
   }
+
   return std::max(std::min(ch, length) + start, 0);
 }
 
 void RenderTextWin::ItemizeLogicalText() {
   STLDeleteContainerPointers(runs_.begin(), runs_.end());
   runs_.clear();
   string_width_ = 0;
   if (text().empty())
     return;
 
@@ skipping 49 matching lines @@
 }
 
 void RenderTextWin::LayoutVisualText() {
   HRESULT hr = E_FAIL;
   base::win::ScopedCreateDC hdc(CreateCompatibleDC(NULL));
   std::vector<internal::TextRun*>::const_iterator run_iter;
   for (run_iter = runs_.begin(); run_iter < runs_.end(); ++run_iter) {
     internal::TextRun* run = *run_iter;
     size_t run_length = run->range.length();
     const wchar_t* run_text = &(text()[run->range.start()]);
+    bool tried_fallback = false;
 
     // Select the font desired for glyph generation.
     SelectObject(hdc, run->font.GetNativeFont());
 
     run->logical_clusters.reset(new WORD[run_length]);
     run->glyph_count = 0;
     // Max glyph guess: http://msdn.microsoft.com/en-us/library/dd368564.aspx
     size_t max_glyphs = static_cast<size_t>(1.5 * run_length + 16);
     while (max_glyphs < kMaxGlyphs) {
       run->glyphs.reset(new WORD[max_glyphs]);
       run->visible_attributes.reset(new SCRIPT_VISATTR[max_glyphs]);
       hr = ScriptShape(hdc,
                        &run->script_cache,
                        run_text,
                        run_length,
                        max_glyphs,
                        &(run->script_analysis),
                        run->glyphs.get(),
                        run->logical_clusters.get(),
                        run->visible_attributes.get(),
                        &(run->glyph_count));
       if (hr == E_OUTOFMEMORY) {
         max_glyphs *= 2;
       } else if (hr == USP_E_SCRIPT_NOT_IN_FONT) {
-        // TODO(msw): Don't use SCRIPT_UNDEFINED. Apparently Uniscribe can crash
-        //            on certain surrogate pairs with SCRIPT_UNDEFINED.
-        //            See https://bugzilla.mozilla.org/show_bug.cgi?id=341500
-        //            And http://maxradi.us/documents/uniscribe/
-        if (run->script_analysis.eScript == SCRIPT_UNDEFINED)
-          break;
+         // Only try font fallback if it hasn't yet been attempted for this run.
+         if (tried_fallback) {
+           // TODO(msw): Don't use SCRIPT_UNDEFINED. Apparently Uniscribe can
+           //            crash on certain surrogate pairs with SCRIPT_UNDEFINED.
+           //            See https://bugzilla.mozilla.org/show_bug.cgi?id=341500
+           //            And http://maxradi.us/documents/uniscribe/
+           run->script_analysis.eScript = SCRIPT_UNDEFINED;
+           break;
+         }
 
         // The run's font doesn't contain the required glyphs, use an alternate.
         if (ChooseFallbackFont(hdc, run->font, run_text, run_length,
                                &run->font)) {
           ScriptFreeCache(&run->script_cache);
           SelectObject(hdc, run->font.GetNativeFont());
         }
 
-        run->script_analysis.eScript = SCRIPT_UNDEFINED;
+        tried_fallback = true;
       } else {
         break;
       }
     }
     DCHECK(SUCCEEDED(hr));
 
     if (run->glyph_count > 0) {
       run->advance_widths.reset(new int[run->glyph_count]);
       run->offsets.reset(new GOFFSET[run->glyph_count]);
       hr = ScriptPlace(hdc,
@@ skipping 144 matching lines @@
   internal::TextRun* next = runs_[visual_to_logical_[visual_index + 1]];
   return next->script_analysis.fRTL ? LastSelectionModelInsideRun(next) :
                                       FirstSelectionModelInsideRun(next);
 }
 
 RenderText* RenderText::CreateRenderText() {
   return new RenderTextWin;
 }
 
 }  // namespace gfx