Fixed suggesting adding more users into multi-profile session when we don't have more

ash/system/user/user_view.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "ash/system/user/user_view.h"
 
 #include <algorithm>
 
 #include "ash/multi_profile_uma.h"
 #include "ash/popup_message.h"
@@ skipping 481 matching lines @@
   add_menu_option_->Show();
 
   AddUserView* add_user_view =
       new AddUserView(static_cast<ButtonFromView*>(user_card_view_));
 
   const SessionStateDelegate* delegate =
       Shell::GetInstance()->session_state_delegate();
 
   bool multi_profile_allowed =
       delegate->IsMultiProfileAllowedByPrimaryUserPolicy();
-  add_user_disabled_ = (delegate->NumberOfLoggedInUsers() >=
-                        delegate->GetMaximumNumberOfLoggedInUsers()) ||
-                       !multi_profile_allowed;
+  int admitted_users_to_be_added =
+      delegate->GetNumberOfUsersAdmittedForMultiProfile();
+  DCHECK(admitted_users_to_be_added >= 0);
+  add_user_disabled_ = !multi_profile_allowed ||

[James Cook, 2014/09/22 19:52:23]

Please change this to add_user_enabled_ and invert the initialization and all
the other conditionals. I know you didn't add this member, but I strongly prefer
positive-sense booleans.

[Roman Sorokin (ftl), 2014/09/23 15:42:37]

Done.

+                       admitted_users_to_be_added == 0 ||
+                       (delegate->NumberOfLoggedInUsers() >=
+                        delegate->GetMaximumNumberOfLoggedInUsers());

[James Cook, 2014/09/22 19:52:23]

Given that all the data to decide whether or not a user should be added lives in
the delegate, can you replace GetNumberOfUsersAdmittedForMultiProfile() with
something like bool CanAddUserToMultiProfile(int* error_message_id)?

The decision whether adding is allowed or not is more of a model/controller
decision than a UI view decision.

Moving the logic will also make it easier for you to add a test, as there are
existing tests of SessionStateDelegate but none for this class.

[Roman Sorokin (ftl), 2014/09/23 15:42:37]

Done.

 
   ButtonFromView* button = new ButtonFromView(
       add_user_view,
       add_user_disabled_ ? NULL : this,
       !add_user_disabled_,
       gfx::Insets(1, 1, 1, 1));
   button->set_request_focus_on_press(false);
   button->SetAccessibleName(
       l10n_util::GetStringUTF16(IDS_ASH_STATUS_TRAY_SIGN_IN_ANOTHER_ACCOUNT));
   button->ForceBorderVisible(true);
   add_menu_option_->SetContentsView(button);
 
   if (add_user_disabled_) {
     ui::ResourceBundle& bundle = ui::ResourceBundle::GetSharedInstance();
     int message_id;
     if (!multi_profile_allowed)
       message_id = IDS_ASH_STATUS_TRAY_MESSAGE_NOT_ALLOWED_PRIMARY_USER;
+    else if (admitted_users_to_be_added == 0)
+      message_id = IDS_ASH_STATUS_TRAY_MESSAGE_OUT_OF_USERS;
     else
       message_id = IDS_ASH_STATUS_TRAY_MESSAGE_CANNOT_ADD_USER;
 
     popup_message_.reset(new PopupMessage(
         bundle.GetLocalizedString(IDS_ASH_STATUS_TRAY_CAPTION_CANNOT_ADD_USER),
         bundle.GetLocalizedString(message_id),
         PopupMessage::ICON_WARNING,
         add_user_view->anchor(),
         views::BubbleBorder::TOP_LEFT,
         gfx::Size(parent()->bounds().width() - kPopupMessageOffset, 0),
@@ skipping 26 matching lines @@
   focus_manager_ = NULL;
   if (user_card_view_->GetFocusManager())
     user_card_view_->GetFocusManager()->ClearFocus();
   popup_message_.reset();
   mouse_watcher_.reset();
   add_menu_option_.reset();
 }
 
 }  // namespace tray
 }  // namespace ash