Change double to float in PDFEngineExports API signatures

GetPDFDocInfo() and GetPDFPageSizeByIndex() call PDFium APIs underneath
that use single precision, not double precision. Change their
signatures to reflect that fact.

Meanwhile, change the output of GetPDFPageSizeByIndex() from two
pointer parameters to a base::Optional<gfx::SizeF> return value.

Change-Id: I91e3645027f2345f930c6ee8ca886f84deed6897
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2393013
Reviewed-by: Sami Kyöstilä <skyostil@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Daniel Hosseinian <dhoss@chromium.org>
Cr-Commit-Position: refs/heads/master@{#804972}

chrome/browser/printing/pdf_nup_converter_client_browsertest.cc

@@ -84,12 +84,12 @@ std::vector<gfx::SizeF> GetPdfPageSizes(base::span<const uint8_t> pdf_data) {
 
   std::vector<gfx::SizeF> sizes;
   for (int i = 0; i < num_pages; ++i) {
-    double width;
+    base::Optional<gfx::SizeF> page_size =
-    double height;
+        chrome_pdf::GetPDFPageSizeByIndex(pdf_data, i);
-    if (!chrome_pdf::GetPDFPageSizeByIndex(pdf_data, i, &width, &height))
+    if (!page_size.has_value())
       return {};
 
-    sizes.push_back({width, height});
+    sizes.push_back(page_size.value());
   }
 
   return sizes;

chrome/browser/printing/print_preview_pdf_generated_browsertest.cc

@@ -24,6 +24,7 @@
 #include "base/hash/md5.h"
 #include "base/location.h"
 #include "base/macros.h"
+#include "base/optional.h"
 #include "base/path_service.h"
 #include "base/run_loop.h"
 #include "base/single_thread_task_runner.h"
@@ -54,6 +55,7 @@
 #include "ui/gfx/codec/png_codec.h"
 #include "ui/gfx/geometry/point.h"
 #include "ui/gfx/geometry/rect.h"
+#include "ui/gfx/geometry/size_f.h"
 #include "url/gurl.h"
 
 #if defined(OS_WIN)
@@ -337,7 +339,7 @@ class PrintPreviewPdfGeneratedBrowserTest : public InProcessBrowserTest {
   // diff on this image and a reference image.
   void PdfToPng() {
     int num_pages;
-    double max_width_in_points = 0;
+    float max_width_in_points = 0;
     std::vector<uint8_t> bitmap_data;
     double total_height_in_pixels = 0;
     std::string pdf_data;
@@ -353,14 +355,14 @@ class PrintPreviewPdfGeneratedBrowserTest : public InProcessBrowserTest {
         ConvertUnitDouble(max_width_in_points, kPointsPerInch, kDpi);
 
     for (int i = 0; i < num_pages; ++i) {
-      double width_in_points, height_in_points;
+      base::Optional<gfx::SizeF> size_in_points =
-      ASSERT_TRUE(chrome_pdf::GetPDFPageSizeByIndex(
+          chrome_pdf::GetPDFPageSizeByIndex(pdf_span, i);
-          pdf_span, i, &width_in_points, &height_in_points));
+      ASSERT_TRUE(size_in_points.has_value());
 
-      double width_in_pixels = ConvertUnitDouble(
+      double width_in_pixels = ConvertUnitDouble(size_in_points.value().width(),
-          width_in_points, kPointsPerInch, kDpi);
+                                                 kPointsPerInch, kDpi);
       double height_in_pixels = ConvertUnitDouble(
-          height_in_points, kPointsPerInch, kDpi);
+          size_in_points.value().height(), kPointsPerInch, kDpi);
 
       // The image will be rotated if |width_in_pixels| is greater than
       // |height_in_pixels|. This is because the page will be rotated to fit

headless/lib/headless_web_contents_browsertest.cc

@@ -10,6 +10,7 @@
 #include "base/check_op.h"
 #include "base/command_line.h"
 #include "base/json/json_writer.h"
+#include "base/optional.h"
 #include "base/run_loop.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
@@ -42,6 +43,7 @@
 #include "third_party/skia/include/core/SkColor.h"
 #include "ui/gfx/codec/png_codec.h"
 #include "ui/gfx/geometry/size.h"
+#include "ui/gfx/geometry/size_f.h"
 #include "url/gurl.h"
 
 #if BUILDFLAG(ENABLE_PRINTING)
@@ -352,13 +354,12 @@ class HeadlessWebContentsPDFTest : public HeadlessAsyncDevTooledBrowserTest {
     EXPECT_EQ(std::ceil(kDocHeight / kPaperHeight), num_pages);
 
     for (int i = 0; i < num_pages; i++) {
-      double width_in_points;
+      base::Optional<gfx::SizeF> size_in_points =
-      double height_in_points;
+          chrome_pdf::GetPDFPageSizeByIndex(pdf_span, i);
-      EXPECT_TRUE(chrome_pdf::GetPDFPageSizeByIndex(
+      ASSERT_TRUE(size_in_points.has_value());
-          pdf_span, i, &width_in_points, &height_in_points));
+      EXPECT_EQ(static_cast<int>(size_in_points.value().width()),
-      EXPECT_EQ(static_cast<int>(width_in_points),
                 static_cast<int>(kPaperWidth * printing::kPointsPerInch));
-      EXPECT_EQ(static_cast<int>(height_in_points),
+      EXPECT_EQ(static_cast<int>(size_in_points.value().height()),
                 static_cast<int>(kPaperHeight * printing::kPointsPerInch));
 
       gfx::Rect rect(kPaperWidth * kDpi, kPaperHeight * kDpi);

pdf/pdf.cc

@@ -11,6 +11,7 @@
 #include "pdf/pdf_engine.h"
 #include "pdf/pdf_init.h"
 #include "ui/gfx/geometry/rect.h"
+#include "ui/gfx/geometry/size_f.h"
 
 namespace chrome_pdf {
 
@@ -85,7 +86,7 @@ void SetPDFUsePrintMode(int mode) {
 
 bool GetPDFDocInfo(base::span<const uint8_t> pdf_buffer,
                    int* page_count,
-                   double* max_page_width) {
+                   float* max_page_width) {
   ScopedSdkInitializer scoped_sdk_initializer(/*enable_v8=*/true);
   PDFEngineExports* engine_exports = PDFEngineExports::Get();
   return engine_exports->GetPDFDocInfo(pdf_buffer, page_count, max_page_width);
@@ -104,15 +105,13 @@ base::Value GetPDFStructTreeForPage(base::span<const uint8_t> pdf_buffer,
   return engine_exports->GetPDFStructTreeForPage(pdf_buffer, page_index);
 }
 
-bool GetPDFPageSizeByIndex(base::span<const uint8_t> pdf_buffer,
+base::Optional<gfx::SizeF> GetPDFPageSizeByIndex(
-                           int page_number,
+    base::span<const uint8_t> pdf_buffer,
-                           double* width,
+    int page_number) {
-                           double* height) {
   ScopedSdkInitializer scoped_sdk_initializer(/*enable_v8=*/true);
   chrome_pdf::PDFEngineExports* engine_exports =
       chrome_pdf::PDFEngineExports::Get();
-  return engine_exports->GetPDFPageSizeByIndex(pdf_buffer, page_number, width,
+  return engine_exports->GetPDFPageSizeByIndex(pdf_buffer, page_number);
-                                               height);
 }
 
 bool RenderPDFPageToBitmap(base::span<const uint8_t> pdf_buffer,

pdf/pdf.h

@@ -25,6 +25,7 @@ typedef void (*PDFEnsureTypefaceCharactersAccessible)(const LOGFONT* font,
 namespace gfx {
 class Rect;
 class Size;
+class SizeF;
 }  // namespace gfx
 
 namespace chrome_pdf {
@@ -103,7 +104,7 @@ void SetPDFUsePrintMode(int mode);
 // Returns false if the document is not valid.
 bool GetPDFDocInfo(base::span<const uint8_t> pdf_buffer,
                    int* page_count,
-                   double* max_page_width);
+                   float* max_page_width);
 
 // Whether the PDF is Tagged (see 10.7 "Tagged PDF" in PDF Reference 1.7).
 // Returns true if it's a tagged (accessible) PDF, false if it's a valid
@@ -120,13 +121,11 @@ base::Value GetPDFStructTreeForPage(base::span<const uint8_t> pdf_buffer,
 //     rendered.
 // |page_number| is the page number that the function will get the dimensions
 //     of.
-// |width| is the output for the width of the page in points.
+// Returns the size of the page in points, or nullopt if the document or the
-// |height| is the output for the height of the page in points.
+// page number are not valid.
-// Returns false if the document or the page number are not valid.
+base::Optional<gfx::SizeF> GetPDFPageSizeByIndex(
-bool GetPDFPageSizeByIndex(base::span<const uint8_t> pdf_buffer,
+    base::span<const uint8_t> pdf_buffer,
-                           int page_number,
+    int page_number);
-                           double* width,
-                           double* height);
 
 // Renders PDF page into 4-byte per pixel BGRA color bitmap.
 // |pdf_buffer| is the buffer that contains the entire PDF document to be

pdf/pdf_engine.h

@@ -47,6 +47,7 @@ namespace gfx {
 class Point;
 class Rect;
 class Size;
+class SizeF;
 class Vector2d;
 }  // namespace gfx
 
@@ -564,7 +565,7 @@ class PDFEngineExports {
 
   virtual bool GetPDFDocInfo(base::span<const uint8_t> pdf_buffer,
                              int* page_count,
-                             double* max_page_width) = 0;
+                             float* max_page_width) = 0;
 
   // Whether the PDF is Tagged (see 10.7 "Tagged PDF" in PDF Reference 1.7).
   // Returns true if it's a tagged (accessible) PDF, false if it's a valid
@@ -579,10 +580,9 @@ class PDFEngineExports {
       int page_index) = 0;
 
   // See the definition of GetPDFPageSizeByIndex in pdf.cc for details.
-  virtual bool GetPDFPageSizeByIndex(base::span<const uint8_t> pdf_buffer,
+  virtual base::Optional<gfx::SizeF> GetPDFPageSizeByIndex(
-                                     int page_number,
+      base::span<const uint8_t> pdf_buffer,
-                                     double* width,
+      int page_number) = 0;
-                                     double* height) = 0;
 };
 
 }  // namespace chrome_pdf

pdf/pdfium/pdfium_engine_exports.cc

@@ -10,6 +10,7 @@
 #include "base/bind.h"
 #include "base/no_destructor.h"
 #include "base/numerics/safe_conversions.h"
+#include "base/optional.h"
 #include "pdf/pdfium/pdfium_api_string_buffer_adapter.h"
 #include "pdf/pdfium/pdfium_mem_buffer_file_write.h"
 #include "pdf/pdfium/pdfium_print.h"
@@ -22,6 +23,7 @@
 #include "third_party/pdfium/public/fpdfview.h"
 #include "ui/gfx/geometry/rect.h"
 #include "ui/gfx/geometry/size.h"
+#include "ui/gfx/geometry/size_f.h"
 #include "ui/gfx/geometry/vector2d.h"
 
 using printing::ConvertUnitDouble;
@@ -376,7 +378,7 @@ std::vector<uint8_t> PDFiumEngineExports::ConvertPdfDocumentToNupPdf(
 
 bool PDFiumEngineExports::GetPDFDocInfo(base::span<const uint8_t> pdf_buffer,
                                         int* page_count,
-                                        double* max_page_width) {
+                                        float* max_page_width) {
   ScopedFPDFDocument doc = LoadPdfData(pdf_buffer);
   if (!doc)
     return false;
@@ -438,22 +440,18 @@ base::Value PDFiumEngineExports::GetPDFStructTreeForPage(
   return RecursiveGetStructTree(struct_root_elem);
 }
 
-bool PDFiumEngineExports::GetPDFPageSizeByIndex(
+base::Optional<gfx::SizeF> PDFiumEngineExports::GetPDFPageSizeByIndex(
     base::span<const uint8_t> pdf_buffer,
-    int page_number,
+    int page_number) {
-    double* width,
-    double* height) {
   ScopedFPDFDocument doc = LoadPdfData(pdf_buffer);
-  if (!doc || !width || !height)
+  if (!doc)
-    return false;
+    return base::nullopt;
 
   FS_SIZEF size;
   if (!FPDF_GetPageSizeByIndexF(doc.get(), page_number, &size))
-    return false;
+    return base::nullopt;
 
-  *width = size.width;
+  return gfx::SizeF(size.width, size.height);
-  *height = size.height;
-  return true;
 }
 
 }  // namespace chrome_pdf

pdf/pdfium/pdfium_engine_exports.h

@@ -52,15 +52,14 @@ class PDFiumEngineExports : public PDFEngineExports {
       const gfx::Rect& printable_area) override;
   bool GetPDFDocInfo(base::span<const uint8_t> pdf_buffer,
                      int* page_count,
-                     double* max_page_width) override;
+                     float* max_page_width) override;
   base::Optional<bool> IsPDFDocTagged(
       base::span<const uint8_t> pdf_buffer) override;
   base::Value GetPDFStructTreeForPage(base::span<const uint8_t> pdf_buffer,
                                       int page_index) override;
-  bool GetPDFPageSizeByIndex(base::span<const uint8_t> pdf_buffer,
+  base::Optional<gfx::SizeF> GetPDFPageSizeByIndex(
-                             int page_number,
+      base::span<const uint8_t> pdf_buffer,
-                             double* width,
+      int page_number) override;
-                             double* height) override;
 };
 
 }  // namespace chrome_pdf

pdf/pdfium/pdfium_engine_exports_unittest.cc

@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 #include "base/files/file_util.h"
+#include "base/optional.h"
 #include "base/path_service.h"
 #include "base/test/test_simple_task_runner.h"
 #include "base/threading/thread_task_runner_handle.h"
@@ -11,6 +12,7 @@
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/gfx/geometry/rect.h"
 #include "ui/gfx/geometry/size.h"
+#include "ui/gfx/geometry/size_f.h"
 
 namespace chrome_pdf {
 
@@ -65,7 +67,7 @@ TEST_F(PDFiumEngineExportsTest, GetPDFDocInfo) {
   ASSERT_TRUE(GetPDFDocInfo(pdf_span, nullptr, nullptr));
 
   int page_count;
-  double max_page_width;
+  float max_page_width;
   ASSERT_TRUE(GetPDFDocInfo(pdf_span, &page_count, &max_page_width));
   EXPECT_EQ(2, page_count);
   EXPECT_DOUBLE_EQ(200.0, max_page_width);
@@ -80,17 +82,14 @@ TEST_F(PDFiumEngineExportsTest, GetPDFPageSizeByIndex) {
   ASSERT_TRUE(base::ReadFileToString(pdf_path, &pdf_data));
 
   auto pdf_span = base::as_bytes(base::make_span(pdf_data));
-  EXPECT_FALSE(GetPDFPageSizeByIndex(pdf_span, 0, nullptr, nullptr));
-
   int page_count;
   ASSERT_TRUE(GetPDFDocInfo(pdf_span, &page_count, nullptr));
   ASSERT_EQ(2, page_count);
   for (int page_number = 0; page_number < page_count; ++page_number) {
-    double width;
+    base::Optional<gfx::SizeF> page_size =
-    double height;
+        GetPDFPageSizeByIndex(pdf_span, page_number);
-    ASSERT_TRUE(GetPDFPageSizeByIndex(pdf_span, page_number, &width, &height));
+    ASSERT_TRUE(page_size.has_value());
-    EXPECT_DOUBLE_EQ(200.0, width);
+    EXPECT_EQ(gfx::SizeF(200, 200), page_size.value());
-    EXPECT_DOUBLE_EQ(200.0, height);
   }
 }
 
@@ -129,11 +128,10 @@ TEST_F(PDFiumEngineExportsTest, ConvertPdfPagesToNupPdf) {
   ASSERT_TRUE(GetPDFDocInfo(output_pdf_span, &page_count, nullptr));
   ASSERT_EQ(1, page_count);
 
-  double width;
+  base::Optional<gfx::SizeF> page_size =
-  double height;
+      GetPDFPageSizeByIndex(output_pdf_span, 0);
-  ASSERT_TRUE(GetPDFPageSizeByIndex(output_pdf_span, 0, &width, &height));
+  ASSERT_TRUE(page_size.has_value());
-  EXPECT_DOUBLE_EQ(792.0, width);
+  EXPECT_EQ(gfx::SizeF(792, 612), page_size.value());
-  EXPECT_DOUBLE_EQ(612.0, height);
 }
 
 TEST_F(PDFiumEngineExportsTest, ConvertPdfDocumentToNupPdf) {
@@ -158,12 +156,10 @@ TEST_F(PDFiumEngineExportsTest, ConvertPdfDocumentToNupPdf) {
   ASSERT_TRUE(GetPDFDocInfo(output_pdf_span, &page_count, nullptr));
   ASSERT_EQ(2, page_count);
   for (int page_number = 0; page_number < page_count; ++page_number) {
-    double width;
+    base::Optional<gfx::SizeF> page_size =
-    double height;
+        GetPDFPageSizeByIndex(output_pdf_span, page_number);
-    ASSERT_TRUE(
+    ASSERT_TRUE(page_size.has_value());
-        GetPDFPageSizeByIndex(output_pdf_span, page_number, &width, &height));
+    EXPECT_EQ(gfx::SizeF(612, 792), page_size.value());
-    EXPECT_DOUBLE_EQ(612.0, width);
-    EXPECT_DOUBLE_EQ(792.0, height);
   }
 }
 

pdf/pdfium/pdfium_print_unittest.cc

@@ -7,6 +7,7 @@
 #include <memory>
 
 #include "base/hash/md5.h"
+#include "base/optional.h"
 #include "base/stl_util.h"
 #include "pdf/pdfium/pdfium_engine.h"
 #include "pdf/pdfium/pdfium_engine_exports.h"
@@ -18,6 +19,7 @@
 #include "printing/units.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "ui/gfx/geometry/rect.h"
+#include "ui/gfx/geometry/size_f.h"
 
 namespace chrome_pdf {
 
@@ -33,12 +35,7 @@ constexpr PP_Size kUSLetterSize = {612, 792};
 constexpr PP_Rect kUSLetterRect = {{0, 0}, kUSLetterSize};
 constexpr PP_Rect kPrintableAreaRect = {{18, 18}, {576, 733}};
 
-struct SizeDouble {
+using ExpectedDimensions = std::vector<gfx::SizeF>;
-  double width;
-  double height;
-};
-
-using ExpectedDimensions = std::vector<SizeDouble>;
 
 void CheckPdfDimensions(const std::vector<uint8_t>& pdf_data,
                         const ExpectedDimensions& expected_dimensions) {
@@ -49,22 +46,22 @@ void CheckPdfDimensions(const std::vector<uint8_t>& pdf_data,
   ASSERT_EQ(expected_dimensions.size(), static_cast<size_t>(page_count));
 
   for (int i = 0; i < page_count; ++i) {
-    double width;
+    base::Optional<gfx::SizeF> page_size =
-    double height;
+        exports.GetPDFPageSizeByIndex(pdf_data, i);
-    ASSERT_TRUE(exports.GetPDFPageSizeByIndex(pdf_data, i, &width, &height));
+    ASSERT_TRUE(page_size.has_value());
-    EXPECT_DOUBLE_EQ(expected_dimensions[i].width, width);
+    EXPECT_EQ(expected_dimensions[i], page_size.value());
-    EXPECT_DOUBLE_EQ(expected_dimensions[i].height, height);
   }
 }
 
 void CheckPdfRendering(const std::vector<uint8_t>& pdf_data,
                        int page_number,
-                       const SizeDouble& size_in_points,
+                       const gfx::SizeF& size_in_points,
                        const char* expected_md5_hash) {
-  int width_in_pixels = printing::ConvertUnit(
+  int width_in_pixels =
-      size_in_points.width, printing::kPointsPerInch, printing::kDefaultPdfDpi);
+      printing::ConvertUnit(size_in_points.width(), printing::kPointsPerInch,
+                            printing::kDefaultPdfDpi);
   int height_in_pixels =
-      printing::ConvertUnit(size_in_points.height, printing::kPointsPerInch,
+      printing::ConvertUnit(size_in_points.height(), printing::kPointsPerInch,
                             printing::kDefaultPdfDpi);
 
   const gfx::Rect page_rect(width_in_pixels, height_in_pixels);