Move test_fonts setup code to //skia

Some blink unittests depend on common fonts which may not be available
on all platforms. This problem is solved in blink layout tests by using
custom font configuration with fonts from //third_party/test_fonts .
Previously the corresponding code was part of content_shell. Moved
it to //skia to make it possible to reuse it for blink_platform_unittests
and skia_unittests.

Bug: 928459
Change-Id: Ic7ea1e53c29ca9aea9014ed6c778ad0f8a2de6fb
Reviewed-on: https://chromium-review.googlesource.com/c/1469287Reviewed-by: Nico Weber <thakis@chromium.org>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Commit-Queue: Sergey Ulanov <sergeyu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#632414}

content/shell/BUILD.gn

@@ -72,8 +72,6 @@ jumbo_static_library("content_shell_lib") {
     "android/shell_manager.cc",
     "android/shell_manager.h",
     "app/blink_test_platform_support.h",
-    "app/blink_test_platform_support_android.cc",
-    "app/blink_test_platform_support_linux.cc",
     "app/blink_test_platform_support_mac.mm",
     "app/blink_test_platform_support_win.cc",
     "app/paths_mac.h",
@@ -314,6 +312,7 @@ jumbo_static_library("content_shell_lib") {
     "//services/test/echo/public/cpp:manifest",
     "//services/test/echo/public/mojom",
     "//skia",
+    "//skia:test_fonts",
     "//storage/browser",
     "//testing/gmock",
     "//testing/gtest",
@@ -340,7 +339,6 @@ jumbo_static_library("content_shell_lib") {
   ]
 
   if (is_fuchsia) {
-    sources += [ "app/blink_test_platform_support_fuchsia.cc" ]
     deps -= [
       "//components/crash/content/app",
       "//components/crash/content/app:test_support",
@@ -498,12 +496,6 @@ group("resources") {
   public_deps = [
     ":copy_shell_resources",
   ]
-  deps = [
-    "//content/shell/test_runner:resources",
-  ]
-  data_deps = [
-    "//content/shell/test_runner:resources",
-  ]
 }
 
 repack("pak") {
@@ -579,11 +571,6 @@ if (is_android) {
     deps = [
       ":content_shell_framework_bundle_data",
       ":content_shell_resources_bundle_data",
-
-      # TODO(rsesek): Remove this after GYP is gone, since it only needs to
-      # be here per the comment in blink_test_platform_support_mac.mm about
-      # the bundle structure.
-      "//content/shell/test_runner:resources",
     ]
     info_plist_target = ":content_shell_plist"
     data_deps = [

content/shell/app/blink_test_platform_support.h

@@ -8,7 +8,7 @@
 namespace content {
 
 bool CheckLayoutSystemDeps();
-bool BlinkTestPlatformInitialize();
+void BlinkTestPlatformInitialize();
 
 }  // namespace content
 

content/shell/app/blink_test_platform_support_mac.mm

@@ -2,11 +2,6 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "base/command_line.h"
-#include "base/logging.h"
-#include "base/mac/bundle_locations.h"
-#include "base/path_service.h"
-#include "base/stl_util.h"
 #include "content/shell/app/blink_test_platform_support.h"
 
 #include <AppKit/AppKit.h>
@@ -17,7 +12,7 @@ namespace content {
 namespace {
 
 void SetDefaultsToWebTestValues(void) {
-  // So we can match the Blnk web tests, we want to force a bunch of
+  // So we can match the Blink web tests, we want to force a bunch of
   // preferences that control appearance to match.
   // (We want to do this as early as possible in application startup so
   // the settings are in before any higher layers could cache values.)
@@ -50,42 +45,8 @@ void SetDefaultsToWebTestValues(void) {
 
 }  // namespace
 
-bool CheckLayoutSystemDeps() {
-  return true;
-}
-
-bool BlinkTestPlatformInitialize() {
+void BlinkTestPlatformInitialize() {
   SetDefaultsToWebTestValues();
-
-  // Load font files in the resource folder.
-  static const char* const kFontFileNames[] = {"Ahem.TTF",
-                                               "ChromiumAATTest.ttf"};
-
-  // mainBundle is Content Shell Helper.app.  Go two levels up to find
-  // Content Shell.app. Due to DumpRenderTree injecting the font files into
-  // its direct dependents, it's not easily possible to put the ttf files into
-  // the helper's resource directory instead of the outer bundle's resource
-  // directory.
-  NSString* bundle = [base::mac::FrameworkBundle() bundlePath];
-  bundle = [bundle stringByAppendingPathComponent:@"../.."];
-  NSURL* resources_directory = [[NSBundle bundleWithPath:bundle] resourceURL];
-
-  NSMutableArray* font_urls = [NSMutableArray array];
-  for (unsigned i = 0; i < base::size(kFontFileNames); ++i) {
-    NSURL* font_url = [resources_directory
-        URLByAppendingPathComponent:
-            [NSString stringWithUTF8String:kFontFileNames[i]]];
-    [font_urls addObject:[font_url absoluteURL]];
-  }
-
-  CFArrayRef errors = 0;
-  if (!CTFontManagerRegisterFontsForURLs((CFArrayRef)font_urls,
-                                         kCTFontManagerScopeProcess,
-                                         &errors)) {
-    DLOG(FATAL) << "Fail to activate fonts.";
-    CFRelease(errors);
-  }
-  return true;
 }
 
 }  // namespace content

content/shell/app/blink_test_platform_support_win.cc

@@ -23,7 +23,7 @@ namespace content {
 
 namespace {
 
-bool SetupFonts() {
+void SetupFonts() {
   // Load Ahem font. Ahem.ttf is copied to the build directory by
   // //third_party/test_fonts .
   base::FilePath base_path;
@@ -36,8 +36,6 @@ bool SetupFonts() {
   base::CommandLine& command_line = *base::CommandLine::ForCurrentProcess();
   command_line.AppendSwitchASCII(kRegisterFontFiles,
                                  base::WideToUTF8(font_path.value()));
-
-  return true;
 }
 
 }  // namespace
@@ -72,8 +70,8 @@ bool CheckLayoutSystemDeps() {
   return errors.empty();
 }
 
-bool BlinkTestPlatformInitialize() {
-  return SetupFonts();
+void BlinkTestPlatformInitialize() {
+  SetupFonts();
 }
 
 }  // namespace content

content/shell/app/shell_main_delegate.cc

@@ -46,6 +46,7 @@
 #include "ppapi/buildflags/buildflags.h"
 #include "services/network/public/cpp/network_switches.h"
 #include "services/service_manager/embedder/switches.h"
+#include "skia/ext/test_fonts.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/base/ui_base_paths.h"
 #include "ui/base/ui_base_switches.h"
@@ -192,15 +193,15 @@ bool ShellMainDelegate::BasicStartupComplete(int* exit_code) {
   }
 
   if (command_line.HasSwitch(switches::kRunWebTests)) {
-    // Only run CheckLayoutSystemDeps on the browser process.
-    if (!command_line.HasSwitch(switches::kProcessType)) {
-      // If CheckLayoutSystemDeps fails, we early exit as there's no point in
-      // running the tests.
-      if (!CheckLayoutSystemDeps()) {
-        *exit_code = 1;
-        return true;
-      }
+#if defined(OS_WIN)
+    // Run CheckLayoutSystemDeps() in the browser process and exit early if it
+    // fails.
+    if (!command_line.HasSwitch(switches::kProcessType) &&
+        !CheckLayoutSystemDeps()) {
+      *exit_code = 1;
+      return true;
     }
+#endif
 
     EnableBrowserWebTestMode();
 
@@ -298,10 +299,15 @@ bool ShellMainDelegate::BasicStartupComplete(int* exit_code) {
     command_line.AppendSwitch(
         switches::kDisableGpuProcessForDX12VulkanInfoCollection);
 
-    if (!BlinkTestPlatformInitialize()) {
-      *exit_code = 1;
-      return true;
-    }
+#if defined(OS_WIN) || defined(OS_MACOSX)
+    BlinkTestPlatformInitialize();
+#endif
+
+#if !defined(OS_WIN)
+    // On Windows BlinkTestPlatformInitialize() is responsible for font
+    // initialization.
+    skia::ConfigureTestFont();
+#endif
   }
 
   content_client_.reset(switches::IsRunWebTestsSwitchPresent()

content/shell/test_runner/BUILD.gn

@@ -92,7 +92,6 @@ jumbo_component("test_runner") {
   configs += [ "//build/config/compiler:no_size_t_to_int_warning" ]
 
   deps = [
-    ":resources",
     "//base:base",
     "//base:i18n",
     "//cc",
@@ -129,59 +128,7 @@ jumbo_component("test_runner") {
     "//v8",
   ]
 
-  data_deps = [
-    ":resources",
-  ]
-
   if (is_mac) {
     libs = [ "AppKit.framework" ]
   }
 }
-
-# Font copies.
-if (is_android || is_fuchsia) {
-  copy("copy_android_fonts_config") {
-    visibility = [ ":*" ]
-    sources = [
-      "resources/fonts/android_fallback_fonts.xml",
-      "resources/fonts/android_main_fonts.xml",
-    ]
-    outputs = [
-      "$root_out_dir/test_fonts/{{source_file_part}}",
-    ]
-  }
-}
-if (is_mac) {
-  bundle_data("test_runner_bundle_data") {
-    public_deps = [
-      "//third_party/test_fonts",
-    ]
-
-    sources = [
-      "$root_out_dir/test_fonts/Ahem.ttf",
-      "resources/fonts/ChromiumAATTest.ttf",
-    ]
-
-    outputs = [
-      "{{bundle_resources_dir}}/{{source_file_part}}",
-    ]
-  }
-}
-
-group("resources") {
-  deps = []
-  data_deps = []
-
-  if (is_mac) {
-    deps += [ ":test_runner_bundle_data" ]
-    data_deps += [ ":test_runner_bundle_data" ]
-  } else {
-    deps += [ "//third_party/test_fonts" ]
-    data_deps += [ "//third_party/test_fonts" ]
-  }
-
-  if (is_android || is_fuchsia) {
-    deps += [ ":copy_android_fonts_config" ]
-    data_deps += [ ":copy_android_fonts_config" ]
-  }
-}

skia/BUILD.gn

@@ -790,6 +790,78 @@ skia_source_set("skia_opts") {
   visibility = [ ":skia" ]
 }
 
+# Font copies.
+if (is_android || is_fuchsia) {
+  copy("copy_android_fonts_config") {
+    sources = [
+      "ext/data/test_fonts/android_fallback_fonts.xml",
+      "ext/data/test_fonts/android_main_fonts.xml",
+    ]
+    outputs = [
+      "$root_out_dir/test_fonts/{{source_file_part}}",
+    ]
+  }
+}
+if (is_mac) {
+  bundle_data("test_fonts_bundle_data") {
+    public_deps = [
+      "//third_party/test_fonts",
+    ]
+
+    sources = [
+      "$root_out_dir/test_fonts/Ahem.ttf",
+      "ext/data/test_fonts/ChromiumAATTest.ttf",
+    ]
+
+    outputs = [
+      "{{bundle_resources_dir}}/{{source_file_part}}",
+    ]
+  }
+}
+
+group("test_fonts_resources") {
+  deps = []
+  data_deps = []
+
+  if (is_mac) {
+    deps += [ ":test_fonts_bundle_data" ]
+    data_deps += [ ":test_fonts_bundle_data" ]
+  } else {
+    deps += [ "//third_party/test_fonts" ]
+    data_deps += [ "//third_party/test_fonts" ]
+  }
+
+  if (is_android || is_fuchsia) {
+    deps += [ ":copy_android_fonts_config" ]
+    data_deps += [ ":copy_android_fonts_config" ]
+  }
+}
+
+source_set("test_fonts") {
+  testonly = true
+  sources = [
+    "ext/test_fonts.h",
+  ]
+  if (is_android) {
+    sources += [ "ext/test_fonts_android.cc" ]
+  }
+  if (is_fuchsia) {
+    sources += [ "ext/test_fonts_fuchsia.cc" ]
+  }
+  if (is_linux) {
+    sources += [ "ext/test_fonts_linux.cc" ]
+  }
+  if (is_mac) {
+    sources += [ "ext/test_fonts_mac.mm" ]
+  }
+
+  deps = [
+    ":test_fonts_resources",
+    "//base/test:test_support",
+    "//skia",
+  ]
+}
+
 test("skia_unittests") {
   sources = [
     "ext/convolver_unittest.cc",

skia/ext/test_fonts.h

+// Copyright 2019 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.
+
+#ifndef SKIA_EXT_TEST_FONTS_H_
+#define SKIA_EXT_TEST_FONTS_H_
+
+namespace skia {
+
+// Configures the process to use //third_party/test_fonts. Should be called
+// early, before default instance of SkFontMgr is created.
+void ConfigureTestFont();
+
+}  // namespace skia
+
+#endif  // SKIA_EXT_TEST_FONTS_H_

content/shell/app/blink_test_platform_support_android.cc → skia/ext/test_fonts_android.cc

-// Copyright 2013 The Chromium Authors. All rights reserved.
+// Copyright 2019 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 "content/shell/app/blink_test_platform_support.h"
+#include "skia/ext/test_fonts.h"
 
 #include "skia/ext/fontmgr_default.h"
 #include "third_party/skia/include/core/SkFontMgr.h"
@@ -27,14 +27,9 @@ const char kFontDirectory[] = DEVICE_SOURCE_ROOT_DIR "fonts/";
 
 }  // namespace
 
-namespace content {
+namespace skia {
 
-bool CheckLayoutSystemDeps() {
-  return true;
-}
-
-bool BlinkTestPlatformInitialize() {
-  // Initialize Skia with the font configuration files crafted for web tests.
+void ConfigureTestFont() {
   SkFontMgr_Android_CustomFonts custom;
   custom.fSystemFontUse = SkFontMgr_Android_CustomFonts::kOnlyCustom;
   custom.fBasePath = kFontDirectory;
@@ -43,7 +38,6 @@ bool BlinkTestPlatformInitialize() {
   custom.fIsolated = false;
 
   skia::OverrideDefaultSkFontMgr(SkFontMgr_New_Android(&custom));
-  return true;
 }
 
-}  // namespace content
+}  // namespace skia

content/shell/app/blink_test_platform_support_fuchsia.cc → skia/ext/test_fonts_fuchsia.cc

-// Copyright 2017 The Chromium Authors. All rights reserved.
+// Copyright 2019 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 "content/shell/app/blink_test_platform_support.h"
+#include "skia/ext/test_fonts.h"
 
 #include "skia/ext/fontmgr_default.h"
 #include "third_party/skia/include/core/SkFontMgr.h"
 #include "third_party/skia/include/ports/SkFontMgr_android.h"
 
-namespace content {
+namespace skia {
 
-bool CheckLayoutSystemDeps() {
-  return true;
-}
-
-bool BlinkTestPlatformInitialize() {
-  // Initialize Skia with the font configuration files crafted for web tests.
+void ConfigureTestFont() {
+  // TODO(https://crbugs.com/927980): Use SkFontMgr_Fuchsia instead of
+  // SkFontMgr_Android. We just need to start a fonts::Provider instance with
+  // a custom manifest files and connect to it instead of the default
+  // fonts::Provider instance..
   SkFontMgr_Android_CustomFonts custom;
   custom.fSystemFontUse = SkFontMgr_Android_CustomFonts::kOnlyCustom;
   custom.fBasePath = "/pkg/test_fonts/";
@@ -24,8 +23,6 @@ bool BlinkTestPlatformInitialize() {
   custom.fIsolated = false;
 
   skia::OverrideDefaultSkFontMgr(SkFontMgr_New_Android(&custom));
-
-  return true;
 }
 
-}  // namespace content
+}  // namespace skia

content/shell/app/blink_test_platform_support_linux.cc → skia/ext/test_fonts_linux.cc

-// Copyright 2013 The Chromium Authors. All rights reserved.
+// Copyright 2019 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 "content/shell/app/blink_test_platform_support.h"
+#include "skia/ext/test_fonts.h"
 
 #include "base/test/fontconfig_util_linux.h"
 
-namespace content {
+namespace skia {
 
-bool CheckLayoutSystemDeps() {
-  return true;
-}
-
-bool BlinkTestPlatformInitialize() {
+void ConfigureTestFont() {
   base::SetUpFontconfig();
-  return true;
 }
 
-}  // namespace content
+}  // namespace skia

skia/ext/test_fonts_mac.mm

+// Copyright 2019 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 "skia/ext/test_fonts.h"
+
+#include <AppKit/AppKit.h>
+#include <Foundation/Foundation.h>
+
+#include "base/logging.h"
+#include "base/mac/bundle_locations.h"
+
+namespace skia {
+
+void ConfigureTestFont() {
+  // Load font files in the resource folder.
+  static const char* const kFontFileNames[] = {"Ahem.TTF",
+                                               "ChromiumAATTest.ttf"};
+
+  // mainBundle is Content Shell Helper.app.  Go two levels up to find
+  // Content Shell.app. Due to DumpRenderTree injecting the font files into
+  // its direct dependents, it's not easily possible to put the ttf files into
+  // the helper's resource directory instead of the outer bundle's resource
+  // directory.
+  NSString* bundle = [base::mac::FrameworkBundle() bundlePath];
+  bundle = [bundle stringByAppendingPathComponent:@"../.."];
+  NSURL* resources_directory = [[NSBundle bundleWithPath:bundle] resourceURL];
+
+  NSMutableArray* font_urls = [NSMutableArray array];
+  for (unsigned i = 0; i < base::size(kFontFileNames); ++i) {
+    NSURL* font_url = [resources_directory
+        URLByAppendingPathComponent:
+            [NSString stringWithUTF8String:kFontFileNames[i]]];
+    [font_urls addObject:[font_url absoluteURL]];
+  }
+
+  CFArrayRef errors = 0;
+  if (!CTFontManagerRegisterFontsForURLs((CFArrayRef)font_urls,
+                                         kCTFontManagerScopeProcess, &errors)) {
+    DLOG(FATAL) << "Fail to activate fonts.";
+    CFRelease(errors);
+  }
+}
+
+}  // namespace skia