Use CTFontCreateForString for RenderTextHarfbuzz on Mac

The approach ends up pretty close to what's already done for Windows.

On Mac, CTFontCreateForString() gives better fallbacks (i.e. they more
often match what native UI does) and requires RenderTextHarfbuzz to do
less work. However, it doesn't guarantee that the returned font can
display every glyph in the provided string.

So still use CTFontCopyDefaultCascadeListForLanguages (which is now a
public API - hooray) to provide a last-resort list of fallback fonts.

This CL causes a font used for Hebrew text in a test to change, which
tickled a pre-existing bug/corner case in the test for some macOS
versions.

BUG=696867, 619684, 735346

Previously landed in r481062, but tickled some OS-specific tests due to
a valid discrepancy between rounding directions.

Review-Url: https://codereview.chromium.org/2927953003
Cr-Commit-Position: refs/heads/master@{#481764}

ui/gfx/font_fallback.h

@@ -7,6 +7,8 @@
 
 #include <vector>
 
+#include "base/strings/string16.h"
+#include "build/build_config.h"
 #include "ui/gfx/font.h"
 #include "ui/gfx/gfx_export.h"
 
@@ -17,6 +19,18 @@ class Font;
 // Given a font, returns the fonts that are suitable for fallback.
 GFX_EXPORT std::vector<Font> GetFallbackFonts(const Font& font);
 
+#if defined(OS_MACOSX) || defined(OS_WIN)
+
+// Finds a fallback font to render the specified |text| with respect to an
+// initial |font|. Returns the resulting font via out param |result|. Returns
+// |true| if a fallback font was found.
+bool GFX_EXPORT GetFallbackFont(const Font& font,
+                                const base::char16* text,
+                                int text_length,
+                                Font* result);
+
+#endif
+
 }  // namespace gfx
 
 #endif  // UI_GFX_FONT_FALLBACK_H_

ui/gfx/font_fallback_mac.mm

@@ -4,32 +4,45 @@
 
 #include "ui/gfx/font_fallback.h"
 
-#include <dlfcn.h>
+#include <CoreText/CoreText.h>
 #import <Foundation/Foundation.h>
-#include <string>
-#include <vector>
 
 #include "base/mac/foundation_util.h"
 #import "base/mac/mac_util.h"
+#include "base/mac/scoped_cftyperef.h"
 #import "base/strings/sys_string_conversions.h"
 #include "ui/gfx/font.h"
 
-// TODO(thakis): Remove this prototype once the deployment target is 10.8+.
-extern "C" CFArrayRef CTFontCopyDefaultCascadeListForLanguages(
-    CTFontRef font,
-    CFArrayRef languagePrefList);
-
 namespace gfx {
+namespace {
+
+// CTFontCreateForString() sometimes re-wraps its result in a new CTFontRef with
+// identical attributes. This wastes time shaping the text run and confounds
+// Skia's internal typeface cache.
+bool FontsEqual(CTFontRef lhs, CTFontRef rhs) {
+  if (lhs == rhs)
+    return true;
+
+  // Compare ATSFontRef typeface IDs. These are typedef uint32_t. Typically if
+  // RenderText decided to hunt for a fallback in the first place, this check
+  // fails and FontsEqual returns here.
+  if (CTFontGetPlatformFont(lhs, nil) != CTFontGetPlatformFont(rhs, nil))
+    return false;
+
+  // Comparing addresses of descriptors seems to be sufficient for other cases.
+  base::ScopedCFTypeRef<CTFontDescriptorRef> lhs_descriptor(
+      CTFontCopyFontDescriptor(lhs));
+  base::ScopedCFTypeRef<CTFontDescriptorRef> rhs_descriptor(
+      CTFontCopyFontDescriptor(rhs));
+  return lhs_descriptor.get() == rhs_descriptor.get();
+}
+
+}  // namespace
 
 std::vector<Font> GetFallbackFonts(const Font& font) {
   // On Mac "There is a system default cascade list (which is polymorphic, based
   // on the user's language setting and current font)" - CoreText Programming
   // Guide.
-  // The CoreText APIs provide CTFontCreateForString(font, string, range), but
-  // it requires a text string "hint", and the returned font can't be
-  // represented by name for easy retrieval later.
-  // In 10.8, CTFontCopyDefaultCascadeListForLanguages(font, language_list)
-  // showed up which is a good fit GetFallbackFonts().
   NSArray* languages = [[NSUserDefaults standardUserDefaults]
       stringArrayForKey:@"AppleLanguages"];
   CFArrayRef languages_cf = base::mac::NSToCFCast(languages);
@@ -56,4 +69,21 @@ std::vector<Font> GetFallbackFonts(const Font& font) {
   return fallback_fonts;
 }
 
+bool GetFallbackFont(const Font& font,
+                     const base::char16* text,
+                     int text_length,
+                     Font* result) {
+  base::ScopedCFTypeRef<CFStringRef> cf_string(
+      CFStringCreateWithCharactersNoCopy(kCFAllocatorDefault, text, text_length,
+                                         kCFAllocatorNull));
+  CTFontRef ct_font = base::mac::NSToCFCast(font.GetNativeFont());
+  base::ScopedCFTypeRef<CTFontRef> ct_result(
+      CTFontCreateForString(ct_font, cf_string, {0, text_length}));
+  if (FontsEqual(ct_font, ct_result))
+    return false;
+
+  *result = Font(base::mac::CFToNSCast(ct_result.get()));
+  return true;
+}
+
 }  // namespace gfx

ui/gfx/font_fallback_mac_unittest.cc

@@ -4,6 +4,7 @@
 
 #include "ui/gfx/font_fallback.h"
 
+#include "base/strings/utf_string_conversions.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/gfx/font.h"
 
@@ -12,7 +13,7 @@ namespace gfx {
 // A targeted test for GetFallbackFonts on Mac. It uses a system API that
 // only became publicly available in the 10.8 SDK. This test is to ensure it
 // behaves sensibly on all supported OS versions.
-GTEST_TEST(FontFallbackMacTest, GetFallbackFonts) {
+TEST(FontFallbackMacTest, GetFallbackFonts) {
   Font font("Arial", 12);
   std::vector<Font> fallback_fonts = GetFallbackFonts(font);
   // If there is only one fallback, it means the only fallback is the font
@@ -20,4 +21,20 @@ GTEST_TEST(FontFallbackMacTest, GetFallbackFonts) {
   EXPECT_LT(1u, fallback_fonts.size());
 }
 
+// Sanity check GetFallbackFont() behavior on Mac. This test makes assumptions
+// about font properties and availability on specific macOS versions.
+TEST(FontFallbackMacTest, GetFallbackFont) {
+  Font arial("Helvetica", 12);
+  const base::string16 ascii = base::ASCIIToUTF16("abc");
+  const base::string16 hebrew = base::WideToUTF16(L"\x5d0\x5d1\x5d2");
+  const base::string16 emoji = base::UTF8ToUTF16("😋");
+
+  gfx::Font fallback;
+  EXPECT_FALSE(GetFallbackFont(arial, ascii.data(), ascii.size(), &fallback));
+  EXPECT_TRUE(GetFallbackFont(arial, hebrew.data(), hebrew.size(), &fallback));
+  EXPECT_EQ("Lucida Grande", fallback.GetFontName());
+  EXPECT_TRUE(GetFallbackFont(arial, emoji.data(), emoji.size(), &fallback));
+  EXPECT_EQ("Apple Color Emoji", fallback.GetFontName());
+}
+
 }  // namespace gfx

ui/gfx/font_fallback_win.cc

@@ -341,7 +341,7 @@ std::vector<Font> GetFallbackFonts(const Font& font) {
 }
 
 bool GetFallbackFont(const Font& font,
-                     const wchar_t* text,
+                     const base::char16* text,
                      int text_length,
                      Font* result) {
   // Creating a DirectWrite font fallback can be expensive. It's ok in the

ui/gfx/font_fallback_win.h

@@ -81,14 +81,6 @@ class GFX_EXPORT LinkedFontsIterator {
 
 }  // namespace internal
 
-// Finds a fallback font to render the specified |text| with respect to an
-// initial |font|. Returns the resulting font via out param |result|. Returns
-// |true| if a fallback font was found.
-bool GFX_EXPORT GetFallbackFont(const Font& font,
-                                const wchar_t* text,
-                                int text_length,
-                                Font* result);
-
 }  // namespace gfx
 
 #endif  // UI_GFX_FONT_FALLBACK_WIN_H_

ui/gfx/render_text_harfbuzz.cc

@@ -36,10 +36,6 @@
 #include "ui/gfx/text_utils.h"
 #include "ui/gfx/utf16_indexing.h"
 
-#if defined(OS_WIN)
-#include "ui/gfx/font_fallback_win.h"
-#endif
-
 namespace gfx {
 
 namespace {
@@ -1503,13 +1499,14 @@ void RenderTextHarfBuzz::ShapeRun(const base::string16& text,
       return;
   }
 
-#if defined(OS_WIN)
+  std::string preferred_fallback_family;
+
+#if defined(OS_WIN) || defined(OS_MACOSX)
   Font fallback_font(primary_font);
-  std::string fallback_family;
   const base::char16* run_text = &(text[run->range.start()]);
   if (GetFallbackFont(primary_font, run_text, run->range.length(),
                       &fallback_font)) {
-    fallback_family = fallback_font.GetFontName();
+    preferred_fallback_family = fallback_font.GetFontName();
     if (CompareFamily(text, fallback_font, fallback_font.GetFontRenderParams(),
                       run, &best_font, &best_render_params,
                       &best_missing_glyphs))
@@ -1520,8 +1517,10 @@ void RenderTextHarfBuzz::ShapeRun(const base::string16& text,
   std::vector<Font> fallback_font_list = GetFallbackFonts(primary_font);
 
 #if defined(OS_WIN)
-  // Append fonts in the fallback list of the fallback font.
-  if (!fallback_family.empty()) {
+  // Append fonts in the fallback list of the preferred fallback font.
+  // TODO(tapted): Investigate whether there's a case that benefits from this on
+  // Mac.
+  if (!preferred_fallback_family.empty()) {
     std::vector<Font> fallback_fonts = GetFallbackFonts(fallback_font);
     fallback_font_list.insert(fallback_font_list.end(), fallback_fonts.begin(),
                               fallback_fonts.end());
@@ -1533,7 +1532,7 @@ void RenderTextHarfBuzz::ShapeRun(const base::string16& text,
   // could be a raster font like System, which would not give us a reasonable
   // fallback font list.
   if (!base::LowerCaseEqualsASCII(primary_font.GetFontName(), "segoe ui") &&
-      !base::LowerCaseEqualsASCII(fallback_family, "segoe ui")) {
+      !base::LowerCaseEqualsASCII(preferred_fallback_family, "segoe ui")) {
     std::vector<Font> default_fallback_families =
         GetFallbackFonts(Font("Segoe UI", 13));
     fallback_font_list.insert(fallback_font_list.end(),
@@ -1548,14 +1547,10 @@ void RenderTextHarfBuzz::ShapeRun(const base::string16& text,
   for (const auto& font : fallback_font_list) {
     std::string font_name = font.GetFontName();
 
-    if (font_name == primary_font.GetFontName())
-      continue;
-#if defined(OS_WIN)
-    if (font_name == fallback_family)
-      continue;
-#endif
-    if (fallback_fonts.find(font) != fallback_fonts.end())
+    if (font_name == primary_font.GetFontName() ||
+        font_name == preferred_fallback_family || fallback_fonts.count(font)) {
       continue;
+    }
 
     fallback_fonts.insert(font);
 

ui/views/controls/label_unittest.cc

@@ -229,12 +229,25 @@ class LabelSelectionTest : public LabelTest {
     const std::vector<gfx::Rect> bounds =
         render_text->GetSubstringBoundsForTesting(gfx::Range(index, index + 1));
     DCHECK_EQ(1u, bounds.size());
-
+    const int mid_y = bounds[0].y() + bounds[0].height() / 2;
+
+    // For single-line text, use the glyph bounds since it's a better
+    // representation of the midpoint between glyphs when considering selection.
+    // TODO(tapted): When GetGlyphBounds() supports returning a vertical range
+    // as well as a horizontal range, just use that here.
+    if (!render_text->multiline())
+      return gfx::Point(render_text->GetGlyphBounds(index).start(), mid_y);
+
+    // Otherwise, GetGlyphBounds() will give incorrect results. Multiline
+    // editing is not supported (http://crbug.com/248597) so there hasn't been
+    // a need to draw a cursor. Instead, derive a point from the selection
+    // bounds, which always rounds up to an integer after the end of a glyph.
+    // This rounding differs to the glyph bounds, which rounds to nearest
+    // integer. See http://crbug.com/735346.
     const bool rtl =
         render_text->GetDisplayTextDirection() == base::i18n::RIGHT_TO_LEFT;
     // Return Point corresponding to the leading edge of the character.
-    return gfx::Point(rtl ? bounds[0].right() - 1 : bounds[0].x() + 1,
-                      bounds[0].y() + bounds[0].height() / 2);
+    return gfx::Point(rtl ? bounds[0].right() - 1 : bounds[0].x() + 1, mid_y);
   }
 
   size_t GetLineCount() {