Remove test dependency to SupervisedUserService for extensions prefs.

Prepares for larger migration in crrev.com/c/4353156. Bug: b/272008112 Change-Id: I599592d8c1bdac1b676eac7872717fa3c26c1382 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4358151 Commit-Queue: Nohemi Fernandez <fernandex@chromium.org> Reviewed-by: Alex Ilin <alexilin@chromium.org> Cr-Commit-Position: refs/heads/main@{#1120484}
chromium · Mar 22, 2023 · dfc72df · dfc72df
1 parent b7f80c4
commit dfc72df
Show file tree

Hide file tree

Showing 12 changed files with 92 additions and 79 deletions.
diff --git a/chrome/browser/ash/child_accounts/family_user_app_metrics_unittest.cc b/chrome/browser/ash/child_accounts/family_user_app_metrics_unittest.cc
@@ -17,6 +17,7 @@
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/supervised_user/supervised_user_service.h"
 #include "chrome/browser/supervised_user/supervised_user_service_factory.h"
+#include "chrome/browser/supervised_user/supervised_user_test_util.h"
 #include "components/services/app_service/public/cpp/app_registry_cache.h"
 #include "components/services/app_service/public/cpp/app_types.h"
 #include "components/services/app_service/public/cpp/instance.h"
@@ -94,8 +95,8 @@ class FamilyUserAppMetricsTest
     EXPECT_EQ(IsFamilyLink(), profile()->IsChild());
 
     supervised_user_service()->Init();
-    supervised_user_service()
-        ->SetSupervisedUserExtensionsMayRequestPermissionsPrefForTesting(true);
+    supervised_user_test_util::
+        SetSupervisedUserExtensionsMayRequestPermissionsPref(profile(), true);
 
     family_user_app_metrics_ =
         std::make_unique<FamilyUserAppMetricsDerivedForTest>(profile());

diff --git a/chrome/browser/extensions/api/developer_private/extension_info_generator_unittest.cc b/chrome/browser/extensions/api/developer_private/extension_info_generator_unittest.cc
@@ -980,8 +980,8 @@ class ExtensionInfoGeneratorUnitTestSupervised
 
     GetSupervisedUserService()->Init();
     // Set the pref to allow the child to request extension install.
-    GetSupervisedUserService()
-        ->SetSupervisedUserExtensionsMayRequestPermissionsPrefForTesting(true);
+    supervised_user_test_util::
+        SetSupervisedUserExtensionsMayRequestPermissionsPref(profile(), true);
   }
 };
 
@@ -1026,8 +1026,8 @@ TEST_F(ExtensionInfoGeneratorUnitTestSupervised,
 
   // Simulate the parent disallowing the child from approving permission
   // updates.
-  GetSupervisedUserService()
-      ->SetSupervisedUserExtensionsMayRequestPermissionsPrefForTesting(false);
+  supervised_user_test_util::
+      SetSupervisedUserExtensionsMayRequestPermissionsPref(profile(), false);
 
   std::unique_ptr<api::developer_private::ExtensionInfo> info =
       GenerateExtensionInfo(extension_id);

diff --git a/chrome/browser/extensions/api/management/management_api_unittest.cc b/chrome/browser/extensions/api/management/management_api_unittest.cc
@@ -1075,8 +1075,8 @@ class ManagementApiSupervisedUserTest : public ManagementApiUnitTest {
 
     GetSupervisedUserService()->Init();
     // Set the pref to allow the child to request extension install.
-    GetSupervisedUserService()
-        ->SetSupervisedUserExtensionsMayRequestPermissionsPrefForTesting(true);
+    supervised_user_test_util::
+        SetSupervisedUserExtensionsMayRequestPermissionsPref(profile(), true);
 
     // Create a WebContents to simulate the Chrome Web Store.
     web_contents_ =
@@ -1118,8 +1118,8 @@ TEST_F(ManagementApiSupervisedUserTest, SetEnabled_BlockedByParent) {
 
   // Simulate disabling Permissions for sites, apps and extensions
   // in the testing supervised user service delegate used by the Management API.
-  GetSupervisedUserService()
-      ->SetSupervisedUserExtensionsMayRequestPermissionsPrefForTesting(false);
+  supervised_user_test_util::
+      SetSupervisedUserExtensionsMayRequestPermissionsPref(profile(), false);
 
   // The supervised user trying to enable while Permissions for sites, apps and
   // extensions is disabled should fail.
@@ -1286,8 +1286,8 @@ TEST_F(ManagementApiSupervisedUserTest,
   // If the "Permissions for sites, apps and extensions" toggle is off, then the
   // enable attempt should fail.
   {
-    GetSupervisedUserService()
-        ->SetSupervisedUserExtensionsMayRequestPermissionsPrefForTesting(false);
+    supervised_user_test_util::
+        SetSupervisedUserExtensionsMayRequestPermissionsPref(profile(), false);
     std::string error;
     bool success = RunSetEnabledFunction(web_contents_.get(), extension_id,
                                          /*use_user_gesture=*/true,
@@ -1745,8 +1745,8 @@ TEST_F(ManagementApiSupervisedUserTestWithSetup,
 
   // Simulate the parent disabling the "Permissions for sites, apps and
   // extensions" toggle.
-  GetSupervisedUserService()
-      ->SetSupervisedUserExtensionsMayRequestPermissionsPrefForTesting(false);
+  supervised_user_test_util::
+      SetSupervisedUserExtensionsMayRequestPermissionsPref(profile(), false);
 
   // Simulate a call to chrome.management.setEnabled(). The enable attempt
   // should be blocked.

diff --git a/chrome/browser/extensions/api/webstore_private/webstore_private_apitest.cc b/chrome/browser/extensions/api/webstore_private/webstore_private_apitest.cc
@@ -371,10 +371,8 @@ class ExtensionWebstorePrivateApiTestChild
     ExtensionWebstorePrivateApiTest::SetUpOnMainThread();
 
     InitializeFamilyData();
-    SupervisedUserService* service =
-        SupervisedUserServiceFactory::GetForProfile(profile());
-    service->SetSupervisedUserExtensionsMayRequestPermissionsPrefForTesting(
-        true);
+    supervised_user_test_util::
+        SetSupervisedUserExtensionsMayRequestPermissionsPref(profile(), true);
   }
 
   ash::LoggedInUserMixin* GetLoggedInUserMixin() {
@@ -488,10 +486,8 @@ IN_PROC_BROWSER_TEST_F(ExtensionWebstorePrivateApiTestChild,
   base::HistogramTester histogram_tester;
   base::UserActionTester user_action_tester;
 
-  SupervisedUserService* service =
-      SupervisedUserServiceFactory::GetForProfile(profile());
-  service->SetSupervisedUserExtensionsMayRequestPermissionsPrefForTesting(
-      false);
+  supervised_user_test_util::
+      SetSupervisedUserExtensionsMayRequestPermissionsPref(profile(), false);
 
   set_next_dialog_action(NextDialogAction::kAccept);
   // Tell the Reauth API client to return a success for the next reauth

diff --git a/chrome/browser/supervised_user/supervised_user_extension_browsertest.cc b/chrome/browser/supervised_user/supervised_user_extension_browsertest.cc
@@ -9,6 +9,7 @@
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/supervised_user/supervised_user_service.h"
 #include "chrome/browser/supervised_user/supervised_user_service_factory.h"
+#include "chrome/browser/supervised_user/supervised_user_test_util.h"
 #include "chrome/test/base/mixin_based_in_process_browser_test.h"
 #include "content/public/test/browser_test.h"
 #include "content/public/test/test_launcher.h"
@@ -92,12 +93,6 @@ class SupervisedUserExtensionTest : public ExtensionBrowserTest {
     return SupervisedUserServiceFactory::GetForProfile(profile());
   }
 
-  void SetSupervisedUserExtensionsMayRequestPermissionsPref(bool enabled) {
-    GetSupervisedUserService()
-        ->SetSupervisedUserExtensionsMayRequestPermissionsPrefForTesting(
-            enabled);
-  }
-
   bool IsDisabledForCustodianApproval(const std::string& extension_id) {
     ExtensionPrefs* extension_prefs = ExtensionPrefs::Get(profile());
     return extension_prefs->HasDisableReason(
@@ -125,7 +120,8 @@ class SupervisedUserExtensionTest : public ExtensionBrowserTest {
 // after removing supervision. Prevents a regression to crbug/1045625.
 IN_PROC_BROWSER_TEST_F(SupervisedUserExtensionTest,
                        PRE_RemovingSupervisionCustodianApprovalRequired) {
-  SetSupervisedUserExtensionsMayRequestPermissionsPref(true);
+  supervised_user_test_util::
+      SetSupervisedUserExtensionsMayRequestPermissionsPref(profile(), true);
 
   EXPECT_TRUE(profile()->IsChild());
 

diff --git a/chrome/browser/supervised_user/supervised_user_extension_unittest.cc b/chrome/browser/supervised_user/supervised_user_extension_unittest.cc
@@ -13,6 +13,7 @@
 #include "chrome/browser/supervised_user/supervised_user_extensions_metrics_recorder.h"
 #include "chrome/browser/supervised_user/supervised_user_service.h"
 #include "chrome/browser/supervised_user/supervised_user_service_factory.h"
+#include "chrome/browser/supervised_user/supervised_user_test_util.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/test/base/testing_profile.h"
 #include "extensions/browser/api_test_utils.h"
@@ -36,12 +37,6 @@ class SupervisedUserExtensionTest : public ExtensionServiceTestWithInstall {
   SupervisedUserExtensionTest() = default;
 
  protected:
-  void SetSupervisedUserExtensionsMayRequestPermissionsPref(bool enabled) {
-    supervised_user_service()
-        ->SetSupervisedUserExtensionsMayRequestPermissionsPrefForTesting(
-            enabled);
-  }
-
   void InitServices(bool profile_is_supervised) {
     ExtensionServiceInitParams params = CreateDefaultInitParams();
     params.profile_is_supervised = profile_is_supervised;
@@ -193,7 +188,8 @@ TEST_F(SupervisedUserExtensionTest,
 TEST_F(SupervisedUserExtensionTest,
        CustodianApprovalDoesNotAffectRegularUsers) {
   InitServices(/*profile_is_supervised=*/false);
-  SetSupervisedUserExtensionsMayRequestPermissionsPref(false);
+  supervised_user_test_util::
+      SetSupervisedUserExtensionsMayRequestPermissionsPref(profile(), false);
 
   // Install an extension.
   base::FilePath path = data_dir().AppendASCII("good.crx");
@@ -214,7 +210,8 @@ TEST_F(SupervisedUserExtensionTest,
 // previously installed extensions.
 TEST_F(SupervisedUserExtensionTest, ExtensionsDisabledAfterGellerization) {
   InitServices(/*profile_is_supervised=*/false);
-  SetSupervisedUserExtensionsMayRequestPermissionsPref(true);
+  supervised_user_test_util::
+      SetSupervisedUserExtensionsMayRequestPermissionsPref(profile(), true);
 
   // Install an extension.
   base::FilePath path = data_dir().AppendASCII("good.crx");
@@ -249,7 +246,8 @@ TEST_F(SupervisedUserExtensionTest, ExtensionsDisabledAfterGellerization) {
 TEST_F(SupervisedUserExtensionTest,
        InstallAllowedButDisabledForSupervisedUser) {
   InitServices(/*profile_is_supervised=*/true);
-  SetSupervisedUserExtensionsMayRequestPermissionsPref(true);
+  supervised_user_test_util::
+      SetSupervisedUserExtensionsMayRequestPermissionsPref(profile(), true);
 
   base::FilePath path = data_dir().AppendASCII("good.crx");
   const Extension* extension = InstallCRX(path, INSTALL_WITHOUT_LOAD);
@@ -276,7 +274,8 @@ TEST_F(SupervisedUserExtensionTest,
 // approval if kSupervisedUserExtensionsMayRequestPermissions is true.
 TEST_F(SupervisedUserExtensionTest, UpdateWithPermissionsIncrease) {
   InitServices(/*profile_is_supervised=*/true);
-  SetSupervisedUserExtensionsMayRequestPermissionsPref(true);
+  supervised_user_test_util::
+      SetSupervisedUserExtensionsMayRequestPermissionsPref(profile(), true);
 
   // Preconditions.
   base::HistogramTester histogram_tester;
@@ -353,7 +352,8 @@ TEST_F(SupervisedUserExtensionTest,
        ChildUserCannotApproveAdditionalPermissions) {
   InitServices(/*profile_is_supervised=*/true);
   // Keep the toggle on initially just to install the extension.
-  SetSupervisedUserExtensionsMayRequestPermissionsPref(true);
+  supervised_user_test_util::
+      SetSupervisedUserExtensionsMayRequestPermissionsPref(profile(), true);
 
   base::HistogramTester histogram_tester;
 
@@ -381,7 +381,8 @@ TEST_F(SupervisedUserExtensionTest,
 
   // Flip toggle to off. Now the extension is blocked since it requires
   // additional permissions and the child can't approve additional permissions.
-  SetSupervisedUserExtensionsMayRequestPermissionsPref(false);
+  supervised_user_test_util::
+      SetSupervisedUserExtensionsMayRequestPermissionsPref(profile(), false);
 
   // Simulate child granting approval for the new permissions.
   service()->GrantPermissionsAndEnableExtension(extension2);
@@ -408,7 +409,8 @@ TEST_F(SupervisedUserExtensionTest,
 // doesn't require additional permissions, it is still enabled.
 TEST_F(SupervisedUserExtensionTest, UpdateWithoutPermissionIncrease) {
   InitServices(/*profile_is_supervised=*/true);
-  SetSupervisedUserExtensionsMayRequestPermissionsPref(true);
+  supervised_user_test_util::
+      SetSupervisedUserExtensionsMayRequestPermissionsPref(profile(), true);
 
   // Save the id, as the extension object will be destroyed during updating.
   std::string id = InstallNoPermissionsTestExtension();
@@ -430,7 +432,8 @@ TEST_F(SupervisedUserExtensionTest, UpdateWithoutPermissionIncrease) {
   // Even though supervised users can't approve additional approvals when the
   // "Permissions for sites, apps and extensions" toggle is off, updates with no
   // additional permissions should be okay.
-  SetSupervisedUserExtensionsMayRequestPermissionsPref(false);
+  supervised_user_test_util::
+      SetSupervisedUserExtensionsMayRequestPermissionsPref(profile(), false);
   std::string version3("3");
   UpdateNoPermissionsTestExtension(id, version3, ENABLED);
 
@@ -453,7 +456,8 @@ TEST_F(SupervisedUserExtensionTest, UpdateWithoutPermissionIncrease) {
 // duplication for the same extension id.
 TEST_F(SupervisedUserExtensionTest, DontTriggerMetricsIfAlreadyApproved) {
   InitServices(/*profile_is_supervised=*/true);
-  SetSupervisedUserExtensionsMayRequestPermissionsPref(true);
+  supervised_user_test_util::
+      SetSupervisedUserExtensionsMayRequestPermissionsPref(profile(), true);
 
   base::HistogramTester histogram_tester;
 
@@ -510,7 +514,8 @@ TEST_F(SupervisedUserExtensionTest, DontTriggerMetricsIfAlreadyApproved) {
 // cannot install new extensions.
 TEST_F(SupervisedUserExtensionTest, SupervisedUserCannotInstallExtension) {
   InitServices(/*profile_is_supervised=*/true);
-  SetSupervisedUserExtensionsMayRequestPermissionsPref(false);
+  supervised_user_test_util::
+      SetSupervisedUserExtensionsMayRequestPermissionsPref(profile(), false);
 
   base::FilePath path = data_dir().AppendASCII("good.crx");
   const Extension* extension = InstallCRX(path, INSTALL_FAILED);
@@ -522,7 +527,8 @@ TEST_F(SupervisedUserExtensionTest, SupervisedUserCannotInstallExtension) {
 // has no effect on regular users.
 TEST_F(SupervisedUserExtensionTest, RegularUserCanInstallExtension) {
   InitServices(/*profile_is_supervised=*/false);
-  SetSupervisedUserExtensionsMayRequestPermissionsPref(false);
+  supervised_user_test_util::
+      SetSupervisedUserExtensionsMayRequestPermissionsPref(profile(), false);
 
   base::FilePath path = data_dir().AppendASCII("good.crx");
   const Extension* extension = InstallCRX(path, INSTALL_NEW);
@@ -537,7 +543,8 @@ TEST_F(SupervisedUserExtensionTest, RegularUserCanInstallExtension) {
 // approved extensions are still enabled.
 TEST_F(SupervisedUserExtensionTest, ToggleOffDoesNotAffectAlreadyEnabled) {
   InitServices(/*profile_is_supervised=*/true);
-  SetSupervisedUserExtensionsMayRequestPermissionsPref(true);
+  supervised_user_test_util::
+      SetSupervisedUserExtensionsMayRequestPermissionsPref(profile(), true);
 
   // The installation helper function checks that the extension is initially
   // disabled.
@@ -551,7 +558,8 @@ TEST_F(SupervisedUserExtensionTest, ToggleOffDoesNotAffectAlreadyEnabled) {
   CheckEnabled(id);
 
   // Custodian toggles "Permissions for sites, apps and extensions" to false.
-  SetSupervisedUserExtensionsMayRequestPermissionsPref(false);
+  supervised_user_test_util::
+      SetSupervisedUserExtensionsMayRequestPermissionsPref(profile(), false);
 
   // Already installed and enabled extensions should remain that way.
   CheckEnabled(id);
@@ -561,7 +569,8 @@ TEST_F(SupervisedUserExtensionTest, ToggleOffDoesNotAffectAlreadyEnabled) {
 // extension itself is installed.
 TEST_F(SupervisedUserExtensionTest, ExtensionApprovalBeforeInstallation) {
   InitServices(/*profile_is_supervised=*/true);
-  SetSupervisedUserExtensionsMayRequestPermissionsPref(true);
+  supervised_user_test_util::
+      SetSupervisedUserExtensionsMayRequestPermissionsPref(profile(), true);
 
   supervised_user_service()->UpdateApprovedExtensionForTesting(
       good_crx, SupervisedUserService::ApprovedExtensionChange::kAdd);
@@ -579,7 +588,8 @@ TEST_F(SupervisedUserExtensionTest, ExtensionApprovalBeforeInstallation) {
 // permissions_increase are present.
 TEST_F(SupervisedUserExtensionTest, ParentApprovalNecessaryButNotSufficient) {
   InitServices(/*profile_is_supervised=*/true);
-  SetSupervisedUserExtensionsMayRequestPermissionsPref(true);
+  supervised_user_test_util::
+      SetSupervisedUserExtensionsMayRequestPermissionsPref(profile(), true);
 
   std::string id = InstallPermissionsTestExtension();
   // Update to a new version with increased permissions.

diff --git a/chrome/browser/supervised_user/supervised_user_service.cc b/chrome/browser/supervised_user/supervised_user_service.cc
@@ -319,19 +319,6 @@ bool SupervisedUserService::
       prefs::kSupervisedUserExtensionsMayRequestPermissions);
 }
 
-void SupervisedUserService::
-    SetSupervisedUserExtensionsMayRequestPermissionsPrefForTesting(
-        bool enabled) {
-  // TODO(crbug/1024646): kSupervisedUserExtensionsMayRequestPermissions is
-  // currently set indirectly by setting geolocation requests. Update Kids
-  // Management server to set a new bit for extension permissions and update
-  // this setter function.
-  settings_service_->SetLocalSetting(supervised_user::kGeolocationDisabled,
-                                     base::Value(!enabled));
-  user_prefs_->SetBoolean(prefs::kSupervisedUserExtensionsMayRequestPermissions,
-                          enabled);
-}
-
 bool SupervisedUserService::CanInstallExtensions() const {
   return HasACustodian() &&
          GetSupervisedUserExtensionsMayRequestPermissionsPref();

diff --git a/chrome/browser/supervised_user/supervised_user_service.h b/chrome/browser/supervised_user/supervised_user_service.h
@@ -228,9 +228,6 @@ class SupervisedUserService
 
   bool GetSupervisedUserExtensionsMayRequestPermissionsPref() const;
 
-  void SetSupervisedUserExtensionsMayRequestPermissionsPrefForTesting(
-      bool enabled);
-
   bool CanInstallExtensions() const;
 
   bool IsExtensionAllowed(const extensions::Extension& extension) const;

diff --git a/chrome/browser/supervised_user/supervised_user_service_unittest.cc b/chrome/browser/supervised_user/supervised_user_service_unittest.cc
@@ -22,6 +22,7 @@
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/signin/identity_test_environment_profile_adaptor.h"
 #include "chrome/browser/supervised_user/supervised_user_service_factory.h"
+#include "chrome/browser/supervised_user/supervised_user_test_util.h"
 #include "chrome/browser/sync/sync_service_factory.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/test/base/testing_browser_process.h"
@@ -323,8 +324,9 @@ TEST_F(SupervisedUserServiceExtensionTest,
        ExtensionManagementPolicyProviderWithoutSUInitiatedInstalls) {
   SupervisedUserService* supervised_user_service =
       SupervisedUserServiceFactory::GetForProfile(profile_.get());
-  supervised_user_service
-      ->SetSupervisedUserExtensionsMayRequestPermissionsPrefForTesting(false);
+  supervised_user_test_util::
+      SetSupervisedUserExtensionsMayRequestPermissionsPref(profile_.get(),
+                                                           false);
   EXPECT_FALSE(profile_->GetPrefs()->GetBoolean(
       prefs::kSupervisedUserExtensionsMayRequestPermissions));
   EXPECT_TRUE(profile_->IsChild());
@@ -376,8 +378,9 @@ TEST_F(SupervisedUserServiceExtensionTest,
       SupervisedUserServiceFactory::GetForProfile(profile_.get());
   // Enable child users to initiate extension installs by simulating the
   // toggling of "Permissions for sites, apps and extensions" to enabled.
-  supervised_user_service
-      ->SetSupervisedUserExtensionsMayRequestPermissionsPrefForTesting(true);
+  supervised_user_test_util::
+      SetSupervisedUserExtensionsMayRequestPermissionsPref(profile_.get(),
+                                                           true);
   EXPECT_TRUE(profile_->GetPrefs()->GetBoolean(
       prefs::kSupervisedUserExtensionsMayRequestPermissions));
   EXPECT_TRUE(profile_->IsChild());