[PDF Ink Signatures] Add stroke brush type metric

Add a metric to track the brush type of a drawn stroke.

To make adding tests easier, provide helper methods for applying quick
strokes and for selecting brushes.

Bug: 380433757
Change-Id: I8612c007c724515ecf6a3dc4beecd0f047335784
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6044359
Reviewed-by: 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>
Commit-Queue: Andy Phan <andyphan@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1388486}

pdf/BUILD.gn

@@ -228,6 +228,8 @@ if (enable_pdf) {
         "pdf_ink_cursor.cc",
         "pdf_ink_cursor.h",
         "pdf_ink_ids.h",
+        "pdf_ink_metrics_handler.cc",
+        "pdf_ink_metrics_handler.h",
         "pdf_ink_module.cc",
         "pdf_ink_module.h",
         "pdf_ink_module_client.h",

pdf/pdf_ink_metrics_handler.cc

@@ -0,0 +1,30 @@
+// Copyright 2024 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "pdf/pdf_ink_metrics_handler.h"
+
+#include "base/metrics/histogram_functions.h"
+#include "pdf/pdf_ink_brush.h"
+
+namespace chrome_pdf {
+
+namespace {
+
+void ReportStrokeType(StrokeMetricBrushType type) {
+  base::UmaHistogramEnumeration("PDF.Ink2StrokeBrushType", type);
+}
+
+}  // namespace
+
+void ReportDrawStroke(PdfInkBrush::Type type) {
+  bool is_pen = type == PdfInkBrush::Type::kPen;
+  ReportStrokeType(is_pen ? StrokeMetricBrushType::kPen
+                          : StrokeMetricBrushType::kHighlighter);
+}
+
+void ReportEraseStroke() {
+  ReportStrokeType(StrokeMetricBrushType::kEraser);
+}
+
+}  // namespace chrome_pdf

pdf/pdf_ink_metrics_handler.h

@@ -0,0 +1,33 @@
+// Copyright 2024 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef PDF_PDF_INK_METRICS_HANDLER_H_
+#define PDF_PDF_INK_METRICS_HANDLER_H_
+
+#include "pdf/buildflags.h"
+#include "pdf/pdf_ink_brush.h"
+
+static_assert(BUILDFLAG(ENABLE_PDF_INK2), "ENABLE_PDF_INK2 not set to true");
+
+namespace chrome_pdf {
+
+// These values are persisted to logs. Entries should not be renumbered and
+// numeric values should never be reused.
+//
+// LINT.IfChange(PDFInk2StrokeBrushType)
+enum class StrokeMetricBrushType {
+  kPen = 0,
+  kHighlighter = 1,
+  kEraser = 2,
+  kMaxValue = 2,
+};
+// LINT.ThenChange(//tools/metrics/histograms/metadata/pdf/enums.xml:PDFInk2StrokeBrushType)
+
+void ReportDrawStroke(PdfInkBrush::Type type);
+
+void ReportEraseStroke();
+
+}  // namespace chrome_pdf
+
+#endif  // PDF_PDF_INK_METRICS_HANDLER_H_

pdf/pdf_ink_module.cc

@@ -31,6 +31,7 @@
 #include "pdf/pdf_ink_brush.h"
 #include "pdf/pdf_ink_conversions.h"
 #include "pdf/pdf_ink_cursor.h"
+#include "pdf/pdf_ink_metrics_handler.h"
 #include "pdf/pdf_ink_module_client.h"
 #include "pdf/pdf_ink_transform.h"
 #include "third_party/blink/public/common/input/web_input_event.h"
@@ -535,6 +536,8 @@ bool PdfInkModule::FinishStroke(const gfx::PointF& position,
   bool undo_redo_success = undo_redo_model_.FinishDraw();
   CHECK(undo_redo_success);
 
+  ReportDrawStroke(state.brush_type);
+
   // Reset `state` now that the stroke operation is done.
   state.inputs.clear();
   state.start_time = std::nullopt;
@@ -604,6 +607,8 @@ bool PdfInkModule::FinishEraseStroke(const gfx::PointF& position) {
     for (int page_index : state.page_indices_with_erasures) {
       client_->UpdateThumbnail(page_index);
     }
+
+    ReportEraseStroke();
   }
 
   // Reset `state` now that the erase operation is done.

pdf/pdf_ink_module_unittest.cc

@@ -16,12 +16,14 @@
 #include "base/containers/to_vector.h"
 #include "base/files/file_path.h"
 #include "base/test/bind.h"
+#include "base/test/metrics/histogram_tester.h"
 #include "base/test/scoped_feature_list.h"
 #include "base/test/values_test_util.h"
 #include "base/values.h"
 #include "pdf/pdf_features.h"
 #include "pdf/pdf_ink_brush.h"
 #include "pdf/pdf_ink_conversions.h"
+#include "pdf/pdf_ink_metrics_handler.h"
 #include "pdf/pdf_ink_module_client.h"
 #include "pdf/pdf_ink_transform.h"
 #include "pdf/pdfium/pdfium_ink_reader.h"
@@ -829,6 +831,14 @@ class PdfInkModuleStrokeTest : public PdfInkModuleTest {
     EXPECT_TRUE(updated_thumbnail_page_indices.empty());
   }
 
+  void SelectBrushTool(PdfInkBrush::Type type,
+                       float size,
+                       const TestAnnotationBrushMessageParams& params) {
+    EXPECT_TRUE(
+        ink_module().OnMessage(CreateSetAnnotationBrushMessageForTesting(
+            PdfInkBrush::TypeToString(type), size, &params)));
+  }
+
   void SelectEraserToolOfSize(float size) {
     EXPECT_TRUE(ink_module().OnMessage(
         CreateSetAnnotationBrushMessageForTesting("eraser", size, nullptr)));
@@ -1973,4 +1983,72 @@ TEST_F(PdfInkModuleGetVisibleStrokesTest, MultiplePageStrokes) {
                             {expected_page1_horz_line_input_batch.value()}))));
 }
 
+class PdfInkModuleMetricsTest : public PdfInkModuleUndoRedoTest {
+ protected:
+  static constexpr char kTypeMetric[] = "PDF.Ink2StrokeBrushType";
+};
+
+TEST_F(PdfInkModuleMetricsTest, StrokeUndoRedoDoesNotAffectMetrics) {
+  InitializeSimpleSinglePageBasicLayout();
+  base::HistogramTester histograms;
+
+  // Draw a pen stroke.
+  RunStrokeCheckTest(/*annotation_mode_enabled=*/true);
+
+  histograms.ExpectUniqueSample(kTypeMetric, StrokeMetricBrushType::kPen, 1);
+
+  // Undo and redo.
+  PerformUndo();
+  PerformRedo();
+
+  // The metrics should stay the same.
+  histograms.ExpectUniqueSample(kTypeMetric, StrokeMetricBrushType::kPen, 1);
+}
+
+TEST_F(PdfInkModuleMetricsTest, StrokeBrushType) {
+  InitializeSimpleSinglePageBasicLayout();
+  base::HistogramTester histograms;
+
+  RunStrokeCheckTest(/*annotation_mode_enabled=*/false);
+
+  histograms.ExpectTotalCount(kTypeMetric, 0);
+
+  // Draw a pen stroke.
+  RunStrokeCheckTest(/*annotation_mode_enabled=*/true);
+
+  histograms.ExpectUniqueSample(kTypeMetric, StrokeMetricBrushType::kPen, 1);
+
+  // Draw a highlighter stroke.
+  TestAnnotationBrushMessageParams params = {/*color_r=*/242, /*color_g=*/139,
+                                             /*color_b=*/130};
+  SelectBrushTool(PdfInkBrush::Type::kHighlighter, 6.0f, params);
+  ApplyStrokeWithMouseAtMouseDownPoint();
+
+  histograms.ExpectBucketCount(kTypeMetric, StrokeMetricBrushType::kHighlighter,
+                               1);
+  histograms.ExpectTotalCount(kTypeMetric, 2);
+
+  // Draw an eraser stroke.
+  SelectEraserToolOfSize(3.0f);
+  ApplyStrokeWithMouseAtMouseDownPoint();
+
+  histograms.ExpectBucketCount(kTypeMetric, StrokeMetricBrushType::kEraser, 1);
+  histograms.ExpectTotalCount(kTypeMetric, 3);
+
+  // Draw an eraser stroke at a different point that does not erase any other
+  // strokes. The metric should stay the same.
+  ApplyStrokeWithMouseAtPoints(
+      kMouseUpPoint, base::span_from_ref(kMouseUpPoint), kMouseUpPoint);
+
+  histograms.ExpectBucketCount(kTypeMetric, StrokeMetricBrushType::kEraser, 1);
+  histograms.ExpectTotalCount(kTypeMetric, 3);
+
+  // Draw another pen stroke.
+  SelectBrushTool(PdfInkBrush::Type::kPen, 3.0f, params);
+  ApplyStrokeWithMouseAtMouseDownPoint();
+
+  histograms.ExpectBucketCount(kTypeMetric, StrokeMetricBrushType::kPen, 2);
+  histograms.ExpectTotalCount(kTypeMetric, 4);
+}
+
 }  // namespace chrome_pdf

tools/metrics/histograms/metadata/pdf/enums.xml

@@ -144,6 +144,16 @@ chromium-metrics-reviews@google.com.
   <int value="3" label="Foreground XFA (XFAF)"/>
 </enum>
 
+<!-- LINT.IfChange(PDFInk2StrokeBrushType) -->
+
+<enum name="PDFInk2StrokeBrushType">
+  <int value="0" label="Pen"/>
+  <int value="1" label="Highlighter"/>
+  <int value="2" label="Eraser"/>
+</enum>
+
+<!-- LINT.ThenChange(//pdf/pdf_ink_metrics_handler.h:PDFInk2StrokeBrushType) -->
+
 <enum name="PDFPostMessageDataType">
 <!-- This must be kept current with PostMessageDataType in
      chrome/browser/resources/pdf/pdf_viewer.ts. -->

tools/metrics/histograms/metadata/pdf/histograms.xml

@@ -77,6 +77,20 @@ chromium-metrics-reviews@google.com.
   </summary>
 </histogram>
 
+<histogram name="PDF.Ink2StrokeBrushType" enum="PDFInk2StrokeBrushType"
+    expires_after="2025-12-01">
+  <owner>andyphan@chromium.org</owner>
+  <owner>thestig@chromium.org</owner>
+  <summary>
+    Tracks the brush type used for Ink2 stroke modifications in the PDF viewer.
+    This includes new drawing strokes as well as erasing strokes. Erase strokes
+    that erase pre-existing strokes in a PDF are also included. This is only
+    recorded when drawing or erasing actions are performed by the user, but not
+    if they occur as part of undo or redo operations. Eraser strokes that do not
+    erase any other strokes are ignored.
+  </summary>
+</histogram>
+
 <histogram name="PDF.LoadStatus2" enum="ChromePDFViewerLoadStatus"
     expires_after="2025-03-30">
   <owner>kmoon@chromium.org</owner>