Servicifying PdfToEmfConverter.

Moving the PdfToEmfConverter interface to the printing service.
As a result, simplified the client of that interface in
chrome/browser/printing/pdf_to_emf_converter.cc:
- merging PdfConverterUtilityProcessHostClient and PdfConverterImpl.
- calls on the service can now happen on the UI thread instead of the
  IO thread

Renamed the mojom::PdfToEmfConverter implementation from
PdfToEmfConverterImpl to PdfToEmfConverter as is now preferred naming
style for Mojo interface implementation.

Bug: 766451
Change-Id: Ic748547f56bb193a558210f97558640ef8d6e44b
Reviewed-on: https://chromium-review.googlesource.com/826308Reviewed-by: Will Harris <wfh@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Jay Civelli <jcivelli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524534}

chrome/browser/BUILD.gn

@@ -3271,6 +3271,7 @@ split_static_library("browser") {
         "printing/pdf_to_emf_converter.cc",
         "printing/pdf_to_emf_converter.h",
       ]
+      deps += [ "//chrome/services/printing/public/interfaces" ]
     }
     if (enable_print_preview) {
       # Full printing on top of the above.

chrome/browser/printing/pdf_to_emf_converter.h

@@ -7,7 +7,7 @@
 
 #include <memory>
 
-#include "base/callback.h"
+#include "base/callback_forward.h"
 #include "base/memory/ref_counted_memory.h"
 
 namespace printing {

chrome/browser/printing/print_job.cc

@@ -275,7 +275,7 @@ void PrintJob::StartPdfToEmfConversion(
       print_text_with_gdi ? PdfRenderSettings::Mode::GDI_TEXT
                           : PdfRenderSettings::Mode::NORMAL);
   pdf_conversion_state_->Start(
-      bytes, settings, base::Bind(&PrintJob::OnPdfConversionStarted, this));
+      bytes, settings, base::BindOnce(&PrintJob::OnPdfConversionStarted, this));
 }
 
 void PrintJob::OnPdfConversionStarted(int page_count) {
@@ -321,7 +321,7 @@ void PrintJob::StartPdfToTextConversion(
                              /*autorotate=*/true,
                              PdfRenderSettings::Mode::TEXTONLY);
   pdf_conversion_state_->Start(
-      bytes, settings, base::Bind(&PrintJob::OnPdfConversionStarted, this));
+      bytes, settings, base::BindOnce(&PrintJob::OnPdfConversionStarted, this));
 }
 
 void PrintJob::StartPdfToPostScriptConversion(
@@ -338,7 +338,7 @@ void PrintJob::StartPdfToPostScriptConversion(
       ps_level2 ? PdfRenderSettings::Mode::POSTSCRIPT_LEVEL2
                 : PdfRenderSettings::Mode::POSTSCRIPT_LEVEL3);
   pdf_conversion_state_->Start(
-      bytes, settings, base::Bind(&PrintJob::OnPdfConversionStarted, this));
+      bytes, settings, base::BindOnce(&PrintJob::OnPdfConversionStarted, this));
 }
 #endif  // defined(OS_WIN)
 

chrome/common/BUILD.gn

@@ -213,7 +213,6 @@ static_library("common") {
     "//chrome/app/theme:theme_resources",
     "//chrome/common:constants",
     "//chrome/common/net",
-    "//chrome/common/printing:interfaces",
     "//chrome/common/profiling",
     "//chrome/installer/util:with_no_strings",
     "//components/cast_certificate",

chrome/common/printing/BUILD.gn

-# Copyright 2017 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.
-
-import("//mojo/public/tools/bindings/mojom.gni")
-
-mojom("interfaces") {
-  sources = [
-    "pdf_to_emf_converter.mojom",
-  ]
-
-  deps = [
-    "//chrome/services/printing/public/interfaces",
-    "//mojo/common:common_custom_types",
-    "//ui/gfx/geometry/mojo",
-  ]
-}

chrome/common/printing/OWNERS

-per-file *.mojom=set noparent
-per-file *.mojom=file://ipc/SECURITY_OWNERS

chrome/services/printing/BUILD.gn

@@ -22,6 +22,17 @@ source_set("lib") {
     "//chrome/services/printing/public/interfaces",
     "//services/service_manager/public/cpp",
   ]
+
+  if (is_win) {
+    sources += [
+      "pdf_to_emf_converter.cc",
+      "pdf_to_emf_converter.h",
+      "pdf_to_emf_converter_factory.cc",
+      "pdf_to_emf_converter_factory.h",
+    ]
+
+    deps += [ "//skia" ]
+  }
 }
 
 service_manifest("manifest") {

chrome/services/printing/manifest.json

 {
   "name": "chrome_printing",
   "display_name": "Printing",
+  "sandbox_type": "utility",
   "interface_provider_specs": {
     "service_manager:connector": {
       "provides": {
-        "converter": [ "printing::mojom::PdfToPwgRasterConverter" ]
+        "converter": [
+            "printing::mojom::PdfToEmfConverterFactory",
+            "printing::mojom::PdfToPwgRasterConverter" ]
        },
        "requires": {
          "service_manager": [ "service_manager:all_users" ]

chrome/utility/printing/pdf_to_emf_converter_impl.cc → chrome/services/printing/pdf_to_emf_converter.cc

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chrome/utility/printing/pdf_to_emf_converter_impl.h"
+#include "chrome/services/printing/pdf_to_emf_converter.h"
 
 #include <algorithm>
 
@@ -61,7 +61,7 @@ void RegisterConverterClient(mojom::PdfToEmfConverterClientPtr client) {
 
 }  // namespace
 
-PdfToEmfConverterImpl::PdfToEmfConverterImpl(
+PdfToEmfConverter::PdfToEmfConverter(
     mojo::ScopedHandle pdf_file_in,
     const printing::PdfRenderSettings& pdf_render_settings,
     mojom::PdfToEmfConverterClientPtr client)
@@ -90,15 +90,15 @@ PdfToEmfConverterImpl::PdfToEmfConverterImpl(
   base::PlatformFile pdf_file;
   if (mojo::UnwrapPlatformFile(std::move(pdf_file_in), &pdf_file) !=
       MOJO_RESULT_OK) {
-    LOG(ERROR) << "Invalid PDF file passed to PdfToEmfConverterImpl";
+    LOG(ERROR) << "Invalid PDF file passed to PdfToEmfConverter.";
     return;
   }
   LoadPdf(base::File(pdf_file));
 }
 
-PdfToEmfConverterImpl::~PdfToEmfConverterImpl() = default;
+PdfToEmfConverter::~PdfToEmfConverter() = default;
 
-void PdfToEmfConverterImpl::LoadPdf(base::File pdf_file) {
+void PdfToEmfConverter::LoadPdf(base::File pdf_file) {
   int64_t length64 = pdf_file.GetLength();
   if (length64 <= 0 || length64 > std::numeric_limits<int>::max())
     return;
@@ -114,10 +114,10 @@ void PdfToEmfConverterImpl::LoadPdf(base::File pdf_file) {
   total_page_count_ = page_count;
 }
 
-bool PdfToEmfConverterImpl::RenderPdfPageToMetafile(int page_number,
-                                                    base::File output_file,
-                                                    float* scale_factor,
-                                                    bool postscript) {
+bool PdfToEmfConverter::RenderPdfPageToMetafile(int page_number,
+                                                base::File output_file,
+                                                float* scale_factor,
+                                                bool postscript) {
   Emf metafile;
   metafile.Init();
 
@@ -160,9 +160,9 @@ bool PdfToEmfConverterImpl::RenderPdfPageToMetafile(int page_number,
   return metafile.SaveTo(&output_file);
 }
 
-void PdfToEmfConverterImpl::ConvertPage(uint32_t page_number,
-                                        mojo::ScopedHandle emf_file_out,
-                                        ConvertPageCallback callback) {
+void PdfToEmfConverter::ConvertPage(uint32_t page_number,
+                                    mojo::ScopedHandle emf_file_out,
+                                    ConvertPageCallback callback) {
   if (page_number >= total_page_count_) {
     std::move(callback).Run(/*success=*/false, /*scale_factor=*/0.0);
     return;

chrome/utility/printing/pdf_to_emf_converter_impl.h → chrome/services/printing/pdf_to_emf_converter.h

@@ -2,24 +2,24 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef CHROME_UTILITY_PRINTING_PDF_TO_EMF_CONVERTER_IMPL_H_
-#define CHROME_UTILITY_PRINTING_PDF_TO_EMF_CONVERTER_IMPL_H_
+#ifndef CHROME_SERVICES_PRINTING_PDF_TO_EMF_CONVERTER_H_
+#define CHROME_SERVICES_PRINTING_PDF_TO_EMF_CONVERTER_H_
 
 #include <vector>
 
 #include "base/files/file.h"
 #include "base/macros.h"
-#include "chrome/common/printing/pdf_to_emf_converter.mojom.h"
+#include "chrome/services/printing/public/interfaces/pdf_to_emf_converter.mojom.h"
 #include "printing/pdf_render_settings.h"
 
 namespace printing {
 
-class PdfToEmfConverterImpl : public mojom::PdfToEmfConverter {
+class PdfToEmfConverter : public mojom::PdfToEmfConverter {
  public:
-  PdfToEmfConverterImpl(mojo::ScopedHandle pdf_file_in,
-                        const PdfRenderSettings& render_settings,
-                        mojom::PdfToEmfConverterClientPtr client);
-  ~PdfToEmfConverterImpl() override;
+  PdfToEmfConverter(mojo::ScopedHandle pdf_file_in,
+                    const PdfRenderSettings& render_settings,
+                    mojom::PdfToEmfConverterClientPtr client);
+  ~PdfToEmfConverter() override;
 
   int total_page_count() const { return total_page_count_; }
 
@@ -39,9 +39,9 @@ class PdfToEmfConverterImpl : public mojom::PdfToEmfConverter {
   PdfRenderSettings pdf_render_settings_;
   std::vector<char> pdf_data_;
 
-  DISALLOW_COPY_AND_ASSIGN(PdfToEmfConverterImpl);
+  DISALLOW_COPY_AND_ASSIGN(PdfToEmfConverter);
 };
 
 }  // namespace printing
 
-#endif  // CHROME_UTILITY_PRINTING_PDF_TO_EMF_CONVERTER_IMPL_H_
+#endif  // CHROME_SERVICES_PRINTING_PDF_TO_EMF_CONVERTER_H_

chrome/utility/printing/pdf_to_emf_converter_factory_impl.cc → chrome/services/printing/pdf_to_emf_converter_factory.cc

@@ -2,24 +2,26 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chrome/utility/printing/pdf_to_emf_converter_factory_impl.h"
+#include "chrome/services/printing/pdf_to_emf_converter_factory.h"
 
-#include "chrome/utility/printing/pdf_to_emf_converter_impl.h"
+#include "chrome/services/printing/pdf_to_emf_converter.h"
 #include "mojo/public/cpp/bindings/strong_binding.h"
 #include "mojo/public/cpp/system/platform_handle.h"
 
 namespace printing {
 
-PdfToEmfConverterFactoryImpl::PdfToEmfConverterFactoryImpl() = default;
+PdfToEmfConverterFactory::PdfToEmfConverterFactory(
+    std::unique_ptr<service_manager::ServiceContextRef> service_ref)
+    : service_ref_(std::move(service_ref)) {}
 
-PdfToEmfConverterFactoryImpl::~PdfToEmfConverterFactoryImpl() = default;
+PdfToEmfConverterFactory::~PdfToEmfConverterFactory() = default;
 
-void PdfToEmfConverterFactoryImpl::CreateConverter(
+void PdfToEmfConverterFactory::CreateConverter(
     mojo::ScopedHandle pdf_file_in,
-    const printing::PdfRenderSettings& render_settings,
+    const PdfRenderSettings& render_settings,
     mojom::PdfToEmfConverterClientPtr client,
     CreateConverterCallback callback) {
-  auto converter = std::make_unique<PdfToEmfConverterImpl>(
+  auto converter = std::make_unique<PdfToEmfConverter>(
       std::move(pdf_file_in), render_settings, std::move(client));
   uint32_t page_count = converter->total_page_count();
   mojom::PdfToEmfConverterPtr converter_ptr;
@@ -29,11 +31,4 @@ void PdfToEmfConverterFactoryImpl::CreateConverter(
   std::move(callback).Run(std::move(converter_ptr), page_count);
 }
 
-// static
-void PdfToEmfConverterFactoryImpl::Create(
-    mojom::PdfToEmfConverterFactoryRequest request) {
-  mojo::MakeStrongBinding(base::MakeUnique<PdfToEmfConverterFactoryImpl>(),
-                          std::move(request));
-}
-
 }  // namespace printing

chrome/utility/printing/pdf_to_emf_converter_factory_impl.h → chrome/services/printing/pdf_to_emf_converter_factory.h

@@ -2,31 +2,33 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef CHROME_UTILITY_PRINTING_PDF_TO_EMF_CONVERTER_FACTORY_IMPL_H_
-#define CHROME_UTILITY_PRINTING_PDF_TO_EMF_CONVERTER_FACTORY_IMPL_H_
+#ifndef CHROME_SERVICES_PRINTING_PDF_TO_EMF_CONVERTER_FACTORY_H_
+#define CHROME_SERVICES_PRINTING_PDF_TO_EMF_CONVERTER_FACTORY_H_
 
 #include "base/macros.h"
-#include "chrome/common/printing/pdf_to_emf_converter.mojom.h"
+#include "chrome/services/printing/public/interfaces/pdf_to_emf_converter.mojom.h"
+#include "services/service_manager/public/cpp/service_context_ref.h"
 
 namespace printing {
 
-class PdfToEmfConverterFactoryImpl : public mojom::PdfToEmfConverterFactory {
+class PdfToEmfConverterFactory : public mojom::PdfToEmfConverterFactory {
  public:
-  PdfToEmfConverterFactoryImpl();
-  ~PdfToEmfConverterFactoryImpl() override;
-
-  static void Create(mojom::PdfToEmfConverterFactoryRequest request);
+  explicit PdfToEmfConverterFactory(
+      std::unique_ptr<service_manager::ServiceContextRef> service_ref);
+  ~PdfToEmfConverterFactory() override;
 
  private:
   // mojom::PdfToEmfConverterFactory implementation.
   void CreateConverter(mojo::ScopedHandle pdf_file_in,
-                       const printing::PdfRenderSettings& render_settings,
+                       const PdfRenderSettings& render_settings,
                        mojom::PdfToEmfConverterClientPtr client,
                        CreateConverterCallback callback) override;
 
-  DISALLOW_COPY_AND_ASSIGN(PdfToEmfConverterFactoryImpl);
+  const std::unique_ptr<service_manager::ServiceContextRef> service_ref_;
+
+  DISALLOW_COPY_AND_ASSIGN(PdfToEmfConverterFactory);
 };
 
 }  // namespace printing
 
-#endif  // CHROME_UTILITY_PRINTING_PDF_TO_EMF_CONVERTER_FACTORY_IMPL_H_
+#endif  // CHROME_SERVICES_PRINTING_PDF_TO_EMF_CONVERTER_FACTORY_H_

chrome/services/printing/pdf_to_pwg_raster_converter.cc

@@ -97,13 +97,13 @@ bool RenderPdfPagesToPwgRaster(base::File pdf_file,
 
 }  // namespace
 
-PdfToPwgRasterConverterImpl::PdfToPwgRasterConverterImpl(
+PdfToPwgRasterConverter::PdfToPwgRasterConverter(
     std::unique_ptr<service_manager::ServiceContextRef> service_ref)
     : service_ref_(std::move(service_ref)) {}
 
-PdfToPwgRasterConverterImpl::~PdfToPwgRasterConverterImpl() {}
+PdfToPwgRasterConverter::~PdfToPwgRasterConverter() {}
 
-void PdfToPwgRasterConverterImpl::Convert(
+void PdfToPwgRasterConverter::Convert(
     mojo::ScopedHandle pdf_file_in,
     const PdfRenderSettings& pdf_settings,
     const PwgRasterSettings& pwg_raster_settings,
@@ -112,7 +112,7 @@ void PdfToPwgRasterConverterImpl::Convert(
   base::PlatformFile pdf_file;
   if (mojo::UnwrapPlatformFile(std::move(pdf_file_in), &pdf_file) !=
       MOJO_RESULT_OK) {
-    LOG(ERROR) << "Invalid PDF file passed to PdfToPwgRasterConverterImpl";
+    LOG(ERROR) << "Invalid PDF file passed to PdfToPwgRasterConverter.";
     std::move(callback).Run(false);
     return;
   }
@@ -120,8 +120,7 @@ void PdfToPwgRasterConverterImpl::Convert(
   base::PlatformFile pwg_raster_file;
   if (mojo::UnwrapPlatformFile(std::move(pwg_raster_file_out),
                                &pwg_raster_file) != MOJO_RESULT_OK) {
-    LOG(ERROR)
-        << "Invalid PWGRaster file passed to PdfToPwgRasterConverterImpl";
+    LOG(ERROR) << "Invalid PWGRaster file passed to PdfToPwgRasterConverter.";
     std::move(callback).Run(false);
     return;
   }

chrome/services/printing/pdf_to_pwg_raster_converter.h

@@ -16,12 +16,12 @@ namespace printing {
 
 struct PdfRenderSettings;
 
-class PdfToPwgRasterConverterImpl
+class PdfToPwgRasterConverter
     : public printing::mojom::PdfToPwgRasterConverter {
  public:
-  explicit PdfToPwgRasterConverterImpl(
+  explicit PdfToPwgRasterConverter(
       std::unique_ptr<service_manager::ServiceContextRef> service_ref);
-  ~PdfToPwgRasterConverterImpl() override;
+  ~PdfToPwgRasterConverter() override;
 
  private:
   // printing::mojom::PdfToPwgRasterConverter
@@ -33,7 +33,7 @@ class PdfToPwgRasterConverterImpl
 
   const std::unique_ptr<service_manager::ServiceContextRef> service_ref_;
 
-  DISALLOW_COPY_AND_ASSIGN(PdfToPwgRasterConverterImpl);
+  DISALLOW_COPY_AND_ASSIGN(PdfToPwgRasterConverter);
 };
 
 }  // namespace printing

chrome/services/printing/printing_service.cc

@@ -8,17 +8,31 @@
 #include "chrome/services/printing/pdf_to_pwg_raster_converter.h"
 #include "mojo/public/cpp/bindings/strong_binding.h"
 
+#if defined(OS_WIN)
+#include "chrome/services/printing/pdf_to_emf_converter.h"
+#include "chrome/services/printing/pdf_to_emf_converter_factory.h"
+#endif
+
 namespace printing {
 
 namespace {
 
+#if defined(OS_WIN)
+void OnPdfToEmfConverterFactoryRequest(
+    service_manager::ServiceContextRefFactory* ref_factory,
+    printing::mojom::PdfToEmfConverterFactoryRequest request) {
+  mojo::MakeStrongBinding(std::make_unique<printing::PdfToEmfConverterFactory>(
+                              ref_factory->CreateRef()),
+                          std::move(request));
+}
+#endif
+
 void OnPdfToPwgRasterConverterRequest(
     service_manager::ServiceContextRefFactory* ref_factory,
     printing::mojom::PdfToPwgRasterConverterRequest request) {
-  mojo::MakeStrongBinding(
-      std::make_unique<printing::PdfToPwgRasterConverterImpl>(
-          ref_factory->CreateRef()),
-      std::move(request));
+  mojo::MakeStrongBinding(std::make_unique<printing::PdfToPwgRasterConverter>(
+                              ref_factory->CreateRef()),
+                          std::move(request));
 }
 
 }  // namespace
@@ -35,6 +49,10 @@ void PrintingService::OnStart() {
   ref_factory_ = std::make_unique<service_manager::ServiceContextRefFactory>(
       base::Bind(&service_manager::ServiceContext::RequestQuit,
                  base::Unretained(context())));
+#if defined(OS_WIN)
+  registry_.AddInterface(
+      base::Bind(&OnPdfToEmfConverterFactoryRequest, ref_factory_.get()));
+#endif
   registry_.AddInterface(
       base::Bind(&OnPdfToPwgRasterConverterRequest, ref_factory_.get()));
 }

chrome/services/printing/public/interfaces/BUILD.gn

@@ -11,6 +11,10 @@ mojom("interfaces") {
     "pdf_to_pwg_raster_converter.mojom",
   ]
 
+  if (is_win) {
+    sources += [ "pdf_to_emf_converter.mojom" ]
+  }
+
   deps = [
     "//mojo/common:common_custom_types",
     "//ui/gfx/geometry/mojo",

chrome/utility/BUILD.gn

@@ -184,15 +184,6 @@ static_library("utility") {
       "printing_handler.cc",
       "printing_handler.h",
     ]
-    deps += [ "//pdf" ]
-    if (is_win) {
-      sources += [
-        "printing/pdf_to_emf_converter_factory_impl.cc",
-        "printing/pdf_to_emf_converter_factory_impl.h",
-        "printing/pdf_to_emf_converter_impl.cc",
-        "printing/pdf_to_emf_converter_impl.h",
-      ]
-    }
   }
 
   if (enable_basic_printing || enable_print_preview) {

chrome/utility/chrome_content_utility_client.cc

@@ -46,7 +46,6 @@
 #if defined(OS_WIN)
 #include "chrome/services/util_win/public/interfaces/constants.mojom.h"
 #include "chrome/services/util_win/util_win_service.h"
-#include "chrome/utility/printing/pdf_to_emf_converter_factory_impl.h"
 #endif
 
 #if BUILDFLAG(ENABLE_EXTENSIONS)
@@ -179,11 +178,6 @@ void ChromeContentUtilityClient::UtilityThreadStarted() {
     registry->AddInterface(base::Bind(CreateResourceUsageReporter),
                            base::ThreadTaskRunnerHandle::Get());
 #endif  // !defined(OS_ANDROID)
-#if defined(OS_WIN)
-    registry->AddInterface(
-        base::Bind(printing::PdfToEmfConverterFactoryImpl::Create),
-        base::ThreadTaskRunnerHandle::Get());
-#endif
   }
 
   connection->AddConnectionFilter(