Refactor force installed extensions unit tests

The unit tests for force installed extensions repeatedly use code to
create an extension using ExtensionBuilder and mark the extension as
loaded/ready. This code can be extracted in a common function and used
from there to reduce some code duplication.

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

chrome/browser/extensions/forced_extensions/force_installed_test_base.cc

@@ -8,7 +8,6 @@
 #include "base/strings/utf_string_conversions.h"
 #include "base/values.h"
 #include "chrome/browser/extensions/external_provider_impl.h"
-#include "chrome/browser/extensions/forced_extensions/force_installed_tracker.h"
 #include "chrome/browser/extensions/forced_extensions/install_stage_tracker.h"
 #include "chrome/test/base/testing_browser_process.h"
 #include "components/policy/core/common/policy_service_impl.h"
@@ -16,7 +15,6 @@
 #include "components/sync_preferences/testing_pref_service_syncable.h"
 #include "extensions/browser/extension_registry.h"
 #include "extensions/browser/pref_names.h"
-#include "extensions/common/extension.h"
 #include "extensions/common/extension_builder.h"
 #include "testing/gmock/include/gmock/gmock.h"
 
@@ -107,4 +105,23 @@ void ForceInstalledTestBase::SetupEmptyForceList() {
   base::RunLoop().RunUntilIdle();
 }
 
+scoped_refptr<const Extension> ForceInstalledTestBase::CreateNewExtension(
+    const std::string& extension_name,
+    const std::string& extension_id,
+    const ForceInstalledTracker::ExtensionStatus& status) {
+  auto ext = ExtensionBuilder(extension_name).SetID(extension_id).Build();
+  switch (status) {
+    case ForceInstalledTracker::ExtensionStatus::PENDING:
+    case ForceInstalledTracker::ExtensionStatus::FAILED:
+      break;
+    case ForceInstalledTracker::ExtensionStatus::LOADED:
+      force_installed_tracker()->OnExtensionLoaded(profile(), ext.get());
+      break;
+    case ForceInstalledTracker::ExtensionStatus::READY:
+      force_installed_tracker()->OnExtensionLoaded(profile(), ext.get());
+      force_installed_tracker()->OnExtensionReady(profile(), ext.get());
+  }
+  return ext;
+}
+
 }  // namespace extensions

chrome/browser/extensions/forced_extensions/force_installed_test_base.h

@@ -5,10 +5,12 @@
 #ifndef CHROME_BROWSER_EXTENSIONS_FORCED_EXTENSIONS_FORCE_INSTALLED_TEST_BASE_H_
 #define CHROME_BROWSER_EXTENSIONS_FORCED_EXTENSIONS_FORCE_INSTALLED_TEST_BASE_H_
 
+#include "chrome/browser/extensions/forced_extensions/force_installed_tracker.h"
 #include "chrome/test/base/testing_profile.h"
 #include "chrome/test/base/testing_profile_manager.h"
 #include "components/policy/core/common/mock_configuration_policy_provider.h"
 #include "content/public/test/browser_task_environment.h"
+#include "extensions/common/extension.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace sync_preferences {
@@ -19,7 +21,6 @@ namespace extensions {
 
 class ExtensionRegistry;
 class InstallStageTracker;
-class ForceInstalledTracker;
 
 // This class is extended by tests to provide a setup for tracking installation
 // of force extensions. It also provides helper functions for creating and
@@ -44,6 +45,14 @@ class ForceInstalledTestBase : public testing::Test {
   // kInstallForceList preference.
   void SetupEmptyForceList();
 
+  // Creates a new extension with |extension_id| and |extension_name| and fakes
+  // its status by calling one of ForceInstalledTracker's
+  // ExtensionRegistryObserver override.
+  scoped_refptr<const Extension> CreateNewExtension(
+      const std::string& extension_name,
+      const std::string& extension_id,
+      const ForceInstalledTracker::ExtensionStatus& status);
+
   Profile* profile() const { return profile_; }
 
   sync_preferences::TestingPrefServiceSyncable* prefs() const { return prefs_; }

chrome/browser/extensions/forced_extensions/force_installed_tracker_unittest.cc

@@ -8,11 +8,12 @@
 #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 {
 
+using ExtensionStatus = ForceInstalledTracker::ExtensionStatus;
+
 class ForceInstalledTrackerTest : public ForceInstalledTestBase,
                                   public ForceInstalledTracker::Observer {
  public:
@@ -56,8 +57,10 @@ TEST_F(ForceInstalledTrackerTest, BeforeForceInstallPolicy) {
 // become ready for use.
 TEST_F(ForceInstalledTrackerTest, AllExtensionsInstalled) {
   SetupForceList(true /*is_from_store */);
-  auto ext1 = ExtensionBuilder(kExtensionName1).SetID(kExtensionId1).Build();
-  auto ext2 = ExtensionBuilder(kExtensionName2).SetID(kExtensionId2).Build();
+  scoped_refptr<const Extension> ext1 = CreateNewExtension(
+      kExtensionName1, kExtensionId1, ExtensionStatus::PENDING);
+  scoped_refptr<const Extension> ext2 = CreateNewExtension(
+      kExtensionName2, kExtensionId2, ExtensionStatus::PENDING);
   EXPECT_FALSE(loaded_called_);
   EXPECT_FALSE(ready_called_);
   EXPECT_FALSE(force_installed_tracker()->IsDoneLoading());
@@ -81,8 +84,8 @@ TEST_F(ForceInstalledTrackerTest, AllExtensionsInstalled) {
 // all extensions have either successfully loaded or failed.
 TEST_F(ForceInstalledTrackerTest, ExtensionPendingInstall) {
   SetupForceList(true /*is_from_store */);
-  auto ext1 = ExtensionBuilder(kExtensionName1).SetID(kExtensionId1).Build();
-  force_installed_tracker()->OnExtensionLoaded(profile(), ext1.get());
+  scoped_refptr<const Extension> ext1 = CreateNewExtension(
+      kExtensionName1, kExtensionId1, ExtensionStatus::LOADED);
   EXPECT_FALSE(loaded_called_);
   EXPECT_FALSE(ready_called_);
   EXPECT_FALSE(force_installed_tracker()->IsDoneLoading());
@@ -100,10 +103,10 @@ TEST_F(ForceInstalledTrackerTest, ObserversOnlyCalledOnce) {
   // Start with a non-empty force-list, and install them, which triggers
   // observer.
   SetupForceList(true /*is_from_store */);
-  auto ext1 = ExtensionBuilder(kExtensionName1).SetID(kExtensionId1).Build();
-  auto ext2 = ExtensionBuilder(kExtensionName2).SetID(kExtensionId2).Build();
-  force_installed_tracker()->OnExtensionLoaded(profile(), ext1.get());
-  force_installed_tracker()->OnExtensionLoaded(profile(), ext2.get());
+  scoped_refptr<const Extension> ext1 = CreateNewExtension(
+      kExtensionName1, kExtensionId1, ExtensionStatus::LOADED);
+  scoped_refptr<const Extension> ext2 = CreateNewExtension(
+      kExtensionName2, kExtensionId2, ExtensionStatus::LOADED);
   EXPECT_TRUE(loaded_called_);
 
   force_installed_tracker()->OnExtensionReady(profile(), ext1.get());
@@ -119,8 +122,8 @@ TEST_F(ForceInstalledTrackerTest, ObserversOnlyCalledOnce) {
 // either successfully loaded or failed.
 TEST_F(ForceInstalledTrackerTest, ExtensionsInstallationFailed) {
   SetupForceList(true /*is_from_store */);
-  auto ext1 = ExtensionBuilder(kExtensionName1).SetID(kExtensionId1).Build();
-  force_installed_tracker()->OnExtensionLoaded(profile(), ext1.get());
+  scoped_refptr<const Extension> ext1 = CreateNewExtension(
+      kExtensionName1, kExtensionId1, ExtensionStatus::LOADED);
   force_installed_tracker()->OnExtensionInstallationFailed(
       kExtensionId2, InstallStageTracker::FailureReason::INVALID_ID);
   EXPECT_TRUE(loaded_called_);
@@ -134,22 +137,22 @@ TEST_F(ForceInstalledTrackerTest, ExtensionsInstallationFailed) {
 TEST_F(ForceInstalledTrackerTest, ExtensionsStatus) {
   SetupForceList(true /*is_from_store */);
   EXPECT_EQ(force_installed_tracker()->extensions().at(kExtensionId1).status,
-            ForceInstalledTracker::ExtensionStatus::PENDING);
+            ExtensionStatus::PENDING);
   EXPECT_EQ(force_installed_tracker()->extensions().at(kExtensionId2).status,
-            ForceInstalledTracker::ExtensionStatus::PENDING);
+            ExtensionStatus::PENDING);
 
-  auto ext1 = ExtensionBuilder(kExtensionName1).SetID(kExtensionId1).Build();
-  force_installed_tracker()->OnExtensionLoaded(profile(), ext1.get());
+  scoped_refptr<const Extension> ext1 = CreateNewExtension(
+      kExtensionName1, kExtensionId1, ExtensionStatus::LOADED);
   force_installed_tracker()->OnExtensionInstallationFailed(
       kExtensionId2, InstallStageTracker::FailureReason::INVALID_ID);
   EXPECT_EQ(force_installed_tracker()->extensions().at(kExtensionId1).status,
-            ForceInstalledTracker::ExtensionStatus::LOADED);
+            ExtensionStatus::LOADED);
   EXPECT_EQ(force_installed_tracker()->extensions().at(kExtensionId2).status,
-            ForceInstalledTracker::ExtensionStatus::FAILED);
+            ExtensionStatus::FAILED);
 
   force_installed_tracker()->OnExtensionReady(profile(), ext1.get());
   EXPECT_EQ(force_installed_tracker()->extensions().at(kExtensionId1).status,
-            ForceInstalledTracker::ExtensionStatus::READY);
+            ExtensionStatus::READY);
 }
 
 // This test verifies that resetting the policy before all force installed
@@ -165,9 +168,8 @@ TEST_F(ForceInstalledTrackerTest, ExtensionsInstallationCancelled) {
 // installed extension is either ready for use or failed.
 TEST_F(ForceInstalledTrackerTest, AllExtensionsReady) {
   SetupForceList(true /*is_from_store */);
-  auto ext1 = ExtensionBuilder(kExtensionName1).SetID(kExtensionId1).Build();
-  force_installed_tracker()->OnExtensionLoaded(profile(), ext1.get());
-  force_installed_tracker()->OnExtensionReady(profile(), ext1.get());
+  scoped_refptr<const Extension> ext1 = CreateNewExtension(
+      kExtensionName1, kExtensionId1, ExtensionStatus::READY);
   force_installed_tracker()->OnExtensionInstallationFailed(
       kExtensionId2, InstallStageTracker::FailureReason::INVALID_ID);
   EXPECT_TRUE(loaded_called_);