Change PDF N-up functions to use gfx::Size.

Instead of taking the dimensions as two separate parameters.

Change-Id: I6a92aa4c21d932f131c0aae39860edfd9b642c2b
Reviewed-on: https://chromium-review.googlesource.com/1185371Reviewed-by: Ian Vollick <vollick@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585868}

chrome/services/printing/pdf_nup_converter.cc

@@ -73,8 +73,8 @@ void PdfNupConverter::NupPageConvert(
   void* output_pdf_buffer = nullptr;
   size_t output_pdf_buffer_size = 0;
   if (!chrome_pdf::ConvertPdfPagesToNupPdf(
-          std::move(input_pdf_buffers), pages_per_sheet, page_size.width(),
-          page_size.height(), &output_pdf_buffer, &output_pdf_buffer_size)) {
+          std::move(input_pdf_buffers), pages_per_sheet, page_size,
+          &output_pdf_buffer, &output_pdf_buffer_size)) {
     std::move(callback).Run(mojom::PdfNupConverter::Status::CONVERSION_FAILURE,
                             base::ReadOnlySharedMemoryRegion());
     return;
@@ -94,9 +94,9 @@ void PdfNupConverter::NupDocumentConvert(
   auto input_pdf_buffer = pdf_document_mapping.GetMemoryAsSpan<const uint8_t>();
   void* output_pdf_buffer = nullptr;
   size_t output_pdf_buffer_size = 0;
-  if (!chrome_pdf::ConvertPdfDocumentToNupPdf(
-          input_pdf_buffer, pages_per_sheet, page_size.width(),
-          page_size.height(), &output_pdf_buffer, &output_pdf_buffer_size)) {
+  if (!chrome_pdf::ConvertPdfDocumentToNupPdf(input_pdf_buffer, pages_per_sheet,
+                                              page_size, &output_pdf_buffer,
+                                              &output_pdf_buffer_size)) {
     std::move(callback).Run(mojom::PdfNupConverter::Status::CONVERSION_FAILURE,
                             base::ReadOnlySharedMemoryRegion());
     return;

pdf/DEPS

@@ -5,8 +5,7 @@ include_rules = [
   "+printing/units.h",
   "+ui/base/window_open_disposition.h",
   "+ui/events/keycodes/keyboard_codes.h",
-  "+ui/gfx/geometry/point_f.h",
-  "+ui/gfx/geometry/rect.h",
+  "+ui/gfx/geometry",
   "+ui/gfx/range/range.h",
   "+v8/include/v8.h"
 ]

pdf/pdf.cc

@@ -139,8 +139,7 @@ bool RenderPDFPageToBitmap(base::span<const uint8_t> pdf_buffer,
 bool ConvertPdfPagesToNupPdf(
     std::vector<base::span<const uint8_t>> input_buffers,
     size_t pages_per_sheet,
-    size_t page_size_width,
-    size_t page_size_height,
+    const gfx::Size& page_size,
     void** dest_pdf_buffer,
     size_t* dest_pdf_buffer_size) {
   ScopedSdkInitializer scoped_sdk_initializer;
@@ -149,14 +148,13 @@ bool ConvertPdfPagesToNupPdf(
 
   PDFEngineExports* engine_exports = PDFEngineExports::Get();
   return engine_exports->ConvertPdfPagesToNupPdf(
-      std::move(input_buffers), pages_per_sheet, page_size_width,
-      page_size_height, dest_pdf_buffer, dest_pdf_buffer_size);
+      std::move(input_buffers), pages_per_sheet, page_size, dest_pdf_buffer,
+      dest_pdf_buffer_size);
 }
 
 bool ConvertPdfDocumentToNupPdf(base::span<const uint8_t> input_buffer,
                                 size_t pages_per_sheet,
-                                size_t page_size_width,
-                                size_t page_size_height,
+                                const gfx::Size& page_size,
                                 void** dest_pdf_buffer,
                                 size_t* dest_pdf_buffer_size) {
   ScopedSdkInitializer scoped_sdk_initializer;
@@ -165,8 +163,8 @@ bool ConvertPdfDocumentToNupPdf(base::span<const uint8_t> input_buffer,
 
   PDFEngineExports* engine_exports = PDFEngineExports::Get();
   return engine_exports->ConvertPdfDocumentToNupPdf(
-      input_buffer, pages_per_sheet, page_size_width, page_size_height,
-      dest_pdf_buffer, dest_pdf_buffer_size);
+      input_buffer, pages_per_sheet, page_size, dest_pdf_buffer,
+      dest_pdf_buffer_size);
 }
 
 }  // namespace chrome_pdf

pdf/pdf.h

@@ -20,6 +20,10 @@ typedef void (*PDFEnsureTypefaceCharactersAccessible)(const LOGFONT* font,
                                                       size_t text_length);
 #endif
 
+namespace gfx {
+class Size;
+}
+
 namespace chrome_pdf {
 
 #if defined(OS_WIN)
@@ -127,29 +131,24 @@ bool RenderPDFPageToBitmap(base::span<const uint8_t> pdf_buffer,
 //     If any of the PDFs contains multiple pages, only the first page of the
 //     document is used.
 // |pages_per_sheet| is the number of pages to put on one sheet.
-// |page_size_width| is the width of the output page size, measured in PDF
-//     "user space" units.
-// |page_size_height| is the height of the output page size, measured in PDF
-//     "user space" units.
+// |page_size| is the output page size, measured in PDF "user space" units.
 // |dest_pdf_buffer| is the output N-up PDF page.  Caller takes ownership, and
 //     needs to free the memory.
 // |dest_pdf_buffer_size| is the size of output N-up PDF page.
 //
-// |page_size_width| and |page_size_height| are the print media size.  The page
-// size of the output N-up PDF is determined by the |pages_per_sheet|, the
-// orientation of the PDF pages contained in the |input_buffers|, and the media
-// page size |page_size_width| and |page_size_height|.  For example, when
-// |page_size_width| = 512, |page_size_height| = 792, |pages_per_sheet| = 2, and
-// the orientation of |input_buffers| = portrait, the output N-up PDF will have
-// |page_size_width| = 792, and |page_size_height| = 512.
+// |page_size| is the print media size.  The page size of the output N-up PDF is
+// determined by the |pages_per_sheet|, the orientation of the PDF pages
+// contained in the |input_buffers|, and the media page size |page_size|. For
+// example, when |page_size| = 512x792, |pages_per_sheet| = 2, and the
+// orientation of |input_buffers| = portrait, the output N-up PDF will be
+// 792x512.
 // See printing::NupParameters for more details on how the output page
-// orientation is determined, to understand why |page_size_width| and
-// |page_size_height| may be swapped in some cases.
+// orientation is determined, to understand why |page_size| may be swapped in
+// some cases.
 bool ConvertPdfPagesToNupPdf(
     std::vector<base::span<const uint8_t>> input_buffers,
     size_t pages_per_sheet,
-    size_t page_size_width,
-    size_t page_size_height,
+    const gfx::Size& page_size,
     void** dest_pdf_buffer,
     size_t* dest_pdf_buffer_size);
 
@@ -157,21 +156,17 @@ bool ConvertPdfPagesToNupPdf(
 // |input_buffer| is the buffer that contains the entire PDF document to be
 //     converted to a N-up PDF document.
 // |pages_per_sheet| is the number of pages to put on one sheet.
-// |page_size_width| is the width of the media page size, measured in PDF
-//     "user space" units.
-// |page_size_height| is the height of the media page size, measured in PDF
-//     "user space" units.
+// |page_size| is the output page size, measured in PDF "user space" units.
 // |dest_pdf_buffer| is the output N-up PDF page.  Caller takes ownership, and
 //     needs to free the memory.
 // |dest_pdf_buffer_size| is the size of output N-up PDF document.
 //
 // Refer to the description of ConvertPdfPagesToNupPdf to understand how the
-// output page size |page_size_width| and |page_size_height| will be calculated.
+// output page size will be calculated.
 // The algorithm used to determine the output page size is the same.
 bool ConvertPdfDocumentToNupPdf(base::span<const uint8_t> input_buffer,
                                 size_t pages_per_sheet,
-                                size_t page_size_width,
-                                size_t page_size_height,
+                                const gfx::Size& page_size,
                                 void** dest_pdf_buffer,
                                 size_t* dest_pdf_buffer_size);
 

pdf/pdf_engine.h

@@ -41,6 +41,10 @@ typedef void (*PDFEnsureTypefaceCharactersAccessible)(const LOGFONT* font,
 
 struct PP_PdfPrintSettings_Dev;
 
+namespace gfx {
+class Size;
+}
+
 namespace pp {
 class InputEvent;
 class VarDictionary;
@@ -466,8 +470,7 @@ class PDFEngineExports {
   virtual bool ConvertPdfPagesToNupPdf(
       std::vector<base::span<const uint8_t>> input_buffers,
       size_t pages_per_sheet,
-      size_t page_size_width,
-      size_t page_size_height,
+      const gfx::Size& page_size,
       void** dest_pdf_buffer,
       size_t* dest_pdf_buffer_size) = 0;
 
@@ -475,8 +478,7 @@ class PDFEngineExports {
   virtual bool ConvertPdfDocumentToNupPdf(
       base::span<const uint8_t> input_buffer,
       size_t pages_per_sheet,
-      size_t page_size_width,
-      size_t page_size_height,
+      const gfx::Size& page_size,
       void** dest_pdf_buffer,
       size_t* dest_pdf_buffer_size) = 0;
 

pdf/pdfium/pdfium_engine_exports.cc

@@ -16,6 +16,7 @@
 #include "third_party/pdfium/public/cpp/fpdf_scopers.h"
 #include "third_party/pdfium/public/fpdf_ppo.h"
 #include "third_party/pdfium/public/fpdfview.h"
+#include "ui/gfx/geometry/size.h"
 
 using printing::ConvertUnitDouble;
 using printing::kPointsPerInch;
@@ -118,13 +119,15 @@ ScopedFPDFDocument CreatePdfDoc(
 
 bool CreateNupPdfDocument(FPDF_DOCUMENT doc,
                           size_t pages_per_sheet,
-                          size_t page_size_width,
-                          size_t page_size_height,
+                          const gfx::Size& page_size,
                           void** dest_pdf_buffer,
                           size_t* dest_pdf_buffer_size) {
   if (!dest_pdf_buffer || !dest_pdf_buffer_size)
     return false;
 
+  int page_size_width = page_size.width();
+  int page_size_height = page_size.height();
+
   printing::NupParameters nup_params;
   bool is_landscape = PDFiumPrint::IsSourcePdfLandscape(doc);
   nup_params.SetParameters(pages_per_sheet, is_landscape);
@@ -308,33 +311,29 @@ bool PDFiumEngineExports::RenderPDFPageToBitmap(
 bool PDFiumEngineExports::ConvertPdfPagesToNupPdf(
     std::vector<base::span<const uint8_t>> input_buffers,
     size_t pages_per_sheet,
-    size_t page_size_width,
-    size_t page_size_height,
+    const gfx::Size& page_size,
     void** dest_pdf_buffer,
     size_t* dest_pdf_buffer_size) {
   ScopedFPDFDocument doc = CreatePdfDoc(std::move(input_buffers));
   if (!doc)
     return false;
 
-  return CreateNupPdfDocument(doc.get(), pages_per_sheet, page_size_width,
-                              page_size_height, dest_pdf_buffer,
-                              dest_pdf_buffer_size);
+  return CreateNupPdfDocument(doc.get(), pages_per_sheet, page_size,
+                              dest_pdf_buffer, dest_pdf_buffer_size);
 }
 
 bool PDFiumEngineExports::ConvertPdfDocumentToNupPdf(
     base::span<const uint8_t> input_buffer,
     size_t pages_per_sheet,
-    size_t page_size_width,
-    size_t page_size_height,
+    const gfx::Size& page_size,
     void** dest_pdf_buffer,
     size_t* dest_pdf_buffer_size) {
   ScopedFPDFDocument doc = LoadPdfData(input_buffer);
   if (!doc)
     return false;
 
-  return CreateNupPdfDocument(doc.get(), pages_per_sheet, page_size_width,
-                              page_size_height, dest_pdf_buffer,
-                              dest_pdf_buffer_size);
+  return CreateNupPdfDocument(doc.get(), pages_per_sheet, page_size,
+                              dest_pdf_buffer, dest_pdf_buffer_size);
 }
 
 bool PDFiumEngineExports::GetPDFDocInfo(base::span<const uint8_t> pdf_buffer,

pdf/pdfium/pdfium_engine_exports.h

@@ -38,14 +38,12 @@ class PDFiumEngineExports : public PDFEngineExports {
   bool ConvertPdfPagesToNupPdf(
       std::vector<base::span<const uint8_t>> input_buffers,
       size_t pages_per_sheet,
-      size_t page_size_width,
-      size_t page_size_height,
+      const gfx::Size& page_size,
       void** dest_pdf_buffer,
       size_t* dest_pdf_buffer_size) override;
   bool ConvertPdfDocumentToNupPdf(base::span<const uint8_t> input_buffer,
                                   size_t pages_per_sheet,
-                                  size_t page_size_width,
-                                  size_t page_size_height,
+                                  const gfx::Size& page_size,
                                   void** dest_pdf_buffer,
                                   size_t* dest_pdf_buffer_size) override;
   bool GetPDFDocInfo(base::span<const uint8_t> pdf_buffer,

pdf/pdfium/pdfium_engine_exports_unittest.cc

@@ -10,6 +10,7 @@
 #include "pdf/pdf.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "ui/gfx/geometry/size.h"
 
 namespace chrome_pdf {
 
@@ -102,16 +103,17 @@ TEST_F(PDFiumEngineExportsTest, ConvertPdfPagesToNupPdf) {
 
   std::vector<base::span<const uint8_t>> pdf_buffers;
 
-  EXPECT_FALSE(
-      ConvertPdfPagesToNupPdf(pdf_buffers, 1, 512, 792, nullptr, nullptr));
+  EXPECT_FALSE(ConvertPdfPagesToNupPdf(pdf_buffers, 1, gfx::Size(512, 792),
+                                       nullptr, nullptr));
 
   pdf_buffers.push_back(base::as_bytes(base::make_span(pdf_data)));
   pdf_buffers.push_back(base::as_bytes(base::make_span(pdf_data)));
 
   void* output_pdf_buffer;
   size_t output_pdf_buffer_size;
-  ASSERT_TRUE(ConvertPdfPagesToNupPdf(
-      pdf_buffers, 2, 512, 792, &output_pdf_buffer, &output_pdf_buffer_size));
+  ASSERT_TRUE(ConvertPdfPagesToNupPdf(pdf_buffers, 2, gfx::Size(512, 792),
+                                      &output_pdf_buffer,
+                                      &output_pdf_buffer_size));
   ASSERT_GT(output_pdf_buffer_size, 0U);
   ASSERT_NE(output_pdf_buffer, nullptr);
 
@@ -138,15 +140,16 @@ TEST_F(PDFiumEngineExportsTest, ConvertPdfDocumentToNupPdf) {
 
   base::span<const uint8_t> pdf_buffer;
 
-  EXPECT_FALSE(
-      ConvertPdfDocumentToNupPdf(pdf_buffer, 1, 512, 792, nullptr, nullptr));
+  EXPECT_FALSE(ConvertPdfDocumentToNupPdf(pdf_buffer, 1, gfx::Size(512, 792),
+                                          nullptr, nullptr));
 
   pdf_buffer = base::as_bytes(base::make_span(pdf_data));
 
   void* output_pdf_buffer;
   size_t output_pdf_buffer_size;
-  ASSERT_TRUE(ConvertPdfDocumentToNupPdf(
-      pdf_buffer, 4, 512, 792, &output_pdf_buffer, &output_pdf_buffer_size));
+  ASSERT_TRUE(ConvertPdfDocumentToNupPdf(pdf_buffer, 4, gfx::Size(512, 792),
+                                         &output_pdf_buffer,
+                                         &output_pdf_buffer_size));
   ASSERT_GT(output_pdf_buffer_size, 0U);
   ASSERT_NE(output_pdf_buffer, nullptr);