[PDF Ink Signatures] Ignore touch events after a pen event

Make the behavior in this scenario more consistent with other apps.
Change PdfInkModule's to remember if it ever received a pen event, and
ignore touch events after that. Update tests with the new expectations,
add ApplyStrokeWithTouchAtPointsNotHandled() as needed, and remove the
now fulfilled TODOs.

Bug: 392650039
Change-Id: I38c45d8fc369748c49619e3448f713651e5e3797
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6216548
Commit-Queue: Lei Zhang <thestig@chromium.org>
Reviewed-by: Alan Screen <awscreen@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1413686}

pdf/pdf_ink_module.cc

@@ -404,8 +404,13 @@ bool PdfInkModule::OnTouchStart(const blink::WebTouchEvent& event) {
     return false;
   }
 
-  gfx::PointF position = event.touches[0].PositionInWidget();
   ink::StrokeInput::ToolType tool_type = GetToolTypeFromTouchEvent(event);
+  MaybeRecordPenInput(tool_type);
+  if (ShouldIgnoreTouchInput(tool_type)) {
+    return false;
+  }
+
+  gfx::PointF position = event.touches[0].PositionInWidget();
   return is_drawing_stroke()
              ? StartStroke(position, event.TimeStamp(), tool_type)
              : StartEraseStroke(position, tool_type);
@@ -418,8 +423,13 @@ bool PdfInkModule::OnTouchEnd(const blink::WebTouchEvent& event) {
     return false;
   }
 
-  gfx::PointF position = event.touches[0].PositionInWidget();
   ink::StrokeInput::ToolType tool_type = GetToolTypeFromTouchEvent(event);
+  MaybeRecordPenInput(tool_type);
+  if (ShouldIgnoreTouchInput(tool_type)) {
+    return false;
+  }
+
+  gfx::PointF position = event.touches[0].PositionInWidget();
   return is_drawing_stroke()
              ? FinishStroke(position, event.TimeStamp(), tool_type)
              : FinishEraseStroke(position, tool_type);
@@ -432,8 +442,13 @@ bool PdfInkModule::OnTouchMove(const blink::WebTouchEvent& event) {
     return false;
   }
 
-  gfx::PointF position = event.touches[0].PositionInWidget();
   ink::StrokeInput::ToolType tool_type = GetToolTypeFromTouchEvent(event);
+  MaybeRecordPenInput(tool_type);
+  if (ShouldIgnoreTouchInput(tool_type)) {
+    return false;
+  }
+
+  gfx::PointF position = event.touches[0].PositionInWidget();
   return is_drawing_stroke()
              ? ContinueStroke(position, event.TimeStamp(), tool_type)
              : ContinueEraseStroke(position, tool_type);
@@ -774,6 +789,18 @@ bool PdfInkModule::EraseHelper(const gfx::PointF& position, int page_index) {
   return true;
 }
 
+void PdfInkModule::MaybeRecordPenInput(ink::StrokeInput::ToolType tool_type) {
+  if (tool_type == ink::StrokeInput::ToolType::kStylus) {
+    using_stylus_instead_of_touch_ = true;
+  }
+}
+
+bool PdfInkModule::ShouldIgnoreTouchInput(
+    ink::StrokeInput::ToolType tool_type) {
+  return using_stylus_instead_of_touch_ &&
+         tool_type == ink::StrokeInput::ToolType::kTouch;
+}
+
 void PdfInkModule::HandleAnnotationRedoMessage(
     const base::Value::Dict& message) {
   ApplyUndoRedoCommands(undo_redo_model_.Redo());

pdf/pdf_ink_module.h

@@ -321,6 +321,14 @@ class PdfInkModule {
   // not.
   bool EraseHelper(const gfx::PointF& position, int page_index);
 
+  // Sets `using_stylus_instead_of_touch_` to true if `tool_type` is
+  // `ink::StrokeInput::ToolType::kStylus`. Otherwise do nothing.
+  void MaybeRecordPenInput(ink::StrokeInput::ToolType tool_type);
+
+  // Returns true if `using_stylus_instead_of_touch_` is set, and `tool_type` is
+  // `ink::StrokeInput::ToolType::kTouch`.
+  bool ShouldIgnoreTouchInput(ink::StrokeInput::ToolType tool_type);
+
   void HandleAnnotationRedoMessage(const base::Value::Dict& message);
   void HandleAnnotationUndoMessage(const base::Value::Dict& message);
   void HandleGetAnnotationBrushMessage(const base::Value::Dict& message);
@@ -388,6 +396,8 @@ class PdfInkModule {
 
   bool enabled_ = false;
 
+  bool using_stylus_instead_of_touch_ = false;
+
   bool loaded_data_from_pdf_ = false;
 
   // Shapes loaded from the PDF.

pdf/pdf_ink_module_unittest.cc

@@ -815,6 +815,15 @@ class PdfInkModuleStrokeTest : public PdfInkModuleTest {
         /*expect_touch_events_handled=*/true);
   }
 
+  void ApplyStrokeWithTouchAtPointsNotHandled(
+      base::span<const gfx::PointF> touch_start_points,
+      std::vector<base::span<const gfx::PointF>> all_touch_move_points,
+      base::span<const gfx::PointF> touch_end_points) {
+    ApplyStrokeWithTouchAtPointsMaybeHandled(
+        touch_start_points, all_touch_move_points, touch_end_points,
+        /*expect_touch_events_handled=*/false);
+  }
+
   // TODO(crbug.com/377733396): Consider refactoring to combine with
   // RunStrokeCheckTest().
   void RunStrokeTouchCheckTest(bool annotation_mode_enabled) {
@@ -1180,11 +1189,10 @@ TEST_F(PdfInkModuleStrokeTest, IgnoreTouchEventsAfterPenEvent) {
   EXPECT_EQ(3, ink_module().GetInputOfTypeCountForPageForTesting(
                    /*page_index=*/0, ink::StrokeInput::ToolType::kStylus));
 
-  ApplyStrokeWithTouchAtPoints(base::span_from_ref(kMouseDownPoint),
-                               all_move_points,
-                               base::span_from_ref(kMouseUpPoint));
-  // TODO(crbug.com/392650039): Ignore touch, so this should be 6 instead of 9.
-  EXPECT_EQ(9, ink_module().GetInputOfTypeCountForPageForTesting(
+  ApplyStrokeWithTouchAtPointsNotHandled(base::span_from_ref(kMouseDownPoint),
+                                         all_move_points,
+                                         base::span_from_ref(kMouseUpPoint));
+  EXPECT_EQ(6, ink_module().GetInputOfTypeCountForPageForTesting(
                    /*page_index=*/0, ink::StrokeInput::ToolType::kTouch));
   EXPECT_EQ(3, ink_module().GetInputOfTypeCountForPageForTesting(
                    /*page_index=*/0, ink::StrokeInput::ToolType::kStylus));
@@ -1192,20 +1200,18 @@ TEST_F(PdfInkModuleStrokeTest, IgnoreTouchEventsAfterPenEvent) {
   ApplyStrokeWithPenAtPoints(base::span_from_ref(kMouseDownPoint),
                              all_move_points,
                              base::span_from_ref(kMouseUpPoint));
-  // TODO(crbug.com/392650039): Ignore touch, so this should be 6 instead of 9.
-  EXPECT_EQ(9, ink_module().GetInputOfTypeCountForPageForTesting(
+  EXPECT_EQ(6, ink_module().GetInputOfTypeCountForPageForTesting(
                    /*page_index=*/0, ink::StrokeInput::ToolType::kTouch));
   EXPECT_EQ(6, ink_module().GetInputOfTypeCountForPageForTesting(
                    /*page_index=*/0, ink::StrokeInput::ToolType::kStylus));
 
-  ApplyStrokeWithTouchAtPoints(base::span_from_ref(kMouseDownPoint),
-                               all_move_points,
-                               base::span_from_ref(kMouseUpPoint));
+  ApplyStrokeWithTouchAtPointsNotHandled(base::span_from_ref(kMouseDownPoint),
+                                         all_move_points,
+                                         base::span_from_ref(kMouseUpPoint));
   EXPECT_EQ(0, ink_module().GetInputOfTypeCountForPageForTesting(
                    /*page_index=*/0, ink::StrokeInput::ToolType::kMouse));
-  // TODO(crbug.com/392650039): Ignore touch, so this should be 6 instead of 12.
-  EXPECT_EQ(12, ink_module().GetInputOfTypeCountForPageForTesting(
-                    /*page_index=*/0, ink::StrokeInput::ToolType::kTouch));
+  EXPECT_EQ(6, ink_module().GetInputOfTypeCountForPageForTesting(
+                   /*page_index=*/0, ink::StrokeInput::ToolType::kTouch));
   EXPECT_EQ(6, ink_module().GetInputOfTypeCountForPageForTesting(
                    /*page_index=*/0, ink::StrokeInput::ToolType::kStylus));
 }