[PDF Ink Signatures] Add stroke brush size metric

Add a metric to track the brush sizes of draw strokes and erase strokes.

Bug: 380433757
Change-Id: I8006fe955c8646c3cd953c5024d48ca09a45fa9b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6044360
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@{#1388488}

pdf/pdf_ink_metrics_handler.cc

@@ -4,27 +4,70 @@
 
 #include "pdf/pdf_ink_metrics_handler.h"
 
+#include "base/containers/fixed_flat_map.h"
 #include "base/metrics/histogram_functions.h"
+#include "base/notreached.h"
 #include "pdf/pdf_ink_brush.h"
 
 namespace chrome_pdf {
 
 namespace {
 
-void ReportStrokeType(StrokeMetricBrushType type) {
+// Pens and erasers share the same sizes.
+constexpr auto kPenAndEraserSizes =
+    base::MakeFixedFlatMap<float, StrokeMetricBrushSize>({
+        {1.0f, StrokeMetricBrushSize::kExtraThin},
+        {2.0f, StrokeMetricBrushSize::kThin},
+        {3.0f, StrokeMetricBrushSize::kMedium},
+        {6.0f, StrokeMetricBrushSize::kThick},
+        {8.0f, StrokeMetricBrushSize::kExtraThick},
+    });
+
+constexpr auto kHighlighterSizes =
+    base::MakeFixedFlatMap<float, StrokeMetricBrushSize>({
+        {4.0f, StrokeMetricBrushSize::kExtraThin},
+        {6.0f, StrokeMetricBrushSize::kThin},
+        {8.0f, StrokeMetricBrushSize::kMedium},
+        {12.0f, StrokeMetricBrushSize::kThick},
+        {16.0f, StrokeMetricBrushSize::kExtraThick},
+    });
+
+void ReportStrokeTypeAndSize(StrokeMetricBrushType type,
+                             StrokeMetricBrushSize size) {
   base::UmaHistogramEnumeration("PDF.Ink2StrokeBrushType", type);
+  const char* size_metric = nullptr;
+  switch (type) {
+    case StrokeMetricBrushType::kPen:
+      size_metric = "PDF.Ink2StrokePenSize";
+      break;
+    case StrokeMetricBrushType::kHighlighter:
+      size_metric = "PDF.Ink2StrokeHighlighterSize";
+      break;
+    case StrokeMetricBrushType::kEraser:
+      size_metric = "PDF.Ink2StrokeEraserSize";
+      break;
+  };
+  CHECK(size_metric);
+  base::UmaHistogramEnumeration(size_metric, size);
 }
 
 }  // namespace
 
-void ReportDrawStroke(PdfInkBrush::Type type) {
+void ReportDrawStroke(PdfInkBrush::Type type, const ink::Brush& brush) {
   bool is_pen = type == PdfInkBrush::Type::kPen;
-  ReportStrokeType(is_pen ? StrokeMetricBrushType::kPen
+  const base::fixed_flat_map<float, StrokeMetricBrushSize, 5>& sizes =
-                          : StrokeMetricBrushType::kHighlighter);
+      is_pen ? kPenAndEraserSizes : kHighlighterSizes;
+  auto size_iter = sizes.find(brush.GetSize());
+  CHECK(size_iter != sizes.end());
+  ReportStrokeTypeAndSize(is_pen ? StrokeMetricBrushType::kPen
+                                 : StrokeMetricBrushType::kHighlighter,
+                          size_iter->second);
 }
 
-void ReportEraseStroke() {
+void ReportEraseStroke(float size) {
-  ReportStrokeType(StrokeMetricBrushType::kEraser);
+  auto iter = kPenAndEraserSizes.find(size);
+  CHECK(iter != kPenAndEraserSizes.end());
+  ReportStrokeTypeAndSize(StrokeMetricBrushType::kEraser, iter->second);
 }
 
 }  // namespace chrome_pdf

pdf/pdf_ink_metrics_handler.h

@@ -12,6 +12,20 @@ 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(PDFInk2StrokeBrushSize)
+enum class StrokeMetricBrushSize {
+  kExtraThin = 0,
+  kThin = 1,
+  kMedium = 2,
+  kThick = 3,
+  kExtraThick = 4,
+  kMaxValue = 4,
+};
+// LINT.ThenChange(//tools/metrics/histograms/metadata/pdf/enums.xml:PDFInk2StrokeBrushSize)
+
 // These values are persisted to logs. Entries should not be renumbered and
 // numeric values should never be reused.
 //
@@ -24,9 +38,9 @@ enum class StrokeMetricBrushType {
 };
 // LINT.ThenChange(//tools/metrics/histograms/metadata/pdf/enums.xml:PDFInk2StrokeBrushType)
 
-void ReportDrawStroke(PdfInkBrush::Type type);
+void ReportDrawStroke(PdfInkBrush::Type type, const ink::Brush& brush);
 
-void ReportEraseStroke();
+void ReportEraseStroke(float size);
 
 }  // namespace chrome_pdf
 

pdf/pdf_ink_module.cc

@@ -536,7 +536,7 @@ bool PdfInkModule::FinishStroke(const gfx::PointF& position,
   bool undo_redo_success = undo_redo_model_.FinishDraw();
   CHECK(undo_redo_success);
 
-  ReportDrawStroke(state.brush_type);
+  ReportDrawStroke(state.brush_type, GetDrawingBrush().ink_brush());
 
   // Reset `state` now that the stroke operation is done.
   state.inputs.clear();
@@ -608,7 +608,7 @@ bool PdfInkModule::FinishEraseStroke(const gfx::PointF& position) {
       client_->UpdateThumbnail(page_index);
     }
 
-    ReportEraseStroke();
+    ReportEraseStroke(eraser_size_);
   }
 
   // Reset `state` now that the erase operation is done.

pdf/pdf_ink_module_unittest.cc

@@ -1986,6 +1986,10 @@ TEST_F(PdfInkModuleGetVisibleStrokesTest, MultiplePageStrokes) {
 class PdfInkModuleMetricsTest : public PdfInkModuleUndoRedoTest {
  protected:
   static constexpr char kTypeMetric[] = "PDF.Ink2StrokeBrushType";
+  static constexpr char kPenSizeMetric[] = "PDF.Ink2StrokePenSize";
+  static constexpr char kHighlighterSizeMetric[] =
+      "PDF.Ink2StrokeHighlighterSize";
+  static constexpr char kEraserSizeMetric[] = "PDF.Ink2StrokeEraserSize";
 };
 
 TEST_F(PdfInkModuleMetricsTest, StrokeUndoRedoDoesNotAffectMetrics) {
@@ -1996,6 +2000,8 @@ TEST_F(PdfInkModuleMetricsTest, StrokeUndoRedoDoesNotAffectMetrics) {
   RunStrokeCheckTest(/*annotation_mode_enabled=*/true);
 
   histograms.ExpectUniqueSample(kTypeMetric, StrokeMetricBrushType::kPen, 1);
+  histograms.ExpectUniqueSample(kPenSizeMetric, StrokeMetricBrushSize::kMedium,
+                                1);
 
   // Undo and redo.
   PerformUndo();
@@ -2003,6 +2009,119 @@ TEST_F(PdfInkModuleMetricsTest, StrokeUndoRedoDoesNotAffectMetrics) {
 
   // The metrics should stay the same.
   histograms.ExpectUniqueSample(kTypeMetric, StrokeMetricBrushType::kPen, 1);
+  histograms.ExpectUniqueSample(kPenSizeMetric, StrokeMetricBrushSize::kMedium,
+                                1);
+}
+
+TEST_F(PdfInkModuleMetricsTest, StrokeBrushSizePen) {
+  InitializeSimpleSinglePageBasicLayout();
+  base::HistogramTester histograms;
+
+  // Draw a stroke.
+  RunStrokeCheckTest(/*annotation_mode_enabled=*/true);
+
+  histograms.ExpectUniqueSample(kPenSizeMetric, StrokeMetricBrushSize::kMedium,
+                                1);
+
+  TestAnnotationBrushMessageParams params = {/*color_r=*/242, /*color_g=*/139,
+                                             /*color_b=*/130};
+  SelectBrushTool(PdfInkBrush::Type::kPen, 1.0f, params);
+  ApplyStrokeWithMouseAtMouseDownPoint();
+
+  histograms.ExpectBucketCount(kPenSizeMetric,
+                               StrokeMetricBrushSize::kExtraThin, 1);
+  histograms.ExpectTotalCount(kPenSizeMetric, 2);
+
+  SelectBrushTool(PdfInkBrush::Type::kPen, 8.0f, params);
+  ApplyStrokeWithMouseAtMouseDownPoint();
+
+  histograms.ExpectBucketCount(kPenSizeMetric,
+                               StrokeMetricBrushSize::kExtraThick, 1);
+  histograms.ExpectTotalCount(kPenSizeMetric, 3);
+  histograms.ExpectTotalCount(kHighlighterSizeMetric, 0);
+  histograms.ExpectTotalCount(kEraserSizeMetric, 0);
+}
+
+TEST_F(PdfInkModuleMetricsTest, StrokeBrushSizeHighlighter) {
+  EnableAnnotationMode();
+  InitializeSimpleSinglePageBasicLayout();
+  base::HistogramTester histograms;
+
+  // Draw a stroke with medium size.
+  TestAnnotationBrushMessageParams params = {/*color_r=*/242, /*color_g=*/139,
+                                             /*color_b=*/130};
+  SelectBrushTool(PdfInkBrush::Type::kHighlighter, 8.0f, params);
+  ApplyStrokeWithMouseAtMouseDownPoint();
+
+  histograms.ExpectUniqueSample(kHighlighterSizeMetric,
+                                StrokeMetricBrushSize::kMedium, 1);
+
+  // Draw a stroke with extra thin size.
+  SelectBrushTool(PdfInkBrush::Type::kHighlighter, 4.0f, params);
+  ApplyStrokeWithMouseAtMouseDownPoint();
+
+  histograms.ExpectBucketCount(kHighlighterSizeMetric,
+                               StrokeMetricBrushSize::kExtraThin, 1);
+  histograms.ExpectTotalCount(kHighlighterSizeMetric, 2);
+
+  // Draw a stroke with extra thick size.
+  SelectBrushTool(PdfInkBrush::Type::kHighlighter, 16.0f, params);
+  ApplyStrokeWithMouseAtMouseDownPoint();
+
+  histograms.ExpectBucketCount(kHighlighterSizeMetric,
+                               StrokeMetricBrushSize::kExtraThick, 1);
+  histograms.ExpectTotalCount(kPenSizeMetric, 0);
+  histograms.ExpectTotalCount(kHighlighterSizeMetric, 3);
+  histograms.ExpectTotalCount(kEraserSizeMetric, 0);
+}
+
+TEST_F(PdfInkModuleMetricsTest, StrokeBrushSizeEraser) {
+  EnableAnnotationMode();
+  InitializeSimpleSinglePageBasicLayout();
+  base::HistogramTester histograms;
+
+  // Draw a pen stroke. Draw an eraser stroke that erases it with medium size.
+  TestAnnotationBrushMessageParams params = {/*color_r=*/242, /*color_g=*/139,
+                                             /*color_b=*/130};
+  SelectBrushTool(PdfInkBrush::Type::kPen, 3.0f, params);
+  ApplyStrokeWithMouseAtMouseDownPoint();
+  SelectEraserToolOfSize(3.0f);
+  ApplyStrokeWithMouseAtMouseDownPoint();
+
+  histograms.ExpectUniqueSample(kEraserSizeMetric,
+                                StrokeMetricBrushSize::kMedium, 1);
+
+  // Draw a pen stroke. Draw an eraser stroke that erases it with extra thin
+  // size.
+  SelectBrushTool(PdfInkBrush::Type::kPen, 3.0f, params);
+  ApplyStrokeWithMouseAtMouseDownPoint();
+  SelectEraserToolOfSize(1.0f);
+  ApplyStrokeWithMouseAtMouseDownPoint();
+
+  histograms.ExpectBucketCount(kEraserSizeMetric,
+                               StrokeMetricBrushSize::kExtraThin, 1);
+  histograms.ExpectTotalCount(kEraserSizeMetric, 2);
+
+  // Draw a pen stroke. Draw an eraser stroke that erases it with extra thick
+  // size.
+  SelectBrushTool(PdfInkBrush::Type::kPen, 3.0f, params);
+  ApplyStrokeWithMouseAtMouseDownPoint();
+  SelectEraserToolOfSize(8.0f);
+  ApplyStrokeWithMouseAtMouseDownPoint();
+
+  histograms.ExpectBucketCount(kEraserSizeMetric,
+                               StrokeMetricBrushSize::kExtraThick, 1);
+
+  // There should be no visible strokes on the page. Draw an eraser stroke that
+  // does not erase any other strokes. The metric should stay the same.
+  ApplyStrokeWithMouseAtMouseDownPoint();
+
+  histograms.ExpectBucketCount(kEraserSizeMetric,
+                               StrokeMetricBrushSize::kExtraThick, 1);
+
+  histograms.ExpectTotalCount(kPenSizeMetric, 3);
+  histograms.ExpectTotalCount(kHighlighterSizeMetric, 0);
+  histograms.ExpectTotalCount(kEraserSizeMetric, 3);
 }
 
 TEST_F(PdfInkModuleMetricsTest, StrokeBrushType) {

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

@@ -144,6 +144,18 @@ chromium-metrics-reviews@google.com.
   <int value="3" label="Foreground XFA (XFAF)"/>
 </enum>
 
+<!-- LINT.IfChange(PDFInk2StrokeBrushSize) -->
+
+<enum name="PDFInk2StrokeBrushSize">
+  <int value="0" label="ExtraThin"/>
+  <int value="1" label="Thin"/>
+  <int value="2" label="Medium"/>
+  <int value="3" label="Thick"/>
+  <int value="4" label="ExtraThick"/>
+</enum>
+
+<!-- LINT.ThenChange(//pdf/pdf_ink_metrics_handler.h:PDFInk2StrokeBrushSize) -->
+
 <!-- LINT.IfChange(PDFInk2StrokeBrushType) -->
 
 <enum name="PDFInk2StrokeBrushType">

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

@@ -91,6 +91,25 @@ chromium-metrics-reviews@google.com.
   </summary>
 </histogram>
 
+<histogram name="PDF.Ink2Stroke{Brush}Size" enum="PDFInk2StrokeBrushSize"
+    expires_after="2025-12-01">
+  <owner>andyphan@chromium.org</owner>
+  <owner>thestig@chromium.org</owner>
+  <summary>
+    Tracks the brush size used for an Ink2 {Brush} stroke modification 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>
+  <token key="Brush">
+    <variant name="Eraser"/>
+    <variant name="Highlighter"/>
+    <variant name="Pen"/>
+  </token>
+</histogram>
+
 <histogram name="PDF.LoadStatus2" enum="ChromePDFViewerLoadStatus"
     expires_after="2025-03-30">
   <owner>kmoon@chromium.org</owner>