Commit 776c3374 authored by Jiewei Qian's avatar Jiewei Qian Committed by Commit Bot

system-web-apps: fix negative install duration

The original CL that introduced the install duration metric contains
one typo, which cause the reported value to be the additive inverse of
the actual time.

Because UMA clamps this value to be within 1 millisecond and 3 minutes,
we always see 1 second in the UMA dashboard (and contradicting metrics
that some installation times out).

Fixed: 1083894
Change-Id: I7529bfb90fc737016d7c9948a7b8b6325ecb24a5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2207375
Auto-Submit: Jiewei Qian  <qjw@chromium.org>
Commit-Queue: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Reviewed-by: default avatarGiovanni Ortuño Urquidi <ortuno@chromium.org>
Cr-Commit-Position: refs/heads/master@{#769909}
parent 6a30d4da
...@@ -543,6 +543,9 @@ const std::string& SystemWebAppManager::CurrentLocale() const { ...@@ -543,6 +543,9 @@ const std::string& SystemWebAppManager::CurrentLocale() const {
void SystemWebAppManager::RecordSystemWebAppInstallMetrics( void SystemWebAppManager::RecordSystemWebAppInstallMetrics(
const std::map<GURL, InstallResultCode>& install_results, const std::map<GURL, InstallResultCode>& install_results,
const base::TimeDelta& install_duration) const { const base::TimeDelta& install_duration) const {
// Install duration should be positive.
DCHECK_GT(install_duration.InMicroseconds(), 0);
// Record the time spent to install system web apps. // Record the time spent to install system web apps.
if (!shutting_down_) { if (!shutting_down_) {
base::UmaHistogramMediumTimes(kInstallDurationHistogramName, base::UmaHistogramMediumTimes(kInstallDurationHistogramName,
...@@ -605,7 +608,7 @@ void SystemWebAppManager::OnAppsSynchronized( ...@@ -605,7 +608,7 @@ void SystemWebAppManager::OnAppsSynchronized(
} }
const base::TimeDelta install_duration = const base::TimeDelta install_duration =
install_start_time - base::TimeTicks::Now(); base::TimeTicks::Now() - install_start_time;
if (IsEnabled()) { if (IsEnabled()) {
// TODO(qjw): Figure out where install_results come from, decide if // TODO(qjw): Figure out where install_results come from, decide if
......
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