[PDF] Remove PDFiumEngine to PDFiumPage pass-through methods

Delete many pass-through methods that existed because of the PDFEngine
interface. Now that the interface is gone, callers can get access to
PDFiumPage from PDFiumEngine, and then call PDFiumPage methods directly.

Upgrade some DCHECKs to CHECKs along the way.

Change-Id: Ib859216cb1bd8f2af1d40528d797d2f6722f27b6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5906281
Commit-Queue: Lei Zhang <thestig@chromium.org>
Reviewed-by: Ramin Halavati <rhalavati@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1363484}

pdf/accessibility.cc

@@ -20,12 +20,10 @@ namespace chrome_pdf {
 namespace {
 
 AccessibilityFormFieldInfo GetAccessibilityFormFieldInfo(
-    PDFiumEngine* engine,
-    int32_t page_index,
+    PDFiumPage* page,
     uint32_t text_run_count) {
   AccessibilityFormFieldInfo form_field_info;
-  form_field_info.text_fields =
-      engine->GetTextFieldInfo(page_index, text_run_count);
+  form_field_info.text_fields = page->GetTextFieldInfo(text_run_count);
   return form_field_info;
 }
 
@@ -37,34 +35,32 @@ void GetAccessibilityInfo(PDFiumEngine* engine,
                           std::vector<AccessibilityTextRunInfo>& text_runs,
                           std::vector<AccessibilityCharInfo>& chars,
                           AccessibilityPageObjects& page_objects) {
-  int page_count = engine->GetNumberOfPages();
-  CHECK_GE(page_index, 0);
-  CHECK_LT(page_index, page_count);
+  PDFiumPage* page = engine->GetPage(page_index);
+  CHECK(page);
 
-  int char_count = engine->GetCharCount(page_index);
+  const int raw_char_count = page->GetCharCount();
 
   // Treat a char count of -1 (error) as 0 (an empty page), since
   // other pages might have valid content.
-  if (char_count < 0)
-    char_count = 0;
+  const uint32_t char_count = std::max<uint32_t>(raw_char_count, 0);
 
   page_info.page_index = page_index;
-  page_info.bounds = engine->GetPageBoundsRect(page_index);
+  page_info.bounds = page->rect();
   page_info.char_count = char_count;
 
   chars.resize(page_info.char_count);
-  for (uint32_t i = 0; i < page_info.char_count; ++i) {
-    chars[i].unicode_character = engine->GetCharUnicode(page_index, i);
+  for (uint32_t i = 0; i < char_count; ++i) {
+    chars[i].unicode_character = page->GetCharUnicode(i);
   }
 
-  int char_index = 0;
+  uint32_t char_index = 0;
   while (char_index < char_count) {
     std::optional<AccessibilityTextRunInfo> text_run_info_result =
-        engine->GetTextRunInfo(page_index, char_index);
-    DCHECK(text_run_info_result.has_value());
+        page->GetTextRunInfo(char_index);
+    CHECK(text_run_info_result.has_value());
     const auto& text_run_info = text_run_info_result.value();
     uint32_t text_run_end = char_index + text_run_info.len;
-    DCHECK_LE(text_run_end, static_cast<uint32_t>(char_count));
+    CHECK_LE(text_run_end, char_count);
     text_runs.push_back(text_run_info);
 
     // We need to provide enough information to draw a bounding box
@@ -76,10 +72,10 @@ void GetAccessibilityInfo(PDFiumEngine* engine,
     // x coordinate of the next. The rest of the bounds of each character
     // can be computed from the bounds of the text run.
     // The same idea is used for RTL, TTB and BTT text direction.
-    gfx::RectF char_bounds = engine->GetCharBounds(page_index, char_index);
+    gfx::RectF char_bounds = page->GetCharBounds(char_index);
     for (uint32_t i = char_index; i < text_run_end - 1; i++) {
-      DCHECK_LT(i + 1, static_cast<uint32_t>(char_count));
-      gfx::RectF next_char_bounds = engine->GetCharBounds(page_index, i + 1);
+      CHECK_LT(i + 1, char_count);
+      gfx::RectF next_char_bounds = page->GetCharBounds(i + 1);
       double& char_width = chars[i].char_width;
       switch (text_run_info.direction) {
         case AccessibilityTextDirection::kNone:
@@ -110,12 +106,11 @@ void GetAccessibilityInfo(PDFiumEngine* engine,
   }
 
   page_info.text_run_count = text_runs.size();
-  page_objects.links = engine->GetLinkInfo(page_index, text_runs);
-  page_objects.images =
-      engine->GetImageInfo(page_index, page_info.text_run_count);
-  page_objects.highlights = engine->GetHighlightInfo(page_index, text_runs);
-  page_objects.form_fields = GetAccessibilityFormFieldInfo(
-      engine, page_index, page_info.text_run_count);
+  page_objects.links = page->GetLinkInfo(text_runs);
+  page_objects.images = page->GetImageInfo(page_info.text_run_count);
+  page_objects.highlights = page->GetHighlightInfo(text_runs);
+  page_objects.form_fields =
+      GetAccessibilityFormFieldInfo(page, page_info.text_run_count);
 }
 
 }  // namespace chrome_pdf

pdf/pdf_view_web_plugin.cc

@@ -1487,8 +1487,10 @@ void PdfViewWebPlugin::HandleGetPageBoundingBoxMessage(
   base::Value::Dict reply =
       PrepareReplyMessage("getPageBoundingBoxReply", message);
 
-  gfx::RectF bounding_box = engine_->GetPageBoundingBox(page_index);
-  gfx::Rect page_bounds = engine_->GetPageBoundsRect(page_index);
+  PDFiumPage* page = engine_->GetPage(page_index);
+  CHECK(page);
+  gfx::RectF bounding_box = page->GetBoundingBox();
+  gfx::Rect page_bounds = page->rect();
 
   // Flip the origin from bottom-left to top-left.
   bounding_box.set_y(static_cast<float>(page_bounds.height()) -

pdf/pdfium/accessibility_unittest.cc

@@ -663,8 +663,11 @@ TEST_P(AccessibilityTest, SetSelectionAndScroll) {
     action_data.selection_start_index.char_index = sel_action.start_char_index;
     action_data.selection_end_index.page_index = sel_action.end_page_index;
     action_data.selection_end_index.char_index = sel_action.end_char_index;
-    gfx::Rect char_bounds = gfx::ToEnclosingRect(engine->GetCharBounds(
-        sel_action.start_page_index, sel_action.start_char_index));
+
+    PDFiumPage& page =
+        GetPDFiumPageForTest(*engine, sel_action.start_page_index);
+    gfx::Rect char_bounds =
+        gfx::ToEnclosingRect(page.GetCharBounds(sel_action.start_char_index));
     action_data.target_rect = {{char_bounds.x(), char_bounds.y() + 400 * index},
                                char_bounds.size()};
 

pdf/pdfium/pdfium_engine.cc

@@ -2591,10 +2591,6 @@ int PDFiumEngine::GetMostVisiblePage() {
   return most_visible_page_;
 }
 
-gfx::Rect PDFiumEngine::GetPageBoundsRect(int index) {
-  return pages_[index]->rect();
-}
-
 gfx::Rect PDFiumEngine::GetPageContentsRect(int index) {
   return GetScreenRect(pages_[index]->rect());
 }
@@ -2619,61 +2615,11 @@ void PDFiumEngine::HandleLongPress(const blink::WebTouchEvent& event) {
   OnMouseDown(mouse_event);
 }
 
-int PDFiumEngine::GetCharCount(int page_index) {
-  DCHECK(PageIndexInBounds(page_index));
-  return pages_[page_index]->GetCharCount();
-}
-
-gfx::RectF PDFiumEngine::GetCharBounds(int page_index, int char_index) {
-  DCHECK(PageIndexInBounds(page_index));
-  return pages_[page_index]->GetCharBounds(char_index);
-}
-
-uint32_t PDFiumEngine::GetCharUnicode(int page_index, int char_index) {
-  DCHECK(PageIndexInBounds(page_index));
-  return pages_[page_index]->GetCharUnicode(char_index);
-}
-
-std::optional<AccessibilityTextRunInfo> PDFiumEngine::GetTextRunInfo(
-    int page_index,
-    int start_char_index) {
-  DCHECK(PageIndexInBounds(page_index));
-  return pages_[page_index]->GetTextRunInfo(start_char_index);
-}
-
-std::vector<AccessibilityLinkInfo> PDFiumEngine::GetLinkInfo(
-    int page_index,
-    const std::vector<AccessibilityTextRunInfo>& text_runs) {
-  DCHECK(PageIndexInBounds(page_index));
-  return pages_[page_index]->GetLinkInfo(text_runs);
-}
-
-std::vector<AccessibilityImageInfo> PDFiumEngine::GetImageInfo(
-    int page_index,
-    uint32_t text_run_count) {
-  DCHECK(PageIndexInBounds(page_index));
-  return pages_[page_index]->GetImageInfo(text_run_count);
-}
-
 SkBitmap PDFiumEngine::GetImageForOcr(int page_index, int image_index) {
   DCHECK(PageIndexInBounds(page_index));
   return pages_[page_index]->GetImageForOcr(image_index);
 }
 
-std::vector<AccessibilityHighlightInfo> PDFiumEngine::GetHighlightInfo(
-    int page_index,
-    const std::vector<AccessibilityTextRunInfo>& text_runs) {
-  DCHECK(PageIndexInBounds(page_index));
-  return pages_[page_index]->GetHighlightInfo(text_runs);
-}
-
-std::vector<AccessibilityTextFieldInfo> PDFiumEngine::GetTextFieldInfo(
-    int page_index,
-    uint32_t text_run_count) {
-  DCHECK(PageIndexInBounds(page_index));
-  return pages_[page_index]->GetTextFieldInfo(text_run_count);
-}
-
 bool PDFiumEngine::GetPrintScaling() {
   return !!FPDF_VIEWERREF_GetPrintScaling(doc());
 }
@@ -3468,14 +3414,6 @@ gfx::Rect PDFiumEngine::GetScreenRect(const gfx::Rect& rect) const {
   return draw_utils::GetScreenRect(rect, position_, current_zoom_);
 }
 
-gfx::RectF PDFiumEngine::GetPageBoundingBox(int page_index) {
-  PDFiumPage* page = GetPage(page_index);
-  if (!page) {
-    return gfx::RectF();
-  }
-  return page->GetBoundingBox();
-}
-
 void PDFiumEngine::Highlight(const RegionData& region,
                              const gfx::Rect& rect,
                              SkColor color,

pdf/pdfium/pdfium_engine.h

@@ -79,11 +79,6 @@ class Thumbnail;
 enum class AccessibilityScrollAlignment;
 struct AccessibilityActionData;
 struct AccessibilityFocusInfo;
-struct AccessibilityHighlightInfo;
-struct AccessibilityImageInfo;
-struct AccessibilityLinkInfo;
-struct AccessibilityTextFieldInfo;
-struct AccessibilityTextRunInfo;
 struct DocumentAttachmentInfo;
 struct DocumentMetadata;
 struct PageCharacterIndex;
@@ -290,9 +285,6 @@ class PDFiumEngine : public DocumentLoader::Client, public IFSDK_PAUSE {
   // Gets the current layout orientation.
   PageOrientation GetCurrentOrientation() const;
 
-  // Gets the rectangle of the page not including the shadow.
-  gfx::Rect GetPageBoundsRect(int index);
-
   // Gets the rectangle of the page excluding any additional areas.
   virtual gfx::Rect GetPageContentsRect(int index);
 
@@ -300,58 +292,12 @@ class PDFiumEngine : public DocumentLoader::Client, public IFSDK_PAUSE {
   // border areas and bottom separator.
   virtual gfx::Rect GetPageScreenRect(int page_index) const;
 
-  // Return a page's bounding box rectangle, or an empty rectangle if
-  // `page_index` is invalid.
-  gfx::RectF GetPageBoundingBox(int page_index);
-
   // Set color / grayscale rendering modes.
   virtual void SetGrayscale(bool grayscale);
 
-  // Get the number of characters on a given page.
-  int GetCharCount(int page_index);
-
-  // Get the bounds in page pixels of a character on a given page.
-  gfx::RectF GetCharBounds(int page_index, int char_index);
-
-  // Get a given unicode character on a given page.
-  uint32_t GetCharUnicode(int page_index, int char_index);
-
-  // Given a start char index, find the longest continuous run of text that's
-  // in a single direction and with the same text style. Return a filled out
-  // AccessibilityTextRunInfo on success or std::nullopt on failure. e.g. When
-  // `start_char_index` is out of bounds.
-  std::optional<AccessibilityTextRunInfo> GetTextRunInfo(int page_index,
-                                                         int start_char_index);
-
-  // For all the links on page `page_index`, get their urls, underlying text
-  // ranges and bounding boxes.
-  std::vector<AccessibilityLinkInfo> GetLinkInfo(
-      int page_index,
-      const std::vector<AccessibilityTextRunInfo>& text_runs);
-
-  // For all the images in page `page_index`, get their alt texts and bounding
-  // boxes. If the alt text is empty or unavailable, and if the user has
-  // requested that the OCR service tag the PDF so that it is made accessible,
-  // transfer the raw image pixels in the `image_data` field. Otherwise do not
-  // populate the `image_data` field.
-  std::vector<AccessibilityImageInfo> GetImageInfo(int page_index,
-                                                   uint32_t text_run_count);
-
   // Returns the image as a 32-bit bitmap format for OCR.
   SkBitmap GetImageForOcr(int page_index, int image_index);
 
-  // For all the highlights in page `page_index`, get their underlying text
-  // ranges and bounding boxes.
-  std::vector<AccessibilityHighlightInfo> GetHighlightInfo(
-      int page_index,
-      const std::vector<AccessibilityTextRunInfo>& text_runs);
-
-  // For all the text fields in page `page_index`, get their properties like
-  // name, value, bounding boxes etc.
-  std::vector<AccessibilityTextFieldInfo> GetTextFieldInfo(
-      int page_index,
-      uint32_t text_run_count);
-
   // Gets the PDF document's print scaling preference. True if the document can
   // be scaled to fit.
   bool GetPrintScaling();

pdf/pdfium/pdfium_page_unittest.cc

@@ -678,19 +678,19 @@ TEST_P(PDFiumPageTextTest, TextRunBounds) {
 
   constexpr int kFirstRunStartIndex = 0;
   constexpr int kFirstRunEndIndex = 20;
-  constexpr int kPageIndex = 0;
+  PDFiumPage& page = GetPDFiumPageForTest(*engine, 0);
   std::optional<AccessibilityTextRunInfo> text_run_info_1 =
-      engine->GetTextRunInfo(kPageIndex, kFirstRunStartIndex);
+      page.GetTextRunInfo(kFirstRunStartIndex);
   ASSERT_TRUE(text_run_info_1.has_value());
 
   const auto& actual_text_run_1 = text_run_info_1.value();
   EXPECT_EQ(21u, actual_text_run_1.len);
 
-  EXPECT_TRUE(base::IsUnicodeWhitespace(
-      engine->GetCharUnicode(kPageIndex, kFirstRunStartIndex)));
+  EXPECT_TRUE(
+      base::IsUnicodeWhitespace(page.GetCharUnicode(kFirstRunStartIndex)));
   gfx::RectF text_run_bounds = actual_text_run_1.bounds;
-  EXPECT_TRUE(text_run_bounds.Contains(
-      engine->GetCharBounds(kPageIndex, kFirstRunStartIndex)));
+  EXPECT_TRUE(
+      text_run_bounds.Contains(page.GetCharBounds(kFirstRunStartIndex)));
 
   // Last non-space character should fall in the bounding box of the text run.
   // Text run looks like this:
@@ -700,14 +700,13 @@ TEST_P(PDFiumPageTextTest, TextRunBounds) {
   // Finally generated text run: " Hello, world! \r\n \r\n "
   constexpr int kFirstRunLastNonSpaceCharIndex = 13;
   EXPECT_FALSE(base::IsUnicodeWhitespace(
-      engine->GetCharUnicode(kPageIndex, kFirstRunLastNonSpaceCharIndex)));
+      page.GetCharUnicode(kFirstRunLastNonSpaceCharIndex)));
   EXPECT_TRUE(text_run_bounds.Contains(
-      engine->GetCharBounds(kPageIndex, kFirstRunLastNonSpaceCharIndex)));
+      page.GetCharBounds(kFirstRunLastNonSpaceCharIndex)));
 
-  EXPECT_TRUE(base::IsUnicodeWhitespace(
-      engine->GetCharUnicode(kPageIndex, kFirstRunEndIndex)));
-  gfx::RectF end_char_rect =
-      engine->GetCharBounds(kPageIndex, kFirstRunEndIndex);
+  EXPECT_TRUE(
+      base::IsUnicodeWhitespace(page.GetCharUnicode(kFirstRunEndIndex)));
+  gfx::RectF end_char_rect = page.GetCharBounds(kFirstRunEndIndex);
   EXPECT_FALSE(text_run_bounds.Contains(end_char_rect));
   // Equals to the length of the previous text run.
   constexpr int kSecondRunStartIndex = 21;
@@ -716,17 +715,17 @@ TEST_P(PDFiumPageTextTest, TextRunBounds) {
   // Note: The leading spaces in second text run are accounted for in the end
   // of first text run. Hence we won't see a space leading the second text run.
   std::optional<AccessibilityTextRunInfo> text_run_info_2 =
-      engine->GetTextRunInfo(kPageIndex, kSecondRunStartIndex);
+      page.GetTextRunInfo(kSecondRunStartIndex);
   ASSERT_TRUE(text_run_info_2.has_value());
 
   const auto& actual_text_run_2 = text_run_info_2.value();
   EXPECT_EQ(16u, actual_text_run_2.len);
 
-  EXPECT_FALSE(base::IsUnicodeWhitespace(
-      engine->GetCharUnicode(kPageIndex, kSecondRunStartIndex)));
+  EXPECT_FALSE(
+      base::IsUnicodeWhitespace(page.GetCharUnicode(kSecondRunStartIndex)));
   text_run_bounds = actual_text_run_2.bounds;
-  EXPECT_TRUE(text_run_bounds.Contains(
-      engine->GetCharBounds(kPageIndex, kSecondRunStartIndex)));
+  EXPECT_TRUE(
+      text_run_bounds.Contains(page.GetCharBounds(kSecondRunStartIndex)));
 
   // Last non-space character should fall in the bounding box of the text run.
   // Text run looks like this:
@@ -734,14 +733,14 @@ TEST_P(PDFiumPageTextTest, TextRunBounds) {
   // Finally generated text run: "Goodbye, world! "
   constexpr int kSecondRunLastNonSpaceCharIndex = 35;
   EXPECT_FALSE(base::IsUnicodeWhitespace(
-      engine->GetCharUnicode(kPageIndex, kSecondRunLastNonSpaceCharIndex)));
+      page.GetCharUnicode(kSecondRunLastNonSpaceCharIndex)));
   EXPECT_TRUE(text_run_bounds.Contains(
-      engine->GetCharBounds(kPageIndex, kSecondRunLastNonSpaceCharIndex)));
+      page.GetCharBounds(kSecondRunLastNonSpaceCharIndex)));
 
-  EXPECT_TRUE(base::IsUnicodeWhitespace(
-      engine->GetCharUnicode(kPageIndex, kSecondRunEndIndex)));
-  EXPECT_FALSE(text_run_bounds.Contains(
-      engine->GetCharBounds(kPageIndex, kSecondRunEndIndex)));
+  EXPECT_TRUE(
+      base::IsUnicodeWhitespace(page.GetCharUnicode(kSecondRunEndIndex)));
+  EXPECT_FALSE(
+      text_run_bounds.Contains(page.GetCharBounds(kSecondRunEndIndex)));
 }
 
 TEST_P(PDFiumPageTextTest, GetTextRunInfo) {
@@ -796,14 +795,15 @@ TEST_P(PDFiumPageTextTest, GetTextRunInfo) {
   }
 
   // Test negative char index returns nullopt
+  PDFiumPage& page = GetPDFiumPageForTest(*engine, 0);
   std::optional<AccessibilityTextRunInfo> text_run_info_result =
-      engine->GetTextRunInfo(0, -1);
+      page.GetTextRunInfo(-1);
   ASSERT_FALSE(text_run_info_result.has_value());
 
   // Test valid char index returns expected text run info and expected text
   // style info
   for (const auto& expected_text_run : expected_text_runs) {
-    text_run_info_result = engine->GetTextRunInfo(0, current_char_index);
+    text_run_info_result = page.GetTextRunInfo(current_char_index);
     ASSERT_TRUE(text_run_info_result.has_value());
     const auto& actual_text_run = text_run_info_result.value();
     CompareTextRuns(expected_text_run, actual_text_run);
@@ -811,9 +811,8 @@ TEST_P(PDFiumPageTextTest, GetTextRunInfo) {
   }
 
   // Test char index outside char range returns nullopt
-  PDFiumPage& page = GetPDFiumPageForTest(*engine, 0);
   EXPECT_EQ(page.GetCharCount(), current_char_index);
-  text_run_info_result = engine->GetTextRunInfo(0, current_char_index);
+  text_run_info_result = page.GetTextRunInfo(current_char_index);
   ASSERT_FALSE(text_run_info_result.has_value());
 }
 
@@ -848,10 +847,11 @@ TEST_P(PDFiumPageTextTest, HighlightTextRunInfo) {
         gfx::RectF(198.66667f, 201.33333f, 21.333328f, 12.000015f);
   }
 
+  PDFiumPage& page = GetPDFiumPageForTest(*engine, 0);
   int current_char_index = 0;
   for (const auto& expected_text_run : expected_text_runs) {
     std::optional<AccessibilityTextRunInfo> text_run_info_result =
-        engine->GetTextRunInfo(0, current_char_index);
+        page.GetTextRunInfo(current_char_index);
     ASSERT_TRUE(text_run_info_result.has_value());
     const auto& actual_text_run = text_run_info_result.value();
     CompareTextRuns(expected_text_run, actual_text_run);