Clarify settings UI in multi-profiles mode

chrome/browser/ui/webui/options/chromeos/core_chromeos_options_handler.cc

Index: chrome/browser/ui/webui/options/chromeos/core_chromeos_options_handler.cc
diff --git a/chrome/browser/ui/webui/options/chromeos/core_chromeos_options_handler.cc b/chrome/browser/ui/webui/options/chromeos/core_chromeos_options_handler.cc
index 0dc8c6110dae4579fa14b1cd66cacbc7b63baf8f..d314e6ae4a0e743b1d1991ff68e5622bf6165968 100644
--- a/chrome/browser/ui/webui/options/chromeos/core_chromeos_options_handler.cc
+++ b/chrome/browser/ui/webui/options/chromeos/core_chromeos_options_handler.cc
@@ -6,11 +6,14 @@
 
 #include <string>
 
+#include "ash/session_state_delegate.h"
+#include "ash/shell.h"
 #include "base/bind.h"
 #include "base/prefs/pref_change_registrar.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/sys_info.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chromeos/login/user_manager.h"
 #include "chrome/browser/chromeos/policy/browser_policy_connector_chromeos.h"
@@ -44,9 +47,7 @@ bool IsSettingOwnerOnly(const std::string& pref) {
 
 // Returns true if |username| is the logged-in owner.
 bool IsLoggedInOwner(const std::string& username) {
-  UserManager* user_manager = UserManager::Get();
-  return user_manager->IsCurrentUserOwner() &&
-      user_manager->GetLoggedInUser()->email() == username;
+  return UserManager::Get()->GetOwnerEmail() == username;

[Dmitry Polukhin, 2014/02/12 18:58:45]

Why you don't check IsCurrentUserOwner() anymore? IsCurrentUserOwner indicates
that owner keys present and singed settings can be changed.

[michaelpg, 2014/02/12 20:29:58]

The existing check doesn't work in multiprofile world:

IsCurrentUserOwner is true if any signed-in user is an owner
GetLoggedInUser (deprecated) checks if the username is the active user

So the existing function returns true if the username is the active user but not
the owner, and it returns false if the username is the owner but not the active
user.

This function is used only to add " - Owner" to the owner in the list of users.
Checking for the key shouldn't be necessary.

[Dmitry Polukhin, 2014/02/12 21:06:31]

of users.
I completely agree that this function need to be changed in multi-profile world.
My point is that we can't relay on email comparison if owner key is not present.
Owner email is stored in local state and potentially could be malicious modified
and we can 100% detect it only when owner key present. But perhaps for this UI
element it is not so important. I'm more worry if it function will be used
somewhere else for owner checking. So I would remove this function and make this
check in directly in CreateUserInfo.

 }
 
 // Creates a user info dictionary to be stored in the |ListValue| that is
@@ -133,6 +134,7 @@ base::Value* CoreChromeOSOptionsHandler::FetchPref(
 
     return value;
   }
+
   if (!CrosSettings::IsCrosSettings(pref_name)) {
     std::string controlling_pref =
         pref_name == prefs::kUseSharedProxies ? prefs::kProxy : std::string();
@@ -152,13 +154,16 @@ base::Value* CoreChromeOSOptionsHandler::FetchPref(
     dict->Set("value", pref_value->DeepCopy());
   policy::BrowserPolicyConnectorChromeOS* connector =
       g_browser_process->platform_part()->browser_policy_connector_chromeos();
-  if (connector->IsEnterpriseManaged())
+  if (connector->IsEnterpriseManaged()) {
+    dict->SetBoolean("disabled", true);
     dict->SetString("controlledBy", "policy");
-  bool disabled_by_owner = IsSettingOwnerOnly(pref_name) &&
-      !ProfileHelper::IsOwnerProfile(Profile::FromWebUI(web_ui()));
-  dict->SetBoolean("disabled", disabled_by_owner);
-  if (disabled_by_owner)
-    dict->SetString("controlledBy", "owner");
+  } else {
+    bool controlled_by_owner = IsSettingOwnerOnly(pref_name) &&
+        !ProfileHelper::IsOwnerProfile(Profile::FromWebUI(web_ui()));

[Dmitry Polukhin, 2014/02/12 18:58:45]

I think we also need check for owner key presence.

[michaelpg, 2014/02/12 20:29:58]

That will only check that the owner is logged in, not that the current user is
the owner. !IsOwnerProfile should be sufficient to disable settings
(IsOwnerProfile is false if we don't have an owner key). Are you saying owner
key presence is important from the UI's perspective in case IsOwnerProfile gets
broken somehow? 

[Dmitry Polukhin, 2014/02/12 21:06:31]

IsOwnerProfile only check that email looks like owner email, it doesn't check
key presence. I would allow settings change only if owner key present in the
session. In some sense it is not only UI because false own will be able to
change on login screen for a while until real owner logs in. But I think the
change should be done in ProfileHelper::IsOwnerProfile not here. Will talk to
Nikita about it.

+    dict->SetBoolean("disabled", controlled_by_owner);
+    if (controlled_by_owner)
+      dict->SetString("controlledBy", "owner");
+  }
   return dict;
 }
 
@@ -209,15 +214,91 @@ void CoreChromeOSOptionsHandler::StopObservingPref(const std::string& path) {
     ::options::CoreOptionsHandler::StopObservingPref(path);
 }
 
+base::Value* CoreChromeOSOptionsHandler::CreateValueForPref(
+    const std::string& pref_name,
+    const std::string& controlling_pref_name) {
+  // The screen lock setting is shared if multiple users are logged in and at
+  // least one has chosen to require passwords.
+  if (pref_name == prefs::kEnableAutoScreenLock &&
+      UserManager::Get()->GetLoggedInUsers().size() > 1 &&
+      controlling_pref_name.empty()) {
+    PrefService* user_prefs = Profile::FromWebUI(web_ui())->GetPrefs();
+    const PrefService::Preference* pref =
+        user_prefs->FindPreference(prefs::kEnableAutoScreenLock);
+
+    ash::SessionStateDelegate* delegate =
+        ash::Shell::GetInstance()->session_state_delegate();
+    if (pref && pref->IsUserModifiable() &&
+        delegate->ShouldLockScreenBeforeSuspending()) {
+      bool screen_lock = false;
+      pref->GetValue()->GetAsBoolean(&screen_lock);
+      if (!screen_lock) {
+        // Screen lock is enabled for the session, but not in the user's
+        // preferences. Show the user's value in the checkbox, but indicate
+        // that the password requirement is enabled by some other user.
+        base::DictionaryValue* dict = new base::DictionaryValue;
+        dict->Set("value", pref->GetValue()->DeepCopy());
+        dict->SetString("controlledBy", "shared");
+        return dict;
+      }
+    }
+  }
+
+  return CoreOptionsHandler::CreateValueForPref(pref_name,
+                                                controlling_pref_name);
+}
+
 void CoreChromeOSOptionsHandler::GetLocalizedValues(
     base::DictionaryValue* localized_strings) {
   DCHECK(localized_strings);
   CoreOptionsHandler::GetLocalizedValues(localized_strings);
 
-  AddAccountUITweaksLocalizedValues(localized_strings,
-                                    Profile::FromWebUI(web_ui()));
-  localized_strings->SetString("controlledSettingOwner",
-      l10n_util::GetStringUTF16(IDS_OPTIONS_CONTROLLED_SETTING_OWNER));
+  Profile* profile = Profile::FromWebUI(web_ui());
+  AddAccountUITweaksLocalizedValues(localized_strings, profile);
+
+  UserManager* user_manager = UserManager::Get();
+  const std::string& primary_email = user_manager->GetPrimaryUser()->email();
+
+  // Check at load time whether this is a secondary user in a multi-profile
+  // session.
+  if (user_manager->GetUserByProfile(profile)->email() != primary_email) {
+    // Set secondaryUser to show the shared icon by the network section header.
+    localized_strings->SetBoolean("secondaryUser", true);
+    localized_strings->SetString("secondaryUserBannerText",
+        l10n_util::GetStringFUTF16(
+            IDS_OPTIONS_SETTINGS_SECONDARY_USER_BANNER,
+            base::ASCIIToUTF16(primary_email)));
+    localized_strings->SetString("controlledSettingShared",
+        l10n_util::GetStringFUTF16(
+            IDS_OPTIONS_CONTROLLED_SETTING_SHARED,
+            base::ASCIIToUTF16(primary_email)));
+    localized_strings->SetString("controlledSettingsShared",
+        l10n_util::GetStringFUTF16(
+            IDS_OPTIONS_CONTROLLED_SETTINGS_SHARED,
+            base::ASCIIToUTF16(primary_email)));
+  } else {
+    localized_strings->SetBoolean("secondaryUser", false);
+    localized_strings->SetString("secondaryUserBannerText", base::string16());
+    localized_strings->SetString("controlledSettingShared", base::string16());
+    localized_strings->SetString("controlledSettingsShared", base::string16());
+  }
+
+  // Screen lock icon can show up as primary or secondary user.
+  localized_strings->SetString("screenLockShared",
+      l10n_util::GetStringUTF16(
+          IDS_OPTIONS_CONTROLLED_SETTING_SHARED_SCREEN_LOCK));
+
+  policy::BrowserPolicyConnectorChromeOS* connector =
+      g_browser_process->platform_part()->browser_policy_connector_chromeos();
+  if (connector->IsEnterpriseManaged()) {
+    // Managed machines have no "owner".
+    localized_strings->SetString("controlledSettingOwner", base::string16());
+  } else {
+    localized_strings->SetString("controlledSettingOwner",
+        l10n_util::GetStringFUTF16(
+            IDS_OPTIONS_CONTROLLED_SETTING_OWNER,
+            base::ASCIIToUTF16(user_manager->GetOwnerEmail())));
+  }
 }
 
 void CoreChromeOSOptionsHandler::SelectNetworkCallback(