Unify desktop Linux and Chrome OS font rendering params.

Use the same path to get font rendering settings on desktop
Linux and Chrome OS: query the LinuxFontDelegate if it's set
and then query Fontconfig.

Also make the LinuxFontDelegate interface return a
PangoFontDescription directly (as opposed to an opaque
string that's actually a stringified PangoFontDescription)
and make it return a cached FontRenderParams struct instead
of exposing separate methods for looking up individual
properties.

Finally, add a missing build dependency on ui_test_pak, which
is needed to get the default font resource when running
gfx_unittests on Chrome OS.

BUG=125235,393067

Review URL: https://codereview.chromium.org/400193004

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

chrome/browser/ui/libgtk2ui/gtk2_ui.cc

@@ -6,6 +6,8 @@
 
 #include <set>
 
+#include <pango/pango.h>
+
 #include "base/command_line.h"
 #include "base/debug/leak_annotations.h"
 #include "base/environment.h"
@@ -40,6 +42,7 @@
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/gfx/canvas.h"
 #include "ui/gfx/image/image.h"
+#include "ui/gfx/pango_util.h"
 #include "ui/gfx/rect.h"
 #include "ui/gfx/size.h"
 #include "ui/gfx/skbitmap_operations.h"
@@ -315,6 +318,58 @@ color_utils::HSL GetDefaultTint(int id) {
   }
 }
 
+// Returns a FontRenderParams corresponding to GTK's configuration.
+gfx::FontRenderParams GetGtkFontRenderParams() {
+  GtkSettings* gtk_settings = gtk_settings_get_default();
+  CHECK(gtk_settings);
+  gint antialias = 0;
+  gint hinting = 0;
+  gchar* hint_style = NULL;
+  gchar* rgba = NULL;
+  g_object_get(gtk_settings,
+               "gtk-xft-antialias", &antialias,
+               "gtk-xft-hinting", &hinting,
+               "gtk-xft-hintstyle", &hint_style,
+               "gtk-xft-rgba", &rgba,
+               NULL);
+
+  gfx::FontRenderParams params;
+  params.antialiasing = antialias != 0;
+
+  if (hinting == 0 || !hint_style || strcmp(hint_style, "hintnone") == 0) {
+    params.hinting = gfx::FontRenderParams::HINTING_NONE;
+  } else if (strcmp(hint_style, "hintslight") == 0) {
+    params.hinting = gfx::FontRenderParams::HINTING_SLIGHT;
+  } else if (strcmp(hint_style, "hintmedium") == 0) {
+    params.hinting = gfx::FontRenderParams::HINTING_MEDIUM;
+  } else if (strcmp(hint_style, "hintfull") == 0) {
+    params.hinting = gfx::FontRenderParams::HINTING_FULL;
+  } else {
+    LOG(WARNING) << "Unexpected gtk-xft-hintstyle \"" << hint_style << "\"";
+    params.hinting = gfx::FontRenderParams::HINTING_NONE;
+  }
+
+  if (!rgba || strcmp(rgba, "none") == 0) {
+    params.subpixel_rendering = gfx::FontRenderParams::SUBPIXEL_RENDERING_NONE;
+  } else if (strcmp(rgba, "rgb") == 0) {
+    params.subpixel_rendering = gfx::FontRenderParams::SUBPIXEL_RENDERING_RGB;
+  } else if (strcmp(rgba, "bgr") == 0) {
+    params.subpixel_rendering = gfx::FontRenderParams::SUBPIXEL_RENDERING_BGR;
+  } else if (strcmp(rgba, "vrgb") == 0) {
+    params.subpixel_rendering = gfx::FontRenderParams::SUBPIXEL_RENDERING_VRGB;
+  } else if (strcmp(rgba, "vbgr") == 0) {
+    params.subpixel_rendering = gfx::FontRenderParams::SUBPIXEL_RENDERING_VBGR;
+  } else {
+    LOG(WARNING) << "Unexpected gtk-xft-rgba \"" << rgba << "\"";
+    params.subpixel_rendering = gfx::FontRenderParams::SUBPIXEL_RENDERING_NONE;
+  }
+
+  g_free(hint_style);
+  g_free(rgba);
+
+  return params;
+}
+
 }  // namespace
 
 Gtk2UI::Gtk2UI() : middle_click_action_(MIDDLE_CLICK_ACTION_LOWER) {
@@ -588,75 +643,16 @@ scoped_ptr<ui::LinuxInputMethodContext> Gtk2UI::CreateInputMethodContext(
       new X11InputMethodContextImplGtk2(delegate));
 }
 
-bool Gtk2UI::UseAntialiasing() const {
-  GtkSettings* gtk_settings = gtk_settings_get_default();
-  CHECK(gtk_settings);
-  gint gtk_antialias = 0;
-  g_object_get(gtk_settings,
-               "gtk-xft-antialias", &gtk_antialias,
-               NULL);
-  return gtk_antialias != 0;
-}
-
-gfx::FontRenderParams::Hinting Gtk2UI::GetHintingStyle() const {
-  GtkSettings* gtk_settings = gtk_settings_get_default();
-  CHECK(gtk_settings);
-  gfx::FontRenderParams::Hinting hinting =
-      gfx::FontRenderParams::HINTING_SLIGHT;
-  gint gtk_hinting = 0;
-  gchar* gtk_hint_style = NULL;
-  g_object_get(gtk_settings,
-               "gtk-xft-hinting", &gtk_hinting,
-               "gtk-xft-hintstyle", &gtk_hint_style,
-               NULL);
-
-  if (gtk_hint_style) {
-    if (gtk_hinting == 0 || strcmp(gtk_hint_style, "hintnone") == 0)
-      hinting = gfx::FontRenderParams::HINTING_NONE;
-    else if (strcmp(gtk_hint_style, "hintslight") == 0)
-      hinting = gfx::FontRenderParams::HINTING_SLIGHT;
-    else if (strcmp(gtk_hint_style, "hintmedium") == 0)
-      hinting = gfx::FontRenderParams::HINTING_MEDIUM;
-    else if (strcmp(gtk_hint_style, "hintfull") == 0)
-      hinting = gfx::FontRenderParams::HINTING_FULL;
-
-    g_free(gtk_hint_style);
-  }
-
-  return hinting;
+gfx::FontRenderParams Gtk2UI::GetDefaultFontRenderParams() const {
+  static gfx::FontRenderParams params = GetGtkFontRenderParams();
+  return params;
 }
 
-gfx::FontRenderParams::SubpixelRendering
-Gtk2UI::GetSubpixelRenderingStyle() const {
-  GtkSettings* gtk_settings = gtk_settings_get_default();
-  CHECK(gtk_settings);
-  gfx::FontRenderParams::SubpixelRendering subpixel_rendering =
-      gfx::FontRenderParams::SUBPIXEL_RENDERING_NONE;
-  gchar* gtk_rgba = NULL;
-  g_object_get(gtk_settings,
-               "gtk-xft-rgba", &gtk_rgba,
-               NULL);
-
-  if (gtk_rgba) {
-    if (strcmp(gtk_rgba, "none") == 0)
-      subpixel_rendering = gfx::FontRenderParams::SUBPIXEL_RENDERING_NONE;
-    else if (strcmp(gtk_rgba, "rgb") == 0)
-      subpixel_rendering = gfx::FontRenderParams::SUBPIXEL_RENDERING_RGB;
-    else if (strcmp(gtk_rgba, "bgr") == 0)
-      subpixel_rendering = gfx::FontRenderParams::SUBPIXEL_RENDERING_BGR;
-    else if (strcmp(gtk_rgba, "vrgb") == 0)
-      subpixel_rendering = gfx::FontRenderParams::SUBPIXEL_RENDERING_VRGB;
-    else if (strcmp(gtk_rgba, "vbgr") == 0)
-      subpixel_rendering = gfx::FontRenderParams::SUBPIXEL_RENDERING_VBGR;
-
-    g_free(gtk_rgba);
-  }
-
-  return subpixel_rendering;
-}
-
-std::string Gtk2UI::GetDefaultFontDescription() const {
-  return default_font_description_;
+scoped_ptr<gfx::ScopedPangoFontDescription>
+Gtk2UI::GetDefaultPangoFontDescription() const {
+  return scoped_ptr<gfx::ScopedPangoFontDescription>(
+      new gfx::ScopedPangoFontDescription(
+          pango_font_description_copy(default_font_description_->get())));
 }
 
 double Gtk2UI::GetFontDPI() const {
@@ -833,27 +829,8 @@ void Gtk2UI::LoadGtkValues() {
   SetThemeColorFromGtk(ThemeProperties::COLOR_BOOKMARK_TEXT, &label_color);
   SetThemeColorFromGtk(ThemeProperties::COLOR_STATUS_BAR_TEXT, &label_color);
 
-  gchar* font_string = pango_font_description_to_string(label_style->font_desc);
-  default_font_description_ = std::string(font_string);
-  g_free(font_string);
-
-  {
-    // TODO(derat): Remove this debugging code if/when http://crbug.com/375824
-    // is resolved.
-    GtkSettings* gtk_settings = gtk_settings_get_default();
-    CHECK(gtk_settings);
-    gchar* font_name = NULL;
-    g_object_get(gtk_settings, "gtk-font-name", &font_name, NULL);
-    if (font_name) {
-      if (std::string(font_name) != default_font_description_) {
-        LOG(ERROR) << "Font specified in gtk-font-name property ("
-                   << font_name << ") does not match font from GtkLabel ("
-                   << default_font_description_ << "); see "
-                   << "http://crbug.com/375824";
-      }
-      g_free(font_name);
-    }
-  }
+  default_font_description_.reset(new gfx::ScopedPangoFontDescription(
+      pango_font_description_copy(label_style->font_desc)));
 
   // Build the various icon tints.
   GetNormalButtonTintHSL(&button_tint_);

chrome/browser/ui/libgtk2ui/gtk2_ui.h

@@ -10,6 +10,7 @@
 
 #include "base/basictypes.h"
 #include "base/compiler_specific.h"
+#include "base/memory/scoped_ptr.h"
 #include "base/observer_list.h"
 #include "chrome/browser/ui/libgtk2ui/gtk2_signal.h"
 #include "chrome/browser/ui/libgtk2ui/gtk2_signal_registrar.h"
@@ -29,6 +30,7 @@ class SkBitmap;
 
 namespace gfx {
 class Image;
+class ScopedPangoFontDescription;
 }
 
 namespace libgtk2ui {
@@ -65,11 +67,9 @@ class Gtk2UI : public views::LinuxUI {
       ui::LinuxInputMethodContextDelegate* delegate) const OVERRIDE;
 
   // gfx::LinuxFontDelegate:
-  virtual bool UseAntialiasing() const OVERRIDE;
-  virtual gfx::FontRenderParams::Hinting GetHintingStyle() const OVERRIDE;
-  virtual gfx::FontRenderParams::SubpixelRendering
-      GetSubpixelRenderingStyle() const OVERRIDE;
-  virtual std::string GetDefaultFontDescription() const OVERRIDE;
+  virtual gfx::FontRenderParams GetDefaultFontRenderParams() const OVERRIDE;
+  virtual scoped_ptr<gfx::ScopedPangoFontDescription>
+      GetDefaultPangoFontDescription() const OVERRIDE;
   virtual double GetFontDPI() const OVERRIDE;
 
   // ui::LinuxShellDialog:
@@ -227,7 +227,7 @@ class Gtk2UI : public views::LinuxUI {
   SkColor inactive_selection_fg_color_;
 
   // Pango description for the default UI font.
-  std::string default_font_description_;
+  scoped_ptr<gfx::ScopedPangoFontDescription> default_font_description_;
 
 #if defined(USE_GCONF)
   // Currently, the only source of window button configuration. This will

ui/gfx/BUILD.gn

@@ -416,6 +416,7 @@ test("gfx_unittests") {
     "//testing/gtest",
     "//ui/base",
     "//ui/gfx/geometry",
+    "//ui/resources:ui_test_pak",
   ]
 
   if (use_pango) {

ui/gfx/font_render_params_linux.cc

@@ -6,24 +6,15 @@
 
 #include "base/command_line.h"
 #include "base/logging.h"
+#include "ui/gfx/linux_font_delegate.h"
 #include "ui/gfx/switches.h"
 
 #include <fontconfig/fontconfig.h>
 
-#if defined(OS_LINUX) && !defined(OS_CHROMEOS)
-#include "ui/gfx/linux_font_delegate.h"
-#endif
-
 namespace gfx {
 
 namespace {
 
-bool SubpixelPositioningRequested(bool for_web_contents) {
-  return CommandLine::ForCurrentProcess()->HasSwitch(
-      for_web_contents ? switches::kEnableWebkitTextSubpixelPositioning
-                       : switches::kEnableBrowserTextSubpixelPositioning);
-}
-
 // Converts Fontconfig FC_HINT_STYLE to FontRenderParams::Hinting.
 FontRenderParams::Hinting ConvertFontconfigHintStyle(int hint_style) {
   switch (hint_style) {
@@ -139,40 +130,21 @@ FontRenderParams GetCustomFontRenderParams(
     const int* pixel_size,
     const int* point_size,
     std::string* family_out) {
-  FontRenderParams params;
   if (family_out)
     family_out->clear();
 
-#if defined(OS_CHROMEOS)
-  // Use reasonable defaults.
-  params.antialiasing = true;
-  params.autohinter = true;
-  params.use_bitmaps = true;
-  params.hinting = FontRenderParams::HINTING_SLIGHT;
-
-  // Query Fontconfig to get the family name and subpixel rendering setting.
-  // In general, we try to limit Chrome OS's dependency on Fontconfig, but it's
-  // used to configure fonts for different scripts and to disable subpixel
-  // rendering on systems that use external displays.
-  // TODO(derat): Decide if we should just use Fontconfig wholeheartedly on
-  // Chrome OS; Blink is using it, after all.
-  FontRenderParams fc_params;
-  QueryFontconfig(family_list, pixel_size, point_size, &fc_params, family_out);
-  params.subpixel_rendering = fc_params.subpixel_rendering;
-#else
   // Start with the delegate's settings, but let Fontconfig have the final say.
-  // TODO(derat): Figure out if we need to query the delegate at all. Does
-  // GtkSettings always get overridden by Fontconfig in GTK apps?
+  FontRenderParams params;
   const LinuxFontDelegate* delegate = LinuxFontDelegate::instance();
-  if (delegate) {
-    params.antialiasing = delegate->UseAntialiasing();
-    params.hinting = delegate->GetHintingStyle();
-    params.subpixel_rendering = delegate->GetSubpixelRenderingStyle();
-  }
+  if (delegate)
+    params = delegate->GetDefaultFontRenderParams();
   QueryFontconfig(family_list, pixel_size, point_size, &params, family_out);
-#endif
 
-  params.subpixel_positioning = SubpixelPositioningRequested(for_web_contents);
+  // Fontconfig doesn't support configuring subpixel positioning; check a flag.
+  params.subpixel_positioning = CommandLine::ForCurrentProcess()->HasSwitch(
+      for_web_contents ?
+      switches::kEnableWebkitTextSubpixelPositioning :
+      switches::kEnableBrowserTextSubpixelPositioning);
 
   // To enable subpixel positioning, we need to disable hinting.
   if (params.subpixel_positioning)

ui/gfx/font_render_params_linux_unittest.cc

@@ -62,27 +62,15 @@ TEST_F(FontRenderParamsTest, Default) {
       kFontconfigFileFooter));
 
   FontRenderParams params = GetDefaultFontRenderParams();
-#if defined(OS_CHROMEOS)
-  // Chrome OS uses its own defaults for everything except subpixel rendering,
-  // which comes from Fontconfig.
-  EXPECT_TRUE(params.antialiasing);
-  EXPECT_TRUE(params.autohinter);
-  EXPECT_TRUE(params.use_bitmaps);
-  EXPECT_EQ(FontRenderParams::HINTING_SLIGHT, params.hinting);
-#else
-  // Desktop Linux gets all settings from fontconfig.
   EXPECT_TRUE(params.antialiasing);
   EXPECT_FALSE(params.autohinter);
   EXPECT_TRUE(params.use_bitmaps);
   EXPECT_EQ(FontRenderParams::HINTING_FULL, params.hinting);
-#endif
   EXPECT_FALSE(params.subpixel_positioning);
   EXPECT_EQ(FontRenderParams::SUBPIXEL_RENDERING_RGB,
             params.subpixel_rendering);
 }
 
-// Chrome OS ignores most Fontconfig settings.
-#if !defined(OS_CHROMEOS)
 TEST_F(FontRenderParamsTest, Size) {
   // Fontconfig needs to know about at least one font to return a match.
   ASSERT_TRUE(LoadSystemFont("arial.ttf"));
@@ -129,6 +117,5 @@ TEST_F(FontRenderParamsTest, Size) {
   EXPECT_EQ(FontRenderParams::SUBPIXEL_RENDERING_RGB,
             params.subpixel_rendering);
 }
-#endif  // !defined(OS_CHROMEOS)
 
 }  // namespace gfx

ui/gfx/gfx_tests.gyp

@@ -76,6 +76,7 @@
         '../../testing/gtest.gyp:gtest',
         '../../third_party/libpng/libpng.gyp:libpng',
         '../base/ui_base.gyp:ui_base',
+        '../resources/ui_resources.gyp:ui_test_pak',
         'gfx.gyp:gfx',
         'gfx.gyp:gfx_geometry',
         'gfx.gyp:gfx_test_support',

ui/gfx/linux_font_delegate.h

@@ -7,12 +7,15 @@
 
 #include <string>
 
+#include "base/memory/scoped_ptr.h"
 #include "ui/gfx/font_render_params.h"
 #include "ui/gfx/gfx_export.h"
 
 namespace gfx {
 
-// Allows a Linux platform specific overriding of font preferences.
+class ScopedPangoFontDescription;
+
+// Allows a Linux platform-specific overriding of font preferences.
 class GFX_EXPORT LinuxFontDelegate {
  public:
   virtual ~LinuxFontDelegate() {}
@@ -29,19 +32,12 @@ class GFX_EXPORT LinuxFontDelegate {
   // running with the "--ash" flag.)
   static const LinuxFontDelegate* instance();
 
-  // Whether we should antialias our text.
-  virtual bool UseAntialiasing() const = 0;
-
-  // What sort of text hinting should we apply?
-  virtual FontRenderParams::Hinting GetHintingStyle() const = 0;
-
-  // What sort of subpixel rendering should we perform.
-  virtual FontRenderParams::SubpixelRendering
-      GetSubpixelRenderingStyle() const = 0;
+  // Returns the default font rendering settings.
+  virtual FontRenderParams GetDefaultFontRenderParams() const = 0;
 
-  // Returns the Pango description for the default UI font. The format matches
-  // that returned by pango_font_description_to_string().
-  virtual std::string GetDefaultFontDescription() const = 0;
+  // Returns the Pango description for the default UI font.
+  virtual scoped_ptr<ScopedPangoFontDescription>
+      GetDefaultPangoFontDescription() const = 0;
 
   // Returns the resolution (as pixels-per-inch) that should be used to convert
   // font sizes between points and pixels. -1 is returned if the DPI is unset.

ui/gfx/pango_util.h

@@ -26,6 +26,11 @@ class ScopedPangoFontDescription {
     DCHECK(description);
   }
 
+  explicit ScopedPangoFontDescription(const std::string& str)
+      : description_(pango_font_description_from_string(str.c_str())) {
+    DCHECK(description_);
+  }
+
   ~ScopedPangoFontDescription() {
     pango_font_description_free(description_);
   }

ui/gfx/platform_font_pango.cc

@@ -74,19 +74,19 @@ std::string* PlatformFontPango::default_font_description_ = NULL;
 
 PlatformFontPango::PlatformFontPango() {
   if (!default_font_) {
-    std::string desc_string;
+    scoped_ptr<ScopedPangoFontDescription> description;
 #if defined(OS_CHROMEOS)
-    // Font name must have been provided by way of SetDefaultFontDescription().
     CHECK(default_font_description_);
-    desc_string = *default_font_description_;
+    description.reset(
+        new ScopedPangoFontDescription(*default_font_description_));
 #else
     const gfx::LinuxFontDelegate* delegate = gfx::LinuxFontDelegate::instance();
-    desc_string = delegate ? delegate->GetDefaultFontDescription() : "sans 10";
+    if (delegate)
+      description = delegate->GetDefaultPangoFontDescription();
 #endif
-
-    ScopedPangoFontDescription desc(
-        pango_font_description_from_string(desc_string.c_str()));
-    default_font_ = new Font(desc.get());
+    if (!description || !description->get())
+      description.reset(new ScopedPangoFontDescription("sans 10"));
+    default_font_ = new Font(description->get());
   }
 
   InitFromPlatformFont(

ui/gfx/platform_font_pango.h

@@ -35,7 +35,8 @@ class GFX_EXPORT PlatformFontPango : public PlatformFont {
   static void ReloadDefaultFont();
 
 #if defined(OS_CHROMEOS)
-  // Sets the default font.
+  // Sets the default font. |font_description| is a Pango font description that
+  // will be passed to pango_font_description_from_string().
   static void SetDefaultFontDescription(const std::string& font_description);
 #endif
 
@@ -120,6 +121,7 @@ class GFX_EXPORT PlatformFontPango : public PlatformFont {
   static Font* default_font_;
 
 #if defined(OS_CHROMEOS)
+  // A Pango font description.
   static std::string* default_font_description_;
 #endif
 

ui/gfx/platform_font_pango_unittest.cc

@@ -26,15 +26,13 @@ TEST(PlatformFontPangoTest, FamilyList) {
   g_type_init();
 #endif
 
-  ScopedPangoFontDescription desc(
-      pango_font_description_from_string("Arial,Times New Roman, 13px"));
+  ScopedPangoFontDescription desc("Arial,Times New Roman, 13px");
   scoped_refptr<gfx::PlatformFontPango> font(
       new gfx::PlatformFontPango(desc.get()));
   EXPECT_EQ("Arial", font->GetFontName());
   EXPECT_EQ(13, font->GetFontSize());
 
-  ScopedPangoFontDescription desc2(
-      pango_font_description_from_string("Times New Roman,Arial, 15px"));
+  ScopedPangoFontDescription desc2("Times New Roman,Arial, 15px");
   scoped_refptr<gfx::PlatformFontPango> font2(
       new gfx::PlatformFontPango(desc2.get()));
   EXPECT_EQ("Times New Roman", font2->GetFontName());

ui/gfx/render_text_pango.cc

@@ -355,9 +355,11 @@ void RenderTextPango::SetupPangoAttributes(PangoLayout* layout) {
     const size_t italic_end = styles()[ITALIC].GetRange(italic).end();
     const size_t style_end = std::min(bold_end, italic_end);
     if (style != font_list().GetFontStyle()) {
+      // TODO(derat): Don't interpret gfx::FontList font descriptions as Pango
+      // font descriptions: http://crbug.com/393067
       FontList derived_font_list = font_list().DeriveWithStyle(style);
-      ScopedPangoFontDescription desc(pango_font_description_from_string(
-          derived_font_list.GetFontDescriptionString().c_str()));
+      ScopedPangoFontDescription desc(
+          derived_font_list.GetFontDescriptionString());
 
       PangoAttribute* pango_attr = pango_attr_font_desc_new(desc.get());
       pango_attr->start_index =