Reland "[PDF Ink Signatures] Switch DrawRenderTransform test case to call Draw()"

This is a reland of commit 9d82486a93

This reland changes the pixel comparators. Where possible, the tests use
cc::ExactPixelComparator(). For build configs where the rendering is not
pixel perfect, use cc::FuzzyPixelComparator, configured as a
"FuzzyPixelOffByTwoComparator".

Original change's description:
> [PDF Ink Signatures] Switch DrawRenderTransform test case to call Draw()
>
> Remove the code that checks the transformed Draw() would have used, and
> instead check the output from Draw(). Add a FuzzyMatchesPngFile() test
> utility to check against new expectation PNGs in //pdf/test/data/ink.
> The existing exact-pixel MatchesPngFile() function mostly works, but not
> for ASAN/TSAN.
>
> Bug: 351990827
> Change-Id: Ie11244bcfe91a413f102b8a3bbc34d4c199f789c
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5871447
> Code-Coverage: findit-for-me@appspot.gserviceaccount.com <findit-for-me@appspot.gserviceaccount.com>
> Reviewed-by: Alan Screen <awscreen@chromium.org>
> Commit-Queue: Lei Zhang <thestig@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1357215}

Bug: 351990827
Change-Id: Ibe0fa6b02cf163b690131711d0d360bf4ec35c48
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5874106
Commit-Queue: Lei Zhang <thestig@chromium.org>
Code-Coverage: findit-for-me@appspot.gserviceaccount.com <findit-for-me@appspot.gserviceaccount.com>
Reviewed-by: Alan Screen <awscreen@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1358916}

pdf/pdf_ink_module.cc

@@ -124,9 +124,6 @@ void PdfInkModule::Draw(SkCanvas& canvas) {
     const gfx::Rect content_rect = client_->GetPageContentsRect(page_index);
     const ink::AffineTransform transform =
         GetInkRenderTransform(origin_offset, rotation, content_rect, zoom);
-    if (draw_render_transform_callback_for_testing_) {
-      draw_render_transform_callback_for_testing_.Run(transform);
-    }
 
     SkAutoCanvasRestore save_restore(&canvas, /*doSave=*/true);
     canvas.clipRect(GetDrawPageClipRect(content_rect, origin_offset));
@@ -149,9 +146,6 @@ void PdfInkModule::Draw(SkCanvas& canvas) {
         client_->GetPageContentsRect(state.page_index);
     const ink::AffineTransform transform =
         GetInkRenderTransform(origin_offset, rotation, content_rect, zoom);
-    if (draw_render_transform_callback_for_testing_) {
-      draw_render_transform_callback_for_testing_.Run(transform);
-    }
 
     SkAutoCanvasRestore save_restore(&canvas, /*doSave=*/true);
     canvas.clipRect(GetDrawPageClipRect(content_rect, origin_offset));
@@ -281,11 +275,6 @@ PdfInkModule::GetVisibleStrokesInputPositionsForTesting() const {
   return all_strokes_points;
 }
 
-void PdfInkModule::SetDrawRenderTransformCallbackForTesting(
-    RenderTransformCallback callback) {
-  draw_render_transform_callback_for_testing_ = std::move(callback);
-}
-
 bool PdfInkModule::OnMouseDown(const blink::WebMouseEvent& event) {
   CHECK(enabled());
 

pdf/pdf_ink_module.h

@@ -14,7 +14,6 @@
 #include <vector>
 
 #include "base/containers/flat_set.h"
-#include "base/functional/callback.h"
 #include "base/memory/raw_ref.h"
 #include "base/time/time.h"
 #include "base/values.h"
@@ -35,10 +34,6 @@ class WebInputEvent;
 class WebMouseEvent;
 }  // namespace blink
 
-namespace ink {
-class AffineTransform;
-}
-
 namespace chrome_pdf {
 
 class PdfInkBrush;
@@ -56,9 +51,6 @@ class PdfInkModule {
   // strokes for that page.
   using DocumentStrokeInputPointsMap = std::map<int, PageStrokeInputPoints>;
 
-  using RenderTransformCallback =
-      base::RepeatingCallback<void(const ink::AffineTransform& transform)>;
-
   explicit PdfInkModule(PdfInkModuleClient& client);
   PdfInkModule(const PdfInkModule&) = delete;
   PdfInkModule& operator=(const PdfInkModule&) = delete;
@@ -98,11 +90,6 @@ class PdfInkModule {
   DocumentStrokeInputPointsMap GetVisibleStrokesInputPositionsForTesting()
       const;
 
-  // For testing only. Provide a callback to use whenever the rendering
-  // transform is determined for `Draw()`.
-  void SetDrawRenderTransformCallbackForTesting(
-      RenderTransformCallback callback);
-
  private:
   using StrokeInputSegment = std::vector<ink::StrokeInput>;
 
@@ -271,8 +258,6 @@ class PdfInkModule {
   DocumentStrokesMap strokes_;
 
   PdfInkUndoRedoModel undo_redo_model_;
-
-  RenderTransformCallback draw_render_transform_callback_for_testing_;
 };
 
 }  // namespace chrome_pdf

pdf/pdf_ink_module_unittest.cc

@@ -12,6 +12,7 @@
 #include "base/containers/contains.h"
 #include "base/containers/span.h"
 #include "base/containers/to_vector.h"
+#include "base/files/file_path.h"
 #include "base/test/bind.h"
 #include "base/test/scoped_feature_list.h"
 #include "base/values.h"
@@ -21,12 +22,15 @@
 #include "pdf/pdf_ink_transform.h"
 #include "pdf/test/mouse_event_builder.h"
 #include "pdf/test/pdf_ink_test_helpers.h"
+#include "pdf/test/test_helpers.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/blink/public/common/input/web_mouse_event.h"
 #include "third_party/ink/src/ink/brush/brush.h"
 #include "third_party/ink/src/ink/geometry/affine_transform.h"
+#include "third_party/skia/include/core/SkBitmap.h"
 #include "third_party/skia/include/core/SkCanvas.h"
+#include "third_party/skia/include/core/SkImage.h"
 #include "ui/gfx/geometry/point_f.h"
 #include "ui/gfx/geometry/rect.h"
 #include "ui/gfx/geometry/rect_conversions.h"
@@ -81,6 +85,10 @@ constexpr gfx::PointF kTwoPageVerticalLayoutPageExitAndReentrySegment2[] = {
     gfx::PointF(10.0f, 0.0f), gfx::PointF(10.0f, 5.0f),
     gfx::PointF(15.0f, 10.0f)};
 
+base::FilePath GetInkTestDataFilePath(std::string_view filename) {
+  return base::FilePath(FILE_PATH_LITERAL("ink")).AppendASCII(filename);
+}
+
 class FakeClient : public PdfInkModuleClient {
  public:
   FakeClient() = default;
@@ -473,6 +481,26 @@ class PdfInkModuleStrokeTest : public PdfInkModuleTest {
     return ink_module().GetVisibleStrokesInputPositionsForTesting();
   }
 
+  void DrawWithSizeAndCompare(const gfx::Size& bitmap_size,
+                              std::string_view expected_png_filename) {
+    // Uses MakeN32Premul() and clear the canvas just like
+    // PdfViewWebPlugin::Paint().
+    SkBitmap bitmap;
+    bitmap.allocPixels(
+        SkImageInfo::MakeN32Premul(bitmap_size.width(), bitmap_size.height()));
+    SkCanvas canvas(bitmap);
+    canvas.clear(SK_ColorTRANSPARENT);
+    ink_module().Draw(canvas);
+#if defined(ADDRESS_SANITIZER) || defined(THREAD_SANITIZER) || !defined(NDEBUG)
+    // Some build configs may not have a pixel-perfect match.
+    EXPECT_TRUE(FuzzyMatchesPngFile(
+        bitmap.asImage().get(), GetInkTestDataFilePath(expected_png_filename)));
+#else
+    EXPECT_TRUE(MatchesPngFile(bitmap.asImage().get(),
+                               GetInkTestDataFilePath(expected_png_filename)));
+#endif
+  }
+
  private:
   void ApplyStrokeWithMouseAtPointsMaybeHandled(
       const gfx::PointF& mouse_down_point,
@@ -554,27 +582,13 @@ TEST_F(PdfInkModuleStrokeTest, DrawRenderTransform) {
 
   RunStrokeCheckTest(/*annotation_mode_enabled=*/true);
 
-  // Simulate drawing the strokes, and verify that the expected transform was
+  constexpr gfx::Size kBitmapSize(100, 100);
-  // used.
+  DrawWithSizeAndCompare(kBitmapSize,
-  std::vector<ink::AffineTransform> draw_render_transforms;
+                         "draw_render_transform_simple_stroke.png");
-  ink_module().SetDrawRenderTransformCallbackForTesting(
-      base::BindLambdaForTesting([&](const ink::AffineTransform& transform) {
-        draw_render_transforms.push_back(transform);
-      }));
-  SkCanvas canvas;
-  ink_module().Draw(canvas);
 
-  // Just one transform provided, to match the captured stroke.
+  // But if the one and only page is not visible, then Draw() does nothing.
-  EXPECT_THAT(draw_render_transforms,
-              ElementsAre(InkAffineTransformEq(-1.0f, 0.0f, 54.0f, 0.0f, -1.0f,
-                                               44.0f)));
-
-  // But if the one and only page is not visible, then Draw() does no transform
-  // calculations.
-  draw_render_transforms.clear();
   client().set_page_visibility(0, false);
-  ink_module().Draw(canvas);
+  DrawWithSizeAndCompare(kBitmapSize, "draw_render_transform_blank.png");
-  EXPECT_TRUE(draw_render_transforms.empty());
 }
 
 TEST_F(PdfInkModuleStrokeTest, InvalidationsFromStroke) {

pdf/test/test_helpers.cc

@@ -20,6 +20,27 @@
 
 namespace chrome_pdf {
 
+namespace {
+
+testing::AssertionResult MatchesPngFileImpl(
+    const SkImage* actual_image,
+    const base::FilePath& expected_png_file,
+    const cc::PixelComparator& comparitor) {
+  SkBitmap actual_bitmap;
+  if (!actual_image->asLegacyBitmap(&actual_bitmap)) {
+    return testing::AssertionFailure() << "Reference: " << expected_png_file;
+  }
+
+  if (!cc::MatchesPNGFile(actual_bitmap, GetTestDataFilePath(expected_png_file),
+                          comparitor)) {
+    return testing::AssertionFailure() << "Reference: " << expected_png_file;
+  }
+
+  return testing::AssertionSuccess();
+}
+
+}  // namespace
+
 base::FilePath GetTestDataFilePath(const base::FilePath& path) {
   return base::PathService::CheckedGet(base::DIR_SRC_TEST_DATA_ROOT)
       .Append(FILE_PATH_LITERAL("pdf"))
@@ -31,16 +52,18 @@ base::FilePath GetTestDataFilePath(const base::FilePath& path) {
 testing::AssertionResult MatchesPngFile(
     const SkImage* actual_image,
     const base::FilePath& expected_png_file) {
-  SkBitmap actual_bitmap;
+  return MatchesPngFileImpl(actual_image, expected_png_file,
-  if (!actual_image->asLegacyBitmap(&actual_bitmap))
+                            cc::ExactPixelComparator());
-    return testing::AssertionFailure() << "Reference: " << expected_png_file;
+}
 
-  if (!cc::MatchesPNGFile(actual_bitmap, GetTestDataFilePath(expected_png_file),
+testing::AssertionResult FuzzyMatchesPngFile(
-                          cc::ExactPixelComparator())) {
+    const SkImage* actual_image,
-    return testing::AssertionFailure() << "Reference: " << expected_png_file;
+    const base::FilePath& expected_png_file) {
-  }
+  // Effectively a "FuzzyPixelOffByTwoComparator".
-
+  cc::FuzzyPixelComparator comparator;
-  return testing::AssertionSuccess();
+  comparator.SetErrorPixelsPercentageLimit(100.0f);
+  comparator.SetAbsErrorLimit(2);
+  return MatchesPngFileImpl(actual_image, expected_png_file, comparator);
 }
 
 sk_sp<SkSurface> CreateSkiaSurfaceForTesting(const gfx::Size& size,

pdf/test/test_helpers.h

@@ -30,6 +30,11 @@ testing::AssertionResult MatchesPngFile(
     const SkImage* actual_image,
     const base::FilePath& expected_png_file);
 
+// Same as MatchesPngFile() above, but with a fuzzy pixel comparator.
+testing::AssertionResult FuzzyMatchesPngFile(
+    const SkImage* actual_image,
+    const base::FilePath& expected_png_file);
+
 // Creates a Skia surface with dimensions `size` and filled with `color`.
 sk_sp<SkSurface> CreateSkiaSurfaceForTesting(const gfx::Size& size,
                                              SkColor color);