Fix installed but not loaded metrics for forced extensions

Currently, only the extensions that had status as LOADED were excluded
from the missing_forced_extensions list. This caused the extensions
with status READY to be included in the missing_forced_extensions
list and thus the metrics are reported wrong for the case when
extensions are installed but not LOADED. Now, we add the extensions
which have status as PENDING or FAILED to missing_forced_extensions
list to remove the false positives.

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

chrome/browser/extensions/forced_extensions/force_installed_metrics.cc

@@ -86,6 +86,22 @@ ForceInstalledMetrics::ForceInstalledMetrics(
 
 ForceInstalledMetrics::~ForceInstalledMetrics() = default;
 
+bool ForceInstalledMetrics::IsStatusGood(ExtensionStatus status) {
+  switch (status) {
+    case ExtensionStatus::PENDING:
+      return false;
+    case ExtensionStatus::LOADED:
+      return true;
+    case ExtensionStatus::READY:
+      return true;
+    case ExtensionStatus::FAILED:
+      return false;
+    default:
+      NOTREACHED();
+  }
+  return false;
+}
+
 bool ForceInstalledMetrics::IsMisconfiguration(
     const InstallStageTracker::InstallationData& installation_data,
     const ExtensionId& id) {
@@ -149,7 +165,7 @@ void ForceInstalledMetrics::ReportMetrics() {
                               tracker_->extensions().size());
   std::set<ExtensionId> missing_forced_extensions;
   for (const auto& extension : tracker_->extensions()) {
-    if (extension.second.status != ExtensionStatus::LOADED)
+    if (!IsStatusGood(extension.second.status))
       missing_forced_extensions.insert(extension.first);
   }
   if (missing_forced_extensions.empty()) {

chrome/browser/extensions/forced_extensions/force_installed_metrics.h

@@ -69,6 +69,10 @@ class ForceInstalledMetrics : public ForceInstalledTracker::Observer {
       ExtensionDownloaderDelegate::CacheStatus cache_status) override;
 
  private:
+  // Returns false if the extension status corresponds to a missing extension
+  // which is not yet installed or loaded.
+  bool IsStatusGood(ForceInstalledTracker::ExtensionStatus status);
+
   // Returns true only in case of some well-known misconfigurations which are
   // easy to detect. Can return false for misconfigurations which are hard to
   // distinguish with other errors.

chrome/browser/extensions/forced_extensions/force_installed_metrics_unittest.cc

@@ -543,6 +543,26 @@ TEST_F(ForceInstalledMetricsTest,
   histogram_tester_.ExpectTotalCount(kTimedOutNotInstalledStats, 0);
 }
 
+// Regression test to check if the metrics are collected properly for the
+// extensions which are in state READY.
+TEST_F(ForceInstalledMetricsTest, ExtensionsReady) {
+  SetupForceList();
+  auto ext1 = ExtensionBuilder(kExtensionName1).SetID(kExtensionId1).Build();
+  tracker_->OnExtensionLoaded(profile_, ext1.get());
+  tracker_->OnExtensionReady(profile_, ext1.get());
+  install_stage_tracker_->ReportFailure(
+      kExtensionId1, InstallStageTracker::FailureReason::ALREADY_INSTALLED);
+  auto ext2 = ExtensionBuilder(kExtensionName2).SetID(kExtensionId2).Build();
+  tracker_->OnExtensionLoaded(profile_, ext2.get());
+  tracker_->OnExtensionReady(profile_, ext2.get());
+  // ForceInstalledMetrics shuts down timer because all extension are either
+  // loaded or failed.
+  EXPECT_FALSE(fake_timer_->IsRunning());
+  histogram_tester_.ExpectTotalCount(kLoadTimeStats, 1);
+  histogram_tester_.ExpectTotalCount(kTimedOutStats, 0);
+  histogram_tester_.ExpectTotalCount(kTimedOutNotInstalledStats, 0);
+}
+
 TEST_F(ForceInstalledMetricsTest, ExtensionsStuck) {
   SetupForceList();
   install_stage_tracker_->ReportInstallationStage(