linux: Add tests for FontRenderParams on Linux.

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 <fontconfig/fontconfig.h>
+
+#include "base/file_util.h"
+#include "base/files/file_path.h"
+#include "base/files/scoped_temp_dir.h"
+#include "base/macros.h"
+#include "base/strings/stringprintf.h"
+#include "net/base/escape.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace gfx {
+
+namespace {
+
+// Strings appearing at the beginning and end of Fontconfig XML files.
+const char kFileHeader[] =
+    "<?xml version=\"1.0\"?>\n"
+    "<!DOCTYPE fontconfig SYSTEM \"fonts.dtd\">\n"
+    "<fontconfig>\n";
+const char kFileFooter[] = "</fontconfig>";
+
+// Strings appearing at the beginning and end of Fontconfig <match> stanzas.
+const char kMatchHeader[] = "  <match>\n";
+const char kMatchFooter[] = "  </match>\n";
+
+// Returns a Fontconfig <edit> stanza.
+std::string CreateEditStanza(const std::string& name,

[Daniel Erat, 2014/07/15 22:09:45]

an alternative would be creating a testdata directory and storing the configs
there, but i have a preference for keeping the data within the test when
possible.

[msw, 2014/07/16 00:53:31]

Test .conf files might be clearer (especially if we test more features in the
future); they might also lead to more [test] code sharing with
content/shell/app/webkit_test_platform_support_linux.cc, which appears to use
content/shell/renderer/test_runner/resources/fonts/fonts.conf. That said, your
self-contained approach does seem okay for now. I don't have a strong opinion,
but maybe ping another reviewer?

+                             const std::string& type,
+                             const std::string& value) {
+  return base::StringPrintf(
+      "    <edit name=\"%s\" mode=\"assign\">\n"
+      "      <%s>%s</%s>\n"
+      "    </edit>\n",
+      name.c_str(), type.c_str(), value.c_str(), type.c_str());
+}
+
+#if !defined(OS_CHROMEOS)
+// Returns a Fontconfig <test> stanza.
+std::string CreateTestStanza(const std::string& name,
+                             const std::string& op,
+                             const std::string& type,
+                             const std::string& value) {
+  return base::StringPrintf(
+      "    <test name=\"%s\" compare=\"%s\" qual=\"any\">\n"
+      "      <%s>%s</%s>\n"
+      "    </test>\n",
+      name.c_str(), op.c_str(), type.c_str(), value.c_str(), type.c_str());
+}
+#endif  // !defined(OS_CHROMEOS)
+
+}  // namespace
+
+class FontRenderParamsTest : public testing::Test {
+ public:
+  FontRenderParamsTest() {

[msw, 2014/07/16 00:53:31]

Should this be done on SetUp instead of the ctor?

[Daniel Erat, 2014/07/16 03:19:47]

these are mostly equivalent, but i believe that c'tor/d'tor are preferred:
https://code.google.com/p/googletest/wiki/FAQ#Should_I_use_the_constructor/de...

+    CHECK(FcInit());
+    CHECK(temp_dir_.CreateUniqueTempDir());
+
+    // Add the original config's fonts to the test config.

[Daniel Erat, 2014/07/15 22:09:45]

the other approach here would be creating a testdata directory and storing some
freely-distributable font there and then calling FcConfigAppFontAddFile() to
load it. that might actually be slower than just using the existing cache,
though, and this doesn't seem like it adds any dependencies
(platform_font_pango_unittest.cc already appears to depend on arial and times
new roman being installed; these tests just need a single arbitrary font to be
present to generate a match).

[msw, 2014/07/16 00:53:31]

It looks like content/shell/app/webkit_test_platform_support_linux.cc already
expects a whole hardcoded list of |kFonts| dirs/files to be accessible and fires
off some warnings/errors if they're not. It also loads a couple other i18n fonts
using FcConfigAppFontAddFile. Can we follow that existing pattern instead?

[Daniel Erat, 2014/07/16 03:19:47]

thanks for pointing that out. i'll switch to that pattern, which means the
font/cache stuff is unneeded. i might send out another change first to try to
make a ui/gfx/fontconfig_test_support or similar file that content/ can use.

+    std::string config_data = kFileHeader;
+
+    FcStrList* font_dirs = FcConfigGetFontDirs(NULL);
+    const FcChar8* font_dir = NULL;
+    while ((font_dir = FcStrListNext(font_dirs))) {
+      const std::string path = net::EscapeForHTML(

[Daniel Erat, 2014/07/15 22:09:45]

ui/base appears to use this for escaping, so i hope it's okay to add to
ui/gfx... i couldn't find anything in base/ for doing this.

[msw, 2014/07/16 00:53:31]

Maybe just make a base::FilePath and get its ::value() instead?
(we should probably avoid adding a direct net dependency if possible)

[willchan no longer on Chromium, 2014/07/16 01:35:38]

That'd be nice if possible :) Let me know if I need to approve a net dependency,
but it'd be nice to avoid it.

[Daniel Erat, 2014/07/16 03:19:47]

this is going into an XML file, so i need to ampersand-escape things like quotes
and angled brackets. putting it into a base::FilePath and then converting it
back to a string won't help there. :-P

+          reinterpret_cast<const char*>(font_dir));
+      config_data += base::StringPrintf("  <dir>%s</dir>\n", path.c_str());
+    }
+    FcStrListDone(font_dirs);
+
+    FcStrList* cache_dirs = FcConfigGetCacheDirs(NULL);

[msw, 2014/07/16 00:53:31]

The <cachedir>s are used to *store* or read the cache of font information. Will
this test have any effect on the actual system font caches by listing these
directories? Should you use the TempDir instead or can we omit this altogether?

[Daniel Erat, 2014/07/16 03:19:47]

the already-built caches are needed to prevent fontconfig from needing to read
all of the font files at the beginning of each test (which takes 15+ seconds on
my z620).

+    const FcChar8* cache_dir = NULL;
+    while ((cache_dir = FcStrListNext(cache_dirs))) {
+      const std::string path = net::EscapeForHTML(
+          reinterpret_cast<const char*>(cache_dir));
+      config_data += base::StringPrintf(
+          "  <cachedir>%s</cachedir>\n", path.c_str());
+    }
+    FcStrListDone(cache_dirs);
+
+    config_data += kFileFooter;
+    config_ = FcConfigCreate();
+    LoadFontconfigData(config_data);

[msw, 2014/07/16 00:53:31]

Could this class instead offer any needed font/dir/cache elements via a helper
(that returns a string), which individual tests append to their configs?

[Daniel Erat, 2014/07/16 03:19:47]

it could, but i'm not sure if there's much benefit. all of the tests will need
fonts to be installed to be able to generate matches (unless there's a future
test for the no-match case).

+    CHECK(FcConfigBuildFonts(config_));
+
+    // Swap in the new config. A primer on undocumented FcConfig

[msw, 2014/07/16 00:53:31]

Ha, this is good to include!

+    // reference-counting:
+    //
+    // - FcConfigCreate() creates a config with a refcount of 1.
+    // - FcConfigReference() increments a config's refcount.
+    // - FcConfigDestroy() decrements a config's refcount, deallocating the
+    //   config when the count reaches 0.
+    // - FcConfigSetCurrent() calls FcConfigDestroy() on the old config, but
+    //   interestingly does not call FcConfigReference() on the new config.
+    CHECK(FcConfigSetCurrent(config_));
+  }
+
+  virtual ~FontRenderParamsTest() {

[msw, 2014/07/16 00:53:31]

Should this be done on TearDown instead of the dtor?

[Daniel Erat, 2014/07/16 03:19:47]

see c'tor comment

+    FcFini();
+  }
+
+ protected:
+  // Writes Fontconfig configuration data to a temporary file and loads it into
+  // |config_|.
+  void LoadFontconfigData(const std::string& data) {
+    VLOG(1) << "Loading config:\n" << data;
+    base::FilePath path;
+    ASSERT_TRUE(CreateTemporaryFileInDir(temp_dir_.path(), &path));
+    ASSERT_EQ(static_cast<int>(data.size()),
+        base::WriteFile(path, data.data(), data.size()));
+    ASSERT_TRUE(FcConfigParseAndLoad(
+        config_, reinterpret_cast<const FcChar8*>(path.value().c_str()),
+        FcTrue));
+  }
+
+ private:
+  FcConfig* config_;
+
+  base::ScopedTempDir temp_dir_;
+
+  DISALLOW_COPY_AND_ASSIGN(FontRenderParamsTest);
+};
+
+TEST_F(FontRenderParamsTest, Default) {
+  LoadFontconfigData(

[msw, 2014/07/16 00:53:31]

Both tests don't modify the same FontRenderParamsTest::config_ instance, right?
Still, perhaps they could use test-local FcConfig instances?

[Daniel Erat, 2014/07/16 03:19:47]

see the googletest faq that i linked to -- each TEST_F generates a new instance
of FontRenderParamsTest

+      std::string(kFileHeader) +
+      kMatchHeader +
+      CreateEditStanza("antialias", "bool", "true") +
+      CreateEditStanza("autohint", "bool", "false") +
+      CreateEditStanza("hinting", "bool", "true") +
+      CreateEditStanza("hintstyle", "const", "hintfull") +
+      CreateEditStanza("rgba", "const", "rgb") +
+      kMatchFooter +
+      kFileFooter);
+
+  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) {
+  LoadFontconfigData(
+      std::string(kFileHeader) +
+      kMatchHeader +
+      CreateEditStanza("antialias", "bool", "true") +
+      CreateEditStanza("hinting", "bool", "true") +
+      CreateEditStanza("hintstyle", "const", "hintfull") +
+      CreateEditStanza("rgba", "const", "none") +
+      kMatchFooter +
+      kMatchHeader +
+      CreateTestStanza("pixelsize", "less_eq", "double", "10") +
+      CreateEditStanza("antialias", "bool", "false") +
+      kMatchFooter +
+      kMatchHeader +
+      CreateTestStanza("size", "more_eq", "double", "20") +
+      CreateEditStanza("hintstyle", "const", "hintslight") +
+      CreateEditStanza("rgba", "const", "rgb") +
+      kMatchFooter +
+      kFileFooter);
+
+  // The defaults should be used when the supplied size isn't matched by the
+  // second or third blocks.
+  int pixel_size = 12;
+  FontRenderParams params = GetCustomFontRenderParams(
+      false, NULL, &pixel_size, NULL, NULL);
+  EXPECT_TRUE(params.antialiasing);
+  EXPECT_EQ(FontRenderParams::HINTING_FULL, params.hinting);
+  EXPECT_EQ(FontRenderParams::SUBPIXEL_RENDERING_NONE,
+            params.subpixel_rendering);
+
+  pixel_size = 10;
+  params = GetCustomFontRenderParams(false, NULL, &pixel_size, NULL, NULL);
+  EXPECT_FALSE(params.antialiasing);
+  EXPECT_EQ(FontRenderParams::HINTING_FULL, params.hinting);
+  EXPECT_EQ(FontRenderParams::SUBPIXEL_RENDERING_NONE,
+            params.subpixel_rendering);
+
+  int point_size = 20;
+  params = GetCustomFontRenderParams(false, NULL, NULL, &point_size, NULL);
+  EXPECT_TRUE(params.antialiasing);
+  EXPECT_EQ(FontRenderParams::HINTING_SLIGHT, params.hinting);
+  EXPECT_EQ(FontRenderParams::SUBPIXEL_RENDERING_RGB,
+            params.subpixel_rendering);
+}
+#endif  // !defined(OS_CHROMEOS)
+
+}  // namespace gfx