Send extension downloading stage to UMA

For further investigation of issues with forceinstalled extensions we need to know what happens with extensions in downloading stage in a more detailed way. We already collect data for this (in InstallationReporter via ExtensionDownloaderDelegate), so this commit introduces an UMA histogram. Change-Id: Id4d13b9e9678d0f7ef5bd30fbfbfebd7e57291bf Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1587032 Commit-Queue: Oleg Davydov <burunduk@chromium.org> Reviewed-by: Mark Pearson <mpearson@chromium.org> Reviewed-by: Devlin <rdevlin.cronin@chromium.org> Cr-Commit-Position: refs/heads/master@{#658505}
Denger-Network · May 10, 2019 · e74cf77 · e74cf77
1 parent bfa78d1
commit e74cf77
Show file tree

Hide file tree

Showing 5 changed files with 91 additions and 16 deletions.
diff --git a/chrome/browser/extensions/forced_extensions/installation_tracker.cc b/chrome/browser/extensions/forced_extensions/installation_tracker.cc
@@ -119,6 +119,13 @@ void InstallationTracker::ReportResults(bool succeeded) {
               installation.install_stage.value();
           UMA_HISTOGRAM_ENUMERATION("Extensions.ForceInstalledStage",
                                     install_stage);
+          if (install_stage == InstallationReporter::Stage::DOWNLOADING) {
+            DCHECK(installation.downloading_stage);
+            ExtensionDownloaderDelegate::Stage downloading_stage =
+                installation.downloading_stage.value();
+            UMA_HISTOGRAM_ENUMERATION(
+                "Extensions.ForceInstalledDownloadingStage", downloading_stage);
+          }
         }
         InstallationReporter::FailureReason failure_reason =
             installation.failure_reason.value_or(

diff --git a/chrome/browser/extensions/forced_extensions/installation_tracker_unittest.cc b/chrome/browser/extensions/forced_extensions/installation_tracker_unittest.cc
@@ -33,6 +33,8 @@ constexpr char kTimedOutNotInstalledStats[] =
     "Extensions.ForceInstalledTimedOutAndNotInstalledCount";
 constexpr char kFailureReasons[] = "Extensions.ForceInstalledFailureReason";
 constexpr char kInstallationStages[] = "Extensions.ForceInstalledStage";
+constexpr char kInstallationDownloadingStages[] =
+    "Extensions.ForceInstalledDownloadingStage";
 constexpr char kFailureCrxInstallErrorStats[] =
     "Extensions.ForceInstalledFailureCrxInstallError";
 }  // namespace
@@ -137,6 +139,8 @@ TEST_F(ForcedExtensionsInstallationTrackerTest, ExtensionsStuck) {
       &profile_, kExtensionId1, InstallationReporter::Stage::PENDING);
   InstallationReporter::ReportInstallationStage(
       &profile_, kExtensionId2, InstallationReporter::Stage::DOWNLOADING);
+  InstallationReporter::ReportDownloadingStage(
+      &profile_, kExtensionId2, ExtensionDownloaderDelegate::PENDING);
   EXPECT_TRUE(fake_timer_->IsRunning());
   fake_timer_->Fire();
   histogram_tester_.ExpectTotalCount(kLoadTimeStats, 0);
@@ -151,6 +155,35 @@ TEST_F(ForcedExtensionsInstallationTrackerTest, ExtensionsStuck) {
   histogram_tester_.ExpectTotalCount(kFailureCrxInstallErrorStats, 0);
 }
 
+TEST_F(ForcedExtensionsInstallationTrackerTest, ExtensionsAreDownloading) {
+  SetupForceList();
+  InstallationReporter::ReportInstallationStage(
+      &profile_, kExtensionId1, InstallationReporter::Stage::DOWNLOADING);
+  InstallationReporter::ReportDownloadingStage(
+      &profile_, kExtensionId1,
+      ExtensionDownloaderDelegate::DOWNLOADING_MANIFEST);
+  InstallationReporter::ReportInstallationStage(
+      &profile_, kExtensionId2, InstallationReporter::Stage::DOWNLOADING);
+  InstallationReporter::ReportDownloadingStage(
+      &profile_, kExtensionId2, ExtensionDownloaderDelegate::DOWNLOADING_CRX);
+  EXPECT_TRUE(fake_timer_->IsRunning());
+  fake_timer_->Fire();
+  histogram_tester_.ExpectTotalCount(kLoadTimeStats, 0);
+  histogram_tester_.ExpectUniqueSample(kTimedOutStats, 2, 1);
+  histogram_tester_.ExpectUniqueSample(kTimedOutNotInstalledStats, 2, 1);
+  histogram_tester_.ExpectUniqueSample(
+      kFailureReasons, InstallationReporter::FailureReason::IN_PROGRESS, 2);
+  histogram_tester_.ExpectUniqueSample(
+      kInstallationStages, InstallationReporter::Stage::DOWNLOADING, 2);
+  histogram_tester_.ExpectTotalCount(kInstallationDownloadingStages, 2);
+  histogram_tester_.ExpectBucketCount(
+      kInstallationDownloadingStages,
+      ExtensionDownloaderDelegate::DOWNLOADING_MANIFEST, 1);
+  histogram_tester_.ExpectBucketCount(
+      kInstallationDownloadingStages,
+      ExtensionDownloaderDelegate::DOWNLOADING_CRX, 1);
+}
+
 TEST_F(ForcedExtensionsInstallationTrackerTest, NoExtensionsConfigured) {
   EXPECT_TRUE(fake_timer_->IsRunning());
   fake_timer_->Fire();

diff --git a/extensions/browser/updater/extension_downloader_delegate.h b/extensions/browser/updater/extension_downloader_delegate.h
@@ -53,38 +53,45 @@ class ExtensionDownloaderDelegate {
   // download failed at least once already. So, this may result in sequence like
   // ... -> MANIFEST_LOADED -> QUEUED_FOR_CRX -> DOWNLOADING_CRX ->
   // DOWNLOADING_CRX_RETRY -> DOWNLOADING_CRX -> FINISHED.
+  // Note: enum used for UMA. Do NOT reorder or remove entries. Don't forget to
+  // update enums.xml (name: ExtensionInstallationDownloadingStage) when adding
+  // new entries.
   enum Stage {
     // Downloader just received extension download request.
-    PENDING,
+    PENDING = 0,
 
     // Extension is in manifest loading queue.
-    QUEUED_FOR_MANIFEST,
+    QUEUED_FOR_MANIFEST = 1,
 
     // There is an active request to download extension's manifest.
-    DOWNLOADING_MANIFEST,
+    DOWNLOADING_MANIFEST = 2,
 
     // There were one or more unsuccessful tries to download manifest, but we'll
     // try more.
-    DOWNLOADING_MANIFEST_RETRY,
+    DOWNLOADING_MANIFEST_RETRY = 3,
 
     // Manifest downloaded and is about to parse.
-    PARSING_MANIFEST,
+    PARSING_MANIFEST = 4,
 
     // Manifest downloaded and successfully parsed.
-    MANIFEST_LOADED,
+    MANIFEST_LOADED = 5,
 
     // Extension in CRX loading queue.
-    QUEUED_FOR_CRX,
+    QUEUED_FOR_CRX = 6,
 
     // There is an active request to download extension archive.
-    DOWNLOADING_CRX,
+    DOWNLOADING_CRX = 7,
 
     // There were one or more unsuccessful tries to download archive, but we'll
     // try more.
-    DOWNLOADING_CRX_RETRY,
+    DOWNLOADING_CRX_RETRY = 8,
 
     // Downloading finished, either successfully or not.
-    FINISHED,
+    FINISHED = 9,
+
+    // Magic constant used by the histogram macros.
+    // Always update it to the max value.
+    kMaxValue = FINISHED,
   };
 
   // Passed as an argument to the completion callbacks to signal whether

diff --git a/tools/metrics/histograms/enums.xml b/tools/metrics/histograms/enums.xml
@@ -19333,6 +19333,19 @@ Called by update_net_error_codes.py.-->
   <int value="24" label="UPDATE_NON_EXISTING_EXTENSION"/>
 </enum>
 
+<enum name="ExtensionInstallationDownloadingStage">
+  <int value="0" label="PENDING"/>
+  <int value="1" label="QUEUED_FOR_MANIFEST"/>
+  <int value="2" label="DOWNLOADING_MANIFEST"/>
+  <int value="3" label="DOWNLOADING_MANIFEST_RETRY"/>
+  <int value="4" label="PARSING_MANIFEST"/>
+  <int value="5" label="MANIFEST_LOADED"/>
+  <int value="6" label="QUEUED_FOR_CRX"/>
+  <int value="7" label="DOWNLOADING_CRX"/>
+  <int value="8" label="DOWNLOADING_CRX_RETRY"/>
+  <int value="9" label="FINISHED"/>
+</enum>
+
 <enum name="ExtensionInstallationFailureReason">
   <int value="0" label="UNKNOWN"/>
   <int value="1" label="INVALID_ID"/>

diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml
@@ -37160,8 +37160,23 @@ uploading your change for review.
   </summary>
 </histogram>
 
+<histogram name="Extensions.ForceInstalledDownloadingStage"
+    enum="ExtensionInstallationDownloadingStage" expires_after="2019-11-01">
+  <owner>burunduk@chromium.org</owner>
+  <owner>poromov@chromium.org</owner>
+  <owner>rdevlin.cronin@chromium.org</owner>
+  <summary>
+    The last known downloading stage of extension downloading process if failure
+    reason was not recorded and installing stage is DOWNLOADING, so it's
+    specification of Extensions.ForceInstalledStage. Recorded for each forced
+    extension that failed to install after 5 minutes. Recorded together with
+    &quot;Extensions.ForceInstalledTimedOutCount&quot; histogram, but for every
+    extension not installed at the moment.
+  </summary>
+</histogram>
+
 <histogram name="Extensions.ForceInstalledFailureCrxInstallError"
-    enum="ExtensionInstallationCrxInstallError" expires_after="2019-07-01">
+    enum="ExtensionInstallationCrxInstallError" expires_after="2019-11-01">
   <owner>burunduk@chromium.org</owner>
   <owner>poromov@chromium.org</owner>
   <owner>rdevlin.cronin@chromium.org</owner>
@@ -37174,7 +37189,7 @@ uploading your change for review.
 </histogram>
 
 <histogram name="Extensions.ForceInstalledFailureReason"
-    enum="ExtensionInstallationFailureReason" expires_after="2019-07-01">
+    enum="ExtensionInstallationFailureReason" expires_after="2019-11-01">
   <owner>burunduk@chromium.org</owner>
   <owner>poromov@chromium.org</owner>
   <owner>rdevlin.cronin@chromium.org</owner>
@@ -37187,7 +37202,7 @@ uploading your change for review.
 </histogram>
 
 <histogram name="Extensions.ForceInstalledLoadTime" units="ms"
-    expires_after="2019-07-01">
+    expires_after="2019-11-01">
   <owner>poromov@chromium.org</owner>
   <summary>
     The amount of time elapsed during installation of enterprise policy forced
@@ -37196,7 +37211,7 @@ uploading your change for review.
 </histogram>
 
 <histogram name="Extensions.ForceInstalledStage"
-    enum="ExtensionInstallationStage" expires_after="2019-07-01">
+    enum="ExtensionInstallationStage" expires_after="2019-11-01">
   <owner>burunduk@chromium.org</owner>
   <owner>poromov@chromium.org</owner>
   <owner>rdevlin.cronin@chromium.org</owner>
@@ -37211,7 +37226,7 @@ uploading your change for review.
 </histogram>
 
 <histogram name="Extensions.ForceInstalledTimedOutAndNotInstalledCount"
-    expires_after="2019-07-01">
+    expires_after="2019-11-01">
   <owner>poromov@chromium.org</owner>
   <summary>
     Number of enterprise policy forced extensions that are not installed after 5
@@ -37220,7 +37235,7 @@ uploading your change for review.
 </histogram>
 
 <histogram name="Extensions.ForceInstalledTimedOutCount"
-    expires_after="2019-07-01">
+    expires_after="2019-11-01">
   <owner>poromov@chromium.org</owner>
   <summary>
     Number of enterprise policy forced extensions that are not enabled after 5