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

This reverts commit 59616a84f2.

Reason for revert: Still causes same bot failure: https://ci.chromium.org/ui/p/chromium/builders/ci/Linux%20CFI/28345/overview

Original change's description:
> Reland "Reland "[PDF Ink Signatures] Switch DrawRenderTransform test case to call Draw()""
>
> This is a reland of commit d46dcdd094
>
> Also do fuzzy matching for MSAN, like ASAN and TSAN.
>
> Original change's description:
> > 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}
>
> Bug: 351990827
> Change-Id: I1c354c16692c3d6916217f7c6a0904fae343577d
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5883508
> Reviewed-by: Alan Screen <awscreen@chromium.org>
> Commit-Queue: Lei Zhang <thestig@chromium.org>
> Code-Coverage: findit-for-me@appspot.gserviceaccount.com <findit-for-me@appspot.gserviceaccount.com>
> Cr-Commit-Position: refs/heads/main@{#1359403}

Bug: 351990827
Change-Id: I00166435bef1408226c8b2c63f74e5f10e6f77f8
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5889353
Commit-Queue: Yoichi Osato <yoichio@chromium.org>
Auto-Submit: Yoichi Osato <yoichio@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Reviewed-by: Yoichi Osato <yoichio@chromium.org>
Owners-Override: Yoichi Osato <yoichio@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1359698}

pdf/pdf_ink_module.cc

@@ -124,6 +124,9 @@ 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));
@@ -146,6 +149,9 @@ 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));
@@ -275,6 +281,11 @@ 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,6 +14,7 @@
 #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"
@@ -34,6 +35,10 @@ class WebInputEvent;
 class WebMouseEvent;
 }  // namespace blink
 
+namespace ink {
+class AffineTransform;
+}
+
 namespace chrome_pdf {
 
 class PdfInkBrush;
@@ -51,6 +56,9 @@ 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;
@@ -90,6 +98,11 @@ 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>;
 
@@ -258,6 +271,8 @@ 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,7 +12,6 @@
 #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"
@@ -22,15 +21,12 @@
 #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"
@@ -85,10 +81,6 @@ 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;
@@ -481,27 +473,6 @@ 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(MEMORY_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,
@@ -583,13 +554,27 @@ TEST_F(PdfInkModuleStrokeTest, DrawRenderTransform) {
 
   RunStrokeCheckTest(/*annotation_mode_enabled=*/true);
 
-  constexpr gfx::Size kBitmapSize(100, 100);
+  // Simulate drawing the strokes, and verify that the expected transform was
-  DrawWithSizeAndCompare(kBitmapSize,
+  // used.
-                         "draw_render_transform_simple_stroke.png");
+  std::vector<ink::AffineTransform> draw_render_transforms;
+  ink_module().SetDrawRenderTransformCallbackForTesting(
+      base::BindLambdaForTesting([&](const ink::AffineTransform& transform) {
+        draw_render_transforms.push_back(transform);
+      }));
+  SkCanvas canvas;
+  ink_module().Draw(canvas);
 
-  // But if the one and only page is not visible, then Draw() does nothing.
+  // Just one transform provided, to match the captured stroke.
+  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);
-  DrawWithSizeAndCompare(kBitmapSize, "draw_render_transform_blank.png");
+  ink_module().Draw(canvas);
+  EXPECT_TRUE(draw_render_transforms.empty());
 }
 
 TEST_F(PdfInkModuleStrokeTest, InvalidationsFromStroke) {

pdf/test/test_helpers.cc

@@ -20,27 +20,6 @@
 
 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"))
@@ -52,18 +31,16 @@ base::FilePath GetTestDataFilePath(const base::FilePath& path) {
 testing::AssertionResult MatchesPngFile(
     const SkImage* actual_image,
     const base::FilePath& expected_png_file) {
-  return MatchesPngFileImpl(actual_image, expected_png_file,
+  SkBitmap actual_bitmap;
-                            cc::ExactPixelComparator());
+  if (!actual_image->asLegacyBitmap(&actual_bitmap))
-}
+    return testing::AssertionFailure() << "Reference: " << expected_png_file;
 
-testing::AssertionResult FuzzyMatchesPngFile(
+  if (!cc::MatchesPNGFile(actual_bitmap, GetTestDataFilePath(expected_png_file),
-    const SkImage* actual_image,
+                          cc::ExactPixelComparator())) {
-    const base::FilePath& expected_png_file) {
+    return testing::AssertionFailure() << "Reference: " << expected_png_file;
-  // Effectively a "FuzzyPixelOffByTwoComparator".
+  }
-  cc::FuzzyPixelComparator comparator;
+
-  comparator.SetErrorPixelsPercentageLimit(100.0f);
+  return testing::AssertionSuccess();
-  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,11 +30,6 @@ 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);