linux: Avoid using unset Fontconfig properties.

ui/gfx/font_render_params_linux_unittest.cc

 // Copyright 2014 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/font_render_params.h"
 
 #include "base/files/file_path.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/logging.h"
 #include "base/macros.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/gfx/font.h"
+#include "ui/gfx/linux_font_delegate.h"
+#include "ui/gfx/pango_util.h"

[msw, 2014/07/21 22:19:48]

Should this (and parts of [Test|Linux]FontDelegate) be exclusive to USE_PANGO
builds? And/or is this test file excluded from non-pango configurations?

[Daniel Erat, 2014/07/21 22:54:42]

whoops, sorry for missing this the first time through. :-(

that's a good question. i believe that desktop linux will always link in pango,
as pango is used by gtk+ to configure fonts and lay out text -- the Gtk2UI impl
of LinuxFontDelegate needs pango to get the default font. do you have any plans
to get rid of PlatformFontPango when switching to RenderTextHarfbuzz? if so, i
haven't seen any signs of them. :-)

font_render_params_linux.cc calls LinuxFontDelegate to get the default params,
so everything that uses font_render_params_linux.cc (just desktop linux and
chrome os, as far as i'm aware) is already pulling in pango.

[msw, 2014/07/21 23:38:30]

Okay, I'm not actually worried beyond breaking any configs (Ozone or something?)
that might have been building this test file on Linux without Pango. You can
proceed if that's not a legit concern.

 #include "ui/gfx/test/fontconfig_util_linux.h"
 
 namespace gfx {
 
 namespace {
 
+// Implementation of LinuxFontDelegate that returns a canned FontRenderParams
+// struct. This is used to isolate tests from the system's local configuration.
+class TestFontDelegate : public LinuxFontDelegate {
+ public:
+  TestFontDelegate() {}
+  virtual ~TestFontDelegate() {}
+
+  void set_params(const FontRenderParams& params) { params_ = params; }
+
+  virtual FontRenderParams GetDefaultFontRenderParams() const OVERRIDE {
+    return params_;
+  }
+  virtual scoped_ptr<ScopedPangoFontDescription>
+      GetDefaultPangoFontDescription() const OVERRIDE {
+    NOTIMPLEMENTED();
+    return scoped_ptr<ScopedPangoFontDescription>();
+  }
+  virtual double GetFontDPI() const OVERRIDE {
+    NOTIMPLEMENTED();
+    return 96.0;
+  }
+
+ private:
+  FontRenderParams params_;
+
+  DISALLOW_COPY_AND_ASSIGN(TestFontDelegate);
+};
+
 // Loads the first system font defined by fontconfig_util_linux.h with a base
 // filename of |basename|. Case is ignored.
 bool LoadSystemFont(const std::string& basename) {
   for (size_t i = 0; i < kNumSystemFontsForFontconfig; ++i) {
     base::FilePath path(gfx::kSystemFontsForFontconfig[i]);
     if (strcasecmp(path.BaseName().value().c_str(), basename.c_str()) == 0)
       return LoadFontIntoFontconfig(path);
   }
   LOG(ERROR) << "Unable to find system font named " << basename;
   return false;
 }
 
 }  // namespace
 
 class FontRenderParamsTest : public testing::Test {
  public:
   FontRenderParamsTest() {
     SetUpFontconfig();
     CHECK(temp_dir_.CreateUniqueTempDir());
+    orig_font_delegate_ = LinuxFontDelegate::instance();
+    LinuxFontDelegate::SetInstance(&test_font_delegate_);
   }
 
   virtual ~FontRenderParamsTest() {
+    LinuxFontDelegate::SetInstance(
+        const_cast<LinuxFontDelegate*>(orig_font_delegate_));
     TearDownFontconfig();
   }
 
  protected:
   base::ScopedTempDir temp_dir_;
+  const LinuxFontDelegate* orig_font_delegate_;

[msw, 2014/07/21 22:19:48]

optional nit: original

[Daniel Erat, 2014/07/21 22:34:33]

Done.

+  TestFontDelegate test_font_delegate_;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(FontRenderParamsTest);
 };
 
 TEST_F(FontRenderParamsTest, Default) {
   // Fontconfig needs to know about at least one font to return a match.
   ASSERT_TRUE(LoadSystemFont("arial.ttf"));
   ASSERT_TRUE(LoadConfigDataIntoFontconfig(temp_dir_.path(),
       std::string(kFontconfigFileHeader) +
@@ skipping 118 matching lines @@
       kFontconfigMatchHeader +
       CreateFontconfigEditStanza("antialias", "bool", "false") +
       kFontconfigMatchFooter +
       kFontconfigMatchHeader +
       CreateFontconfigTestStanza("scalable", "eq", "bool", "true") +
       CreateFontconfigEditStanza("antialias", "bool", "true") +
       kFontconfigMatchFooter +
       kFontconfigFileFooter));
 
   // Check that we specifically ask how scalable fonts should be rendered.
   FontRenderParams params = GetCustomFontRenderParams(

[msw, 2014/07/21 22:19:48]

nit: use GetDefaultFontRenderParams.

[Daniel Erat, 2014/07/21 22:34:33]

i'm actually wondering if i should just delete that method.
GetDefaultWebKitFontRenderParams() makes sense; it is (or at least was) used to
get default settings for web fonts. but we should be able to call
GetCustomFontRenderParams() everywhere else, since we know which font we're
going to render.

       false, NULL, NULL, NULL, NULL, NULL);
   EXPECT_TRUE(params.antialiasing);
 }
 
 TEST_F(FontRenderParamsTest, UseBitmaps) {
   ASSERT_TRUE(LoadSystemFont("arial.ttf"));
   // Load a config that enables embedded bitmaps for fonts <= 10 pixels.
   ASSERT_TRUE(LoadConfigDataIntoFontconfig(temp_dir_.path(),
       std::string(kFontconfigFileHeader) +
       kFontconfigMatchHeader +
       CreateFontconfigEditStanza("embeddedbitmap", "bool", "false") +
       kFontconfigMatchFooter +
       kFontconfigMatchHeader +
       CreateFontconfigTestStanza("pixelsize", "less_eq", "double", "10") +
       CreateFontconfigEditStanza("embeddedbitmap", "bool", "true") +
       kFontconfigMatchFooter +
       kFontconfigFileFooter));
 
   FontRenderParams params = GetCustomFontRenderParams(

[msw, 2014/07/21 22:19:48]

nit: use GetDefaultFontRenderParams.

       false, NULL, NULL, NULL, NULL, NULL);
   EXPECT_FALSE(params.use_bitmaps);
 
   const int pixel_size = 5;
   params = GetCustomFontRenderParams(
       false, NULL, &pixel_size, NULL, NULL, NULL);
   EXPECT_TRUE(params.use_bitmaps);
 }
 
+TEST_F(FontRenderParamsTest, OnlySetConfiguredValues) {
+  // Configure the LinuxFontDelegate (which queries GtkSettings on desktop
+  // Linux) to request subpixel rendering.
+  FontRenderParams system_params;
+  system_params.subpixel_rendering = FontRenderParams::SUBPIXEL_RENDERING_RGB;
+  test_font_delegate_.set_params(system_params);
+
+  // Load a Fontconfig config that enables antialiasing but doesn't say anything
+  // about subpixel rendering.
+  ASSERT_TRUE(LoadSystemFont("arial.ttf"));
+  ASSERT_TRUE(LoadConfigDataIntoFontconfig(temp_dir_.path(),
+      std::string(kFontconfigFileHeader) +
+      kFontconfigMatchHeader +
+      CreateFontconfigEditStanza("antialias", "bool", "true") +
+      kFontconfigMatchFooter +
+      kFontconfigFileFooter));
+
+  // The subpixel rendering setting from the delegate should make it through.
+  FontRenderParams params = GetCustomFontRenderParams(

[msw, 2014/07/21 22:19:48]

nit: use GetDefaultFontRenderParams.

+      false, NULL, NULL, NULL, NULL, NULL);
+  EXPECT_EQ(system_params.subpixel_rendering, params.subpixel_rendering);
+}
+
 }  // namespace gfx