[PDF Ink Signatures] Refactor PdfInkModuleClient::UpdateInkCursorImage()

Refactor PdfInkModuleClient::UpdateInkCursorImage() by changing the
parameter from an SkBitmap to a ui::Cursor. Rename accordingly. This
allows PdfInkModule to set the cursor to pre-defined types, rather than
only custom cursors.

This CL should not cause any visible change in behavior. This change is
needed to use existing cursors in https://crrev.com/c/6269750.

Bug: 342445982
Change-Id: If349cd5f428d966a9be108f7556569a5ff90e554
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6310886
Commit-Queue: Andy Phan <andyphan@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1426666}

pdf/pdf_ink_module.cc

@@ -51,6 +51,7 @@
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "third_party/skia/include/core/SkCanvas.h"
 #include "third_party/skia/include/core/SkColor.h"
+#include "ui/base/cursor/cursor.h"
 #include "ui/gfx/geometry/point_f.h"
 #include "ui/gfx/geometry/rect.h"
 #include "ui/gfx/geometry/rect_f.h"
@@ -1348,9 +1349,12 @@ void PdfInkModule::MaybeSetCursor() {
     brush_size = kEraserSize;
   }
 
-  client_->UpdateInkCursorImage(GenerateToolCursor(
+  SkBitmap bitmap = GenerateToolCursor(
       color,
-      CursorDiameterFromBrushSizeAndZoom(brush_size, client_->GetZoom())));
+      CursorDiameterFromBrushSizeAndZoom(brush_size, client_->GetZoom()));
+  gfx::Point hotspot(bitmap.width() / 2, bitmap.height() / 2);
+  client_->UpdateInkCursor(
+      ui::Cursor::NewCustom(std::move(bitmap), std::move(hotspot)));
 }
 
 PdfInkModule::DrawingStrokeState::DrawingStrokeState() = default;

pdf/pdf_ink_module_client.h

@@ -12,7 +12,6 @@
 #include "pdf/pdf_ink_ids.h"
 #include "pdf/ui/thumbnail.h"
 #include "third_party/ink/src/ink/geometry/partitioned_mesh.h"
-#include "third_party/skia/include/core/SkBitmap.h"
 #include "ui/gfx/geometry/rect.h"
 #include "ui/gfx/geometry/vector2d.h"
 
@@ -26,6 +25,10 @@ namespace ink {
 class Stroke;
 }
 
+namespace ui {
+class Cursor;
+}
+
 namespace chrome_pdf {
 
 class PdfInkModuleClient {
@@ -104,8 +107,8 @@ class PdfInkModuleClient {
   // Notifies the client that a stroke has finished drawing or erasing.
   virtual void StrokeFinished() {}
 
-  // Asks the client to change the cursor to `bitmap`.
+  // Asks the client to change the cursor to `cursor`.
-  virtual void UpdateInkCursorImage(SkBitmap bitmap) {}
+  virtual void UpdateInkCursor(const ui::Cursor& cursor) {}
 
   // Notifies that an existing shape identified by `id` on the page at
   // `page_index` should update its active state.

pdf/pdf_ink_module_unittest.cc

@@ -40,6 +40,8 @@
 #include "third_party/ink/src/ink/geometry/affine_transform.h"
 #include "third_party/ink/src/ink/strokes/input/type_matchers.h"
 #include "third_party/skia/include/core/SkBitmap.h"
+#include "ui/base/cursor/cursor.h"
+#include "ui/base/cursor/mojom/cursor_type.mojom.h"
 #include "ui/gfx/geometry/point_f.h"
 #include "ui/gfx/geometry/rect.h"
 #include "ui/gfx/geometry/rect_conversions.h"
@@ -174,9 +176,10 @@ MATCHER_P(InkStrokeDrawingBrushTypeEq, expected_type, "") {
   return opacity == 0.4f;
 }
 
-// Matcher for bitmap against expected dimensions.
+// Matcher for cursor with a custom bitmap against expected dimensions.
-MATCHER_P(BitmapImageSizeEq, dimensions, "") {
+MATCHER_P(CursorBitmapImageSizeEq, dimensions, "") {
-  return arg.dimensions() == dimensions;
+  return arg.type() == ui::mojom::CursorType::kCustom &&
+         arg.custom_bitmap().dimensions() == dimensions;
 }
 
 std::map<int, std::vector<raw_ref<const ink::Stroke>>> CollectVisibleStrokes(
@@ -305,7 +308,7 @@ class FakeClient : public PdfInkModuleClient {
 
   void StrokeFinished() override { ++stroke_finished_count_; }
 
-  MOCK_METHOD(void, UpdateInkCursorImage, (SkBitmap bitmap), (override));
+  MOCK_METHOD(void, UpdateInkCursor, (const ui::Cursor&), (override));
 
   MOCK_METHOD(void,
               UpdateShapeActive,
@@ -650,9 +653,9 @@ TEST_F(PdfInkModuleTest, HandleSetAnnotationModeMessage) {
 TEST_F(PdfInkModuleTest, MaybeSetCursorWhenTogglingAnnotationMode) {
   EXPECT_FALSE(ink_module().enabled());
 
-  EXPECT_CALL(client(), UpdateInkCursorImage(_))
+  EXPECT_CALL(client(), UpdateInkCursor(_)).WillOnce([this]() {
-      .WillOnce(
+    EXPECT_TRUE(ink_module().enabled());
-          [this](SkBitmap bitmap) { EXPECT_TRUE(ink_module().enabled()); });
+  });
 
   base::Value::Dict message =
       CreateSetAnnotationModeMessageForTesting(/*enable=*/true);
@@ -667,18 +670,24 @@ TEST_F(PdfInkModuleTest, MaybeSetCursorWhenTogglingAnnotationMode) {
 TEST_F(PdfInkModuleTest, MaybeSetCursorWhenChangingBrushes) {
   {
     InSequence seq;
-    EXPECT_CALL(client(), UpdateInkCursorImage(_))
+    EXPECT_CALL(client(), UpdateInkCursor(_))
-        .WillOnce([](SkBitmap bitmap) {
+        .WillOnce([](const ui::Cursor& cursor) {
+          ASSERT_EQ(ui::mojom::CursorType::kCustom, cursor.type());
+          const SkBitmap& bitmap = cursor.custom_bitmap();
           EXPECT_EQ(6, bitmap.width());
           EXPECT_EQ(6, bitmap.height());
         });
-    EXPECT_CALL(client(), UpdateInkCursorImage(_))
+    EXPECT_CALL(client(), UpdateInkCursor(_))
-        .WillOnce([](SkBitmap bitmap) {
+        .WillOnce([](const ui::Cursor& cursor) {
+          ASSERT_EQ(ui::mojom::CursorType::kCustom, cursor.type());
+          const SkBitmap& bitmap = cursor.custom_bitmap();
           EXPECT_EQ(20, bitmap.width());
           EXPECT_EQ(20, bitmap.height());
         });
-    EXPECT_CALL(client(), UpdateInkCursorImage(_))
+    EXPECT_CALL(client(), UpdateInkCursor(_))
-        .WillOnce([](SkBitmap bitmap) {
+        .WillOnce([](const ui::Cursor& cursor) {
+          ASSERT_EQ(ui::mojom::CursorType::kCustom, cursor.type());
+          const SkBitmap& bitmap = cursor.custom_bitmap();
           EXPECT_EQ(6, bitmap.width());
           EXPECT_EQ(6, bitmap.height());
         });
@@ -700,18 +709,24 @@ TEST_F(PdfInkModuleTest, MaybeSetCursorWhenChangingBrushes) {
 TEST_F(PdfInkModuleTest, MaybeSetCursorWhenChangingZoom) {
   {
     InSequence seq;
-    EXPECT_CALL(client(), UpdateInkCursorImage(_))
+    EXPECT_CALL(client(), UpdateInkCursor(_))
-        .WillOnce([](SkBitmap bitmap) {
+        .WillOnce([](const ui::Cursor& cursor) {
+          ASSERT_EQ(ui::mojom::CursorType::kCustom, cursor.type());
+          const SkBitmap& bitmap = cursor.custom_bitmap();
           EXPECT_EQ(6, bitmap.width());
           EXPECT_EQ(6, bitmap.height());
         });
-    EXPECT_CALL(client(), UpdateInkCursorImage(_))
+    EXPECT_CALL(client(), UpdateInkCursor(_))
-        .WillOnce([](SkBitmap bitmap) {
+        .WillOnce([](const ui::Cursor& cursor) {
+          ASSERT_EQ(ui::mojom::CursorType::kCustom, cursor.type());
+          const SkBitmap& bitmap = cursor.custom_bitmap();
           EXPECT_EQ(20, bitmap.width());
           EXPECT_EQ(20, bitmap.height());
         });
-    EXPECT_CALL(client(), UpdateInkCursorImage(_))
+    EXPECT_CALL(client(), UpdateInkCursor(_))
-        .WillOnce([](SkBitmap bitmap) {
+        .WillOnce([](const ui::Cursor& cursor) {
+          ASSERT_EQ(ui::mojom::CursorType::kCustom, cursor.type());
+          const SkBitmap& bitmap = cursor.custom_bitmap();
           EXPECT_EQ(10, bitmap.width());
           EXPECT_EQ(10, bitmap.height());
         });
@@ -1918,7 +1933,8 @@ TEST_F(PdfInkModuleStrokeTest, ChangeBrushSizeDuringDrawing) {
   // until the mouse-up event.  The cursor image will be updated only when
   // there is not a stroke in progress.
   EXPECT_CALL(client(), StrokeAdded(_, _, _)).Times(0);
-  EXPECT_CALL(client(), UpdateInkCursorImage(BitmapImageSizeEq(SkISize(6, 6))));
+  EXPECT_CALL(client(),
+              UpdateInkCursor(CursorBitmapImageSizeEq(SkISize(6, 6))));
   TestAnnotationBrushMessageParams message_params{/*color_r=*/0,
                                                   /*color_g=*/0,
                                                   /*color_b=*/0, /*size=*/2.0};
@@ -1945,7 +1961,7 @@ TEST_F(PdfInkModuleStrokeTest, ChangeBrushSizeDuringDrawing) {
     EXPECT_CALL(client(), StrokeAdded(kPageIndex, InkStrokeId(0),
                                       InkStrokeBrushSizeEq(2.0f)));
     EXPECT_CALL(client(),
-                UpdateInkCursorImage(BitmapImageSizeEq(SkISize(8, 8))));
+                UpdateInkCursor(CursorBitmapImageSizeEq(SkISize(8, 8))));
   }
   blink::WebMouseEvent mouse_move_event =
       CreateMouseMoveWithLeftButtonEventAtPoint(kLeftVerticalStrokePoint2);
@@ -2082,7 +2098,8 @@ TEST_F(PdfInkModuleStrokeTest, ChangeDrawingBrushTypeDuringDrawing) {
   // until the mouse-up event.  The cursor image will be updated only if a
   // stroke is not in progress.
   EXPECT_CALL(client(), StrokeAdded(_, _, _)).Times(0);
-  EXPECT_CALL(client(), UpdateInkCursorImage(BitmapImageSizeEq(SkISize(6, 6))));
+  EXPECT_CALL(client(),
+              UpdateInkCursor(CursorBitmapImageSizeEq(SkISize(6, 6))));
   TestAnnotationBrushMessageParams pen_message_params{/*color_r=*/0,
                                                       /*color_g=*/0,
                                                       /*color_b=*/0,
@@ -2116,7 +2133,7 @@ TEST_F(PdfInkModuleStrokeTest, ChangeDrawingBrushTypeDuringDrawing) {
         StrokeAdded(kPageIndex, InkStrokeId(0),
                     InkStrokeDrawingBrushTypeEq(PdfInkBrush::Type::kPen)));
     EXPECT_CALL(client(),
-                UpdateInkCursorImage(BitmapImageSizeEq(SkISize(10, 10))));
+                UpdateInkCursor(CursorBitmapImageSizeEq(SkISize(10, 10))));
   }
   blink::WebMouseEvent mouse_move_event =
       CreateMouseMoveWithLeftButtonEventAtPoint(kLeftVerticalStrokePoint2);

pdf/pdf_view_web_plugin.cc

@@ -365,10 +365,8 @@ class PdfViewWebPlugin::PdfInkModuleClientImpl : public PdfInkModuleClient {
     plugin_->SetPluginCanSave(true);
   }
 
-  void UpdateInkCursorImage(SkBitmap bitmap) override {
+  void UpdateInkCursor(const ui::Cursor& cursor) override {
-    gfx::Point hotspot(bitmap.width() / 2, bitmap.height() / 2);
+    plugin_->cursor_ = cursor;
-    plugin_->cursor_ =
-        ui::Cursor::NewCustom(std::move(bitmap), std::move(hotspot));
   }
 
   void UpdateShapeActive(int page_index,