Convert printing IPCs to Mojo

chrome/utility/printing_handler.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 "chrome/utility/printing_handler.h"
 
 #include <stdint.h>
 #include <utility>
 
 #include "base/files/file_util.h"
 #include "build/build_config.h"
 #include "chrome/common/chrome_paths.h"
-#include "chrome/common/chrome_utility_printing_messages.h"
 #include "chrome/utility/cloud_print/bitmap_image.h"
 #include "chrome/utility/cloud_print/pwg_encoder.h"
 #include "content/public/utility/utility_thread.h"
+#include "mojo/public/cpp/bindings/strong_binding.h"
+#include "mojo/public/cpp/system/platform_handle.h"
 #include "pdf/pdf.h"
 #include "printing/features/features.h"
 #include "printing/page_range.h"
 #include "printing/pdf_render_settings.h"
 
 #if defined(OS_WIN)
 #include "printing/emf_win.h"
 #include "ui/gfx/gdi_util.h"
 #endif
 
 #if BUILDFLAG(ENABLE_PRINT_PREVIEW)
 #include "chrome/common/crash_keys.h"
 #include "printing/backend/print_backend.h"
 #endif
 
 namespace printing {
 
 namespace {
 
-bool Send(IPC::Message* message) {
-  return content::UtilityThread::Get()->Send(message);
-}
-
 void ReleaseProcessIfNeeded() {
   content::UtilityThread::Get()->ReleaseProcessIfNeeded();
 }
 
+static mojom::FontPreCaching* g_font_pre_caching;
+
 #if defined(OS_WIN)
 void PreCacheFontCharacters(const LOGFONT* logfont,
                             const wchar_t* text,
                             size_t text_length) {
-  Send(new ChromeUtilityHostMsg_PreCacheFontCharacters(
-      *logfont, base::string16(text, text_length)));
+  if (g_font_pre_caching) {
+    g_font_pre_caching->PreCacheFontCharacters(
+        *logfont, base::string16(text, text_length));
+  }
 }
 #endif
 
 }  // namespace
 
-PrintingHandler::PrintingHandler() {
+PrintingHandler::PrintingHandler(mojom::FontPreCachingPtr font_pre_caching)
+    : font_pre_caching_(std::move(font_pre_caching)) {
+  g_font_pre_caching = font_pre_caching_.get();
 #if defined(OS_WIN)
   chrome_pdf::SetPDFEnsureTypefaceCharactersAccessible(PreCacheFontCharacters);
 #endif
 }
 
-PrintingHandler::~PrintingHandler() {}
+PrintingHandler::~PrintingHandler() {
+  g_font_pre_caching = nullptr;

[groby-ooo-7-16, 2016/12/14 17:28:29]

I have no idea what the threading implications around the font precaching being
called from pdfium - just to be sure, I'd suggest calling
chrome_pdf::SetPDFEnsureTypefaceCharactersAccessible(nullptr) first. 

(It likely should never be called once this DTOR is hit - if that's true, then
we should DCHECK in PreCacheFontCharacters instead of using the if())

[tibell, 2016/12/14 23:12:19]

Addressed the first paragraph.

I don't know the code well enough (i.e. at all :)) to address the second
paragraph. Could we punt on adding the DCHECK to someone who understands this
code/chrome_pdf better? That would mean that we're keeping whatever the current
behavior is.

+}
 
 bool PrintingHandler::OnMessageReceived(const IPC::Message& message) {

[groby-ooo-7-16, 2016/12/14 17:28:29]

Do we still need this? Technically, this is not a message handler any more, is
it?

[tibell, 2016/12/14 23:12:19]

Done.

-  bool handled = true;
-  IPC_BEGIN_MESSAGE_MAP(PrintingHandler, message)
+  return false;
+}
+
+void PrintingHandler::PrintingHandlerFactory::MakePrinting(
+    mojom::PrintingRequest request,
+    mojom::FontPreCachingPtr font_pre_caching) {
+  auto impl = base::MakeUnique<PrintingHandler>(std::move(font_pre_caching));
+  base::Closure error_handler = base::Bind(&PrintingHandler::OnConnectionClosed,
+                                           base::Unretained(impl.get()));
+  auto binding = mojo::MakeStrongBinding(std::move(impl), std::move(request));
+  binding->set_connection_error_handler(error_handler);
+}
+
+// static
+void PrintingHandler::Create(mojom::PrintingFactoryRequest request) {
+  mojo::MakeStrongBinding(base::MakeUnique<PrintingHandlerFactory>(),
+                          std::move(request));
+}
+
+void PrintingHandler::RenderPDFPagesToMetafiles(

[groby-ooo-7-16, 2016/12/14 17:28:29]

This seems kind of odd from a non-mojo-educated point of view.

We used to have a fairly strict stance from security folk that if a message
doesn't do anything for a given platform, it shouldn't have a handler in the
first place. (Hence the #ifdef OS_WIN)

Have we changed that stance with mojo?

[tibell, 2016/12/14 23:12:19]

I will bring this up during the Mojo meeting today to see if we can define a
policy.

In the mean time I've changed the NOTREACHED macros to mojo::ReportBadMessage,
which will kill this process and report the error to the caller if a bad message
is received.

Somewhat more philosophically:

Mojo doesn't support conditionally defining methods and I don't think we want it
to (at least not using the C preprocessor). We're trying to move to a service
architecture with coherent services and interfaces and conditional interfaces
can be confusing to use and it's hard to know what it means if the client and
service were compiled on different platforms.

It is possible to conditionally define interfaces (using build system ifs on the
mojom targets) so it is possible to split out OS-specific messages to a
different interface, but see previous paragraph.

[tibell, 2016/12/21 03:52:31]

We've now discussed this
(https://groups.google.com/a/chromium.org/forum/#!topic/chromium-mojo/BfvXCPBpcUs).
We will add technical support for preprocessing mojoms. Until that has been
implemented I will add a TODO and keep this as is (I took this pattern from
existing platform-specific Mojo code). 

+    base::File pdf_file,
+    const PdfRenderSettings& settings,
+    bool print_text_with_gdi,
+    const RenderPDFPagesToMetafilesCallback& callback) {
 #if defined(OS_WIN)
-    IPC_MESSAGE_HANDLER(ChromeUtilityMsg_RenderPDFPagesToMetafiles,
-                        OnRenderPDFPagesToMetafile)
-    IPC_MESSAGE_HANDLER(ChromeUtilityMsg_RenderPDFPagesToMetafiles_GetPage,
-                        OnRenderPDFPagesToMetafileGetPage)
-    IPC_MESSAGE_HANDLER(ChromeUtilityMsg_RenderPDFPagesToMetafiles_Stop,
-                        OnRenderPDFPagesToMetafileStop)
-#endif  // OS_WIN
+  pdf_rendering_settings_ = settings;
+  chrome_pdf::SetPDFUseGDIPrinting(print_text_with_gdi);
+  int page_count = LoadPDF(std::move(pdf_file));
+  callback.Run(page_count);
+#else
+  NOTREACHED();
+#endif
+}
+
+void PrintingHandler::RenderPDFPagesToMetafilesGetPage(
+    int page_number,
+    base::File emf_file,
+    const RenderPDFPagesToMetafilesGetPageCallback& callback) {
+#if defined(OS_WIN)
+  float scale_factor = 1.0f;
+  bool success =
+      RenderPdfPageToMetafile(page_number, std::move(emf_file), &scale_factor);
+  callback.Run(success, scale_factor);
+#else
+  NOTREACHED();
+#endif
+}
+
+void PrintingHandler::OnConnectionClosed() {
+#if defined(OS_WIN)
+  ReleaseProcessIfNeeded();
+#else
+  NOTREACHED();
+#endif
+}
+
+void PrintingHandler::RenderPDFPagesToPWGRaster(
+    base::File pdf,
+    const PdfRenderSettings& settings,
+    const PwgRasterSettings& bitmap_settings,
+    base::File bitmap,
+    const RenderPDFPagesToPWGRasterCallback& callback) {
 #if BUILDFLAG(ENABLE_PRINT_PREVIEW)
-    IPC_MESSAGE_HANDLER(ChromeUtilityMsg_RenderPDFPagesToPWGRaster,
-                        OnRenderPDFPagesToPWGRaster)
-    IPC_MESSAGE_HANDLER(ChromeUtilityMsg_GetPrinterCapsAndDefaults,
-                        OnGetPrinterCapsAndDefaults)
-    IPC_MESSAGE_HANDLER(ChromeUtilityMsg_GetPrinterSemanticCapsAndDefaults,
-                        OnGetPrinterSemanticCapsAndDefaults)
+  if (RenderPDFPagesToPWGRaster(std::move(pdf), settings, bitmap_settings,
+                                std::move(bitmap))) {
+    callback.Run(true);
+  } else {
+    callback.Run(false);
+  }
+  ReleaseProcessIfNeeded();
+#else
+  NOTREACHED();
 #endif  // ENABLE_PRINT_PREVIEW
-    IPC_MESSAGE_UNHANDLED(handled = false)
-  IPC_END_MESSAGE_MAP()
-  return handled;
 }
 
 #if defined(OS_WIN)
-void PrintingHandler::OnRenderPDFPagesToMetafile(
-    IPC::PlatformFileForTransit pdf_transit,
-    const PdfRenderSettings& settings,
-    bool print_text_with_gdi) {
-  pdf_rendering_settings_ = settings;
-  chrome_pdf::SetPDFUseGDIPrinting(print_text_with_gdi);
-  base::File pdf_file = IPC::PlatformFileForTransitToFile(pdf_transit);
-  int page_count = LoadPDF(std::move(pdf_file));
-  Send(
-      new ChromeUtilityHostMsg_RenderPDFPagesToMetafiles_PageCount(page_count));
-}
-
-void PrintingHandler::OnRenderPDFPagesToMetafileGetPage(
-    int page_number,
-    IPC::PlatformFileForTransit output_file) {
-  base::File emf_file = IPC::PlatformFileForTransitToFile(output_file);
-  float scale_factor = 1.0f;
-  bool success =
-      RenderPdfPageToMetafile(page_number, std::move(emf_file), &scale_factor);
-  Send(new ChromeUtilityHostMsg_RenderPDFPagesToMetafiles_PageDone(
-      success, scale_factor));
-}
-
-void PrintingHandler::OnRenderPDFPagesToMetafileStop() {
-  ReleaseProcessIfNeeded();
-}
-
-#endif  // OS_WIN
-
-#if BUILDFLAG(ENABLE_PRINT_PREVIEW)
-void PrintingHandler::OnRenderPDFPagesToPWGRaster(
-    IPC::PlatformFileForTransit pdf_transit,
-    const PdfRenderSettings& settings,
-    const PwgRasterSettings& bitmap_settings,
-    IPC::PlatformFileForTransit bitmap_transit) {
-  base::File pdf = IPC::PlatformFileForTransitToFile(pdf_transit);
-  base::File bitmap = IPC::PlatformFileForTransitToFile(bitmap_transit);
-  if (RenderPDFPagesToPWGRaster(std::move(pdf), settings, bitmap_settings,
-                                std::move(bitmap))) {
-    Send(new ChromeUtilityHostMsg_RenderPDFPagesToPWGRaster_Succeeded());
-  } else {
-    Send(new ChromeUtilityHostMsg_RenderPDFPagesToPWGRaster_Failed());
-  }
-  ReleaseProcessIfNeeded();
-}
-#endif  // ENABLE_PRINT_PREVIEW
-
-#if defined(OS_WIN)
 int PrintingHandler::LoadPDF(base::File pdf_file) {
   int64_t length64 = pdf_file.GetLength();
   if (length64 <= 0 || length64 > std::numeric_limits<int>::max())
     return 0;
   int length = static_cast<int>(length64);
 
   pdf_data_.resize(length);
   if (length != pdf_file.Read(0, pdf_data_.data(), pdf_data_.size()))
     return 0;
 
@@ skipping 117 matching lines @@
     std::string pwg_page;
     if (!encoder.EncodePage(image, header_info, &pwg_page))
       return false;
     bytes_written = bitmap_file.WriteAtCurrentPos(pwg_page.data(),
                                                   pwg_page.size());
     if (bytes_written != static_cast<int>(pwg_page.size()))
       return false;
   }
   return true;
 }
+#endif  // ENABLE_PRINT_PREVIEW
 
-void PrintingHandler::OnGetPrinterCapsAndDefaults(
-    const std::string& printer_name) {
+void PrintingHandler::GetPrinterCapsAndDefaults(
+    const std::string& printer_name,
+    const GetPrinterCapsAndDefaultsCallback& callback) {
+#if BUILDFLAG(ENABLE_PRINT_PREVIEW)
   scoped_refptr<PrintBackend> print_backend =
       PrintBackend::CreateInstance(nullptr);
   PrinterCapsAndDefaults printer_info;
 
   crash_keys::ScopedPrinterInfo crash_key(
       print_backend->GetPrinterDriverInfo(printer_name));
 
   if (print_backend->GetPrinterCapsAndDefaults(printer_name, &printer_info)) {
-    Send(new ChromeUtilityHostMsg_GetPrinterCapsAndDefaults_Succeeded(
-        printer_name, printer_info));
+    callback.Run(printer_info);
   } else  {
-    Send(new ChromeUtilityHostMsg_GetPrinterCapsAndDefaults_Failed(
-        printer_name));
+    callback.Run(base::nullopt);
   }
   ReleaseProcessIfNeeded();
+#else
+  NOTREACHED();
+#endif  // ENABLE_PRINT_PREVIEW
 }
 
-void PrintingHandler::OnGetPrinterSemanticCapsAndDefaults(
-    const std::string& printer_name) {
+void PrintingHandler::GetPrinterSemanticCapsAndDefaults(

[groby-ooo-7-16, 2016/12/14 17:28:29]

Same comment as above - I'd be much happier if the print preview used a separate
mojo interface that simply isn't present if print preview doesn't exist.

[tibell, 2016/12/14 23:12:19]

If print preview does make sense as a separate interface (regardless of
OS-specific concerns) we should probably split it out. Would such an interface
make sense even if the rest of the printing functionality wasn't available?

+    const std::string& printer_name,
+    const GetPrinterSemanticCapsAndDefaultsCallback& callback) {
+#if BUILDFLAG(ENABLE_PRINT_PREVIEW)
   scoped_refptr<PrintBackend> print_backend =
       PrintBackend::CreateInstance(nullptr);
   PrinterSemanticCapsAndDefaults printer_info;
 
   crash_keys::ScopedPrinterInfo crash_key(
       print_backend->GetPrinterDriverInfo(printer_name));
 
   if (print_backend->GetPrinterSemanticCapsAndDefaults(printer_name,
                                                        &printer_info)) {
-    Send(new ChromeUtilityHostMsg_GetPrinterSemanticCapsAndDefaults_Succeeded(
-        printer_name, printer_info));
+    callback.Run(printer_info);
   } else {
-    Send(new ChromeUtilityHostMsg_GetPrinterSemanticCapsAndDefaults_Failed(
-        printer_name));
+    callback.Run(base::nullopt);
   }
   ReleaseProcessIfNeeded();
+#else
+  NOTREACHED();
+#endif  // ENABLE_PRINT_PREVIEW
 }
-#endif  // ENABLE_PRINT_PREVIEW
 
 }  // namespace printing