Simplify PDFEngine::GetTextRunInfo().

Change it to return base::Optional<PP_PrivateAccessibilityTextRunInfo>,
instead of returning the individual elements of
PP_PrivateAccessibilityTextRunInfo as out parameters. Update the caller
of GetTextRunInfo() to std::move() the text run into the vector that
holds all the text runs, and simplify calculations for char indices in
the caller.

Change-Id: I719119935ea201f5aed7d1d41e706831a1696c90
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1769220
Commit-Queue: Lei Zhang <thestig@chromium.org>
Reviewed-by: Ian Prest <iapres@microsoft.com>
Reviewed-by: Kevin Babbitt <kbabbitt@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#690901}

pdf/accessibility.cc

@@ -39,10 +39,12 @@ bool GetAccessibilityInfo(
 
   int char_index = 0;
   while (char_index < char_count) {
-    PP_PrivateAccessibilityTextRunInfo text_run_info;
-    engine->GetTextRunInfo(page_index, char_index, &text_run_info);
-    DCHECK_LE(char_index + text_run_info.len,
-              static_cast<uint32_t>(char_count));
+    base::Optional<PP_PrivateAccessibilityTextRunInfo> text_run_info_result =
+        engine->GetTextRunInfo(page_index, char_index);
+    DCHECK(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));
     text_runs->push_back(text_run_info);
 
     // We need to provide enough information to draw a bounding box
@@ -55,11 +57,10 @@ bool GetAccessibilityInfo(
     // can be computed from the bounds of the text run.
     // The same idea is used for RTL, TTB and BTT text direction.
     pp::FloatRect char_bounds = engine->GetCharBounds(page_index, char_index);
-    for (uint32_t i = 0; i < text_run_info.len - 1; i++) {
-      DCHECK_LT(char_index + i + 1, static_cast<uint32_t>(char_count));
-      pp::FloatRect next_char_bounds =
-          engine->GetCharBounds(page_index, char_index + i + 1);
-      double& char_width = (*chars)[char_index + i].char_width;
+    for (uint32_t i = char_index; i < text_run_end - 1; i++) {
+      DCHECK_LT(i + 1, static_cast<uint32_t>(char_count));
+      pp::FloatRect next_char_bounds = engine->GetCharBounds(page_index, i + 1);
+      double& char_width = (*chars)[i].char_width;
       switch (text_run_info.direction) {
         case PP_PRIVATEDIRECTION_NONE:
         case PP_PRIVATEDIRECTION_LTR:
@@ -77,8 +78,7 @@ bool GetAccessibilityInfo(
       }
       char_bounds = next_char_bounds;
     }
-    double& char_width =
-        (*chars)[char_index + text_run_info.len - 1].char_width;
+    double& char_width = (*chars)[text_run_end - 1].char_width;
     if (text_run_info.direction == PP_PRIVATEDIRECTION_BTT ||
         text_run_info.direction == PP_PRIVATEDIRECTION_TTB) {
       char_width = char_bounds.height();

pdf/pdf_engine.h

@@ -359,13 +359,12 @@ class PDFEngine {
   // Get a given unicode character on a given page.
   virtual uint32_t GetCharUnicode(int page_index, int char_index) = 0;
   // Given a start char index, find the longest continuous run of text that's
-  // in a single direction and with the same style and font size. Fill the
-  // |text_run_info| with the length of that sequence, text direction, bounding
-  // box and font size.
-  virtual void GetTextRunInfo(
+  // in a single direction and with the same style and font size. Return a
+  // filled out PP_PrivateAccessibilityTextRunInfo on success or base::nullopt
+  // on failure. e.g. When |start_char_index| is out of bounds.
+  virtual base::Optional<PP_PrivateAccessibilityTextRunInfo> GetTextRunInfo(
       int page_index,
-      int start_char_index,
-      PP_PrivateAccessibilityTextRunInfo* text_run_info) = 0;
+      int start_char_index) = 0;
   // Gets the PDF document's print scaling preference. True if the document can
   // be scaled to fit.
   virtual bool GetPrintScaling() = 0;

pdf/pdfium/pdfium_engine.cc

@@ -2189,12 +2189,11 @@ uint32_t PDFiumEngine::GetCharUnicode(int page_index, int char_index) {
   return pages_[page_index]->GetCharUnicode(char_index);
 }
 
-void PDFiumEngine::GetTextRunInfo(
+base::Optional<PP_PrivateAccessibilityTextRunInfo> PDFiumEngine::GetTextRunInfo(
     int page_index,
-    int start_char_index,
-    PP_PrivateAccessibilityTextRunInfo* text_run_info) {
+    int start_char_index) {
   DCHECK(PageIndexInBounds(page_index));
-  return pages_[page_index]->GetTextRunInfo(start_char_index, text_run_info);
+  return pages_[page_index]->GetTextRunInfo(start_char_index);
 }
 
 bool PDFiumEngine::GetPrintScaling() {

pdf/pdfium/pdfium_engine.h

@@ -113,10 +113,9 @@ class PDFiumEngine : public PDFEngine,
   int GetCharCount(int page_index) override;
   pp::FloatRect GetCharBounds(int page_index, int char_index) override;
   uint32_t GetCharUnicode(int page_index, int char_index) override;
-  void GetTextRunInfo(
+  base::Optional<PP_PrivateAccessibilityTextRunInfo> GetTextRunInfo(
       int page_index,
-      int start_char_index,
-      PP_PrivateAccessibilityTextRunInfo* text_run_info) override;
+      int start_char_index) override;
   bool GetPrintScaling() override;
   int GetCopiesToPrint() override;
   int GetDuplexType() override;

pdf/pdfium/pdfium_page.cc

@@ -251,20 +251,14 @@ FPDF_TEXTPAGE PDFiumPage::GetTextPage() {
   return text_page();
 }
 
-void PDFiumPage::GetTextRunInfo(
-    int start_char_index,
-    PP_PrivateAccessibilityTextRunInfo* text_run_info) {
+base::Optional<PP_PrivateAccessibilityTextRunInfo> PDFiumPage::GetTextRunInfo(
+    int start_char_index) {
   FPDF_PAGE page = GetPage();
   FPDF_TEXTPAGE text_page = GetTextPage();
   int chars_count = FPDFText_CountChars(text_page);
   // Check to make sure |start_char_index| is within bounds.
-  if (start_char_index < 0 || start_char_index >= chars_count) {
-    text_run_info->len = 0;
-    text_run_info->font_size = 0;
-    text_run_info->bounds = pp::FloatRect();
-    text_run_info->direction = PP_PRIVATEDIRECTION_NONE;
-    return;
-  }
+  if (start_char_index < 0 || start_char_index >= chars_count)
+    return base::nullopt;
 
   int actual_start_char_index = GetFirstNonUnicodeWhiteSpaceCharIndex(
       text_page, start_char_index, chars_count);
@@ -274,11 +268,12 @@ void PDFiumPage::GetTextRunInfo(
     // If so, |text_run_info->len| needs to take the number of characters
     // iterated into account.
     DCHECK_GT(actual_start_char_index, start_char_index);
-    text_run_info->len = chars_count - start_char_index;
-    text_run_info->font_size = 0;
-    text_run_info->bounds = pp::FloatRect();
-    text_run_info->direction = PP_PRIVATEDIRECTION_NONE;
-    return;
+    PP_PrivateAccessibilityTextRunInfo info;
+    info.len = chars_count - start_char_index;
+    info.font_size = 0;
+    info.bounds = pp::FloatRect();
+    info.direction = PP_PRIVATEDIRECTION_NONE;
+    return info;
   }
   int char_index = actual_start_char_index;
 
@@ -383,16 +378,18 @@ void PDFiumPage::GetTextRunInfo(
     text_run_font_size = estimated_font_size;
   }
 
+  PP_PrivateAccessibilityTextRunInfo info;
+  info.len = char_index - start_char_index;
+  info.font_size = text_run_font_size;
+  info.bounds = text_run_bounds;
   // Infer text direction from first and last character of the text run. We
   // can't base our decision on the character direction, since a character of a
   // RTL language will have an angle of 0 when not rotated, just like a
   // character in a LTR language.
-  text_run_info->direction = char_index - actual_start_char_index > 1
-                                 ? GetDirectionFromAngle(text_run_angle)
-                                 : PP_PRIVATEDIRECTION_NONE;
-  text_run_info->len = char_index - start_char_index;
-  text_run_info->font_size = text_run_font_size;
-  text_run_info->bounds = text_run_bounds;
+  info.direction = char_index - actual_start_char_index > 1
+                       ? GetDirectionFromAngle(text_run_angle)
+                       : PP_PRIVATEDIRECTION_NONE;
+  return info;
 }
 
 uint32_t PDFiumPage::GetCharUnicode(int char_index) {

pdf/pdfium/pdfium_page.h

@@ -44,12 +44,9 @@ class PDFiumPage {
   // Returns FPDF_TEXTPAGE for the page, loading and parsing it if necessary.
   FPDF_TEXTPAGE GetTextPage();
 
-  // Given a start char index, find the longest continuous run of text that's
-  // in a single direction and with the same style and font size. Fill the
-  // |text_run_info| with the length of that sequence, text direction, bounding
-  // box and font size.
-  void GetTextRunInfo(int start_char_index,
-                      PP_PrivateAccessibilityTextRunInfo* text_run_info);
+  // See definition of PDFEngine::GetTextRunInfo().
+  base::Optional<PP_PrivateAccessibilityTextRunInfo> GetTextRunInfo(
+      int start_char_index);
   // Get a unicode character from the page.
   uint32_t GetCharUnicode(int char_index);
   // Get the bounds of a character in page pixels.