Migrate configuration of SupervisedUserSettingsService.
Moves the configuration of the supervised state to the settings
initialization in SupervisedUserService to modularize the actions
taken by each child service.
Bug: 1324946
Change-Id: I621e4b53b0d95b1d78774b87fe8f9612572adc58
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3700171
Commit-Queue: Nohemi Fernandez <fernandex@chromium.org>
Reviewed-by: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1016136}
diff --git a/chrome/browser/content_settings/content_settings_supervised_provider_unittest.cc b/chrome/browser/content_settings/content_settings_supervised_provider_unittest.cc
index eb46022b..121dfa5 100644
--- a/chrome/browser/content_settings/content_settings_supervised_provider_unittest.cc
+++ b/chrome/browser/content_settings/content_settings_supervised_provider_unittest.cc
@@ -84,16 +84,7 @@
TEST_F(SupervisedUserProviderTest, CookiesTest) {
std::unique_ptr<RuleIterator> rule_iterator =
provider_->GetRuleIterator(ContentSettingsType::COOKIES, false);
- EXPECT_FALSE(rule_iterator);
- // Allow cookies everywhere.
- EXPECT_CALL(mock_observer_,
- OnContentSettingChanged(_, _, ContentSettingsType::COOKIES));
- service_.SetLocalSetting(supervised_users::kCookiesAlwaysAllowed,
- std::make_unique<base::Value>(true));
-
- rule_iterator =
- provider_->GetRuleIterator(ContentSettingsType::COOKIES, false);
ASSERT_TRUE(rule_iterator->HasNext());
Rule rule = rule_iterator->Next();
EXPECT_FALSE(rule_iterator->HasNext());
diff --git a/chrome/browser/supervised_user/child_accounts/child_account_service.cc b/chrome/browser/supervised_user/child_accounts/child_account_service.cc
index aa75902..4f02764 100644
--- a/chrome/browser/supervised_user/child_accounts/child_account_service.cc
+++ b/chrome/browser/supervised_user/child_accounts/child_account_service.cc
@@ -23,7 +23,6 @@
#include "chrome/browser/supervised_user/supervised_user_settings_service.h"
#include "chrome/browser/supervised_user/supervised_user_settings_service_factory.h"
#include "chrome/browser/supervised_user/web_approvals_manager.h"
-#include "chrome/browser/sync/sync_service_factory.h"
#include "chrome/common/pref_names.h"
#include "components/pref_registry/pref_registry_syncable.h"
#include "components/prefs/pref_service.h"
@@ -31,8 +30,6 @@
#include "components/signin/public/identity_manager/account_info.h"
#include "components/signin/public/identity_manager/accounts_in_cookie_jar_info.h"
#include "components/signin/public/identity_manager/tribool.h"
-#include "components/sync/driver/sync_service.h"
-#include "components/sync/driver/sync_user_settings.h"
#include "content/public/browser/browser_context.h"
#include "content/public/browser/storage_partition.h"
@@ -161,28 +158,6 @@
active_ = active;
if (active_) {
- SupervisedUserSettingsService* settings_service =
- SupervisedUserSettingsServiceFactory::GetForKey(
- profile_->GetProfileKey());
-
- // In contrast to deprecated legacy SUs, child account SUs must sign in.
- settings_service->SetLocalSetting(supervised_users::kSigninAllowed,
- std::make_unique<base::Value>(true));
-
- // Always allow cookies, to avoid website compatibility issues.
- settings_service->SetLocalSetting(supervised_users::kCookiesAlwaysAllowed,
- std::make_unique<base::Value>(true));
-
- // SafeSearch is controlled at the account level, so don't override it
- // client-side.
- settings_service->SetLocalSetting(supervised_users::kForceSafeSearch,
- std::make_unique<base::Value>(false));
-
- // GeolocationDisabled is controlled at the account level, so don't override
- // it client-side.
- settings_service->SetLocalSetting(supervised_users::kGeolocationDisabled,
- std::make_unique<base::Value>(false));
-
#if !BUILDFLAG(IS_CHROMEOS)
// This is also used by user policies (UserPolicySigninService), but since
// child accounts can not also be Dasher accounts, there shouldn't be any
@@ -197,18 +172,6 @@
service->web_approvals_manager().AddRemoteApprovalRequestCreator(
PermissionRequestCreatorApiary::CreateWithProfile(profile_));
} else {
- SupervisedUserSettingsService* settings_service =
- SupervisedUserSettingsServiceFactory::GetForKey(
- profile_->GetProfileKey());
- settings_service->SetLocalSetting(supervised_users::kSigninAllowed,
- nullptr);
- settings_service->SetLocalSetting(supervised_users::kCookiesAlwaysAllowed,
- nullptr);
- settings_service->SetLocalSetting(supervised_users::kForceSafeSearch,
- nullptr);
- settings_service->SetLocalSetting(supervised_users::kGeolocationDisabled,
- nullptr);
-
#if !BUILDFLAG(IS_CHROMEOS)
signin_util::SetUserSignoutAllowedForProfile(profile_, true);
#endif
@@ -216,18 +179,6 @@
CancelFetchingFamilyInfo();
}
- // Trigger a sync reconfig to enable/disable the right SU data types.
- // The logic to do this lives in the SupervisedUserSyncModelTypeController.
- // TODO(crbug.com/946473): Get rid of this hack and instead call
- // DataTypePreconditionChanged from the controller.
- syncer::SyncService* sync_service =
- SyncServiceFactory::GetForProfile(profile_);
- if (sync_service->GetUserSettings()->IsFirstSetupComplete()) {
- // Trigger a reconfig by grabbing a SyncSetupInProgressHandle and
- // immediately releasing it again (via the temporary unique_ptr going away).
- sync_service->GetSetupInProgressHandle();
- }
-
return true;
}
diff --git a/chrome/browser/supervised_user/parental_control_metrics_unittest.cc b/chrome/browser/supervised_user/parental_control_metrics_unittest.cc
index 7f75683..b2200b5 100644
--- a/chrome/browser/supervised_user/parental_control_metrics_unittest.cc
+++ b/chrome/browser/supervised_user/parental_control_metrics_unittest.cc
@@ -19,6 +19,7 @@
#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_url_filter.h"
+#include "chrome/browser/sync/sync_service_factory.h"
#include "chrome/common/chrome_features.h"
#include "chrome/common/pref_names.h"
#include "chrome/test/base/testing_profile.h"
@@ -52,6 +53,9 @@
TestingProfile::Builder profile_builder;
profile_builder.SetPrefService(std::move(prefs));
profile_builder.SetIsSupervisedProfile();
+ profile_builder.AddTestingFactory(SyncServiceFactory::GetInstance(),
+ SyncServiceFactory::GetDefaultFactory());
+
profile_ = profile_builder.Build();
EXPECT_TRUE(profile_->IsChild());
supervised_user_service_ =
diff --git a/chrome/browser/supervised_user/supervised_user_service.cc b/chrome/browser/supervised_user/supervised_user_service.cc
index 7095942..29d46b3 100644
--- a/chrome/browser/supervised_user/supervised_user_service.cc
+++ b/chrome/browser/supervised_user/supervised_user_service.cc
@@ -37,12 +37,15 @@
#include "chrome/browser/supervised_user/supervised_user_service_observer.h"
#include "chrome/browser/supervised_user/supervised_user_settings_service.h"
#include "chrome/browser/supervised_user/supervised_user_settings_service_factory.h"
+#include "chrome/browser/sync/sync_service_factory.h"
#include "chrome/common/chrome_paths.h"
#include "chrome/common/pref_names.h"
#include "chrome/common/webui_url_constants.h"
#include "chrome/grit/generated_resources.h"
#include "components/pref_registry/pref_registry_syncable.h"
#include "components/prefs/pref_service.h"
+#include "components/sync/driver/sync_service.h"
+#include "components/sync/driver/sync_user_settings.h"
#include "content/public/browser/browser_context.h"
#include "content/public/browser/storage_partition.h"
#include "extensions/buildflags/buildflags.h"
@@ -379,6 +382,18 @@
theme_service->UseDefaultTheme();
#endif
+ // Trigger a sync reconfig to enable/disable the right SU data types.
+ // The logic to do this lives in the SupervisedUserSyncModelTypeController.
+ // TODO(crbug.com/946473): Get rid of this hack and instead call
+ // DataTypePreconditionChanged from the controller.
+ syncer::SyncService* sync_service =
+ SyncServiceFactory::GetForProfile(profile_);
+ if (sync_service->GetUserSettings()->IsFirstSetupComplete()) {
+ // Trigger a reconfig by grabbing a SyncSetupInProgressHandle and
+ // immediately releasing it again (via the temporary unique_ptr going away).
+ sync_service->GetSetupInProgressHandle();
+ }
+
GetSettingsService()->SetActive(active_);
#if BUILDFLAG(ENABLE_EXTENSIONS)
diff --git a/chrome/browser/supervised_user/supervised_user_service_browsertest.cc b/chrome/browser/supervised_user/supervised_user_service_browsertest.cc
index 86431b42..7ace6fa 100644
--- a/chrome/browser/supervised_user/supervised_user_service_browsertest.cc
+++ b/chrome/browser/supervised_user/supervised_user_service_browsertest.cc
@@ -82,9 +82,9 @@
logged_in_user_mixin_.LogInUser();
Profile* profile = browser()->profile();
PrefService* prefs = profile->GetPrefs();
- EXPECT_FALSE(prefs->GetBoolean(prefs::kForceGoogleSafeSearch));
+ EXPECT_TRUE(prefs->GetBoolean(prefs::kForceGoogleSafeSearch));
EXPECT_EQ(prefs->GetInteger(prefs::kForceYouTubeRestrict),
- safe_search_util::YOUTUBE_RESTRICT_OFF);
+ safe_search_util::YOUTUBE_RESTRICT_MODERATE);
EXPECT_FALSE(
prefs->IsUserModifiablePreference(prefs::kForceGoogleSafeSearch));
EXPECT_FALSE(prefs->IsUserModifiablePreference(prefs::kForceYouTubeRestrict));
diff --git a/chrome/browser/supervised_user/supervised_user_service_unittest.cc b/chrome/browser/supervised_user/supervised_user_service_unittest.cc
index 9978fc20..7896631 100644
--- 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/sync/sync_service_factory.h"
#include "chrome/common/pref_names.h"
#include "chrome/test/base/testing_browser_process.h"
#include "chrome/test/base/testing_profile.h"
@@ -130,6 +131,8 @@
// Build supervised profile.
TestingProfile::Builder builder;
+ builder.AddTestingFactory(SyncServiceFactory::GetInstance(),
+ SyncServiceFactory::GetDefaultFactory());
builder.SetIsSupervisedProfile();
profile_ = builder.Build();
diff --git a/chrome/browser/supervised_user/supervised_user_settings_service.cc b/chrome/browser/supervised_user/supervised_user_settings_service.cc
index bae1163..dee41b4a 100644
--- a/chrome/browser/supervised_user/supervised_user_settings_service.cc
+++ b/chrome/browser/supervised_user/supervised_user_settings_service.cc
@@ -156,6 +156,25 @@
void SupervisedUserSettingsService::SetActive(bool active) {
active_ = active;
+
+ if (active_) {
+ // Child account supervised users must be signed in.
+ SetLocalSetting(supervised_users::kSigninAllowed,
+ std::make_unique<base::Value>(true));
+
+ // Always allow cookies, to avoid website compatibility issues.
+ SetLocalSetting(supervised_users::kCookiesAlwaysAllowed,
+ std::make_unique<base::Value>(true));
+
+ // SafeSearch and GeolocationDisabled are controlled at the account level,
+ // so don't override them client-side.
+ } else {
+ SetLocalSetting(supervised_users::kSigninAllowed, nullptr);
+ SetLocalSetting(supervised_users::kCookiesAlwaysAllowed, nullptr);
+ SetLocalSetting(supervised_users::kForceSafeSearch, nullptr);
+ SetLocalSetting(supervised_users::kGeolocationDisabled, nullptr);
+ }
+
InformSubscribers();
}
