Commit bffb04d8 authored by Sammie Quon's avatar Sammie Quon Committed by Commit Bot

capture_mode: Metric to track if user has switched modes in a session.

Also moves some histogram codes to capture_mode_metrics.

Bug: 1140182
Test: added tests
Change-Id: I14fcb45512333ac3978884aae6db6a83cb1e8110
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2530220
Commit-Queue: Sammie Quon <sammiequon@chromium.org>
Reviewed-by: default avatarAhmed Fakhry <afakhry@chromium.org>
Reviewed-by: default avatarRobert Kaplow <rkaplow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#826938}
parent ef54488d
...@@ -54,9 +54,6 @@ namespace { ...@@ -54,9 +54,6 @@ namespace {
CaptureModeController* g_instance = nullptr; CaptureModeController* g_instance = nullptr;
constexpr char kRecordTimeHistogramName[] =
"Ash.CaptureModeController.ScreenRecordingLength";
constexpr char kScreenCaptureNotificationId[] = "capture_mode_notification"; constexpr char kScreenCaptureNotificationId[] = "capture_mode_notification";
constexpr char kScreenCaptureStoppedNotificationId[] = constexpr char kScreenCaptureStoppedNotificationId[] =
"capture_mode_stopped_notification"; "capture_mode_stopped_notification";
...@@ -333,8 +330,8 @@ void CaptureModeController::PerformCapture() { ...@@ -333,8 +330,8 @@ void CaptureModeController::PerformCapture() {
return; return;
} }
if (source_ == CaptureModeSource::kRegion) DCHECK(capture_mode_session_);
capture_mode_session_->ReportRegionCaptureHistograms(); capture_mode_session_->ReportSessionHistograms();
if (type_ == CaptureModeType::kImage) if (type_ == CaptureModeType::kImage)
CaptureImage(); CaptureImage();
...@@ -587,13 +584,8 @@ void CaptureModeController::OnVideoFileSaved(bool success) { ...@@ -587,13 +584,8 @@ void CaptureModeController::OnVideoFileSaved(bool success) {
ShowPreviewNotification(current_video_file_path_, gfx::Image(), ShowPreviewNotification(current_video_file_path_, gfx::Image(),
CaptureModeType::kVideo); CaptureModeType::kVideo);
DCHECK(!recording_start_time_.is_null()); DCHECK(!recording_start_time_.is_null());
// Use custom counts macro instead of custom times so we can record in RecordCaptureModeRecordTime(
// seconds instead of milliseconds. The max bucket is 3 hours. (base::TimeTicks::Now() - recording_start_time_).InSeconds());
base::UmaHistogramCustomCounts(
kRecordTimeHistogramName,
(base::TimeTicks::Now() - recording_start_time_).InSeconds(), /*min=*/1,
/*max=*/base::TimeDelta::FromHours(3).InSeconds(),
/*bucket_count=*/50);
} }
if (!on_file_saved_callback_.is_null()) if (!on_file_saved_callback_.is_null())
......
...@@ -16,6 +16,10 @@ constexpr char kCaptureRegionAdjustmentHistogramName[] = ...@@ -16,6 +16,10 @@ constexpr char kCaptureRegionAdjustmentHistogramName[] =
constexpr char kBarButtonHistogramName[] = constexpr char kBarButtonHistogramName[] =
"Ash.CaptureModeController.BarButtons"; "Ash.CaptureModeController.BarButtons";
constexpr char kEntryHistogramName[] = "Ash.CaptureModeController.EntryPoint"; constexpr char kEntryHistogramName[] = "Ash.CaptureModeController.EntryPoint";
constexpr char kRecordTimeHistogramName[] =
"Ash.CaptureModeController.ScreenRecordingLength";
constexpr char kSwitchesFromInitialModeHistogramName[] =
"Ash.CaptureModeController.SwitchesFromInitialCaptureMode";
// Appends the proper suffix to |prefix| based on whether the user is in tablet // Appends the proper suffix to |prefix| based on whether the user is in tablet
// mode or not. // mode or not.
...@@ -43,4 +47,17 @@ void RecordNumberOfCaptureRegionAdjustments(int num_adjustments) { ...@@ -43,4 +47,17 @@ void RecordNumberOfCaptureRegionAdjustments(int num_adjustments) {
num_adjustments); 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.
base::UmaHistogramCustomCounts(
kRecordTimeHistogramName, length_in_seconds, /*min=*/1,
/*max=*/base::TimeDelta::FromHours(3).InSeconds(),
/*bucket_count=*/50);
}
void RecordCaptureModeSwitchesFromInitialMode(bool switched) {
base::UmaHistogramBoolean(kSwitchesFromInitialModeHistogramName, switched);
}
} // namespace ash } // namespace ash
...@@ -5,6 +5,8 @@ ...@@ -5,6 +5,8 @@
#ifndef ASH_CAPTURE_MODE_CAPTURE_MODE_METRICS_H_ #ifndef ASH_CAPTURE_MODE_CAPTURE_MODE_METRICS_H_
#define ASH_CAPTURE_MODE_CAPTURE_MODE_METRICS_H_ #define ASH_CAPTURE_MODE_CAPTURE_MODE_METRICS_H_
#include <stdint.h>
namespace ash { namespace ash {
// Enumeration of capture bar buttons that can be pressed while in capture mode. // Enumeration of capture bar buttons that can be pressed while in capture mode.
...@@ -45,6 +47,12 @@ void RecordCaptureModeEntryType(CaptureModeEntryType entry_type); ...@@ -45,6 +47,12 @@ void RecordCaptureModeEntryType(CaptureModeEntryType entry_type);
// capture sources. // capture sources.
void RecordNumberOfCaptureRegionAdjustments(int num_adjustments); 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 if the user has switched modes during a capture session.
void RecordCaptureModeSwitchesFromInitialMode(bool switched);
} // namespace ash } // namespace ash
#endif // ASH_CAPTURE_MODE_CAPTURE_MODE_METRICS_H_ #endif // ASH_CAPTURE_MODE_CAPTURE_MODE_METRICS_H_
...@@ -310,6 +310,8 @@ aura::Window* CaptureModeSession::GetSelectedWindow() const { ...@@ -310,6 +310,8 @@ aura::Window* CaptureModeSession::GetSelectedWindow() const {
} }
void CaptureModeSession::OnCaptureSourceChanged(CaptureModeSource new_source) { void CaptureModeSession::OnCaptureSourceChanged(CaptureModeSource new_source) {
capture_source_changed_ = true;
if (new_source == CaptureModeSource::kWindow) { if (new_source == CaptureModeSource::kWindow) {
capture_window_observer_ = capture_window_observer_ =
std::make_unique<CaptureWindowObserver>(this, controller_->type()); std::make_unique<CaptureWindowObserver>(this, controller_->type());
...@@ -341,10 +343,12 @@ void CaptureModeSession::OnCaptureTypeChanged(CaptureModeType new_type) { ...@@ -341,10 +343,12 @@ void CaptureModeSession::OnCaptureTypeChanged(CaptureModeType new_type) {
UpdateCaptureLabelWidget(); UpdateCaptureLabelWidget();
} }
void CaptureModeSession::ReportRegionCaptureHistograms() { void CaptureModeSession::ReportSessionHistograms() {
DCHECK_EQ(controller_->source(), CaptureModeSource::kRegion); if (controller_->source() == CaptureModeSource::kRegion)
RecordNumberOfCaptureRegionAdjustments(num_capture_region_adjusted_); RecordNumberOfCaptureRegionAdjustments(num_capture_region_adjusted_);
num_capture_region_adjusted_ = 0; num_capture_region_adjusted_ = 0;
RecordCaptureModeSwitchesFromInitialMode(capture_source_changed_);
} }
void CaptureModeSession::StartCountDown( void CaptureModeSession::StartCountDown(
......
...@@ -71,9 +71,9 @@ class ASH_EXPORT CaptureModeSession : public ui::LayerOwner, ...@@ -71,9 +71,9 @@ class ASH_EXPORT CaptureModeSession : public ui::LayerOwner,
void OnCaptureSourceChanged(CaptureModeSource new_source); void OnCaptureSourceChanged(CaptureModeSource new_source);
void OnCaptureTypeChanged(CaptureModeType new_type); void OnCaptureTypeChanged(CaptureModeType new_type);
// Called when the capture source is region and the user performs a capture. // Called when the user performs a capture. Records histograms related to this
// Records |num_capture_region_adjusted_| to corresponding histogram. // session.
void ReportRegionCaptureHistograms(); void ReportSessionHistograms();
// Called when starting 3-seconds count down before recording video. // Called when starting 3-seconds count down before recording video.
void StartCountDown(base::OnceClosure countdown_finished_callback); void StartCountDown(base::OnceClosure countdown_finished_callback);
...@@ -256,6 +256,10 @@ class ASH_EXPORT CaptureModeSession : public ui::LayerOwner, ...@@ -256,6 +256,10 @@ class ASH_EXPORT CaptureModeSession : public ui::LayerOwner,
// This should be reset when a user creates a new capture region, changes // This should be reset when a user creates a new capture region, changes
// capture sources or when a user performs a capture. // capture sources or when a user performs a capture.
int num_capture_region_adjusted_ = 0; int num_capture_region_adjusted_ = 0;
// True if at any point during the lifetime of |this|, the capture source
// changed. Used for metrics collection.
bool capture_source_changed_ = false;
}; };
} // namespace ash } // namespace ash
......
...@@ -1269,7 +1269,7 @@ TEST_F(CaptureModeTest, ShuttingDownWhileRecording) { ...@@ -1269,7 +1269,7 @@ TEST_F(CaptureModeTest, ShuttingDownWhileRecording) {
// VideoRecordingWatcher::OnChromeTerminating() terminates the recording. // VideoRecordingWatcher::OnChromeTerminating() terminates the recording.
} }
// Tests that metrics are recorded properly for capture mode snap buttons. // Tests that metrics are recorded properly for capture mode bar buttons.
TEST_F(CaptureModeTest, CaptureModeBarButtonTypeHistograms) { TEST_F(CaptureModeTest, CaptureModeBarButtonTypeHistograms) {
constexpr char kClamshellHistogram[] = constexpr char kClamshellHistogram[] =
"Ash.CaptureModeController.BarButtons.ClamshellMode"; "Ash.CaptureModeController.BarButtons.ClamshellMode";
...@@ -1280,7 +1280,7 @@ TEST_F(CaptureModeTest, CaptureModeBarButtonTypeHistograms) { ...@@ -1280,7 +1280,7 @@ TEST_F(CaptureModeTest, CaptureModeBarButtonTypeHistograms) {
CaptureModeController::Get()->Start(CaptureModeEntryType::kQuickSettings); CaptureModeController::Get()->Start(CaptureModeEntryType::kQuickSettings);
auto* event_generator = GetEventGenerator(); auto* event_generator = GetEventGenerator();
// Tests each snap button in clamshell mode. // Tests each bar button in clamshell mode.
ClickOnView(GetImageToggleButton(), event_generator); ClickOnView(GetImageToggleButton(), event_generator);
histogram_tester.ExpectBucketCount( histogram_tester.ExpectBucketCount(
kClamshellHistogram, CaptureModeBarButtonType::kScreenCapture, 1); kClamshellHistogram, CaptureModeBarButtonType::kScreenCapture, 1);
...@@ -1301,7 +1301,7 @@ TEST_F(CaptureModeTest, CaptureModeBarButtonTypeHistograms) { ...@@ -1301,7 +1301,7 @@ TEST_F(CaptureModeTest, CaptureModeBarButtonTypeHistograms) {
histogram_tester.ExpectBucketCount(kClamshellHistogram, histogram_tester.ExpectBucketCount(kClamshellHistogram,
CaptureModeBarButtonType::kWindow, 1); CaptureModeBarButtonType::kWindow, 1);
// Enter tablet mode and test the snap buttons. // Enter tablet mode and test the bar buttons.
auto* tablet_mode_controller = Shell::Get()->tablet_mode_controller(); auto* tablet_mode_controller = Shell::Get()->tablet_mode_controller();
tablet_mode_controller->SetEnabledForTest(true); tablet_mode_controller->SetEnabledForTest(true);
ASSERT_TRUE(tablet_mode_controller->InTabletMode()); ASSERT_TRUE(tablet_mode_controller->InTabletMode());
...@@ -1327,6 +1327,39 @@ TEST_F(CaptureModeTest, CaptureModeBarButtonTypeHistograms) { ...@@ -1327,6 +1327,39 @@ TEST_F(CaptureModeTest, CaptureModeBarButtonTypeHistograms) {
CaptureModeBarButtonType::kWindow, 1); CaptureModeBarButtonType::kWindow, 1);
} }
TEST_F(CaptureModeTest, CaptureSessionSwitchedModeMetric) {
constexpr char kHistogramName[] =
"Ash.CaptureModeController.SwitchesFromInitialCaptureMode";
base::HistogramTester histogram_tester;
histogram_tester.ExpectBucketCount(kHistogramName, false, 0);
histogram_tester.ExpectBucketCount(kHistogramName, true, 0);
// Perform a capture without switching modes. A false should be recorded.
auto* controller = StartImageRegionCapture();
SelectRegion(gfx::Rect(100, 100));
auto* event_generator = GetEventGenerator();
SendKey(ui::VKEY_RETURN, event_generator);
histogram_tester.ExpectBucketCount(kHistogramName, false, 1);
histogram_tester.ExpectBucketCount(kHistogramName, true, 0);
// Perform a capture after switching to fullscreen mode. A true should be
// recorded.
controller->Start(CaptureModeEntryType::kQuickSettings);
ClickOnView(GetFullscreenToggleButton(), event_generator);
SendKey(ui::VKEY_RETURN, event_generator);
histogram_tester.ExpectBucketCount(kHistogramName, false, 1);
histogram_tester.ExpectBucketCount(kHistogramName, true, 1);
// Perform a capture after switching to another mode and back to the original
// mode. A true should still be recorded as there was some switching done.
controller->Start(CaptureModeEntryType::kQuickSettings);
ClickOnView(GetRegionToggleButton(), event_generator);
ClickOnView(GetFullscreenToggleButton(), event_generator);
SendKey(ui::VKEY_RETURN, event_generator);
histogram_tester.ExpectBucketCount(kHistogramName, false, 1);
histogram_tester.ExpectBucketCount(kHistogramName, true, 2);
}
// Test that cancel recording during countdown won't cause crash. // Test that cancel recording during countdown won't cause crash.
TEST_F(CaptureModeTest, CancelCaptureDuringCountDown) { TEST_F(CaptureModeTest, CancelCaptureDuringCountDown) {
ui::ScopedAnimationDurationScaleMode animatin_scale( ui::ScopedAnimationDurationScaleMode animatin_scale(
......
...@@ -258,6 +258,16 @@ reviews. Googlers can read more about this at go/gwsq-gerrit. ...@@ -258,6 +258,16 @@ reviews. Googlers can read more about this at go/gwsq-gerrit.
</summary> </summary>
</histogram> </histogram>
<histogram name="Ash.CaptureModeController.SwitchesFromInitialCaptureMode"
enum="Boolean" expires_after="2021-11-11">
<owner>chinsenj@chromium.org</owner>
<owner>gzadina@google.com</owner>
<summary>
Emits true if a user has switched capture modes (fullscreen, region, window)
while in a capture mode session, false otherwise.
</summary>
</histogram>
<histogram name="Ash.ClipboardHistory.ContextMenu.DisplayFormatDeleted" <histogram name="Ash.ClipboardHistory.ContextMenu.DisplayFormatDeleted"
enum="ClipboardHistoryDisplayFormat" expires_after="2021-09-01"> enum="ClipboardHistoryDisplayFormat" expires_after="2021-09-01">
<owner>andrewxu@chromium.org</owner> <owner>andrewxu@chromium.org</owner>
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment