Preference dis/allowing supervised users creation is now available as owner setting, not only as de…

chrome/browser/chromeos/settings/device_settings_provider.cc

Index: chrome/browser/chromeos/settings/device_settings_provider.cc
diff --git a/chrome/browser/chromeos/settings/device_settings_provider.cc b/chrome/browser/chromeos/settings/device_settings_provider.cc
index b122ae04e7020d06da11f4bcd5839d291c30b3c3..806fa1f22301c09a908e32c87e6a0eba9ab1387f 100644
--- a/chrome/browser/chromeos/settings/device_settings_provider.cc
+++ b/chrome/browser/chromeos/settings/device_settings_provider.cc
@@ -216,6 +216,14 @@ void DeviceSettingsProvider::SetInPolicy() {
       guest->set_guest_mode_enabled(guest_value);
     else
       NOTREACHED();
+  } else if (prop == kAccountsPrefSupervisedUsersEnabled) {
+    em::SupervisedUsersSettingsProto* supervised =
+        device_settings_.mutable_supervised_users_settings();
+    bool supervised_value;
+    if (value->GetAsBoolean(&supervised_value))
+      supervised->set_supervised_users_enabled(supervised_value);
+    else
+      NOTREACHED();
   } else if (prop == kAccountsPrefShowUserNamesOnSignIn) {
     em::ShowUserNamesOnSigninProto* show =
         device_settings_.mutable_show_user_names();
@@ -387,7 +395,6 @@ void DeviceSettingsProvider::SetInPolicy() {
   } else {
     // The remaining settings don't support Set(), since they are not
     // intended to be customizable by the user:
-    //   kAccountsPrefSupervisedUsersEnabled
     //   kAppPack
     //   kDeviceAttestationEnabled
     //   kDeviceOwner
@@ -437,6 +444,8 @@ void DeviceSettingsProvider::DecodeLoginPolicies(
   // true is default permissive value and false is safe prohibitive value.
   // Exceptions:
   //   kAccountsPrefEphemeralUsersEnabled has a default value of false.
+  //   kAccountsPrefSupervisedUsersEnabled has a default value of false
+  //     for enterprise devices and true for consumer devices.
   if (policy.has_allow_new_users() &&
       policy.allow_new_users().has_allow_new_users()) {
     if (policy.allow_new_users().allow_new_users()) {
@@ -460,6 +469,28 @@ void DeviceSettingsProvider::DecodeLoginPolicies(
       !policy.guest_mode_enabled().has_guest_mode_enabled() ||
       policy.guest_mode_enabled().guest_mode_enabled());
 
+  if (g_browser_process->is_browser_policy_connector_created()) {

[jam, 2014/05/13 21:09:11]

why this check? it's not deterministic when you're in this method whether this
object is created or not?

if it's always created, then this check isn't needed.
if it's not created, can its constructor be made cheap, and any expensive work
can be done lazily?

[merkulova, 2014/05/14 06:30:10]

It's a check that covers unit tests where browser is not created. Implicit
creation at this step led to tests failing due to wrong Shutdown order.

Had a discussion with Mattias on it. Comments #55 and #59 contain the info that
might help understand the case.

[jam, 2014/05/14 17:29:46]

Can you find another way of doing this without adding yet another method to
BrowserProcess? why can't this unit test check be localized to policy code
without adding another getter to the global BrowserProcess? 

[merkulova, 2014/05/23 09:27:15]

Done.

[jam, 2014/05/28 17:38:40]

thanks, removing myself as reviewer

+    policy::BrowserPolicyConnectorChromeOS* connector =
+        g_browser_process->platform_part()->browser_policy_connector_chromeos();
+    bool supervised_users_enabled = false;
+    if (connector->IsEnterpriseManaged()) {
+      supervised_users_enabled =
+          policy.has_supervised_users_settings() &&
+          policy.supervised_users_settings().has_supervised_users_enabled() &&
+          policy.supervised_users_settings().supervised_users_enabled();
+    } else {
+      supervised_users_enabled =
+          !policy.has_supervised_users_settings() ||
+          !policy.supervised_users_settings().has_supervised_users_enabled() ||
+          policy.supervised_users_settings().supervised_users_enabled();
+    }
+    new_values_cache->SetBoolean(
+        kAccountsPrefSupervisedUsersEnabled, supervised_users_enabled);
+    } else {
+      // For unit tests.
+      new_values_cache->SetBoolean(kAccountsPrefSupervisedUsersEnabled, true);
+  }
+
   new_values_cache->SetBoolean(
       kAccountsPrefShowUserNamesOnSignIn,
       !policy.has_show_user_names() ||
@@ -472,11 +503,6 @@ void DeviceSettingsProvider::DecodeLoginPolicies(
       policy.ephemeral_users_enabled().has_ephemeral_users_enabled() &&
       policy.ephemeral_users_enabled().ephemeral_users_enabled());
 
-  new_values_cache->SetBoolean(
-      kAccountsPrefSupervisedUsersEnabled,
-      policy.has_supervised_users_settings() &&
-      policy.supervised_users_settings().supervised_users_enabled());
-
   base::ListValue* list = new base::ListValue();
   const em::UserWhitelistProto& whitelist_proto = policy.user_whitelist();
   const RepeatedPtrField<std::string>& whitelist =