capture_mode: Add metrics for the number of consecutive screenshots.

This CL adds metrics for counting the number of consecutive screenshots
a user takes. Visit go/capture-mode-metrics to see other planned
metrics.

Test: manual + added
Bug: 1140182
Change-Id: Ic1b39e3601fd890ad42b9d9e18678b60793cd7cb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2532997
Commit-Queue: Jeremy Chinsen <chinsenj@chromium.org>
Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Reviewed-by: Weilun Shi <sweilun@chromium.org>
Reviewed-by: Sammie Quon <sammiequon@chromium.org>
Cr-Commit-Position: refs/heads/master@{#827364}

ash/capture_mode/capture_mode_controller.cc

@@ -54,6 +54,11 @@ namespace {
 
 CaptureModeController* g_instance = nullptr;
 
+// The amount of time that can elapse from the prior screenshot to be considered
+// consecutive.
+constexpr base::TimeDelta kConsecutiveScreenshotThreshold =
+    base::TimeDelta::FromSeconds(5);
+
 constexpr char kScreenCaptureNotificationId[] = "capture_mode_notification";
 constexpr char kScreenCaptureStoppedNotificationId[] =
     "capture_mode_stopped_notification";
@@ -247,7 +252,12 @@ CaptureModeController::CaptureModeController(
           // service.
           {base::MayBlock(), base::TaskPriority::BEST_EFFORT,
            base::TaskShutdownBehavior::SKIP_ON_SHUTDOWN})),
-      recording_service_client_receiver_(this) {
+      recording_service_client_receiver_(this),
+      num_consecutive_screenshots_scheduler_(
+          FROM_HERE,
+          kConsecutiveScreenshotThreshold,
+          this,
+          &CaptureModeController::RecordAndResetConsecutiveScreenshots) {
   DCHECK_EQ(g_instance, nullptr);
   g_instance = this;
 
@@ -259,14 +269,14 @@ CaptureModeController::CaptureModeController(
   num_screenshots_taken_in_last_day_scheduler_.Start(
       FROM_HERE, base::TimeDelta::FromDays(1),
       base::BindRepeating(
-          &CaptureModeController::RecordNumberOfScreenshotsTakenInLastDay,
+          &CaptureModeController::RecordAndResetScreenshotsTakenInLastDay,
           weak_ptr_factory_.GetWeakPtr()));
 
   // Schedule recording of the number of screenshots taken per week.
   num_screenshots_taken_in_last_week_scheduler_.Start(
       FROM_HERE, base::TimeDelta::FromDays(7),
       base::BindRepeating(
-          &CaptureModeController::RecordNumberOfScreenshotsTakenInLastWeek,
+          &CaptureModeController::RecordAndResetScreenshotsTakenInLastWeek,
           weak_ptr_factory_.GetWeakPtr()));
 
   // TODO(afakhry): Explore starting this only when a video recording starts, so
@@ -513,6 +523,9 @@ void CaptureModeController::CaptureImage() {
 
   ++num_screenshots_taken_in_last_day_;
   ++num_screenshots_taken_in_last_week_;
+
+  ++num_consecutive_screenshots_;
+  num_consecutive_screenshots_scheduler_.Reset();
 }
 
 void CaptureModeController::CaptureVideo() {
@@ -684,18 +697,21 @@ base::FilePath CaptureModeController::BuildPath(const char* const format_string,
       GetTimeStr(exploded_time, delegate_->Uses24HourFormat()).c_str()));
 }
 
-void CaptureModeController::RecordNumberOfScreenshotsTakenInLastDay() {
-  base::UmaHistogramCounts100("Ash.CaptureModeController.ScreenshotsPerDay",
-                              num_screenshots_taken_in_last_day_);
+void CaptureModeController::RecordAndResetScreenshotsTakenInLastDay() {
+  RecordNumberOfScreenshotsTakenInLastDay(num_screenshots_taken_in_last_day_);
   num_screenshots_taken_in_last_day_ = 0;
 }
 
-void CaptureModeController::RecordNumberOfScreenshotsTakenInLastWeek() {
-  base::UmaHistogramCounts1000("Ash.CaptureModeController.ScreenshotsPerWeek",
-                               num_screenshots_taken_in_last_week_);
+void CaptureModeController::RecordAndResetScreenshotsTakenInLastWeek() {
+  RecordNumberOfScreenshotsTakenInLastWeek(num_screenshots_taken_in_last_week_);
   num_screenshots_taken_in_last_week_ = 0;
 }
 
+void CaptureModeController::RecordAndResetConsecutiveScreenshots() {
+  RecordNumberOfConsecutiveScreenshots(num_consecutive_screenshots_);
+  num_consecutive_screenshots_ = 0;
+}
+
 void CaptureModeController::OnVideoRecordCountDownFinished() {
   // If this event is dispatched after the capture session was cancelled or
   // destroyed, this should be a no-op.

ash/capture_mode/capture_mode_controller.h

@@ -185,8 +185,12 @@ class ASH_EXPORT CaptureModeController
                            base::Time timestamp) const;
 
   // Records the number of screenshots taken.
-  void RecordNumberOfScreenshotsTakenInLastDay();
-  void RecordNumberOfScreenshotsTakenInLastWeek();
+  void RecordAndResetScreenshotsTakenInLastDay();
+  void RecordAndResetScreenshotsTakenInLastWeek();
+
+  // Records the number of consecutive screenshots taken within 5s of each
+  // other.
+  void RecordAndResetConsecutiveScreenshots();
 
   // Called when the video record 3-seconds count down finishes.
   void OnVideoRecordCountDownFinished();
@@ -250,10 +254,16 @@ class ASH_EXPORT CaptureModeController
   base::RepeatingTimer num_screenshots_taken_in_last_day_scheduler_;
   base::RepeatingTimer num_screenshots_taken_in_last_week_scheduler_;
 
-  // Counters used to track the number of screenshots taken.
+  // Counters used to track the number of screenshots taken. These values are
+  // not persisted across crashes, restarts or sessions so they only provide a
+  // rough approximation.
   int num_screenshots_taken_in_last_day_ = 0;
   int num_screenshots_taken_in_last_week_ = 0;
 
+  // Counter used to track the number of consecutive screenshots taken.
+  int num_consecutive_screenshots_ = 0;
+  base::DelayTimer num_consecutive_screenshots_scheduler_;
+
   // The time when OnVideoRecordCountDownFinished is called and video has
   // started recording. It is used when video has finished recording for metrics
   // collection.

ash/capture_mode/capture_mode_metrics.cc

@@ -11,13 +11,19 @@ namespace ash {
 
 namespace {
 
-constexpr char kCaptureRegionAdjustmentHistogramName[] =
-    "Ash.CaptureModeController.CaptureRegionAdjusted";
 constexpr char kBarButtonHistogramName[] =
     "Ash.CaptureModeController.BarButtons";
+constexpr char kCaptureRegionAdjustmentHistogramName[] =
+    "Ash.CaptureModeController.CaptureRegionAdjusted";
+constexpr char kConsecutiveScreenshotHistogramName[] =
+    "Ash.CaptureModeController.ConsecutiveScreenshots";
 constexpr char kEntryHistogramName[] = "Ash.CaptureModeController.EntryPoint";
 constexpr char kRecordTimeHistogramName[] =
     "Ash.CaptureModeController.ScreenRecordingLength";
+constexpr char kScreenshotsPerDayHistogramName[] =
+    "Ash.CaptureModeController.ScreenshotsPerDay";
+constexpr char kScreenshotsPerWeekHistogramName[] =
+    "Ash.CaptureModeController.ScreenshotsPerWeek";
 constexpr char kSwitchesFromInitialModeHistogramName[] =
     "Ash.CaptureModeController.SwitchesFromInitialCaptureMode";
 
@@ -41,12 +47,6 @@ void RecordCaptureModeEntryType(CaptureModeEntryType entry_type) {
       GetCaptureModeHistogramName(kEntryHistogramName), entry_type);
 }
 
-void RecordNumberOfCaptureRegionAdjustments(int num_adjustments) {
-  base::UmaHistogramCounts100(
-      GetCaptureModeHistogramName(kCaptureRegionAdjustmentHistogramName),
-      num_adjustments);
-}
-
 void RecordCaptureModeRecordTime(int64_t length_in_seconds) {
   // Use custom counts macro instead of custom times so we can record in
   // seconds instead of milliseconds. The max bucket is 3 hours.
@@ -60,4 +60,29 @@ void RecordCaptureModeSwitchesFromInitialMode(bool switched) {
   base::UmaHistogramBoolean(kSwitchesFromInitialModeHistogramName, switched);
 }
 
+void RecordNumberOfCaptureRegionAdjustments(int num_adjustments) {
+  base::UmaHistogramCounts100(
+      GetCaptureModeHistogramName(kCaptureRegionAdjustmentHistogramName),
+      num_adjustments);
+}
+
+void RecordNumberOfConsecutiveScreenshots(int num_consecutive_screenshots) {
+  if (num_consecutive_screenshots > 1) {
+    base::UmaHistogramCounts100(kConsecutiveScreenshotHistogramName,
+                                num_consecutive_screenshots);
+  }
+}
+
+void RecordNumberOfScreenshotsTakenInLastDay(
+    int num_screenshots_taken_in_last_day) {
+  base::UmaHistogramCounts100(kScreenshotsPerDayHistogramName,
+                              num_screenshots_taken_in_last_day);
+}
+
+void RecordNumberOfScreenshotsTakenInLastWeek(
+    int num_screenshots_taken_in_last_week) {
+  base::UmaHistogramCounts100(kScreenshotsPerWeekHistogramName,
+                              num_screenshots_taken_in_last_week);
+}
+
 }  // namespace ash

ash/capture_mode/capture_mode_metrics.h

@@ -40,6 +40,12 @@ void RecordCaptureModeBarButtonType(CaptureModeBarButtonType button_type);
 // Records the method the user enters capture mode given by |entry_type|.
 void RecordCaptureModeEntryType(CaptureModeEntryType entry_type);
 
+// Records the length in seconds of a recording taken by capture mode.
+void RecordCaptureModeRecordTime(int64_t length_in_seconds);
+
+// Records if the user has switched modes during a capture session.
+void RecordCaptureModeSwitchesFromInitialMode(bool switched);
+
 // Records the number of times a user adjusts a capture region. This includes
 // moving and resizing. The count is started when a user sets the capture source
 // as a region. The count is recorded and reset when a user performs a capture.
@@ -47,11 +53,17 @@ void RecordCaptureModeEntryType(CaptureModeEntryType entry_type);
 // capture sources.
 void RecordNumberOfCaptureRegionAdjustments(int num_adjustments);
 
-// Records the length in seconds of a recording taken by capture mode.
-void RecordCaptureModeRecordTime(int64_t length_in_seconds);
+// Records the number of times a user consecutively screenshots. Only records a
+// sample if |num_consecutive_screenshots| is greater than 1.
+void RecordNumberOfConsecutiveScreenshots(int num_consecutive_screenshots);
 
-// Records if the user has switched modes during a capture session.
-void RecordCaptureModeSwitchesFromInitialMode(bool switched);
+// Records the number of screenshots taken. This metric is meant to be a rough
+// approximation so its counts are not persisted across crashes, restarts or
+// sessions.
+void RecordNumberOfScreenshotsTakenInLastDay(
+    int num_screenshots_taken_in_last_day);
+void RecordNumberOfScreenshotsTakenInLastWeek(
+    int num_screenshots_taken_in_last_week);
 
 }  // namespace ash
 

ash/capture_mode/capture_mode_unittests.cc

@@ -115,6 +115,8 @@ class CaptureModeSessionTestApi {
 class CaptureModeTest : public AshTestBase {
  public:
   CaptureModeTest() = default;
+  CaptureModeTest(base::test::TaskEnvironment::TimeSource time)
+      : AshTestBase(time) {}
   CaptureModeTest(const CaptureModeTest&) = delete;
   CaptureModeTest& operator=(const CaptureModeTest&) = delete;
   ~CaptureModeTest() override = default;
@@ -1480,4 +1482,53 @@ TEST_F(CaptureModeTest, FullscreenCapture) {
   EXPECT_FALSE(controller->IsActive());
 }
 
+// A test class that uses a mock time task environment.
+class CaptureModeMockTimeTest : public CaptureModeTest {
+ public:
+  CaptureModeMockTimeTest()
+      : CaptureModeTest(base::test::TaskEnvironment::TimeSource::MOCK_TIME) {}
+  CaptureModeMockTimeTest(const CaptureModeMockTimeTest&) = delete;
+  CaptureModeMockTimeTest& operator=(const CaptureModeMockTimeTest&) = delete;
+  ~CaptureModeMockTimeTest() override = default;
+};
+
+// Tests that the consecutive screenshots histogram is recorded properly.
+TEST_F(CaptureModeMockTimeTest, ConsecutiveScreenshotsHistograms) {
+  constexpr char kConsecutiveScreenshotsHistogram[] =
+      "Ash.CaptureModeController.ConsecutiveScreenshots";
+  base::HistogramTester histogram_tester;
+
+  auto take_n_screenshots = [this](int n) {
+    for (int i = 0; i < n; ++i) {
+      auto* controller = StartImageRegionCapture();
+      controller->PerformCapture();
+    }
+  };
+
+  // Take three consecutive screenshots. Should only record after 5 seconds.
+  StartImageRegionCapture();
+  const gfx::Rect capture_region(200, 200, 400, 400);
+  SelectRegion(capture_region);
+  take_n_screenshots(3);
+  histogram_tester.ExpectBucketCount(kConsecutiveScreenshotsHistogram, 3, 0);
+  task_environment()->FastForwardBy(base::TimeDelta::FromSeconds(5));
+  histogram_tester.ExpectBucketCount(kConsecutiveScreenshotsHistogram, 3, 1);
+
+  // Take only one screenshot. This should not be recorded.
+  take_n_screenshots(1);
+  histogram_tester.ExpectBucketCount(kConsecutiveScreenshotsHistogram, 1, 0);
+  task_environment()->FastForwardBy(base::TimeDelta::FromSeconds(5));
+  histogram_tester.ExpectBucketCount(kConsecutiveScreenshotsHistogram, 1, 0);
+
+  // Take a screenshot, change source and take another screenshot. This should
+  // count as 2 consecutive screenshots.
+  take_n_screenshots(1);
+  auto* controller = StartCaptureSession(CaptureModeSource::kFullscreen,
+                                         CaptureModeType::kImage);
+  controller->PerformCapture();
+  histogram_tester.ExpectBucketCount(kConsecutiveScreenshotsHistogram, 2, 0);
+  task_environment()->FastForwardBy(base::TimeDelta::FromSeconds(5));
+  histogram_tester.ExpectBucketCount(kConsecutiveScreenshotsHistogram, 2, 1);
+}
+
 }  // namespace ash

tools/metrics/histograms/histograms_xml/ash/histograms.xml

@@ -184,7 +184,7 @@ reviews. Googlers can read more about this at go/gwsq-gerrit.
 </histogram>
 
 <histogram name="Ash.CaptureModeController.BarButtons.{TabletOrClamshell}"
-    enum="CaptureModeEntryType" expires_after="2021-09-29">
+    enum="CaptureModeBarButtonType" expires_after="2021-09-29">
   <owner>chinsenj@chromium.org</owner>
   <owner>gzadina@google.com</owner>
   <summary>
@@ -210,6 +210,16 @@ reviews. Googlers can read more about this at go/gwsq-gerrit.
   <token key="TabletOrClamshell" variants="DisplayModes"/>
 </histogram>
 
+<histogram name="Ash.CaptureModeController.ConsecutiveScreenshots"
+    units="consecutive screenshots" expires_after="2021-11-10">
+  <owner>chinsenj@chromium.org</owner>
+  <owner>gzadina@google.com</owner>
+  <summary>
+    Records the number of consecutive screenshots the user takes within 5s of
+    each other, regardless of capture options.
+  </summary>
+</histogram>
+
 <histogram name="Ash.CaptureModeController.EntryPoint.{TabletOrClamshell}"
     enum="CaptureModeEntryType" expires_after="2021-09-29">
   <owner>chinsenj@chromium.org</owner>