Revert of printing::Metafile: Simplify OS_MACOSX-specific code path (patchset...

Revert of printing::Metafile:  Simplify OS_MACOSX-specific code path (patchset #3 id:40001 of https://codereview.chromium.org/2802093006/ )

Reason for revert:
Likely causing https://crbug.com/712309

Original issue's description:
> printing::Metafile:  Simplify OS_MACOSX-specific code path
>
>   - printing::Metafile
>       * move MacRenderPageParams to PdfMetafileCg
>       * remove virtual RenderPage()
>   - printing::PdfMetafileCg
>       * Add MacRenderPageParams
>       * RenderPage no longer an override
>   - printing::PdfMetafileSkia
>       * no longer implment RenderPage()
>       * code moved to printing::PrintedDocument::RenderPrintedPage()
>
> This will help isolate BUG=chromium:374359
>
> R=thestig@chromium.org
>
> Review-Url: https://codereview.chromium.org/2802093006
> Cr-Commit-Position: refs/heads/master@{#463828}
> Committed: https://chromium.googlesource.com/chromium/src/+/c843c89f3051e4b01cf032e52c478000f7ded0ee

TBR=halcanary@google.com
# Not skipping CQ checks because original CL landed more than 1 days ago.

Review-Url: https://codereview.chromium.org/2839323002
Cr-Commit-Position: refs/heads/master@{#467568}

printing/image_mac.cc

@@ -5,13 +5,11 @@
 #include "printing/image.h"
 
 #include <ApplicationServices/ApplicationServices.h>
-#include <CoreFoundation/CoreFoundation.h>
 #include <stddef.h>
 #include <stdint.h>
 
 #include "base/mac/scoped_cftyperef.h"
 #include "printing/metafile.h"
-#include "printing/pdf_metafile_cg_mac.h"
 #include "ui/gfx/geometry/rect.h"
 
 namespace printing {
@@ -41,13 +39,12 @@ bool Image::LoadMetafile(const Metafile& metafile) {
                             kCGImageAlphaPremultipliedLast));
   DCHECK(bitmap_context.get());
 
-  PdfMetafileCg::RenderPageParams params;
+  struct Metafile::MacRenderPageParams params;
   params.shrink_to_fit = true;
-  CGRect cg_rect = CGRectMake(0, 0, size_.width(), size_.height());
-  std::vector<char> buffer;
-  return metafile.GetDataAsVector(&buffer) &&
-         PdfMetafileCg::RenderPage(buffer, page_number, bitmap_context, cg_rect,
-                                   params);
+  metafile.RenderPage(page_number, bitmap_context,
+                      CGRectMake(0, 0, size_.width(), size_.height()), params);
+
+  return true;
 }
 
 }  // namespace printing

printing/metafile.h

@@ -16,6 +16,10 @@
 
 #if defined(OS_WIN)
 #include <windows.h>
+#elif defined(OS_MACOSX)
+#include <ApplicationServices/ApplicationServices.h>
+#include <CoreFoundation/CoreFoundation.h>
+#include "base/mac/scoped_cftyperef.h"
 #endif
 
 namespace base {
@@ -32,6 +36,36 @@ namespace printing {
 // This class plays metafiles from data stream (usually PDF or EMF).
 class PRINTING_EXPORT MetafilePlayer {
  public:
+#if defined(OS_MACOSX)
+  // |shrink_to_fit| specifies whether the output should be shrunk to fit a
+  // destination page if the source PDF is bigger than the destination page in
+  // any dimension. If this is false, parts of the source PDF page that lie
+  // outside the bounds will be clipped.
+  // |stretch_to_fit| specifies whether the output should be stretched to fit
+  // the destination page if the source page size is smaller in all dimensions.
+  // |center_horizontally| specifies whether the output (after any scaling is
+  // done) should be centered horizontally within the destination page.
+  // |center_vertically| specifies whether the output (after any scaling is
+  // done) should be centered vertically within the destination page.
+  // Note that all scaling preserves the original aspect ratio of the page.
+  // |autorotate| specifies whether the source PDF should be autorotated to fit
+  // on the destination page.
+  struct MacRenderPageParams {
+    MacRenderPageParams()
+        : shrink_to_fit(false),
+          stretch_to_fit(false),
+          center_horizontally(false),
+          center_vertically(false),
+          autorotate(false) {
+    }
+
+    bool shrink_to_fit;
+    bool stretch_to_fit;
+    bool center_horizontally;
+    bool center_vertically;
+    bool autorotate;
+  };
+#endif  // defined(OS_MACOSX)
   MetafilePlayer();
   virtual ~MetafilePlayer();
 
@@ -41,6 +75,15 @@ class PRINTING_EXPORT MetafilePlayer {
   // issue with some printers. See Emf::Record::SafePlayback implementation for
   // details.
   virtual bool SafePlayback(skia::NativeDrawingContext hdc) const = 0;
+
+#elif defined(OS_MACOSX)
+  // Renders the given page into |rect| in the given context.
+  // Pages use a 1-based index. The rendering uses the arguments in
+  // |params| to determine scaling, translation, and rotation.
+  virtual bool RenderPage(unsigned int page_number,
+                          skia::NativeDrawingContext context,
+                          const CGRect rect,
+                          const MacRenderPageParams& params) const = 0;
 #endif  // if defined(OS_WIN)
 
   // Populates the buffer with the underlying data. This function should ONLY be

printing/pdf_metafile_cg_mac.cc

@@ -157,24 +157,11 @@ bool PdfMetafileCg::FinishDocument() {
   return true;
 }
 
-/* TODO(caryclark): The set up of PluginInstance::PrintPDFOutput may result in
-   rasterized output.  Even if that flow uses PdfMetafileCg::RenderPage,
-   the drawing of the PDF into the canvas may result in a rasterized output.
-   PDFMetafileSkia::RenderPage should be not implemented as shown and instead
-   should do something like the following CL in PluginInstance::PrintPDFOutput:
-   http://codereview.chromium.org/7200040/diff/1/webkit/plugins/ppapi/ppapi_plugin_instance.cc
-*/
-bool PdfMetafileCg::RenderPage(const std::vector<char>& src_buffer,
-                               unsigned int page_number,
+bool PdfMetafileCg::RenderPage(unsigned int page_number,
                                CGContextRef context,
                                const CGRect rect,
-                               const PdfMetafileCg::RenderPageParams& params) {
-  PdfMetafileCg metafile;
-  if (!metafile.InitFromData(src_buffer.data(), src_buffer.size())) {
-    LOG(ERROR) << "Unable to initialize PDF document from data";
-    return false;
-  }
-  CGPDFDocumentRef pdf_doc = metafile.GetPDFDocument();
+                               const MacRenderPageParams& params) const {
+  CGPDFDocumentRef pdf_doc = GetPDFDocument();
   if (!pdf_doc) {
     LOG(ERROR) << "Unable to create PDF document from data";
     return false;

printing/pdf_metafile_cg_mac.h

@@ -45,38 +45,10 @@ class PRINTING_EXPORT PdfMetafileCg : public Metafile {
   // the data returned from GetData will not be valid PDF data.
   CGContextRef context() const override;
 
-  // |shrink_to_fit| specifies whether the output should be shrunk to fit a
-  // destination page if the source PDF is bigger than the destination page in
-  // any dimension. If this is false, parts of the source PDF page that lie
-  // outside the bounds will be clipped.
-  //
-  // |stretch_to_fit| specifies whether the output should be stretched to fit
-  // the destination page if the source page size is smaller in all dimensions.
-  //
-  // |center_horizontally| specifies whether the output (after any scaling is
-  // done) should be centered horizontally within the destination page.
-  //
-  // |center_vertically| specifies whether the output (after any scaling is
-  // done) should be centered vertically within the destination page.
-  // Note that all scaling preserves the original aspect ratio of the page.
-  //
-  // |autorotate| specifies whether the source PDF should be autorotated to fit
-  // on the destination page.
-  struct RenderPageParams {
-    bool shrink_to_fit = false;
-    bool stretch_to_fit = false;
-    bool center_horizontally = false;
-    bool center_vertically = false;
-    bool autorotate = false;
-  };
-  // Renders the given page from |src_buffer| into |rect| in the given context.
-  // Pages use a 1-based index. The rendering uses the arguments in
-  // |params| to determine scaling, translation, and rotation.
-  static bool RenderPage(const std::vector<char>& src_buffer,
-                         unsigned int page_number,
-                         CGContextRef context,
-                         const CGRect rect,
-                         const RenderPageParams& params);
+  bool RenderPage(unsigned int page_number,
+                  skia::NativeDrawingContext context,
+                  const CGRect rect,
+                  const MacRenderPageParams& params) const override;
 
  private:
   // Returns a CGPDFDocumentRef version of |pdf_data_|.

printing/pdf_metafile_skia.cc

@@ -24,6 +24,14 @@
 #include "ui/gfx/geometry/safe_integer_conversions.h"
 #include "ui/gfx/skia_util.h"
 
+#if defined(OS_MACOSX)
+#include "printing/pdf_metafile_cg_mac.h"
+#endif
+
+#if defined(OS_POSIX)
+#include "base/file_descriptor_posix.h"
+#endif
+
 namespace {
 
 bool WriteAssetToBuffer(const SkStreamAsset* asset,
@@ -96,6 +104,10 @@ struct PdfMetafileSkiaData {
   float scale_factor_;
   SkSize size_;
   SkiaDocumentType type_;
+
+#if defined(OS_MACOSX)
+  PdfMetafileCg pdf_cg_;
+#endif
 };
 
 PdfMetafileSkia::~PdfMetafileSkia() {}
@@ -243,6 +255,30 @@ bool PdfMetafileSkia::SafePlayback(skia::NativeDrawingContext hdc) const {
   NOTREACHED();
   return false;
 }
+
+#elif defined(OS_MACOSX)
+/* TODO(caryclark): The set up of PluginInstance::PrintPDFOutput may result in
+   rasterized output.  Even if that flow uses PdfMetafileCg::RenderPage,
+   the drawing of the PDF into the canvas may result in a rasterized output.
+   PDFMetafileSkia::RenderPage should be not implemented as shown and instead
+   should do something like the following CL in PluginInstance::PrintPDFOutput:
+http://codereview.chromium.org/7200040/diff/1/webkit/plugins/ppapi/ppapi_plugin_instance.cc
+*/
+bool PdfMetafileSkia::RenderPage(unsigned int page_number,
+                                 CGContextRef context,
+                                 const CGRect rect,
+                                 const MacRenderPageParams& params) const {
+  DCHECK_GT(GetDataSize(), 0U);
+  if (data_->pdf_cg_.GetDataSize() == 0) {
+    if (GetDataSize() == 0)
+      return false;
+    size_t length = data_->pdf_data_->getLength();
+    std::vector<uint8_t> buffer(length);
+    (void)WriteAssetToBuffer(data_->pdf_data_.get(), &buffer[0], length);
+    data_->pdf_cg_.InitFromData(&buffer[0], length);
+  }
+  return data_->pdf_cg_.RenderPage(page_number, context, rect, params);
+}
 #endif
 
 bool PdfMetafileSkia::SaveTo(base::File* file) const {

printing/pdf_metafile_skia.h

@@ -57,6 +57,11 @@ class PRINTING_EXPORT PdfMetafileSkia : public Metafile {
   bool Playback(skia::NativeDrawingContext hdc,
                 const RECT* rect) const override;
   bool SafePlayback(skia::NativeDrawingContext hdc) const override;
+#elif defined(OS_MACOSX)
+  bool RenderPage(unsigned int page_number,
+                  skia::NativeDrawingContext context,
+                  const CGRect rect,
+                  const MacRenderPageParams& params) const override;
 #endif
 
   bool SaveTo(base::File* file) const override;

printing/printed_document_mac.cc

@@ -4,12 +4,11 @@
 
 #include "printing/printed_document.h"
 
-#include <ApplicationServices/ApplicationServices.h>
-#include <CoreFoundation/CoreFoundation.h>
+#import <ApplicationServices/ApplicationServices.h>
+#import <CoreFoundation/CoreFoundation.h>
 
 #include "base/logging.h"
 #include "printing/page_number.h"
-#include "printing/pdf_metafile_cg_mac.h"
 #include "printing/printed_page.h"
 
 namespace printing {
@@ -32,16 +31,11 @@ void PrintedDocument::RenderPrintedPage(
   page.GetCenteredPageContentRect(page_setup.physical_size(), &content_area);
 
   const MetafilePlayer* metafile = page.metafile();
-
   // Each Metafile is a one-page PDF, and pages use 1-based indexing.
   const int page_number = 1;
-  PdfMetafileCg::RenderPageParams params;
+  struct Metafile::MacRenderPageParams params;
   params.autorotate = true;
-  std::vector<char> buffer;
-  if (metafile->GetDataAsVector(&buffer)) {
-    PdfMetafileCg::RenderPage(buffer, page_number, context,
-                              content_area.ToCGRect(), params);
-  }
+  metafile->RenderPage(page_number, context, content_area.ToCGRect(), params);
 }
 
 }  // namespace printing