Remove chrome_pdf::CompareRect() functions.

Updates tests that call these functions to use gfx equivalents. Make
direct conversions when possible. For pp::PDF structs used as test
expectations, clone the structs and switch the clones to use gfx::RectF
instead of pp::FloatRect. Then call RectFFromPPFloatRect() on the actual
pp::PDF rects from production code to make comparisons possible.

Bug: 1101101
Change-Id: I82f5d19a0cf0c355a45f57370120e071fe08a018
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2425746
Commit-Queue: Lei Zhang <thestig@chromium.org>
Reviewed-by: Ankit Kumar 🌪️ <ankk@microsoft.com>
Reviewed-by: K. Moon <kmoon@chromium.org>
Cr-Commit-Position: refs/heads/master@{#809977}

pdf/BUILD.gn

@@ -271,8 +271,6 @@ if (enable_pdf) {
       "test/test_client.h",
       "test/test_document_loader.cc",
       "test/test_document_loader.h",
-      "test/test_utils.cc",
-      "test/test_utils.h",
     ]
 
     configs += [ "//build/config/compiler:noshadowing" ]

pdf/pdfium/accessibility_unittest.cc

@@ -4,10 +4,12 @@
 
 #include "pdf/accessibility.h"
 
+#include <string>
+
 #include "pdf/pdfium/pdfium_engine.h"
 #include "pdf/pdfium/pdfium_test_base.h"
+#include "pdf/ppapi_migration/geometry_conversions.h"
 #include "pdf/test/test_client.h"
-#include "pdf/test/test_utils.h"
 #include "ppapi/c/private/ppp_pdf.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/gfx/geometry/rect_f.h"
@@ -112,12 +114,16 @@ TEST_F(AccessibilityTest, GetAccessibilityPage) {
 }
 
 TEST_F(AccessibilityTest, GetAccessibilityImageInfo) {
-  static const pp::PDF::PrivateAccessibilityImageInfo kExpectedImageInfo[] = {
-      {"Image 1", 0, {{380, 78}, {67, 68}}},
-      {"Image 2", 0, {{380, 385}, {27, 28}}},
-      {"Image 3", 0, {{380, 678}, {1, 1}}}};
+  // Clone of pp::PDF::PrivateAccessibilityImageInfo.
+  static const struct {
+    std::string alt_text;
+    uint32_t text_run_index;
+    gfx::RectF bounds;
+  } kExpectedImageInfo[] = {{"Image 1", 0, {380, 78, 67, 68}},
+                            {"Image 2", 0, {380, 385, 27, 28}},
+                            {"Image 3", 0, {380, 678, 1, 1}}};
 
-  static const pp::Rect kExpectedPageRect = {{5, 3}, {816, 1056}};
+  static constexpr gfx::Rect kExpectedPageRect(5, 3, 816, 1056);
 
   TestClient client;
   std::unique_ptr<PDFiumEngine> engine =
@@ -132,14 +138,15 @@ TEST_F(AccessibilityTest, GetAccessibilityImageInfo) {
   ASSERT_TRUE(GetAccessibilityInfo(engine.get(), 0, &page_info, &text_runs,
                                    &chars, &page_objects));
   EXPECT_EQ(0u, page_info.page_index);
-  CompareRect(kExpectedPageRect, page_info.bounds);
+  EXPECT_EQ(kExpectedPageRect, RectFromPPRect(page_info.bounds));
   EXPECT_EQ(text_runs.size(), page_info.text_run_count);
   EXPECT_EQ(chars.size(), page_info.char_count);
   ASSERT_EQ(page_objects.images.size(), base::size(kExpectedImageInfo));
 
   for (size_t i = 0; i < page_objects.images.size(); ++i) {
     EXPECT_EQ(page_objects.images[i].alt_text, kExpectedImageInfo[i].alt_text);
-    CompareRect(kExpectedImageInfo[i].bounds, page_objects.images[i].bounds);
+    EXPECT_EQ(kExpectedImageInfo[i].bounds,
+              RectFFromPPFloatRect(page_objects.images[i].bounds));
     EXPECT_EQ(page_objects.images[i].text_run_index,
               kExpectedImageInfo[i].text_run_index);
   }
@@ -445,17 +452,23 @@ TEST_F(AccessibilityTest, TestInternalLinkClickActionHandling) {
 }
 
 TEST_F(AccessibilityTest, GetAccessibilityLinkInfo) {
-  static pp::PDF::PrivateAccessibilityLinkInfo expected_link_info[] = {
-      {"http://yahoo.com", 0, 1, 1, {{75, 191}, {110, 16}}},
-      {"http://bing.com", 1, 4, 1, {{131, 121}, {138, 20}}},
-      {"http://google.com", 2, 7, 1, {{82, 67}, {161, 21}}}};
+  // Clone of pp::PDF::PrivateAccessibilityLinkInfo.
+  struct {
+    std::string url;
+    uint32_t index_in_page;
+    uint32_t text_run_index;
+    uint32_t text_run_count;
+    gfx::RectF bounds;
+  } expected_link_info[] = {{"http://yahoo.com", 0, 1, 1, {75, 191, 110, 16}},
+                            {"http://bing.com", 1, 4, 1, {131, 121, 138, 20}},
+                            {"http://google.com", 2, 7, 1, {82, 67, 161, 21}}};
 
   if (IsRunningOnChromeOS()) {
-    expected_link_info[0].bounds = {{75, 192}, {110, 15}};
-    expected_link_info[1].bounds = {{131, 120}, {138, 22}};
+    expected_link_info[0].bounds = {75, 192, 110, 15};
+    expected_link_info[1].bounds = {131, 120, 138, 22};
   }
 
-  static const pp::Rect kExpectedPageRect = {{5, 3}, {533, 266}};
+  static constexpr gfx::Rect kExpectedPageRect(5, 3, 533, 266);
 
   TestClient client;
   std::unique_ptr<PDFiumEngine> engine =
@@ -470,7 +483,7 @@ TEST_F(AccessibilityTest, GetAccessibilityLinkInfo) {
   ASSERT_TRUE(GetAccessibilityInfo(engine.get(), 0, &page_info, &text_runs,
                                    &chars, &page_objects));
   EXPECT_EQ(0u, page_info.page_index);
-  CompareRect(kExpectedPageRect, page_info.bounds);
+  EXPECT_EQ(kExpectedPageRect, RectFromPPRect(page_info.bounds));
   EXPECT_EQ(text_runs.size(), page_info.text_run_count);
   EXPECT_EQ(chars.size(), page_info.char_count);
   ASSERT_EQ(page_objects.links.size(), base::size(expected_link_info));
@@ -480,7 +493,8 @@ TEST_F(AccessibilityTest, GetAccessibilityLinkInfo) {
         page_objects.links[i];
     EXPECT_EQ(link_info.url, expected_link_info[i].url);
     EXPECT_EQ(link_info.index_in_page, expected_link_info[i].index_in_page);
-    CompareRect(expected_link_info[i].bounds, link_info.bounds);
+    EXPECT_EQ(expected_link_info[i].bounds,
+              RectFFromPPFloatRect(link_info.bounds));
     EXPECT_EQ(link_info.text_run_index, expected_link_info[i].text_run_index);
     EXPECT_EQ(link_info.text_run_count, expected_link_info[i].text_run_count);
   }
@@ -490,13 +504,20 @@ TEST_F(AccessibilityTest, GetAccessibilityHighlightInfo) {
   constexpr uint32_t kHighlightDefaultColor = MakeARGB(255, 255, 255, 0);
   constexpr uint32_t kHighlightRedColor = MakeARGB(102, 230, 0, 0);
   constexpr uint32_t kHighlightNoColor = MakeARGB(0, 0, 0, 0);
-  static const pp::PDF::PrivateAccessibilityHighlightInfo
-      kExpectedHighlightInfo[] = {
-          {"Text Note", 0, 0, 1, {{5, 196}, {49, 26}}, kHighlightDefaultColor},
-          {"", 1, 2, 1, {{110, 196}, {77, 26}}, kHighlightRedColor},
-          {"", 2, 3, 1, {{192, 196}, {13, 26}}, kHighlightNoColor}};
+  // Clone of pp::PDF::PrivateAccessibilityHighlightInfo.
+  static const struct {
+    std::string note_text;
+    uint32_t index_in_page;
+    uint32_t text_run_index;
+    uint32_t text_run_count;
+    gfx::RectF bounds;
+    uint32_t color;
+  } kExpectedHighlightInfo[] = {
+      {"Text Note", 0, 0, 1, {5, 196, 49, 26}, kHighlightDefaultColor},
+      {"", 1, 2, 1, {110, 196, 77, 26}, kHighlightRedColor},
+      {"", 2, 3, 1, {192, 196, 13, 26}, kHighlightNoColor}};
 
-  static const pp::Rect kExpectedPageRect = {{5, 3}, {533, 266}};
+  static constexpr gfx::Rect kExpectedPageRect(5, 3, 533, 266);
 
   TestClient client;
   std::unique_ptr<PDFiumEngine> engine =
@@ -511,7 +532,7 @@ TEST_F(AccessibilityTest, GetAccessibilityHighlightInfo) {
   ASSERT_TRUE(GetAccessibilityInfo(engine.get(), 0, &page_info, &text_runs,
                                    &chars, &page_objects));
   EXPECT_EQ(0u, page_info.page_index);
-  CompareRect(kExpectedPageRect, page_info.bounds);
+  EXPECT_EQ(kExpectedPageRect, RectFromPPRect(page_info.bounds));
   EXPECT_EQ(text_runs.size(), page_info.text_run_count);
   EXPECT_EQ(chars.size(), page_info.char_count);
   ASSERT_EQ(page_objects.highlights.size(), base::size(kExpectedHighlightInfo));
@@ -521,7 +542,8 @@ TEST_F(AccessibilityTest, GetAccessibilityHighlightInfo) {
         page_objects.highlights[i];
     EXPECT_EQ(highlight_info.index_in_page,
               kExpectedHighlightInfo[i].index_in_page);
-    CompareRect(kExpectedHighlightInfo[i].bounds, highlight_info.bounds);
+    EXPECT_EQ(kExpectedHighlightInfo[i].bounds,
+              RectFFromPPFloatRect(highlight_info.bounds));
     EXPECT_EQ(highlight_info.text_run_index,
               kExpectedHighlightInfo[i].text_run_index);
     EXPECT_EQ(highlight_info.text_run_count,
@@ -532,28 +554,30 @@ TEST_F(AccessibilityTest, GetAccessibilityHighlightInfo) {
 }
 
 TEST_F(AccessibilityTest, GetAccessibilityTextFieldInfo) {
-  static const pp::PDF::PrivateAccessibilityTextFieldInfo
-      kExpectedTextFieldInfo[] = {
-          {"Text Box", "Text", false, false, false, 0, 5, {138, 230, 135, 41}},
-          {"ReadOnly",
-           "Elephant",
-           true,
-           false,
-           false,
-           1,
-           5,
-           {138, 163, 135, 41}},
-          {"Required",
-           "Required Field",
-           false,
-           true,
-           false,
-           2,
-           5,
-           {138, 303, 135, 34}},
-          {"Password", "", false, false, true, 3, 5, {138, 356, 135, 35}}};
+  // Clone of pp::PDF::PrivateAccessibilityTextFieldInfo.
+  static const struct {
+    std::string name;
+    std::string value;
+    bool is_read_only;
+    bool is_required;
+    bool is_password;
+    uint32_t index_in_page;
+    uint32_t text_run_index;
+    gfx::RectF bounds;
+  } kExpectedTextFieldInfo[] = {
+      {"Text Box", "Text", false, false, false, 0, 5, {138, 230, 135, 41}},
+      {"ReadOnly", "Elephant", true, false, false, 1, 5, {138, 163, 135, 41}},
+      {"Required",
+       "Required Field",
+       false,
+       true,
+       false,
+       2,
+       5,
+       {138, 303, 135, 34}},
+      {"Password", "", false, false, true, 3, 5, {138, 356, 135, 35}}};
 
-  static const pp::Rect kExpectedPageRect = {{5, 3}, {400, 400}};
+  static constexpr gfx::Rect kExpectedPageRect(5, 3, 400, 400);
 
   TestClient client;
   std::unique_ptr<PDFiumEngine> engine =
@@ -568,7 +592,7 @@ TEST_F(AccessibilityTest, GetAccessibilityTextFieldInfo) {
   ASSERT_TRUE(GetAccessibilityInfo(engine.get(), 0, &page_info, &text_runs,
                                    &chars, &page_objects));
   EXPECT_EQ(0u, page_info.page_index);
-  CompareRect(kExpectedPageRect, page_info.bounds);
+  EXPECT_EQ(kExpectedPageRect, RectFromPPRect(page_info.bounds));
   EXPECT_EQ(text_runs.size(), page_info.text_run_count);
   EXPECT_EQ(chars.size(), page_info.char_count);
   ASSERT_EQ(page_objects.form_fields.text_fields.size(),
@@ -589,7 +613,8 @@ TEST_F(AccessibilityTest, GetAccessibilityTextFieldInfo) {
               text_field_info.index_in_page);
     EXPECT_EQ(kExpectedTextFieldInfo[i].text_run_index,
               text_field_info.text_run_index);
-    CompareRect(kExpectedTextFieldInfo[i].bounds, text_field_info.bounds);
+    EXPECT_EQ(kExpectedTextFieldInfo[i].bounds,
+              RectFFromPPFloatRect(text_field_info.bounds));
   }
 }
 

pdf/pdfium/pdfium_page_unittest.cc

@@ -20,7 +20,6 @@
 #include "pdf/pdfium/pdfium_test_base.h"
 #include "pdf/ppapi_migration/geometry_conversions.h"
 #include "pdf/test/test_client.h"
-#include "pdf/test/test_utils.h"
 #include "pdf/thumbnail.h"
 #include "ppapi/c/private/ppb_pdf.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -50,11 +49,27 @@ TEST(PDFiumPageHelperDeathTest, ToPDFiumRotation) {
 #endif
 }
 
+// Clone of pp::PDF::PrivateAccessibilityTextRunInfo.
+struct ExpectedAccessibilityTextRunInfo {
+  uint32_t len;
+  gfx::RectF bounds;
+  PP_PrivateDirection direction;
+  pp::PDF::PrivateAccessibilityTextStyleInfo style;
+};
+
 void CompareTextRuns(
-    const pp::PDF::PrivateAccessibilityTextRunInfo& expected_text_run,
-    const pp::PDF::PrivateAccessibilityTextRunInfo actual_text_run) {
+    const ExpectedAccessibilityTextRunInfo& expected_text_run,
+    const pp::PDF::PrivateAccessibilityTextRunInfo& actual_text_run) {
   EXPECT_EQ(expected_text_run.len, actual_text_run.len);
-  CompareRect(expected_text_run.bounds, actual_text_run.bounds);
+
+  // Use EXPECT_FLOAT_EQ() here instead of direct gfx::RectF comparisons to
+  // avoid having to deal with float rounding errors.
+  gfx::RectF actual_bounds = RectFFromPPFloatRect(actual_text_run.bounds);
+  EXPECT_FLOAT_EQ(expected_text_run.bounds.x(), actual_bounds.x());
+  EXPECT_FLOAT_EQ(expected_text_run.bounds.y(), actual_bounds.y());
+  EXPECT_FLOAT_EQ(expected_text_run.bounds.width(), actual_bounds.width());
+  EXPECT_FLOAT_EQ(expected_text_run.bounds.height(), actual_bounds.height());
+
   EXPECT_EQ(expected_text_run.direction, actual_text_run.direction);
 
   const pp::PDF::PrivateAccessibilityTextStyleInfo& expected_style =
@@ -358,37 +373,31 @@ TEST_F(PDFiumPageTextTest, GetTextRunInfo) {
   // The links span from [7, 22], [52, 66] and [92, 108] with 16, 15 and 17
   // text run lengths respectively. There are text runs preceding and
   // succeeding them.
-  pp::PDF::PrivateAccessibilityTextRunInfo expected_text_runs[] = {
-      {7,
-       PP_MakeFloatRectFromXYWH(26.666666f, 189.333333f, 38.666672f,
-                                13.333344f),
+  ExpectedAccessibilityTextRunInfo expected_text_runs[] = {
+      {7, gfx::RectF(26.666666f, 189.333333f, 38.666672f, 13.333344f),
        PP_PrivateDirection::PP_PRIVATEDIRECTION_LTR, expected_style_1},
-      {16,
-       PP_MakeFloatRectFromXYWH(70.666664f, 189.333333f, 108.0f, 14.666672f),
+      {16, gfx::RectF(70.666664f, 189.333333f, 108.0f, 14.666672f),
        PP_PrivateDirection::PP_PRIVATEDIRECTION_LTR, expected_style_1},
-      {20,
-       PP_MakeFloatRectFromXYWH(181.333333f, 189.333333f, 117.333333f,
-                                14.666672f),
+      {20, gfx::RectF(181.333333f, 189.333333f, 117.333333f, 14.666672f),
        PP_PrivateDirection::PP_PRIVATEDIRECTION_LTR, expected_style_1},
-      {9, PP_MakeFloatRectFromXYWH(28.0f, 117.33334f, 89.333328f, 20.0f),
+      {9, gfx::RectF(28.0f, 117.33334f, 89.333328f, 20.0f),
        PP_PrivateDirection::PP_PRIVATEDIRECTION_LTR, expected_style_2},
-      {15, PP_MakeFloatRectFromXYWH(126.66666f, 117.33334f, 137.33334f, 20.0f),
+      {15, gfx::RectF(126.66666f, 117.33334f, 137.33334f, 20.0f),
        PP_PrivateDirection::PP_PRIVATEDIRECTION_LTR, expected_style_2},
-      {20,
-       PP_MakeFloatRectFromXYWH(266.66666f, 118.66666f, 169.33334f, 18.666664f),
+      {20, gfx::RectF(266.66666f, 118.66666f, 169.33334f, 18.666664f),
        PP_PrivateDirection::PP_PRIVATEDIRECTION_LTR, expected_style_2},
-      {5, PP_MakeFloatRectFromXYWH(28.0f, 65.333336f, 40.0f, 18.666664f),
+      {5, gfx::RectF(28.0f, 65.333336f, 40.0f, 18.666664f),
        PP_PrivateDirection::PP_PRIVATEDIRECTION_LTR, expected_style_2},
-      {17, PP_MakeFloatRectFromXYWH(77.333336f, 64.0f, 160.0f, 20.0f),
+      {17, gfx::RectF(77.333336f, 64.0f, 160.0f, 20.0f),
        PP_PrivateDirection::PP_PRIVATEDIRECTION_LTR, expected_style_2}};
 
   if (IsRunningOnChromeOS()) {
     expected_text_runs[4].bounds =
-        PP_MakeFloatRectFromXYWH(126.66666f, 117.33334f, 137.33334f, 21.33334f);
+        gfx::RectF(126.66666f, 117.33334f, 137.33334f, 21.33334f);
     expected_text_runs[5].bounds =
-        PP_MakeFloatRectFromXYWH(266.66666f, 118.66666f, 170.66666f, 20.0f);
+        gfx::RectF(266.66666f, 118.66666f, 170.66666f, 20.0f);
     expected_text_runs[7].bounds =
-        PP_MakeFloatRectFromXYWH(77.333336f, 64.0f, 160.0f, 21.33333f);
+        gfx::RectF(77.333336f, 64.0f, 160.0f, 21.33333f);
   }
 
   // Test negative char index returns nullopt
@@ -424,27 +433,23 @@ TEST_F(PDFiumPageTextTest, TestHighlightTextRunInfo) {
   static const pp::PDF::PrivateAccessibilityTextStyleInfo kExpectedStyle = {
       "Helvetica", 0,    PP_TEXTRENDERINGMODE_FILL, 16, 0xff000000, 0xff000000,
       false,       false};
-  pp::PDF::PrivateAccessibilityTextRunInfo expected_text_runs[] = {
-      {5,
-       PP_MakeFloatRectFromXYWH(1.3333334f, 198.66667f, 46.666668f, 14.666672f),
+  ExpectedAccessibilityTextRunInfo expected_text_runs[] = {
+      {5, gfx::RectF(1.3333334f, 198.66667f, 46.666668f, 14.666672f),
        PP_PrivateDirection::PP_PRIVATEDIRECTION_LTR, kExpectedStyle},
-      {7,
-       PP_MakeFloatRectFromXYWH(50.666668f, 198.66667f, 47.999996f, 17.333328f),
+      {7, gfx::RectF(50.666668f, 198.66667f, 47.999996f, 17.333328f),
        PP_PrivateDirection::PP_PRIVATEDIRECTION_LTR, kExpectedStyle},
-      {7,
-       PP_MakeFloatRectFromXYWH(106.66666f, 198.66667f, 73.333336f, 18.666672f),
+      {7, gfx::RectF(106.66666f, 198.66667f, 73.333336f, 18.666672f),
        PP_PrivateDirection::PP_PRIVATEDIRECTION_LTR, kExpectedStyle},
-      {2, PP_MakeFloatRectFromXYWH(181.33333f, 192.0f, 16.0f, 25.333344f),
+      {2, gfx::RectF(181.33333f, 192.0f, 16.0f, 25.333344f),
        PP_PrivateDirection::PP_PRIVATEDIRECTION_NONE, kExpectedStyle},
-      {2,
-       PP_MakeFloatRectFromXYWH(198.66667f, 202.66667f, 21.333328f, 10.666672f),
+      {2, gfx::RectF(198.66667f, 202.66667f, 21.333328f, 10.666672f),
        PP_PrivateDirection::PP_PRIVATEDIRECTION_LTR, kExpectedStyle}};
 
   if (IsRunningOnChromeOS()) {
-    expected_text_runs[2].bounds = PP_MakeFloatRectFromXYWH(
-        106.66666f, 198.66667f, 73.333336f, 19.999985f);
-    expected_text_runs[4].bounds = PP_MakeFloatRectFromXYWH(
-        198.66667f, 201.33333f, 21.333328f, 12.000015f);
+    expected_text_runs[2].bounds =
+        gfx::RectF(106.66666f, 198.66667f, 73.333336f, 19.999985f);
+    expected_text_runs[4].bounds =
+        gfx::RectF(198.66667f, 201.33333f, 21.333328f, 12.000015f);
   }
 
   int current_char_index = 0;

pdf/test/test_utils.cc

@@ -1,27 +0,0 @@
-// Copyright 2019 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "pdf/test/test_utils.h"
-
-#include "ppapi/cpp/rect.h"
-#include "testing/gtest/include/gtest/gtest.h"
-
-namespace chrome_pdf {
-
-void CompareRect(const pp::Rect& expected_rect, const pp::Rect& given_rect) {
-  EXPECT_EQ(expected_rect.x(), given_rect.x());
-  EXPECT_EQ(expected_rect.y(), given_rect.y());
-  EXPECT_EQ(expected_rect.width(), given_rect.width());
-  EXPECT_EQ(expected_rect.height(), given_rect.height());
-}
-
-void CompareRect(const pp::FloatRect& expected_rect,
-                 const pp::FloatRect& given_rect) {
-  EXPECT_FLOAT_EQ(expected_rect.x(), given_rect.x());
-  EXPECT_FLOAT_EQ(expected_rect.y(), given_rect.y());
-  EXPECT_FLOAT_EQ(expected_rect.width(), given_rect.width());
-  EXPECT_FLOAT_EQ(expected_rect.height(), given_rect.height());
-}
-
-}  // namespace chrome_pdf

pdf/test/test_utils.h

@@ -1,21 +0,0 @@
-// Copyright 2019 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#ifndef PDF_TEST_TEST_UTILS_H_
-#define PDF_TEST_TEST_UTILS_H_
-
-namespace pp {
-class FloatRect;
-class Rect;
-}  // namespace pp
-
-namespace chrome_pdf {
-
-void CompareRect(const pp::Rect& expected_rect, const pp::Rect& given_rect);
-void CompareRect(const pp::FloatRect& expected_rect,
-                 const pp::FloatRect& given_rect);
-
-}  // namespace chrome_pdf
-
-#endif  // PDF_TEST_TEST_UTILS_H_