Providing more information on why certain users can't be added to multi-profile

chrome/browser/chromeos/login/users/multi_profile_user_controller.cc

Index: chrome/browser/chromeos/login/users/multi_profile_user_controller.cc
diff --git a/chrome/browser/chromeos/login/users/multi_profile_user_controller.cc b/chrome/browser/chromeos/login/users/multi_profile_user_controller.cc
index 4f325edcb8eeb6dfbca139054af1994c268919b7..eacadaf5736b81edfd893f3470c7a8441918f9eb 100644
--- a/chrome/browser/chromeos/login/users/multi_profile_user_controller.cc
+++ b/chrome/browser/chromeos/login/users/multi_profile_user_controller.cc
@@ -87,30 +87,18 @@ void MultiProfileUserController::RegisterProfilePrefs(
       user_prefs::PrefRegistrySyncable::SYNCABLE_PREF);
 }
 
-bool MultiProfileUserController::IsUserAllowedInSession(
-    const std::string& user_email,
-    MultiProfileUserController::UserAllowedInSessionReason* reason) const {
-  UserManager* user_manager = UserManager::Get();
+MultiProfileUserController::UserAllowedInSessionReason

[James Cook, 2014/08/11 17:01:34]

can this function be static?  or moved into the anonymous namespace?

[Roman Sorokin (ftl), 2014/08/11 17:41:45]

made static

+MultiProfileUserController::GetPrimaryUserPolicy() const {

[James Cook, 2014/08/11 17:01:34]

Pass in |primary_user| since you've already looked it up in the calling
function?

[Roman Sorokin (ftl), 2014/08/11 17:41:45]

Actually I call this function from session_state_delegate_chromeos as well and
don't look up |primary_user| there.

+  chromeos::UserManager* user_manager = chromeos::UserManager::Get();

[James Cook, 2014/08/11 17:01:34]

"chromeos::" not needed

[Roman Sorokin (ftl), 2014/08/11 17:41:45]

Done.

   CHECK(user_manager);
 
   const user_manager::User* primary_user = user_manager->GetPrimaryUser();
-  std::string primary_user_email;
-  if (primary_user)
-    primary_user_email = primary_user->email();
-
-  // Always allow if there is no primary user or user being checked is the
-  // primary user.
-  if (primary_user_email.empty() || primary_user_email == user_email)
-    return SetUserAllowedReason(reason, ALLOWED);
-
-  // Owner is not allowed to be secondary user.
-  if (user_manager->GetOwnerEmail() == user_email)
-    return SetUserAllowedReason(reason, NOT_ALLOWED_OWNER_AS_SECONDARY);
+  if (!primary_user)
+    return MultiProfileUserController::ALLOWED;

[James Cook, 2014/08/11 17:01:34]

No need for MultiProfileUserController:: (unless you put this in anonymous
namespace)

[Roman Sorokin (ftl), 2014/08/11 17:41:45]

Done.

+  Profile* primary_user_profile =
+      chromeos::ProfileHelper::Get()->GetProfileByUser(primary_user);
 
-  // Don't allow profiles potentially tainted by data fetched with policy-pushed
-  // certificates to join a multiprofile session.
-  if (policy::PolicyCertServiceFactory::UsedPolicyCertificates(user_email))
-    return SetUserAllowedReason(reason, NOT_ALLOWED_POLICY_CERT_TAINTED);
+  std::string primary_user_email = primary_user->email();
 
   // Don't allow any secondary profiles if the primary profile is tainted.
   if (policy::PolicyCertServiceFactory::UsedPolicyCertificates(
@@ -118,31 +106,57 @@ bool MultiProfileUserController::IsUserAllowedInSession(
     // Check directly in local_state before checking if the primary user has
     // a PolicyCertService. His profile may have been tainted previously though
     // he didn't get a PolicyCertService created for this session.
-    return SetUserAllowedReason(reason,
-                                NOT_ALLOWED_PRIMARY_POLICY_CERT_TAINTED);
+    return NOT_ALLOWED_PRIMARY_POLICY_CERT_TAINTED;
   }
 
   // If the primary profile already has policy certificates installed but hasn't
   // used them yet then it can become tainted at any time during this session;
   // disable secondary profiles in this case too.
-  Profile* primary_user_profile =
-      primary_user ? ProfileHelper::Get()->GetProfileByUser(primary_user)
-                   : NULL;
   policy::PolicyCertService* service =
       primary_user_profile ? policy::PolicyCertServiceFactory::GetForProfile(
                                  primary_user_profile)
                            : NULL;
   if (service && service->has_policy_certificates())
-    return SetUserAllowedReason(reason,
-                                NOT_ALLOWED_PRIMARY_POLICY_CERT_TAINTED);
+    return NOT_ALLOWED_PRIMARY_POLICY_CERT_TAINTED;
 
   // No user is allowed if the primary user policy forbids it.
   const std::string primary_user_behavior =
       primary_user_profile->GetPrefs()->GetString(
           prefs::kMultiProfileUserBehavior);
   if (primary_user_behavior == kBehaviorNotAllowed)
-    return SetUserAllowedReason(reason,
-                                NOT_ALLOWED_PRIMARY_USER_POLICY_FORBIDS);
+    return NOT_ALLOWED_PRIMARY_USER_POLICY_FORBIDS;
+
+  return MultiProfileUserController::ALLOWED;

[James Cook, 2014/08/11 17:01:34]

ditto

[Roman Sorokin (ftl), 2014/08/11 17:41:45]

Done.

+}
+
+bool MultiProfileUserController::IsUserAllowedInSession(
+    const std::string& user_email,
+    MultiProfileUserController::UserAllowedInSessionReason* reason) const {
+  UserManager* user_manager = UserManager::Get();
+  CHECK(user_manager);
+
+  const user_manager::User* primary_user = user_manager->GetPrimaryUser();
+  std::string primary_user_email;
+  if (primary_user)
+    primary_user_email = primary_user->email();
+
+  // Always allow if there is no primary user or user being checked is the
+  // primary user.
+  if (primary_user_email.empty() || primary_user_email == user_email)
+    return SetUserAllowedReason(reason, ALLOWED);
+
+  // Owner is not allowed to be secondary user.
+  if (user_manager->GetOwnerEmail() == user_email)
+    return SetUserAllowedReason(reason, NOT_ALLOWED_OWNER_AS_SECONDARY);
+
+  // Don't allow profiles potentially tainted by data fetched with policy-pushed
+  // certificates to join a multiprofile session.
+  if (policy::PolicyCertServiceFactory::UsedPolicyCertificates(user_email))
+    return SetUserAllowedReason(reason, NOT_ALLOWED_POLICY_CERT_TAINTED);
+
+  UserAllowedInSessionReason primary_user_policy = GetPrimaryUserPolicy();
+  if (primary_user_policy != ALLOWED)
+    return SetUserAllowedReason(reason, primary_user_policy);
 
   // The user must have 'unrestricted' policy to be a secondary user.
   const std::string behavior = GetCachedValue(user_email);