[PDF] Add Thumbnail::CalculateImageSize() to avoid unneeded memory alloc

Currently, several places instantiate the Thumbnail class, which
allocates memory for the thumbnail image data, only to never use the
allocated memory. This is because the Thumbnail instances are being used
solely to get the image size.

Add a dedicated CalculateImageSize() method for doing just that, so
callers that only care about the image size can get it without
instantiating a Thumbnail.

Change-Id: I3f49c4e5902f3832448fb973e481e34e27ce03b5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6290593
Reviewed-by: Alan Screen <awscreen@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1423294}

pdf/pdf_ink_module_unittest.cc

@@ -255,9 +255,8 @@ class FakeClient : public PdfInkModuleClient {
   gfx::Size GetThumbnailSize(int page_index) override {
     CHECK_GE(page_index, 0);
     CHECK_LT(static_cast<size_t>(page_index), page_layouts_.size());
-    Thumbnail thumbnail(page_layouts_[page_index].size(),
-                        /*device_pixel_ratio=*/1);
-    return thumbnail.image_size();
+    return Thumbnail::CalculateImageSize(page_layouts_[page_index].size(),
+                                         /*device_pixel_ratio=*/1);
   }
 
   gfx::Vector2dF GetViewportOriginOffset() override {

pdf/pdfium/pdfium_page.cc

@@ -93,6 +93,10 @@ std::optional<Rotation> GetRotationFromRawValue(int rotation) {
   }
 }
 
+gfx::SizeF GetPageSizeInPoints(FPDF_PAGE page) {
+  return gfx::SizeF(FPDF_GetPageWidthF(page), FPDF_GetPageHeightF(page));
+}
+
 gfx::RectF FloatPageRectToPixelRect(FPDF_PAGE page, const gfx::RectF& input) {
   int output_width = FPDF_GetPageWidthF(page);
   int output_height = FPDF_GetPageHeightF(page);
@@ -769,7 +773,7 @@ gfx::RectF PDFiumPage::GetBoundingBox() {
   }
 
   // Page width and height are already swapped based on page rotation.
-  gfx::SizeF page_size(FPDF_GetPageWidthF(page), FPDF_GetPageHeightF(page));
+  gfx::SizeF page_size = GetPageSizeInPoints(page);
 
   // Start with bounds with the left and bottom values at the max possible
   // bounds and the right and top values at the min possible bounds. Bounds are
@@ -1834,7 +1838,7 @@ Thumbnail PDFiumPage::GenerateThumbnail(float device_pixel_ratio) {
   const bool has_alpha = !!FPDFPage_HasTransparency(page);
   const int format = has_alpha ? FPDFBitmap_BGRA : FPDFBitmap_BGRx;
 
-  Thumbnail thumbnail = CreateThumbnail(device_pixel_ratio);
+  Thumbnail thumbnail(GetPageSizeInPoints(page), device_pixel_ratio);
   const gfx::Size& image_size = thumbnail.image_size();
 
   // Create and initialize the bitmap.
@@ -1865,7 +1869,10 @@ Thumbnail PDFiumPage::GenerateThumbnail(float device_pixel_ratio) {
 
 #if BUILDFLAG(ENABLE_PDF_INK2)
 gfx::Size PDFiumPage::GetThumbnailSize(float device_pixel_ratio) {
-  return CreateThumbnail(device_pixel_ratio).image_size();
+  CHECK(available());
+  FPDF_PAGE page = GetPage();
+  return Thumbnail::CalculateImageSize(GetPageSizeInPoints(page),
+                                       device_pixel_ratio);
 }
 #endif
 
@@ -1874,15 +1881,6 @@ void PDFiumPage::GenerateAndSendThumbnail(float device_pixel_ratio,
   std::move(send_callback).Run(GenerateThumbnail(device_pixel_ratio));
 }
 
-Thumbnail PDFiumPage::CreateThumbnail(float device_pixel_ratio) {
-  CHECK(available());
-
-  FPDF_PAGE page = GetPage();
-  return Thumbnail(
-      gfx::SizeF(FPDF_GetPageWidthF(page), FPDF_GetPageHeightF(page)),
-      device_pixel_ratio);
-}
-
 void PDFiumPage::MarkAvailable() {
   available_ = true;
 

pdf/pdfium/pdfium_page.h

@@ -464,11 +464,6 @@ class PDFiumPage {
   void GenerateAndSendThumbnail(float device_pixel_ratio,
                                 SendThumbnailCallback send_callback);
 
-  // Creates a `Thumbnail` for a given `device_pixel_ratio` using this page's
-  // size. The caller is responsible for rendering the page content into the
-  // thumbnail.
-  Thumbnail CreateThumbnail(float device_pixel_ratio);
-
   raw_ptr<PDFiumEngine> engine_;
   ScopedFPDFPage page_;
   ScopedFPDFTextPage text_page_;

pdf/ui/thumbnail.cc

@@ -22,9 +22,6 @@ namespace chrome_pdf {
 
 namespace {
 
-constexpr float kMinDevicePixelRatio = 0.25;
-constexpr float kMaxDevicePixelRatio = 2;
-
 constexpr int kImageColorChannels = 4;
 
 // TODO(crbug.com/40511452): Reevaluate the thumbnail size cap when the PDF
@@ -49,6 +46,13 @@ constexpr int kPdfPageMinDimension = 3;
 constexpr int kPdfPageMaxDimension = 14400;
 constexpr int kPdfMaxAspectRatio = kPdfPageMaxDimension / kPdfPageMinDimension;
 
+float ClampDevicePixelRatio(float device_pixel_ratio) {
+  static constexpr float kMinDevicePixelRatio = 0.25;
+  static constexpr float kMaxDevicePixelRatio = 2;
+  return std::clamp(device_pixel_ratio, kMinDevicePixelRatio,
+                    kMaxDevicePixelRatio);
+}
+
 // Limit the proportions within PDF limits to handle pathological PDF pages.
 gfx::SizeF LimitAspectRatio(gfx::SizeF page_size) {
   // Bump up any lengths of 0 to 1.
@@ -111,10 +115,15 @@ size_t CalculateImageDataSize(int stride, int height) {
 
 }  // namespace
 
+// static
+gfx::Size Thumbnail::CalculateImageSize(const gfx::SizeF& page_size,
+                                        float device_pixel_ratio) {
+  return CalculateBestFitSize(page_size,
+                              ClampDevicePixelRatio(device_pixel_ratio));
+}
+
 Thumbnail::Thumbnail(const gfx::SizeF& page_size, float device_pixel_ratio)
-    : device_pixel_ratio_(std::clamp(device_pixel_ratio,
-                                     kMinDevicePixelRatio,
-                                     kMaxDevicePixelRatio)),
+    : device_pixel_ratio_(ClampDevicePixelRatio(device_pixel_ratio)),
       image_size_(CalculateBestFitSize(page_size, device_pixel_ratio_)),
       stride_(CalculateStride(image_size_.width())),
       image_data_(CalculateImageDataSize(stride(), image_size().height())) {

pdf/ui/thumbnail.h

@@ -21,6 +21,11 @@ using SendThumbnailCallback = base::OnceCallback<void(Thumbnail)>;
 
 class Thumbnail final {
  public:
+  // This is the equivalent to Thumbnail(...).image_size(), without the need to
+  // allocate memory for `image_data_`.
+  static gfx::Size CalculateImageSize(const gfx::SizeF& page_size,
+                                      float device_pixel_ratio);
+
   // `page_size` is in points.
   Thumbnail(const gfx::SizeF& page_size, float device_pixel_ratio);
   Thumbnail(Thumbnail&& other) noexcept;

pdf/ui/thumbnail_unittest.cc

@@ -24,8 +24,14 @@ struct BestFitSizeParams {
 void TestBestFitSize(const BestFitSizeParams& params) {
   Thumbnail thumbnail(params.page_size, params.device_pixel_ratio);
   EXPECT_EQ(thumbnail.image_size(), params.expected_thumbnail_size)
-      << "Failed for page of size of " << params.page_size.ToString()
+      << "Failed for ctor with page of size of " << params.page_size.ToString()
       << " and device to pixel ratio of " << params.device_pixel_ratio;
+  EXPECT_EQ(Thumbnail::CalculateImageSize(params.page_size,
+                                          params.device_pixel_ratio),
+            params.expected_thumbnail_size)
+      << "Failed for CalculateImageSize() with page of size of "
+      << params.page_size.ToString() << " and device to pixel ratio of "
+      << params.device_pixel_ratio;
 }
 
 TEST(ThumbnailTest, CalculateBestFitSizeNormal) {