system-web-app: add install duration metric

Bug: 1021813
Change-Id: Ie148398f2e670164c3acd82c666335c9d2a71781
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1921600
Commit-Queue: Jiewei Qian  <qjw@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Reviewed-by: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Cr-Commit-Position: refs/heads/master@{#719087}

chrome/browser/web_applications/extensions/system_web_app_manager_unittest.cc

@@ -395,6 +395,8 @@ TEST_F(SystemWebAppManagerTest, InstallResultHistogram) {
         SystemWebAppManager::kInstallResultHistogramName, 0);
     histograms.ExpectTotalCount(settings_app_install_result_histogram, 0);
     histograms.ExpectTotalCount(profile_install_result_histogram, 0);
+    histograms.ExpectTotalCount(
+        SystemWebAppManager::kInstallDurationHistogramName, 0);
 
     system_web_app_manager()->Start();
     base::RunLoop().RunUntilIdle();
@@ -410,6 +412,8 @@ TEST_F(SystemWebAppManagerTest, InstallResultHistogram) {
     histograms.ExpectTotalCount(profile_install_result_histogram, 1);
     histograms.ExpectBucketCount(profile_install_result_histogram,
                                  InstallResultCode::kSuccessNewInstall, 1);
+    histograms.ExpectTotalCount(
+        SystemWebAppManager::kInstallDurationHistogramName, 1);
   }
   {
     base::flat_map<SystemAppType, SystemAppInfo> system_apps;
@@ -447,6 +451,8 @@ TEST_F(SystemWebAppManagerTest, InstallResultHistogram) {
     pending_app_manager()->SetInstallResultCode(
         InstallResultCode::kProfileDestroyed);
 
+    histograms.ExpectTotalCount(
+        SystemWebAppManager::kInstallDurationHistogramName, 2);
     histograms.ExpectBucketCount(settings_app_install_result_histogram,
                                  InstallResultCode::kFailedShuttingDown, 0);
     histograms.ExpectBucketCount(profile_install_result_histogram,
@@ -467,6 +473,9 @@ TEST_F(SystemWebAppManagerTest, InstallResultHistogram) {
                                  InstallResultCode::kFailedShuttingDown, 1);
     histograms.ExpectBucketCount(profile_install_result_histogram,
                                  InstallResultCode::kFailedShuttingDown, 1);
+    // If install was interrupted by shutdown, do not report duration.
+    histograms.ExpectTotalCount(
+        SystemWebAppManager::kInstallDurationHistogramName, 2);
   }
 }
 

chrome/browser/web_applications/system_web_app_manager.cc

@@ -129,6 +129,7 @@ SystemAppInfo::~SystemAppInfo() = default;
 
 // static
 const char SystemWebAppManager::kInstallResultHistogramName[];
+const char SystemWebAppManager::kInstallDurationHistogramName[];
 
 // static
 bool SystemWebAppManager::IsAppEnabled(SystemAppType type) {
@@ -191,6 +192,8 @@ void SystemWebAppManager::SetSubsystems(PendingAppManager* pending_app_manager,
 }
 
 void SystemWebAppManager::Start() {
+  const base::TimeTicks install_start_time = base::TimeTicks::Now();
+
   std::vector<ExternalInstallOptions> install_options_list;
   if (IsEnabled()) {
     // Skipping this will uninstall all System Apps currently installed.
@@ -199,11 +202,10 @@ void SystemWebAppManager::Start() {
           CreateInstallOptionsForSystemApp(app.second, NeedsUpdate()));
     }
   }
-
   pending_app_manager_->SynchronizeInstalledApps(
       std::move(install_options_list), ExternalInstallSource::kSystemInstalled,
       base::BindOnce(&SystemWebAppManager::OnAppsSynchronized,
-                     weak_ptr_factory_.GetWeakPtr()));
+                     weak_ptr_factory_.GetWeakPtr(), install_start_time));
 }
 
 void SystemWebAppManager::InstallSystemAppsForTesting() {
@@ -281,8 +283,15 @@ const std::string& SystemWebAppManager::CurrentLocale() const {
   return g_browser_process->GetApplicationLocale();
 }
 
-void SystemWebAppManager::RecordSystemWebAppInstallResultCode(
-    const std::map<GURL, InstallResultCode>& install_results) const {
+void SystemWebAppManager::RecordSystemWebAppInstallMetrics(
+    const std::map<GURL, InstallResultCode>& install_results,
+    const base::TimeDelta& install_duration) const {
+  // Record the time spent to install system web apps.
+  if (!shutting_down_) {
+    base::UmaHistogramMediumTimes(kInstallDurationHistogramName,
+                                  install_duration);
+  }
+
   // Record aggregate result.
   for (const auto& url_and_result : install_results)
     base::UmaHistogramEnumeration(kInstallResultHistogramName,
@@ -315,8 +324,12 @@ void SystemWebAppManager::RecordSystemWebAppInstallResultCode(
 }
 
 void SystemWebAppManager::OnAppsSynchronized(
+    const base::TimeTicks& install_start_time,
     std::map<GURL, InstallResultCode> install_results,
     std::map<GURL, bool> uninstall_results) {
+  const base::TimeDelta install_duration =
+      install_start_time - base::TimeTicks::Now();
+
   if (IsEnabled()) {
     // TODO(qjw): Figure out where install_results come from, decide if
     // installation failures need to be handled
@@ -326,7 +339,7 @@ void SystemWebAppManager::OnAppsSynchronized(
                              CurrentLocale());
   }
 
-  RecordSystemWebAppInstallResultCode(install_results);
+  RecordSystemWebAppInstallMetrics(install_results, install_duration);
 
   // Build the map from installed app id to app type.
   for (const auto& it : system_app_infos_) {

chrome/browser/web_applications/system_web_app_manager.h

@@ -81,6 +81,8 @@ class SystemWebAppManager {
 
   static constexpr char kInstallResultHistogramName[] =
       "Webapp.InstallResult.System";
+  static constexpr char kInstallDurationHistogramName[] =
+      "Webapp.InstallDuration.System";
 
   // Returns whether the given app type is enabled.
   static bool IsAppEnabled(SystemAppType type);
@@ -138,12 +140,14 @@ class SystemWebAppManager {
   virtual const std::string& CurrentLocale() const;
 
  private:
-  void OnAppsSynchronized(std::map<GURL, InstallResultCode> install_results,
+  void OnAppsSynchronized(const base::TimeTicks& install_start_time,
+                          std::map<GURL, InstallResultCode> install_results,
                           std::map<GURL, bool> uninstall_results);
   bool NeedsUpdate() const;
 
-  void RecordSystemWebAppInstallResultCode(
-      const std::map<GURL, InstallResultCode>& install_results) const;
+  void RecordSystemWebAppInstallMetrics(
+      const std::map<GURL, InstallResultCode>& install_results,
+      const base::TimeDelta& install_duration) const;
 
   std::unique_ptr<base::OneShotEvent> on_apps_synchronized_;
 

tools/metrics/histograms/histograms.xml

@@ -162465,6 +162465,16 @@ regressions. -->
   </summary>
 </histogram>
 
+<histogram name="Webapp.InstallDuration.System" units="seconds"
+    expires_after="2020-06-30">
+  <owner>calamity@chromium.org</owner>
+  <owner>ortuno@chromium.org</owner>
+  <summary>
+    Records the number of seconds taken to install system web apps, from when we
+    dispatch a call to install them, until we get all the installation results.
+  </summary>
+</histogram>
+
 <histogram name="Webapp.InstallResult" enum="WebAppInstallResultCode"
     expires_after="2020-06-30">
 <!-- Name completed by histogram_suffixes name="WebAppType" -->