Fix font_service registration on Linux

r572930 landed a bunch of changes to have font_service registered on the Linux platform to support all sandboxed font config usage. There are a few problems with how this was done, namely: - It was not necessary to register the service individually with each content embedder's implementation of ContentBrowserClient. Instead registration should simply be done in content's ServiceManagerContext (which calls out to ContentBrowserClient only to support registrations content itself can't know about, as is the purpose of all Content*Client APIs). - There are quite a few superfluous DEPS and GN deps entries that were added by the CL. This CL fixes that stuff. Bug: 855021 Change-Id: Ica2b023a06763f2442c4e2dce76bf23426547e62 Reviewed-on: https://chromium-review.googlesource.com/1130466 Commit-Queue: Ken Rockot <rockot@chromium.org> Reviewed-by: Lei Zhang <thestig@chromium.org> Reviewed-by: Jochen Eisinger <jochen@chromium.org> Reviewed-by: Rahul Chaturvedi <rkc@chromium.org> Reviewed-by: Antoine Labour <piman@chromium.org> Reviewed-by: Emil A Eklund <eae@chromium.org> Reviewed-by: Luke Halliwell <halliwell@chromium.org> Cr-Commit-Position: refs/heads/master@{#573895}
hylschwy · Jul 10, 2018 · 7a295c6 · 7a295c6
1 parent 0dfa679
commit 7a295c6
Show file tree

Hide file tree

Showing 27 changed files with 41 additions and 123 deletions.
diff --git a/chrome/app/generated_resources.grd b/chrome/app/generated_resources.grd
@@ -3180,11 +3180,6 @@ are declared in tools/grit/grit_rule.gni.
       <message name="IDS_UTILITY_PROCESS_FILE_UTILITY_NAME" desc="The name of the utility process used for various Chrome specific file operations.">
           Chrome File Utilities
       </message>
-      <if expr="is_linux">
-        <message name="IDS_UTILITY_PROCESS_FONT_SERVICE_UTILITY_NAME" desc="The name of the utility process used for FontConfig operations for Chrome on Linux.">
-          Linux Font Service
-        </message>
-      </if>
       <if expr="not is_android">
         <message name="IDS_UTILITY_PROCESS_PROFILE_IMPORTER_NAME" desc="The name of the utility process used for importing profiles.">
           Profile Importer

diff --git a/chrome/browser/chrome_content_browser_client.cc b/chrome/browser/chrome_content_browser_client.cc
@@ -501,10 +501,6 @@
 #include "chrome/browser/offline_pages/offline_page_url_loader_request_interceptor.h"
 #endif
 
-#if defined(OS_LINUX)
-#include "components/services/font/public/interfaces/constants.mojom.h"
-#endif
-
 using base::FileDescriptor;
 using content::BrowserThread;
 using content::BrowserURLHandler;
@@ -3655,12 +3651,6 @@ void ChromeContentBrowserClient::RegisterOutOfProcessServices(
       &l10n_util::GetStringUTF16, IDS_UTILITY_PROCESS_FILE_UTILITY_NAME);
 #endif
 
-#if defined(OS_LINUX)
-  (*services)[font_service::mojom::kServiceName] =
-      base::BindRepeating(&l10n_util::GetStringUTF16,
-                          IDS_UTILITY_PROCESS_FONT_SERVICE_UTILITY_NAME);
-#endif
-
   (*services)[patch::mojom::kServiceName] = base::BindRepeating(
       &l10n_util::GetStringUTF16, IDS_UTILITY_PROCESS_PATCH_NAME);
 

diff --git a/chrome/utility/BUILD.gn b/chrome/utility/BUILD.gn
@@ -153,8 +153,6 @@ static_library("utility") {
       "//ash/components/tap_visualizer:lib",
       "//ash/components/tap_visualizer/public/mojom",
       "//chrome/services/file_util:lib",
-      "//components/services/font:lib",
-      "//components/services/font/public/interfaces",
       "//services/ui:lib",
       "//services/ui/public/interfaces",
     ]
@@ -199,10 +197,6 @@ static_library("utility") {
   if (is_android || enable_extensions) {
     deps += [ "//chrome/services/media_gallery_util:lib" ]
   }
-
-  if (is_linux && !is_android) {
-    deps += [ "//components/services/font:lib" ]
-  }
 }
 
 if (!is_android) {

diff --git a/chrome/utility/DEPS b/chrome/utility/DEPS
@@ -18,7 +18,6 @@ include_rules = [
   "+chrome/services/wifi_util_win/public/mojom",
   "+components/crash/core/common/crash_keys.h",
   "+components/payments/content/utility",
-  "+components/services/font/font_service_app.h",
   "+components/services/heap_profiling/heap_profiling_service.h",
   "+components/services/heap_profiling/public",
   "+components/services/patch",
@@ -53,8 +52,6 @@ specific_include_rules = {
     "+ash/components/tap_visualizer/tap_visualizer_app.h",
     "+ash/public/interfaces",
     "+ash/window_manager_service.h",
-    "+components/services/font/font_service_app.h",
-    "+components/services/font/public/interfaces",
     "+services/ui/common/image_cursors_set.h",
     "+services/ui/public",
     "+services/ui/service.h",

diff --git a/chrome/utility/mash_service_factory.cc b/chrome/utility/mash_service_factory.cc
@@ -21,8 +21,6 @@
 #include "base/message_loop/message_loop.h"
 #include "base/metrics/histogram_macros.h"
 #include "build/build_config.h"
-#include "components/services/font/font_service_app.h"
-#include "components/services/font/public/interfaces/constants.mojom.h"
 #include "ui/base/ui_base_features.h"
 
 namespace {
@@ -86,11 +84,6 @@ std::unique_ptr<service_manager::Service> CreateTapVisualizerApp() {
   return std::make_unique<tap_visualizer::TapVisualizerApp>();
 }
 
-std::unique_ptr<service_manager::Service> CreateFontService() {
-  RecordMashServiceLaunch(MashService::kFont);
-  return std::make_unique<font_service::FontServiceApp>();
-}
-
 }  // namespace
 
 MashServiceFactory::MashServiceFactory() = default;
@@ -107,8 +100,6 @@ void MashServiceFactory::RegisterOutOfProcessServices(
                       &CreateShortcutViewerApp);
   RegisterMashService(services, tap_visualizer::mojom::kServiceName,
                       &CreateTapVisualizerApp);
-  RegisterMashService(services, font_service::mojom::kServiceName,
-                      &CreateFontService);
 
   keyboard_shortcut_viewer::ShortcutViewerApplication::RegisterForTraceEvents();
 }
diff --git a/chromecast/browser/BUILD.gn b/chromecast/browser/BUILD.gn
@@ -201,7 +201,6 @@ cast_source_set("browser") {
 
     deps += [
       "//components/metrics:serialization",
-      "//components/services/font:lib",
       "//third_party/fontconfig",
     ]
   }

diff --git a/chromecast/browser/DEPS b/chromecast/browser/DEPS
@@ -16,7 +16,6 @@ include_rules = [
   "+components/prefs",
   "+components/pref_registry",
   "+components/proxy_config",
-  "+components/services/font",
   "+components/storage_monitor",
   "+components/user_prefs",
   "+components/version_info",

diff --git a/chromecast/browser/cast_content_browser_client.cc b/chromecast/browser/cast_content_browser_client.cc
@@ -76,11 +76,6 @@
 #include "media/mojo/services/media_service.h"      // nogncheck
 #endif  // ENABLE_MOJO_MEDIA_IN_BROWSER_PROCESS
 
-#if defined(OS_LINUX)
-#include "components/services/font/font_service_app.h"  // nogncheck
-#include "components/services/font/public/interfaces/constants.mojom.h"  // nogncheck
-#endif
-
 #if defined(OS_LINUX) || defined(OS_ANDROID)
 #include "components/crash/content/app/breakpad_linux.h"
 #include "components/crash/content/browser/crash_handler_host_linux.h"
@@ -688,14 +683,6 @@ void CastContentBrowserClient::RegisterInProcessServices(
 #endif
 }
 
-void CastContentBrowserClient::RegisterOutOfProcessServices(
-    OutOfProcessServiceMap* services) {
-#if defined(OS_LINUX)
-  (*services)[font_service::mojom::kServiceName] =
-      base::BindRepeating(&base::ASCIIToUTF16, "Font Service");
-#endif
-}
-
 std::unique_ptr<base::Value>
 CastContentBrowserClient::GetServiceManifestOverlay(
     base::StringPiece service_name) {

diff --git a/chromecast/browser/cast_content_browser_client.h b/chromecast/browser/cast_content_browser_client.h
@@ -183,7 +183,6 @@ class CastContentBrowserClient : public content::ContentBrowserClient {
   void RegisterInProcessServices(
       StaticServiceMap* services,
       content::ServiceManagerConnection* connection) override;
-  void RegisterOutOfProcessServices(OutOfProcessServiceMap* services) override;
   std::unique_ptr<base::Value> GetServiceManifestOverlay(
       base::StringPiece service_name) override;
   void GetAdditionalMappedFilesForChildProcess(

diff --git a/content/DEPS b/content/DEPS
@@ -24,7 +24,7 @@ include_rules = [
   # settings, packaging details, installation or crash reporting.
 
   "+components/services/filesystem",
-  "+components/services/font",
+  "+components/services/font/public",
 
   "+crypto",
   "+grit/blink_resources.h",

diff --git a/content/app/strings/content_strings.grd b/content/app/strings/content_strings.grd
@@ -792,6 +792,12 @@ below:
         Year
       </message>
 
+      <if expr="is_linux">
+        <message name="IDS_FONT_SERVICE_PROCESS_TITLE" desc="The user-visible process title displayed for the font_service process used by Chrome on Linux.">
+          Linux Font Service
+        </message>
+      </if>
+
       <message name="IDS_FORM_INPUT_WEEK_TEMPLATE" desc="A specific week (1-53) in a specific year shown in a form control">
         Week <ph name="WEEKNUMBER">$2<ex>51</ex></ph>, <ph name="YEAR">$1<ex>2012</ex></ph>
       </message>

diff --git a/content/browser/BUILD.gn b/content/browser/BUILD.gn
@@ -1795,7 +1795,10 @@ jumbo_source_set("browser") {
   }
 
   if (is_linux) {
-    deps += [ "//services/service_manager/zygote" ]
+    deps += [
+      "//components/services/font/public/interfaces",
+      "//services/service_manager/zygote",
+    ]
   }
 
   # ChromeOS also defines linux but their memory-monitors conflict.

diff --git a/content/browser/DEPS b/content/browser/DEPS
@@ -8,6 +8,7 @@ include_rules = [
   "+components/download/quarantine/quarantine.h",
   "+components/filename_generation",
   "+components/services/filesystem",
+  "+components/services/font/ppapi_fontconfig_matching.h",
   "+components/services/leveldb",
   "+components/link_header_util",
   "+components/metrics",

diff --git a/content/browser/service_manager/service_manager_context.cc b/content/browser/service_manager/service_manager_context.cc
@@ -25,6 +25,7 @@
 #include "base/strings/utf_string_conversions.h"
 #include "base/task_scheduler/post_task.h"
 #include "build/build_config.h"
+#include "content/app/strings/grit/content_strings.h"
 #include "content/browser/browser_main_loop.h"
 #include "content/browser/child_process_launcher.h"
 #include "content/browser/gpu/gpu_process_host.h"
@@ -94,6 +95,10 @@
 #include "ui/aura/env.h"
 #endif
 
+#if defined(OS_LINUX)
+#include "components/services/font/public/interfaces/constants.mojom.h"
+#endif
+
 namespace content {
 
 namespace {
@@ -603,6 +608,19 @@ ServiceManagerContext::ServiceManagerContext(
   out_of_process_services[data_decoder::mojom::kServiceName] =
       base::BindRepeating(&base::ASCIIToUTF16, "Data Decoder Service");
 
+#if defined(OS_LINUX)
+  out_of_process_services[font_service::mojom::kServiceName] =
+      base::BindRepeating([] {
+        auto title = GetContentClient()->GetLocalizedString(
+            IDS_FONT_SERVICE_PROCESS_TITLE);
+        if (!title.empty())
+          return title;
+
+        // Default to a non-localized string if we have to.
+        return base::ASCIIToUTF16("Linux Font Service");
+      });
+#endif
+
   bool network_service_enabled =
       base::FeatureList::IsEnabled(network::features::kNetworkService);
   bool network_service_in_process =

diff --git a/content/child/BUILD.gn b/content/child/BUILD.gn
@@ -140,19 +140,16 @@ target(link_target_type, "child") {
     ]
   }
 
-  if (is_linux && !is_android) {
-    deps += [
-      "//components/services/font:lib",
-      "//components/services/font/public/cpp",
-    ]
-  }
-
   if (is_android) {
     deps += [ "//third_party/android_tools:cpu_features" ]
   }
 
   if (is_linux) {
-    deps += [ "//services/service_manager/zygote" ]
+    deps += [
+      "//components/services/font/public/cpp",
+      "//components/services/font/public/interfaces",
+      "//services/service_manager/zygote",
+    ]
   }
 
   if (is_win) {

diff --git a/content/child/DEPS b/content/child/DEPS
@@ -4,7 +4,7 @@ include_rules = [
   "+components/tracing",
   "+components/variations/child_process_field_trial_syncer.h",
   "+components/webcrypto",
-  "+components/services/font",
+  "+components/services/font/public",
 
   "+content/app/strings/grit",  # For generated headers
   "+content/public/child",

diff --git a/content/renderer/BUILD.gn b/content/renderer/BUILD.gn
@@ -818,7 +818,10 @@ target(link_target_type, "renderer") {
   }
 
   if (is_linux) {
-    deps += [ "//services/service_manager/zygote" ]
+    deps += [
+      "//components/services/font/public/cpp",
+      "//services/service_manager/zygote",
+    ]
   }
 
   if (is_fuchsia) {
@@ -1023,16 +1026,6 @@ target(link_target_type, "renderer") {
     deps += [ "//sandbox:sandbox_buildflags" ]
   }
 
-  if (is_linux && !is_android) {
-    deps += [
-      "//components/services/font:lib",
-      "//components/services/font/public/cpp",
-    ]
-    data_deps = [
-      "//components/services/font:font_service",
-    ]
-  }
-
   if (use_ozone) {
     deps += [ "//ui/ozone" ]
   }

diff --git a/content/shell/BUILD.gn b/content/shell/BUILD.gn
@@ -352,10 +352,6 @@ static_library("content_shell_lib") {
     ]
   }
 
-  if (is_linux && !is_android) {
-    deps += [ "//components/services/font:lib" ]
-  }
-
   if (use_x11) {
     # Some tests rely on this tool at runtime. Note: it might be better if
     # the tests that needed it had this as a dep instead of adding it here.
@@ -897,10 +893,6 @@ mojom("mojo_bindings") {
 service_manifest("content_shell_packaged_services_manifest_overlay") {
   source = "//content/shell/browser/content_shell_packaged_services_manifest_overlay.json"
   packaged_services = [ "//services/test/echo:manifest" ]
-
-  if (is_linux && !is_android) {
-    packaged_services += [ "//components/services/font:manifest" ]
-  }
 }
 
 group("content_shell_crash_test") {

diff --git a/content/shell/browser/shell_content_browser_client.cc b/content/shell/browser/shell_content_browser_client.cc
@@ -65,11 +65,6 @@
 #include "content/public/common/content_descriptors.h"
 #endif
 
-#if defined(OS_LINUX)
-#include "components/services/font/font_service_app.h"  // nogncheck
-#include "components/services/font/public/interfaces/constants.mojom.h"  // nogncheck
-#endif
-
 #if defined(OS_WIN)
 #include "sandbox/win/src/sandbox.h"
 #include "services/service_manager/sandbox/win/sandbox_win.h"
@@ -232,10 +227,6 @@ void ShellContentBrowserClient::RegisterOutOfProcessServices(
       base::BindRepeating(&base::ASCIIToUTF16, "Test Service");
   (*services)[echo::mojom::kServiceName] =
       base::BindRepeating(&base::ASCIIToUTF16, "Echo Service");
-#if defined(OS_LINUX)
-  (*services)[font_service::mojom::kServiceName] =
-      base::BindRepeating(&base::ASCIIToUTF16, "Font Service");
-#endif
 }
 
 bool ShellContentBrowserClient::ShouldTerminateOnServiceQuit(

diff --git a/content/utility/DEPS b/content/utility/DEPS
@@ -1,4 +1,5 @@
 include_rules = [
+  "+components/services/font",
   "+content/child",
   "+content/public/utility",
   "+services/audio",

diff --git a/extensions/shell/BUILD.gn b/extensions/shell/BUILD.gn
@@ -222,10 +222,6 @@ source_set("app_shell_lib") {
     ]
   }
 
-  if (is_linux) {
-    deps += [ "//components/services/font/public/cpp" ]
-  }
-
   if (is_chromeos) {
     sources += [
       "browser/api/vpn_provider/vpn_service_factory.cc",

diff --git a/extensions/shell/browser/DEPS b/extensions/shell/browser/DEPS
@@ -8,7 +8,6 @@ include_rules = [
   "+components/network_session_configurator/common",
   "+components/pref_registry",
   "+components/sessions",
-  "+components/services/font",
   "+components/update_client",
   "+components/user_prefs",
   "+components/web_modal",