Add separate unit tests for ForceInstalledTracker

Due to historical reasons, force_install_metrics_unittest.cc contains
tests for both ForceInstalledMetrics and ForceInstalledTracker classes.
This CL moves the tests related to ForceInstalledTracker out of
force_installed_metrics_unittest.cc into a separate file. Some new tests
for ForceInstalledTracker are also added in this CL.

Bug: 1086430
Change-Id: I8c49c61b0e1a4c25bbf57e7645fd5440acf970f1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2414309
Commit-Queue: Saurabh Nijhara <snijhara@google.com>
Reviewed-by: Oleg Davydov <burunduk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#809280}

chrome/browser/extensions/forced_extensions/force_installed_metrics_unittest.cc

@@ -6,7 +6,6 @@
 
 #include "base/memory/ptr_util.h"
 #include "base/optional.h"
-#include "base/scoped_observer.h"
 #include "base/test/metrics/histogram_tester.h"
 #include "base/time/time.h"
 #include "base/timer/mock_timer.h"
@@ -119,8 +118,7 @@ namespace extensions {
 using testing::_;
 using testing::Return;
 
-class ForceInstalledMetricsTest : public ForceInstalledTestBase,
-                                  public ForceInstalledTracker::Observer {
+class ForceInstalledMetricsTest : public ForceInstalledTestBase {
  public:
   ForceInstalledMetricsTest() = default;
 
@@ -132,7 +130,6 @@ class ForceInstalledMetricsTest : public ForceInstalledTestBase,
     ForceInstalledTestBase::SetUp();
     auto fake_timer = std::make_unique<base::MockOneShotTimer>();
     fake_timer_ = fake_timer.get();
-    scoped_observer_.Add(tracker_.get());
     metrics_ = std::make_unique<ForceInstalledMetrics>(
         registry_, profile_, tracker_.get(), std::move(fake_timer));
   }
@@ -172,26 +169,15 @@ class ForceInstalledMetricsTest : public ForceInstalledTestBase,
         kExtensionId1, InstallStageTracker::Stage::INSTALLING);
   }
 
-  // ForceInstalledTracker::Observer overrides:
-  void OnForceInstalledExtensionsLoaded() override { loaded_call_count_++; }
-  void OnForceInstalledExtensionsReady() override { ready_call_count_++; }
-
  protected:
   base::HistogramTester histogram_tester_;
   base::MockOneShotTimer* fake_timer_;
   std::unique_ptr<ForceInstalledMetrics> metrics_;
-
-  ScopedObserver<ForceInstalledTracker, ForceInstalledTracker::Observer>
-      scoped_observer_{this};
-  size_t loaded_call_count_ = 0;
-  size_t ready_call_count_ = 0;
 };
 
 TEST_F(ForceInstalledMetricsTest, EmptyForcelist) {
   SetupEmptyForceList();
   EXPECT_FALSE(fake_timer_->IsRunning());
-  EXPECT_EQ(1u, loaded_call_count_);
-  EXPECT_EQ(1u, ready_call_count_);
   // Don't report metrics when the Forcelist is empty.
   histogram_tester_.ExpectTotalCount(kLoadTimeStats, 0);
   histogram_tester_.ExpectTotalCount(kTimedOutStats, 0);
@@ -208,16 +194,11 @@ TEST_F(ForceInstalledMetricsTest, ExtensionsInstalled) {
   auto ext1 = ExtensionBuilder(kExtensionName1).SetID(kExtensionId1).Build();
   auto ext2 = ExtensionBuilder(kExtensionName2).SetID(kExtensionId2).Build();
 
-  EXPECT_EQ(0u, loaded_call_count_);
-  EXPECT_EQ(0u, ready_call_count_);
   histogram_tester_.ExpectTotalCount(kLoadTimeStats, 0);
   tracker_->OnExtensionLoaded(profile_, ext1.get());
-  EXPECT_EQ(0u, loaded_call_count_);
-  EXPECT_EQ(0u, ready_call_count_);
   histogram_tester_.ExpectTotalCount(kLoadTimeStats, 0);
   tracker_->OnExtensionLoaded(profile_, ext2.get());
-  EXPECT_EQ(1u, loaded_call_count_);
-  EXPECT_EQ(0u, ready_call_count_);
+
   histogram_tester_.ExpectTotalCount(kLoadTimeStats, 1);
   histogram_tester_.ExpectTotalCount(kTimedOutStats, 0);
   histogram_tester_.ExpectTotalCount(kTimedOutNotInstalledStats, 0);
@@ -228,10 +209,6 @@ TEST_F(ForceInstalledMetricsTest, ExtensionsInstalled) {
   histogram_tester_.ExpectUniqueSample(
       kTotalCountStats,
       prefs_->GetManagedPref(pref_names::kInstallForceList)->DictSize(), 1);
-  tracker_->OnExtensionReady(profile_, ext1.get());
-  tracker_->OnExtensionReady(profile_, ext2.get());
-  EXPECT_EQ(1u, loaded_call_count_);
-  EXPECT_EQ(1u, ready_call_count_);
 }
 
 // Verifies that failure is reported for the extensions which are listed in
@@ -245,42 +222,18 @@ TEST_F(ForceInstalledMetricsTest, ExtensionSettingsOverrideForcedList) {
   // ForceInstalledMetrics shuts down timer because all extension are either
   // loaded or failed.
   EXPECT_FALSE(fake_timer_->IsRunning());
-  EXPECT_EQ(1u, loaded_call_count_);
   histogram_tester_.ExpectBucketCount(
       kFailureReasonsCWS,
       InstallStageTracker::FailureReason::OVERRIDDEN_BY_SETTINGS, 1);
 }
 
-TEST_F(ForceInstalledMetricsTest, ObserversOnlyCalledOnce) {
-  // Start with a non-empty force-list, and install them, which triggers
-  // observer.
-  SetupForceList();
-  auto ext1 = ExtensionBuilder(kExtensionName1).SetID(kExtensionId1).Build();
-  auto ext2 = ExtensionBuilder(kExtensionName2).SetID(kExtensionId2).Build();
-  tracker_->OnExtensionLoaded(profile_, ext1.get());
-  tracker_->OnExtensionLoaded(profile_, ext2.get());
-  EXPECT_EQ(1u, loaded_call_count_);
-  tracker_->OnExtensionReady(profile_, ext1.get());
-  tracker_->OnExtensionReady(profile_, ext2.get());
-  EXPECT_EQ(1u, ready_call_count_);
-
-  // Then apply a new set of policies, which shouldn't trigger observers again.
-  SetupEmptyForceList();
-  EXPECT_EQ(1u, loaded_call_count_);
-  EXPECT_EQ(1u, ready_call_count_);
-}
-
 TEST_F(ForceInstalledMetricsTest, ExtensionsInstallationTimedOut) {
   SetupForceList();
-  EXPECT_EQ(0u, loaded_call_count_);
   auto ext1 = ExtensionBuilder(kExtensionName1).SetID(kExtensionId1).Build();
   registry_->AddEnabled(ext1.get());
   EXPECT_TRUE(fake_timer_->IsRunning());
-  EXPECT_EQ(0u, loaded_call_count_);
   fake_timer_->Fire();
-  // Metrics are reported due to timeout, but ForceInstalledTracker::Observer
-  // never fired.
-  EXPECT_EQ(0u, loaded_call_count_);
+  // Metrics are reported due to timeout.
   histogram_tester_.ExpectTotalCount(kLoadTimeStats, 0);
   histogram_tester_.ExpectUniqueSample(kTimedOutStats, 2, 1);
   histogram_tester_.ExpectUniqueSample(kTimedOutNotInstalledStats, 1, 1);
@@ -494,8 +447,6 @@ TEST_F(ForceInstalledMetricsTest, ExtensionsInstallationCancelled) {
   // ForceInstalledMetrics does not shut down the timer, because it's still
   // waiting for the initial extensions to install.
   EXPECT_TRUE(fake_timer_->IsRunning());
-  EXPECT_EQ(0u, loaded_call_count_);
-  EXPECT_EQ(0u, ready_call_count_);
   histogram_tester_.ExpectTotalCount(kLoadTimeStats, 0);
   histogram_tester_.ExpectTotalCount(kTimedOutStats, 0);
   histogram_tester_.ExpectTotalCount(kTimedOutNotInstalledStats, 0);
@@ -512,8 +463,6 @@ TEST_F(ForceInstalledMetricsTest, ForcedExtensionsAddedAfterManualExtensions) {
   // ForceInstalledMetrics should keep running as the forced extensions are
   // still not loaded.
   EXPECT_TRUE(fake_timer_->IsRunning());
-  EXPECT_EQ(0u, loaded_call_count_);
-  EXPECT_EQ(0u, ready_call_count_);
   SetupForceList();
 
   auto ext = ExtensionBuilder(kExtensionName1).SetID(kExtensionId1).Build();
@@ -524,8 +473,6 @@ TEST_F(ForceInstalledMetricsTest, ForcedExtensionsAddedAfterManualExtensions) {
   // ForceInstalledMetrics shuts down timer because kExtensionId1 was loaded and
   // kExtensionId2 was failed.
   EXPECT_FALSE(fake_timer_->IsRunning());
-  EXPECT_EQ(1u, loaded_call_count_);
-  EXPECT_EQ(1u, ready_call_count_);
   histogram_tester_.ExpectBucketCount(
       kFailureReasonsCWS, InstallStageTracker::FailureReason::INVALID_ID, 1);
 }

chrome/browser/extensions/forced_extensions/force_installed_tracker_unittest.cc

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/extensions/forced_extensions/force_installed_tracker.h"
+
+#include "base/scoped_observer.h"
+#include "base/values.h"
+#include "chrome/browser/extensions/forced_extensions/force_installed_test_base.h"
+#include "extensions/common/extension.h"
+#include "extensions/common/extension_builder.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace extensions {
+
+class ForceInstalledTrackerTest : public ForceInstalledTestBase,
+                                  public ForceInstalledTracker::Observer {
+ public:
+  ForceInstalledTrackerTest() = default;
+
+  ForceInstalledTrackerTest(const ForceInstalledTrackerTest&) = delete;
+  ForceInstalledTrackerTest& operator=(const ForceInstalledTrackerTest&) =
+      delete;
+
+  void SetUp() override {
+    ForceInstalledTestBase::SetUp();
+    scoped_observer_.Add(tracker_.get());
+  }
+
+  // ForceInstalledTracker::Observer overrides:
+  void OnForceInstalledExtensionsLoaded() override { loaded_called_ = true; }
+  void OnForceInstalledExtensionsReady() override { ready_called_ = true; }
+
+ protected:
+  ScopedObserver<ForceInstalledTracker, ForceInstalledTracker::Observer>
+      scoped_observer_{this};
+  bool loaded_called_ = false;
+  bool ready_called_ = false;
+};
+
+TEST_F(ForceInstalledTrackerTest, EmptyForcelist) {
+  SetupEmptyForceList();
+  EXPECT_TRUE(loaded_called_);
+  EXPECT_TRUE(ready_called_);
+}
+
+TEST_F(ForceInstalledTrackerTest, BeforeForceInstallPolicy) {
+  EXPECT_FALSE(loaded_called_);
+  EXPECT_FALSE(ready_called_);
+  SetupForceList();
+}
+
+// This test verifies that OnForceInstalledExtensionsLoaded() is called once all
+// force installed extensions have successfully loaded and
+// OnForceInstalledExtensionsReady() is called once all those extensions have
+// become ready for use.
+TEST_F(ForceInstalledTrackerTest, AllExtensionsInstalled) {
+  SetupForceList();
+  auto ext1 = ExtensionBuilder(kExtensionName1).SetID(kExtensionId1).Build();
+  auto ext2 = ExtensionBuilder(kExtensionName2).SetID(kExtensionId2).Build();
+  EXPECT_FALSE(loaded_called_);
+  EXPECT_FALSE(ready_called_);
+  EXPECT_FALSE(tracker_->IsDoneLoading());
+
+  tracker_->OnExtensionLoaded(profile_, ext1.get());
+  tracker_->OnExtensionLoaded(profile_, ext2.get());
+  EXPECT_TRUE(loaded_called_);
+  EXPECT_FALSE(ready_called_);
+  EXPECT_TRUE(tracker_->IsDoneLoading());
+  EXPECT_FALSE(tracker_->IsReady());
+
+  tracker_->OnExtensionReady(profile_, ext1.get());
+  tracker_->OnExtensionReady(profile_, ext2.get());
+  EXPECT_TRUE(loaded_called_);
+  EXPECT_TRUE(ready_called_);
+  EXPECT_TRUE(tracker_->IsDoneLoading());
+  EXPECT_TRUE(tracker_->IsReady());
+}
+
+// This test verifies that OnForceInstalledExtensionsLoaded() is not called till
+// all extensions have either successfully loaded or failed.
+TEST_F(ForceInstalledTrackerTest, ExtensionPendingInstall) {
+  SetupForceList();
+  auto ext1 = ExtensionBuilder(kExtensionName1).SetID(kExtensionId1).Build();
+  tracker_->OnExtensionLoaded(profile_, ext1.get());
+  EXPECT_FALSE(loaded_called_);
+  EXPECT_FALSE(ready_called_);
+  EXPECT_FALSE(tracker_->IsDoneLoading());
+
+  tracker_->OnExtensionReady(profile_, ext1.get());
+  EXPECT_FALSE(loaded_called_);
+  EXPECT_FALSE(ready_called_);
+  EXPECT_FALSE(tracker_->IsDoneLoading());
+}
+
+// This test verifies that applying a new policy value for force installed
+// extensions once all the extensions in the previous value have loaded does not
+// trigger the observers again.
+TEST_F(ForceInstalledTrackerTest, ObserversOnlyCalledOnce) {
+  // Start with a non-empty force-list, and install them, which triggers
+  // observer.
+  SetupForceList();
+  auto ext1 = ExtensionBuilder(kExtensionName1).SetID(kExtensionId1).Build();
+  auto ext2 = ExtensionBuilder(kExtensionName2).SetID(kExtensionId2).Build();
+  tracker_->OnExtensionLoaded(profile_, ext1.get());
+  tracker_->OnExtensionLoaded(profile_, ext2.get());
+  EXPECT_TRUE(loaded_called_);
+
+  tracker_->OnExtensionReady(profile_, ext1.get());
+  tracker_->OnExtensionReady(profile_, ext2.get());
+  EXPECT_TRUE(ready_called_);
+
+  SetupEmptyForceList();
+  EXPECT_TRUE(loaded_called_);
+  EXPECT_TRUE(ready_called_);
+}
+
+// This test verifies that observer is called if force installed extensions are
+// either successfully loaded or failed.
+TEST_F(ForceInstalledTrackerTest, ExtensionsInstallationFailed) {
+  SetupForceList();
+  auto ext1 = ExtensionBuilder(kExtensionName1).SetID(kExtensionId1).Build();
+  tracker_->OnExtensionLoaded(profile_, ext1.get());
+  tracker_->OnExtensionInstallationFailed(
+      kExtensionId2, InstallStageTracker::FailureReason::INVALID_ID);
+  EXPECT_TRUE(loaded_called_);
+  EXPECT_FALSE(ready_called_);
+  EXPECT_TRUE(tracker_->IsDoneLoading());
+}
+
+// This test tracks the status of the force installed extensions in
+// |ForceInstalledTracker::extensions_| as the extensions are either loaded or
+// failed.
+TEST_F(ForceInstalledTrackerTest, ExtensionsStatus) {
+  SetupForceList();
+  EXPECT_EQ(tracker_->extensions().at(kExtensionId1).status,
+            ForceInstalledTracker::ExtensionStatus::PENDING);
+  EXPECT_EQ(tracker_->extensions().at(kExtensionId2).status,
+            ForceInstalledTracker::ExtensionStatus::PENDING);
+
+  auto ext1 = ExtensionBuilder(kExtensionName1).SetID(kExtensionId1).Build();
+  tracker_->OnExtensionLoaded(profile_, ext1.get());
+  tracker_->OnExtensionInstallationFailed(
+      kExtensionId2, InstallStageTracker::FailureReason::INVALID_ID);
+  EXPECT_EQ(tracker_->extensions().at(kExtensionId1).status,
+            ForceInstalledTracker::ExtensionStatus::LOADED);
+  EXPECT_EQ(tracker_->extensions().at(kExtensionId2).status,
+            ForceInstalledTracker::ExtensionStatus::FAILED);
+
+  tracker_->OnExtensionReady(profile_, ext1.get());
+  EXPECT_EQ(tracker_->extensions().at(kExtensionId1).status,
+            ForceInstalledTracker::ExtensionStatus::READY);
+}
+
+// This test verifies that resetting the policy before all force installed
+// extensions are either loaded or failed does not call the observers.
+TEST_F(ForceInstalledTrackerTest, ExtensionsInstallationCancelled) {
+  SetupForceList();
+  SetupEmptyForceList();
+  EXPECT_FALSE(loaded_called_);
+  EXPECT_FALSE(ready_called_);
+}
+
+}  // namespace extensions

chrome/test/BUILD.gn

@@ -5064,6 +5064,7 @@ test("unit_tests") {
       "../browser/extensions/forced_extensions/force_installed_metrics_unittest.cc",
       "../browser/extensions/forced_extensions/force_installed_test_base.cc",
       "../browser/extensions/forced_extensions/force_installed_test_base.h",
+      "../browser/extensions/forced_extensions/force_installed_tracker_unittest.cc",
       "../browser/extensions/install_tracker_unittest.cc",
       "../browser/extensions/install_verifier_unittest.cc",
       "../browser/extensions/installed_loader_unittest.cc",