Unit tests for supervised users policy allowlist extensions install

In response to the COVID-19 crisis, we launched the "extensions lite"
feature that enables supervised users to install extensions from the
ExtensionInstallWhitelist policy. In the hurry, we didn't add unit
tests for the original CL. This CL adds unit tests for
crrev.com/c/2109977.

Bug: 1063099
Change-Id: Iaca12f16aedf6b1bd7e9a0b2c406ed7631f9942d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2121463Reviewed-by: James Cook <jamescook@chromium.org>
Reviewed-by: Marc Treib <treib@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Commit-Queue: Toby Huang <tobyhuang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#754167}

ash/policy/policy_recommendation_restorer_unittest.cc

@@ -24,9 +24,10 @@ class PolicyRecommendationRestorerTest : public NoSessionAshTestBase {
   PolicyRecommendationRestorerTest()
       : recommended_prefs_(new TestingPrefStore),
         prefs_(new sync_preferences::TestingPrefServiceSyncable(
-            new TestingPrefStore,
-            new TestingPrefStore,
-            new TestingPrefStore,
+            /*managed_prefs=*/new TestingPrefStore,
+            /*supervised_user_prefs=*/new TestingPrefStore,
+            /*extension_prefs=*/new TestingPrefStore,
+            /*user_prefs=*/new TestingPrefStore,
             recommended_prefs_,
             new user_prefs::PrefRegistrySyncable,
             new PrefNotifierImpl)) {}

chrome/browser/ui/sync/profile_signin_confirmation_helper_unittest.cc

@@ -140,9 +140,11 @@ class ProfileSigninConfirmationHelperTest : public testing::Test {
     user_prefs_ = new TestingPrefStoreWithCustomReadError;
     sync_preferences::TestingPrefServiceSyncable* pref_service =
         new sync_preferences::TestingPrefServiceSyncable(
-            new TestingPrefStore(), new TestingPrefStore(), user_prefs_,
-            new TestingPrefStore(), new user_prefs::PrefRegistrySyncable(),
-            new PrefNotifierImpl());
+            /*managed_prefs=*/new TestingPrefStore(),
+            /*supervised_user_prefs=*/new TestingPrefStore(),
+            /*extension_prefs=*/new TestingPrefStore(), user_prefs_,
+            /*recommended_prefs=*/new TestingPrefStore(),
+            new user_prefs::PrefRegistrySyncable(), new PrefNotifierImpl());
     RegisterUserProfilePrefs(pref_service->registry());
     builder.SetPrefService(
         base::WrapUnique<sync_preferences::PrefServiceSyncable>(pref_service));

chrome/test/base/testing_profile.cc

@@ -94,11 +94,11 @@
 #include "components/policy/core/common/policy_service.h"
 #include "components/policy/core/common/policy_service_impl.h"
 #include "components/policy/core/common/schema.h"
+#include "components/prefs/pref_notifier_impl.h"
 #include "components/prefs/testing_pref_store.h"
 #include "components/signin/public/identity_manager/identity_test_utils.h"
 #include "components/sync/model/fake_sync_change_processor.h"
 #include "components/sync/model/sync_error_factory_mock.h"
-#include "components/sync_preferences/pref_service_mock_factory.h"
 #include "components/sync_preferences/pref_service_syncable.h"
 #include "components/sync_preferences/testing_pref_service_syncable.h"
 #include "components/user_prefs/user_prefs.h"
@@ -153,7 +153,6 @@
 
 #if BUILDFLAG(ENABLE_SUPERVISED_USERS)
 #include "chrome/browser/supervised_user/supervised_user_constants.h"
-#include "chrome/browser/supervised_user/supervised_user_pref_store.h"
 #include "chrome/browser/supervised_user/supervised_user_settings_service.h"
 #include "chrome/browser/supervised_user/supervised_user_settings_service_factory.h"
 #endif
@@ -417,8 +416,8 @@ void TestingProfile::Init() {
   if (!IsOffTheRecord()) {
     SupervisedUserSettingsService* settings_service =
         SupervisedUserSettingsServiceFactory::GetForKey(key_.get());
-    TestingPrefStore* store = new TestingPrefStore();
-    settings_service->Init(store);
+    supervised_user_pref_store_ = new TestingPrefStore();
+    settings_service->Init(supervised_user_pref_store_);
     settings_service->MergeDataAndStartSyncing(
         syncer::SUPERVISED_USER_SETTINGS, syncer::SyncDataList(),
         std::unique_ptr<syncer::SyncChangeProcessor>(
@@ -426,7 +425,7 @@ void TestingProfile::Init() {
         std::unique_ptr<syncer::SyncErrorFactory>(
             new syncer::SyncErrorFactoryMock));
 
-    store->SetInitializationCompleted();
+    supervised_user_pref_store_->SetInitializationCompleted();
   }
 #endif
 
@@ -852,20 +851,17 @@ void TestingProfile::CreateTestingPrefService() {
 void TestingProfile::CreatePrefServiceForSupervisedUser() {
   DCHECK(!prefs_.get());
   DCHECK(!supervised_user_id_.empty());
-  sync_preferences::PrefServiceMockFactory factory;
-  SupervisedUserSettingsService* supervised_user_settings =
-      SupervisedUserSettingsServiceFactory::GetForKey(GetProfileKey());
-  scoped_refptr<PrefStore> supervised_user_prefs =
-      base::MakeRefCounted<SupervisedUserPrefStore>(supervised_user_settings);
 
-  factory.set_supervised_user_prefs(supervised_user_prefs);
-
-  scoped_refptr<user_prefs::PrefRegistrySyncable> registry(
-      new user_prefs::PrefRegistrySyncable);
-
-  prefs_ = factory.CreateSyncable(registry.get());
-  RegisterUserProfilePrefs(registry.get());
+  // Construct testing_prefs_ by hand to add the supervised user pref store.
+  testing_prefs_ = new sync_preferences::TestingPrefServiceSyncable(
+      /*managed_prefs=*/new TestingPrefStore, supervised_user_pref_store_,
+      /*extension_prefs=*/new TestingPrefStore,
+      /*user_prefs=*/new TestingPrefStore,
+      /*recommended_prefs=*/new TestingPrefStore,
+      new user_prefs::PrefRegistrySyncable, new PrefNotifierImpl);
+  prefs_.reset(testing_prefs_);
   user_prefs::UserPrefs::Set(this, prefs_.get());
+  RegisterUserProfilePrefs(testing_prefs_->registry());
 }
 #endif  // BUILDFLAG(ENABLE_SUPERVISED_USERS)
 

chrome/test/base/testing_profile.h

@@ -36,6 +36,7 @@ class BrowserContextDependencyManager;
 class SimpleDependencyManager;
 class ExtensionSpecialStoragePolicy;
 class HostContentSettingsMap;
+class TestingPrefStore;
 
 namespace content {
 class MockResourceContext;
@@ -500,6 +501,10 @@ class TestingProfile : public Profile {
 #endif  // defined(OS_CHROMEOS)
 
   std::unique_ptr<policy::PolicyService> policy_service_;
+
+#if BUILDFLAG(ENABLE_SUPERVISED_USERS)
+  TestingPrefStore* supervised_user_pref_store_ = nullptr;
+#endif  // BUILDFLAG(ENABLE_SUPERVISED_USERS)
 };
 
 #endif  // CHROME_TEST_BASE_TESTING_PROFILE_H_

components/prefs/testing_pref_service.cc

@@ -17,6 +17,7 @@
 template <>
 TestingPrefServiceBase<PrefService, PrefRegistry>::TestingPrefServiceBase(
     TestingPrefStore* managed_prefs,
+    TestingPrefStore* supervised_user_prefs,
     TestingPrefStore* extension_prefs,
     TestingPrefStore* user_prefs,
     TestingPrefStore* recommended_prefs,
@@ -25,9 +26,9 @@ TestingPrefServiceBase<PrefService, PrefRegistry>::TestingPrefServiceBase(
     : PrefService(
           std::unique_ptr<PrefNotifierImpl>(pref_notifier),
           std::make_unique<PrefValueStore>(managed_prefs,
-                                           nullptr,
+                                           supervised_user_prefs,
                                            extension_prefs,
-                                           nullptr,
+                                           /*command_line_prefs=*/nullptr,
                                            user_prefs,
                                            recommended_prefs,
                                            pref_registry->defaults().get(),
@@ -39,16 +40,18 @@ TestingPrefServiceBase<PrefService, PrefRegistry>::TestingPrefServiceBase(
                                       PrefRegistry>::HandleReadError),
           false),
       managed_prefs_(managed_prefs),
+      supervised_user_prefs_(supervised_user_prefs),
       extension_prefs_(extension_prefs),
       user_prefs_(user_prefs),
       recommended_prefs_(recommended_prefs) {}
 
 TestingPrefServiceSimple::TestingPrefServiceSimple()
     : TestingPrefServiceBase<PrefService, PrefRegistry>(
-          new TestingPrefStore(),
-          new TestingPrefStore(),
-          new TestingPrefStore(),
-          new TestingPrefStore(),
+          /*managed_prefs=*/new TestingPrefStore(),
+          /*supervised_user_prefs=*/new TestingPrefStore(),
+          /*extension_prefs=*/new TestingPrefStore(),
+          /*user_prefs=*/new TestingPrefStore(),
+          /*recommended_prefs=*/new TestingPrefStore(),
           new PrefRegistrySimple(),
           new PrefNotifierImpl()) {}
 

components/prefs/testing_pref_service.h

@@ -41,6 +41,12 @@ class TestingPrefServiceBase : public SuperPrefService {
   // preference has been defined previously.
   void RemoveManagedPref(const std::string& path);
 
+  // Similar to the above, but for supervised user preferences.
+  const base::Value* GetSupervisedUserPref(const std::string& path) const;
+  void SetSupervisedUserPref(const std::string& path,
+                             std::unique_ptr<base::Value> value);
+  void RemoveSupervisedUserPref(const std::string& path);
+
   // Similar to the above, but for extension preferences.
   // Does not really know about extensions and their order of installation.
   // Useful in tests that only check that a preference is overridden by an
@@ -69,6 +75,7 @@ class TestingPrefServiceBase : public SuperPrefService {
 
  protected:
   TestingPrefServiceBase(TestingPrefStore* managed_prefs,
+                         TestingPrefStore* supervised_user_prefs,
                          TestingPrefStore* extension_prefs,
                          TestingPrefStore* user_prefs,
                          TestingPrefStore* recommended_prefs,
@@ -91,6 +98,7 @@ class TestingPrefServiceBase : public SuperPrefService {
 
   // Pointers to the pref stores our value store uses.
   scoped_refptr<TestingPrefStore> managed_prefs_;
+  scoped_refptr<TestingPrefStore> supervised_user_prefs_;
   scoped_refptr<TestingPrefStore> extension_prefs_;
   scoped_refptr<TestingPrefStore> user_prefs_;
   scoped_refptr<TestingPrefStore> recommended_prefs_;
@@ -116,9 +124,10 @@ class TestingPrefServiceSimple
   DISALLOW_COPY_AND_ASSIGN(TestingPrefServiceSimple);
 };
 
-template<>
+template <>
 TestingPrefServiceBase<PrefService, PrefRegistry>::TestingPrefServiceBase(
     TestingPrefStore* managed_prefs,
+    TestingPrefStore* supervised_user_prefs,
     TestingPrefStore* extension_prefs,
     TestingPrefStore* user_prefs,
     TestingPrefStore* recommended_prefs,
@@ -150,6 +159,26 @@ void TestingPrefServiceBase<SuperPrefService, ConstructionPrefRegistry>::
   RemovePref(managed_prefs_.get(), path);
 }
 
+template <class SuperPrefService, class ConstructionPrefRegistry>
+const base::Value*
+TestingPrefServiceBase<SuperPrefService, ConstructionPrefRegistry>::
+    GetSupervisedUserPref(const std::string& path) const {
+  return GetPref(supervised_user_prefs_.get(), path);
+}
+
+template <class SuperPrefService, class ConstructionPrefRegistry>
+void TestingPrefServiceBase<SuperPrefService, ConstructionPrefRegistry>::
+    SetSupervisedUserPref(const std::string& path,
+                          std::unique_ptr<base::Value> value) {
+  SetPref(supervised_user_prefs_.get(), path, std::move(value));
+}
+
+template <class SuperPrefService, class ConstructionPrefRegistry>
+void TestingPrefServiceBase<SuperPrefService, ConstructionPrefRegistry>::
+    RemoveSupervisedUserPref(const std::string& path) {
+  RemovePref(supervised_user_prefs_.get(), path);
+}
+
 template <class SuperPrefService, class ConstructionPrefRegistry>
 const base::Value* TestingPrefServiceBase<
     SuperPrefService,
@@ -237,6 +266,7 @@ template <class SuperPrefService, class ConstructionPrefRegistry>
 void TestingPrefServiceBase<SuperPrefService, ConstructionPrefRegistry>::
     SetInitializationCompleted() {
   managed_prefs_->SetInitializationCompleted();
+  supervised_user_prefs_->SetInitializationCompleted();
   extension_prefs_->SetInitializationCompleted();
   recommended_prefs_->SetInitializationCompleted();
   // |user_prefs_| is initialized in PrefService constructor so no need to

components/sync_preferences/testing_pref_service_syncable.cc

@@ -16,6 +16,7 @@ template <>
 TestingPrefServiceBase<sync_preferences::PrefServiceSyncable,
                        user_prefs::PrefRegistrySyncable>::
     TestingPrefServiceBase(TestingPrefStore* managed_prefs,
+                           TestingPrefStore* supervised_user_prefs,
                            TestingPrefStore* extension_prefs,
                            TestingPrefStore* user_prefs,
                            TestingPrefStore* recommended_prefs,
@@ -24,9 +25,9 @@ TestingPrefServiceBase<sync_preferences::PrefServiceSyncable,
     : sync_preferences::PrefServiceSyncable(
           std::unique_ptr<PrefNotifierImpl>(pref_notifier),
           std::make_unique<PrefValueStore>(managed_prefs,
-                                           nullptr,  // supervised_user_prefs
-                                           extension_prefs,  // extension_prefs
-                                           nullptr,  // command_line_prefs
+                                           supervised_user_prefs,
+                                           extension_prefs,
+                                           /*command_line_prefs=*/nullptr,
                                            user_prefs,
                                            recommended_prefs,
                                            pref_registry->defaults().get(),
@@ -49,15 +50,17 @@ namespace sync_preferences {
 TestingPrefServiceSyncable::TestingPrefServiceSyncable()
     : TestingPrefServiceBase<PrefServiceSyncable,
                              user_prefs::PrefRegistrySyncable>(
-          new TestingPrefStore(),
-          new TestingPrefStore(),
-          new TestingPrefStore(),
-          new TestingPrefStore(),
+          /*managed_prefs=*/new TestingPrefStore(),
+          /*supervised_user_prefs=*/new TestingPrefStore(),
+          /*extension_prefs=*/new TestingPrefStore(),
+          /*user_prefs=*/new TestingPrefStore(),
+          /*recommended_prefs=*/new TestingPrefStore(),
           new user_prefs::PrefRegistrySyncable(),
           new PrefNotifierImpl()) {}
 
 TestingPrefServiceSyncable::TestingPrefServiceSyncable(
     TestingPrefStore* managed_prefs,
+    TestingPrefStore* supervised_user_prefs,
     TestingPrefStore* extension_prefs,
     TestingPrefStore* user_prefs,
     TestingPrefStore* recommended_prefs,
@@ -66,6 +69,7 @@ TestingPrefServiceSyncable::TestingPrefServiceSyncable(
     : TestingPrefServiceBase<PrefServiceSyncable,
                              user_prefs::PrefRegistrySyncable>(
           managed_prefs,
+          supervised_user_prefs,
           extension_prefs,
           user_prefs,
           recommended_prefs,

components/sync_preferences/testing_pref_service_syncable.h

@@ -23,8 +23,8 @@ namespace sync_preferences {
 //  a PrefModelAssociatorClient.
 // TODO(tschumann) The whole purpose of TestingPrefServiceBase is questionable
 // and I'd be in favor of removing it completely:
-//  -- it hides the dependency injetion of the different stores
-//  -- just to later offer ways to manipulate speficic stores.
+//  -- it hides the dependency injection of the different stores
+//  -- just to later offer ways to manipulate specific stores.
 //  -- if tests just dependency injects the individual stores directly, they
 //     already have full control and won't need that indirection at all.
 // See PrefServiceSyncableMergeTest as an example of a cleaner way.
@@ -34,6 +34,7 @@ class TestingPrefServiceSyncable
  public:
   TestingPrefServiceSyncable();
   TestingPrefServiceSyncable(TestingPrefStore* managed_prefs,
+                             TestingPrefStore* supervised_user_prefs,
                              TestingPrefStore* extension_prefs,
                              TestingPrefStore* user_prefs,
                              TestingPrefStore* recommended_prefs,
@@ -56,6 +57,7 @@ template <>
 TestingPrefServiceBase<sync_preferences::PrefServiceSyncable,
                        user_prefs::PrefRegistrySyncable>::
     TestingPrefServiceBase(TestingPrefStore* managed_prefs,
+                           TestingPrefStore* supervised_user_prefs,
                            TestingPrefStore* extension_prefs,
                            TestingPrefStore* user_prefs,
                            TestingPrefStore* recommended_prefs,