From 2d5ba6a2f17a24343dbb51589308743074585b17 Mon Sep 17 00:00:00 2001 From: Caitlin Fischer Date: Fri, 6 May 2022 14:31:29 +0000 Subject: [PATCH] [M102] Add a metric for the extended browser crash monitoring duration. (cherry picked from commit 49058f972ce3a7217edfaca0547a6a50fc778eec) Bug: 1321465 Change-Id: I808216e2d9fe5afe36ff999d0421b771a07e49a9 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3615364 Reviewed-by: Siddhartha S Commit-Queue: Caitlin Fischer Reviewed-by: Ilya Sherman Reviewed-by: Peter Wen Cr-Original-Commit-Position: refs/heads/main@{#998717} Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3629706 Cr-Commit-Position: refs/branch-heads/5005@{#494} Cr-Branched-From: 5b4d9450fee01f821b6400e947b3839727643a71-refs/heads/main@{#992738} --- components/metrics/clean_exit_beacon.cc | 20 ++++++ components/metrics/clean_exit_beacon.h | 6 ++ .../metrics/clean_exit_beacon_unittest.cc | 63 +++++++++++++++++++ .../histograms/metadata/uma/histograms.xml | 14 +++++ 4 files changed, 103 insertions(+) diff --git a/components/metrics/clean_exit_beacon.cc b/components/metrics/clean_exit_beacon.cc index da22bceb9613ec..557b53b9147a20 100644 --- a/components/metrics/clean_exit_beacon.cc +++ b/components/metrics/clean_exit_beacon.cc @@ -366,6 +366,11 @@ void CleanExitBeacon::WriteBeaconValue(bool exited_cleanly, if (is_extended_safe_mode) { DCHECK_EQ(group_name, kEnabledGroup); DCHECK(!exited_cleanly); + +#if BUILDFLAG(IS_ANDROID) + extended_monitoring_stage_start_time_ = base::TimeTicks::Now(); +#endif + SCOPED_UMA_HISTOGRAM_TIMER_MICROS( "Variations.ExtendedSafeMode.WritePrefsTime"); // The beacon value is written to disk synchronously twice during @@ -390,6 +395,21 @@ void CleanExitBeacon::WriteBeaconValue(bool exited_cleanly, } else { local_state_->SetBoolean(prefs::kStabilityExitedCleanly, exited_cleanly); local_state_->CommitPendingWrite(); // Schedule a write. +#if BUILDFLAG(IS_ANDROID) + if (!extended_monitoring_stage_start_time_.is_null()) { + // The time exists, so this is the transition from the extended browser + // crash monitoring stage to the status quo stage. Only Extended + // Variations Safe Mode enabled-group clients have the extended monitoring + // stage. + // TODO(crbug/1321989): Clean up this metric and + // |extended_monitoring_stage_start_time_| once Android Chrome + // stakeholders have enough data on the duration. + base::UmaHistogramLongTimes( + "UMA.CleanExitBeacon.ExtendedMonitoringStageDuration", + base::TimeTicks::Now() - extended_monitoring_stage_start_time_); + extended_monitoring_stage_start_time_ = base::TimeTicks(); // Null time. + } +#endif // BUILDFLAG(IS_ANDROID) if (group_name == kEnabledGroup) { // Clients in this group write to the Variations Safe Mode file whenever // |kStabilityExitedCleanly| is updated. The file is kept in sync with the diff --git a/components/metrics/clean_exit_beacon.h b/components/metrics/clean_exit_beacon.h index c3f49037287616..63f273c56b23a8 100644 --- a/components/metrics/clean_exit_beacon.h +++ b/components/metrics/clean_exit_beacon.h @@ -196,6 +196,12 @@ class CleanExitBeacon { static void ResetUserDefaultsBeacon(); #endif // BUILDFLAG(IS_IOS) +#if BUILDFLAG(IS_ANDROID) + // Denotes the time at which Chrome clients in the Extended Variations Safe + // Mode experiment's enabled group start watching for browser crashes. + base::TimeTicks extended_monitoring_stage_start_time_; +#endif + // Indicates whether the CleanExitBeacon has been initialized. bool initialized_ = false; diff --git a/components/metrics/clean_exit_beacon_unittest.cc b/components/metrics/clean_exit_beacon_unittest.cc index 69227d64bfde7d..e4ae9b7db1c5d3 100644 --- a/components/metrics/clean_exit_beacon_unittest.cc +++ b/components/metrics/clean_exit_beacon_unittest.cc @@ -822,4 +822,67 @@ TEST_P(BeaconFileAndPlatformBeaconConsistencyTest, BeaconConsistency) { } #endif // BUILDFLAG(IS_IOS) +#if BUILDFLAG(IS_ANDROID) +TEST_F(CleanExitBeaconTest, EnabledGroupEmitsStageDurationMetric) { + // Force the client into the Extended Variations Safe Mode experiment's + // enabled group. + SetUpExtendedSafeModeExperiment(variations::kEnabledGroup); + + // Create and initialize the CleanExitBeacon. + TestCleanExitBeacon clean_exit_beacon(&prefs_); + + // Simulate Chrome starting to watch for browser crashes for enabled-group + // clients. + clean_exit_beacon.WriteBeaconValue(/*exited_cleanly=*/false, + /*is_extended_safe_mode=*/true); + // Verify that the metric has not yet been emitted. + histogram_tester_.ExpectTotalCount( + "UMA.CleanExitBeacon.ExtendedMonitoringStageDuration", 0); + + // Simulate Chrome continuing to watch for crashes once the app enters the + // foreground. + clean_exit_beacon.WriteBeaconValue(/*exited_cleanly=*/false, + /*is_extended_safe_mode=*/false); + // Verify that the metric was emitted. + histogram_tester_.ExpectTotalCount( + "UMA.CleanExitBeacon.ExtendedMonitoringStageDuration", 1); + + // Make the same call. Note that these two identical, consecutive calls to + // WriteBeaconValue() shouldn't actually happen, but this is done for the + // purpose of the test. + clean_exit_beacon.WriteBeaconValue(/*exited_cleanly=*/false, + /*is_extended_safe_mode=*/false); + // Verify that the metric was not emitted again. + histogram_tester_.ExpectTotalCount( + "UMA.CleanExitBeacon.ExtendedMonitoringStageDuration", 1); +} + +TEST_F(CleanExitBeaconTest, ControlGroupDoesNotEmitStageDurationMetric) { + // Force the client into the Extended Variations Safe Mode experiment's + // control group. + SetUpExtendedSafeModeExperiment(variations::kControlGroup); + + // Create and initialize the CleanExitBeacon. + TestCleanExitBeacon clean_exit_beacon(&prefs_); + + // Simulate Chrome starting to watch for browser crashes for control-group + // clients once the app enters the foreground. + clean_exit_beacon.WriteBeaconValue(/*exited_cleanly=*/false, + /*is_extended_safe_mode=*/false); + // Verify that the metric was not emitted. + histogram_tester_.ExpectTotalCount( + "UMA.CleanExitBeacon.ExtendedMonitoringStageDuration", 0); + + // Make the same call. Note that these two identical, consecutive calls to + // WriteBeaconValue() shouldn't actually happen, but this is done for the + // purpose of the test. + clean_exit_beacon.WriteBeaconValue(/*exited_cleanly=*/false, + /*is_extended_safe_mode=*/false); + // Verify that the metric was not emitted. + histogram_tester_.ExpectTotalCount( + "UMA.CleanExitBeacon.ExtendedMonitoringStageDuration", 0); +} + +#endif // BUILDFLAG(IS_ANDROID) + } // namespace metrics diff --git a/tools/metrics/histograms/metadata/uma/histograms.xml b/tools/metrics/histograms/metadata/uma/histograms.xml index c6cd3637ad886c..91b1b6974d533f 100644 --- a/tools/metrics/histograms/metadata/uma/histograms.xml +++ b/tools/metrics/histograms/metadata/uma/histograms.xml @@ -114,6 +114,20 @@ chromium-metrics-reviews@google.com. + + ssid@chromium.org + wnwen@chromium.org + + The amount of time in ms between the extended and status quo browser crash + monitoring stages. The extended monitoring stage may begin in + MaybeExtendVariationsSafeMode(), and the status quo stage begins in + MetricsService::OnAppEnterForeground(). Emitted when the status quo + monitoring stage begins. Recorded by Extended Variations Safe Mode + enabled-group clients on Android Chrome. + + + caitlinfischer@google.com