linux/chromeos: Improve queries for Fontconfig defaults.

r302898 made FontRenderParams avoid calling FcFontMatch()
for empty queries, instead just sticking to the default
pattern. Many users don't have sane default patterns,
though, so instead call FcConfigSubstituteWithPat() to load
settings that aren't tied to particular font families.

Also use scoped_ptr to simplify FcPattern management and add
myself as an owner for font_render_params*.

BUG=442443,435277,423056

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

Cr-Commit-Position: refs/heads/master@{#309560}

ui/gfx/OWNERS

@@ -11,6 +11,9 @@ msw@chromium.org
 per-file display*=oshima@chromium.org
 per-file screen*=oshima@chromium.org
 
+# Font-rendering configuration.
+per-file font_render_params*=derat@chromium.org
+
 # Transform, interpolated transform and transform util.
 per-file transform*=vollick@chromium.org
 per-file interpolated_transform*=vollick@chromium.org

ui/gfx/font_render_params_linux.cc

@@ -12,6 +12,7 @@
 #include "base/lazy_instance.h"
 #include "base/logging.h"
 #include "base/macros.h"
+#include "base/memory/scoped_ptr.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/synchronization/lock.h"
@@ -96,84 +97,97 @@ FontRenderParams::SubpixelRendering ConvertFontconfigRgba(int rgba) {
 bool QueryFontconfig(const FontRenderParamsQuery& query,
                      FontRenderParams* params_out,
                      std::string* family_out) {
-  FcPattern* query_pattern = FcPatternCreate();
+  struct FcPatternDeleter {
+    void operator()(FcPattern* ptr) const { FcPatternDestroy(ptr); }
+  };
+  typedef scoped_ptr<FcPattern, FcPatternDeleter> ScopedFcPattern;
+
+  ScopedFcPattern query_pattern(FcPatternCreate());
   CHECK(query_pattern);
 
-  FcPatternAddBool(query_pattern, FC_SCALABLE, FcTrue);
+  FcPatternAddBool(query_pattern.get(), FC_SCALABLE, FcTrue);
 
   for (std::vector<std::string>::const_iterator it = query.families.begin();
        it != query.families.end(); ++it) {
-    FcPatternAddString(query_pattern,
+    FcPatternAddString(query_pattern.get(),
         FC_FAMILY, reinterpret_cast<const FcChar8*>(it->c_str()));
   }
   if (query.pixel_size > 0)
-    FcPatternAddDouble(query_pattern, FC_PIXEL_SIZE, query.pixel_size);
+    FcPatternAddDouble(query_pattern.get(), FC_PIXEL_SIZE, query.pixel_size);
   if (query.point_size > 0)
-    FcPatternAddInteger(query_pattern, FC_SIZE, query.point_size);
+    FcPatternAddInteger(query_pattern.get(), FC_SIZE, query.point_size);
   if (query.style >= 0) {
-    FcPatternAddInteger(query_pattern, FC_SLANT,
+    FcPatternAddInteger(query_pattern.get(), FC_SLANT,
         (query.style & Font::ITALIC) ? FC_SLANT_ITALIC : FC_SLANT_ROMAN);
-    FcPatternAddInteger(query_pattern, FC_WEIGHT,
+    FcPatternAddInteger(query_pattern.get(), FC_WEIGHT,
         (query.style & Font::BOLD) ? FC_WEIGHT_BOLD : FC_WEIGHT_NORMAL);
   }
 
-  FcConfigSubstitute(NULL, query_pattern, FcMatchPattern);
-  FcDefaultSubstitute(query_pattern);
+  FcConfigSubstitute(NULL, query_pattern.get(), FcMatchPattern);
+  FcDefaultSubstitute(query_pattern.get());
 
-  // If the query was non-empty, match a specific font and destroy the query
-  // pattern. Otherwise, just use the query pattern.
-  FcPattern* result_pattern = query_pattern;
-  if (!query.is_empty()) {
+  ScopedFcPattern result_pattern;
+  if (query.is_empty()) {
+    // If the query was empty, call FcConfigSubstituteWithPat() to get
+    // non-family-specific configuration so it can be used as the default.
+    result_pattern.reset(FcPatternDuplicate(query_pattern.get()));
+    if (!result_pattern)
+      return false;
+    FcPatternDel(result_pattern.get(), FC_FAMILY);
+    FcConfigSubstituteWithPat(NULL, result_pattern.get(), query_pattern.get(),
+                              FcMatchFont);
+  } else {
     FcResult result;
-    result_pattern = FcFontMatch(NULL, query_pattern, &result);
-    FcPatternDestroy(query_pattern);
-    query_pattern = NULL;
+    result_pattern.reset(FcFontMatch(NULL, query_pattern.get(), &result));
     if (!result_pattern)
       return false;
   }
+  DCHECK(result_pattern);
 
   if (family_out) {
     FcChar8* family = NULL;
-    FcPatternGetString(result_pattern, FC_FAMILY, 0, &family);
+    FcPatternGetString(result_pattern.get(), FC_FAMILY, 0, &family);
     if (family)
       family_out->assign(reinterpret_cast<const char*>(family));
   }
 
   if (params_out) {
     FcBool fc_antialias = 0;
-    if (FcPatternGetBool(result_pattern, FC_ANTIALIAS, 0, &fc_antialias) ==
-        FcResultMatch) {
+    if (FcPatternGetBool(result_pattern.get(), FC_ANTIALIAS, 0,
+                         &fc_antialias) == FcResultMatch) {
       params_out->antialiasing = fc_antialias;
     }
 
     FcBool fc_autohint = 0;
-    if (FcPatternGetBool(result_pattern, FC_AUTOHINT, 0, &fc_autohint) ==
+    if (FcPatternGetBool(result_pattern.get(), FC_AUTOHINT, 0, &fc_autohint) ==
         FcResultMatch) {
       params_out->autohinter = fc_autohint;
     }
 
     FcBool fc_bitmap = 0;
-    if (FcPatternGetBool(result_pattern, FC_EMBEDDED_BITMAP, 0, &fc_bitmap) ==
+    if (FcPatternGetBool(result_pattern.get(), FC_EMBEDDED_BITMAP, 0,
+                         &fc_bitmap) ==
         FcResultMatch) {
       params_out->use_bitmaps = fc_bitmap;
     }
 
     FcBool fc_hinting = 0;
-    if (FcPatternGetBool(result_pattern, FC_HINTING, 0, &fc_hinting) ==
+    if (FcPatternGetBool(result_pattern.get(), FC_HINTING, 0, &fc_hinting) ==
         FcResultMatch) {
       int fc_hint_style = FC_HINT_NONE;
-      if (fc_hinting)
-        FcPatternGetInteger(result_pattern, FC_HINT_STYLE, 0, &fc_hint_style);
+      if (fc_hinting) {
+        FcPatternGetInteger(
+            result_pattern.get(), FC_HINT_STYLE, 0, &fc_hint_style);
+      }
       params_out->hinting = ConvertFontconfigHintStyle(fc_hint_style);
     }
 
     int fc_rgba = FC_RGBA_NONE;
-    if (FcPatternGetInteger(result_pattern, FC_RGBA, 0, &fc_rgba) ==
+    if (FcPatternGetInteger(result_pattern.get(), FC_RGBA, 0, &fc_rgba) ==
         FcResultMatch)
       params_out->subpixel_rendering = ConvertFontconfigRgba(fc_rgba);
   }
 
-  FcPatternDestroy(result_pattern);
   return true;
 }
 

ui/gfx/font_render_params_linux_unittest.cc

@@ -47,7 +47,8 @@ class TestFontDelegate : public LinuxFontDelegate {
 };
 
 // Loads the first system font defined by fontconfig_util_linux.h with a base
-// filename of |basename|. Case is ignored.
+// filename of |basename|. Case is ignored. FcFontMatch() requires there to be
+// at least one font present.
 bool LoadSystemFont(const std::string& basename) {
   for (size_t i = 0; i < kNumSystemFontsForFontconfig; ++i) {
     base::FilePath path(gfx::kSystemFontsForFontconfig[i]);
@@ -89,21 +90,24 @@ TEST_F(FontRenderParamsTest, Default) {
   ASSERT_TRUE(LoadSystemFont("arial.ttf"));
   ASSERT_TRUE(LoadConfigDataIntoFontconfig(temp_dir_.path(),
       std::string(kFontconfigFileHeader) +
-      kFontconfigMatchPatternHeader +
+      // Specify the desired defaults via a font match rather than a pattern
+      // match (since this is the style generally used in /etc/fonts/conf.d).
+      kFontconfigMatchFontHeader +
       CreateFontconfigEditStanza("antialias", "bool", "true") +
       CreateFontconfigEditStanza("autohint", "bool", "true") +
       CreateFontconfigEditStanza("hinting", "bool", "true") +
       CreateFontconfigEditStanza("hintstyle", "const", "hintslight") +
       CreateFontconfigEditStanza("rgba", "const", "rgb") +
       kFontconfigMatchFooter +
-      // Add a font match for Arial; it shouldn't be used when querying for
-      // default settings: http://crbug.com/421247
+      // Add a font match for Arial. Since it specifies a family, it shouldn't
+      // take effect when querying default settings.
       kFontconfigMatchFontHeader +
       CreateFontconfigTestStanza("family", "eq", "string", "Arial") +
       CreateFontconfigEditStanza("antialias", "bool", "true") +
       CreateFontconfigEditStanza("autohint", "bool", "false") +
       CreateFontconfigEditStanza("hinting", "bool", "true") +
       CreateFontconfigEditStanza("hintstyle", "const", "hintfull") +
+      CreateFontconfigEditStanza("rgba", "const", "none") +
       kFontconfigMatchFooter +
       kFontconfigFileFooter));