Report installation stages in increasing order

This CL reports the installation stages for force installed extensions
in an increasing order. This is done in order to investigate
the extensions stuck in CREATED stage.

Bug: 989526
Change-Id: I82e07f30c6768c7bde30a76b61b9987173f76fbd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2421375
Commit-Queue: Swapnil Gupta <swapnilgupta@google.com>
Reviewed-by: Oleg Davydov <burunduk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#809777}

chrome/browser/extensions/forced_extensions/force_installed_metrics_unittest.cc

@@ -592,6 +592,50 @@ TEST_F(ForceInstalledMetricsTest, ExtensionsReady) {
   histogram_tester_.ExpectTotalCount(kTimedOutNotInstalledStats, 0);
 }
 
+// Verifies that the installation stage is not overwritten by a previous stage.
+TEST_F(ForceInstalledMetricsTest,
+       ExtensionsPreviousInstallationStageReportedAgain) {
+  SetupForceList();
+  auto extension =
+      ExtensionBuilder(kExtensionName1).SetID(kExtensionId1).Build();
+  tracker_->OnExtensionLoaded(profile_, extension.get());
+  install_stage_tracker_->ReportInstallationStage(
+      kExtensionId2, InstallStageTracker::Stage::CREATED);
+  install_stage_tracker_->ReportInstallationStage(
+      kExtensionId2, InstallStageTracker::Stage::PENDING);
+  install_stage_tracker_->ReportInstallationStage(
+      kExtensionId2, InstallStageTracker::Stage::CREATED);
+  EXPECT_TRUE(fake_timer_->IsRunning());
+  fake_timer_->Fire();
+  histogram_tester_.ExpectUniqueSample(
+      kFailureReasonsCWS, InstallStageTracker::FailureReason::IN_PROGRESS, 1);
+  histogram_tester_.ExpectBucketCount(kInstallationStages,
+                                      InstallStageTracker::Stage::PENDING, 1);
+}
+
+// Verifies that the installation stage is overwritten if DOWNLOADING stage is
+// reported again after INSTALLING stage.
+TEST_F(ForceInstalledMetricsTest, ExtensionsDownloadingStageReportedAgain) {
+  SetupForceList();
+  auto extension =
+      ExtensionBuilder(kExtensionName1).SetID(kExtensionId1).Build();
+  tracker_->OnExtensionLoaded(profile_, extension.get());
+  install_stage_tracker_->ReportInstallationStage(
+      kExtensionId2, InstallStageTracker::Stage::DOWNLOADING);
+  install_stage_tracker_->ReportInstallationStage(
+      kExtensionId2, InstallStageTracker::Stage::INSTALLING);
+  install_stage_tracker_->ReportInstallationStage(
+      kExtensionId2, InstallStageTracker::Stage::DOWNLOADING);
+  install_stage_tracker_->ReportDownloadingStage(
+      kExtensionId2, ExtensionDownloaderDelegate::Stage::PENDING);
+  EXPECT_TRUE(fake_timer_->IsRunning());
+  fake_timer_->Fire();
+  histogram_tester_.ExpectUniqueSample(
+      kFailureReasonsCWS, InstallStageTracker::FailureReason::IN_PROGRESS, 1);
+  histogram_tester_.ExpectBucketCount(
+      kInstallationStages, InstallStageTracker::Stage::DOWNLOADING, 1);
+}
+
 TEST_F(ForceInstalledMetricsTest, ExtensionsStuck) {
   SetupForceList();
   install_stage_tracker()->ReportInstallationStage(

chrome/browser/extensions/forced_extensions/install_stage_tracker.cc

@@ -15,6 +15,24 @@
 
 namespace extensions {
 
+namespace {
+// Returns true if the |current_stage| should be overridden by the
+// |new_stage|.
+bool ShouldOverrideCurrentStage(
+    base::Optional<InstallStageTracker::Stage> current_stage,
+    InstallStageTracker::Stage new_stage) {
+  if (!current_stage)
+    return true;
+  // If CRX was from the cache and was damaged as a file, we would try to
+  // download the CRX after reporting the INSTALLING stage.
+  if (current_stage == InstallStageTracker::Stage::INSTALLING &&
+      new_stage == InstallStageTracker::Stage::DOWNLOADING)
+    return true;
+  return new_stage > current_stage;
+}
+
+}  // namespace
+
 #if defined(OS_CHROMEOS)
 InstallStageTracker::UserInfo::UserInfo(const UserInfo&) = default;
 InstallStageTracker::UserInfo::UserInfo(user_manager::UserType user_type,
@@ -162,6 +180,8 @@ void InstallStageTracker::ReportInstallCreationStage(
 void InstallStageTracker::ReportInstallationStage(const ExtensionId& id,
                                                   Stage stage) {
   InstallationData& data = installation_data_map_[id];
+  if (!ShouldOverrideCurrentStage(data.install_stage, stage))
+    return;
   data.install_stage = stage;
   for (auto& observer : observers_) {
     observer.OnExtensionInstallationStageChanged(id, stage);

chrome/browser/extensions/forced_extensions/install_stage_tracker.h

@@ -38,10 +38,13 @@ class InstallStageTracker : public KeyedService {
  public:
   // Stage of extension installing process. Typically forced extensions from
   // policies should go through all stages in this order, other extensions skip
-  // CREATED stage.
-  // Note: enum used for UMA. Do NOT reorder or remove entries. Don't forget to
-  // update enums.xml (name: ExtensionInstallationStage) when adding new
-  // entries. Don't forget to update device_management_backend.proto (name:
+  // CREATED stage. The stages are recorded in the increasing order of their
+  // values, therefore always verify that values are in increasing order and
+  // items are in order in which they appear. Exceptions are handled in
+  // ShouldOverrideCurrentStage method. Note: enum used for UMA. Do NOT reorder
+  // or remove entries. Don't forget to update enums.xml (name:
+  // ExtensionInstallationStage) when adding new entries. Don't forget to update
+  // device_management_backend.proto (name:
   // ExtensionInstallReportLogEvent::InstallationStage) when adding new entries.
   // Don't forget to update ConvertInstallationStageToProto method in
   // ExtensionInstallEventLogCollector.