Move AppListModel::Users to AppListViewDelegate

This is some minor re-factoring to simplify AppListModel in preparation
for making it associated with a Profile for syncing. While not strictly
necessary, it is confusing to have a list of "Users" associated with a
model that describes the list of apps for a specific user. Also, the
list of Users is only used by AppListMenu and can be maintained by
AppListViewDelegate.

One side effect of this change is that views will not be notified if the
user list changes and therefore can not invalidate the menu. While this
could be addressed, it shoudln't generally be possible to add a user
while the app list UI is visible, so the added complexity did not seem
worth supporting a theoretical edge case.

BUG=315887
For ash/shell/app_list.cc:
TBR=jamescook@chromium.org

Review URL: https://codereview.chromium.org/63283003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@233996 0039d316-1c4b-4281-b951-d872f2087c98

ash/shell/app_list.cc

@@ -236,6 +236,10 @@ class ExampleAppListViewDelegate : public app_list::AppListViewDelegate {
     // Nothing needs to be done.
   }
 
+  virtual const Users& GetUsers() const OVERRIDE {
+    return users_;
+  }
+
   virtual void InitModel(app_list::AppListModel* model) OVERRIDE {
     model_ = model;
     PopulateApps(model_->item_list());
@@ -328,6 +332,7 @@ class ExampleAppListViewDelegate : public app_list::AppListViewDelegate {
   }
 
   app_list::AppListModel* model_;
+  Users users_;
 
   DISALLOW_COPY_AND_ASSIGN(ExampleAppListViewDelegate);
 };

chrome/browser/ui/app_list/app_list_view_delegate.cc

@@ -59,14 +59,14 @@ void CreateShortcutInWebAppDir(
 
 void PopulateUsers(const ProfileInfoCache& profile_info,
                    const base::FilePath& active_profile_path,
-                   app_list::AppListModel::Users* users) {
+                   app_list::AppListViewDelegate::Users* users) {
   const size_t count = profile_info.GetNumberOfProfiles();
   for (size_t i = 0; i < count; ++i) {
     // Don't display managed users.
     if (profile_info.ProfileIsManagedAtIndex(i))
       continue;
 
-    app_list::AppListModel::User user;
+    app_list::AppListViewDelegate::User user;
     user.name = profile_info.GetNameOfProfileAtIndex(i);
     user.email = profile_info.GetUserNameOfProfileAtIndex(i);
     user.profile_path = profile_info.GetPathOfProfileAtIndex(i);
@@ -125,10 +125,8 @@ void AppListViewDelegate::OnProfileChanged() {
     return;
 
   // Populate the app list users.
-  app_list::AppListModel::Users users;
   PopulateUsers(g_browser_process->profile_manager()->GetProfileInfoCache(),
-                profile_->GetPath(), &users);
-  model_->SetUsers(users);
+                profile_->GetPath(), &users_);
 }
 
 bool AppListViewDelegate::ForceNativeDesktop() const {
@@ -300,3 +298,8 @@ content::WebContents* AppListViewDelegate::GetStartPageContents() {
 
   return service->contents();
 }
+
+const app_list::AppListViewDelegate::Users&
+AppListViewDelegate::GetUsers() const {
+  return users_;
+}

chrome/browser/ui/app_list/app_list_view_delegate.h

@@ -81,6 +81,7 @@ class AppListViewDelegate : public app_list::AppListViewDelegate,
   virtual void ShowForProfileByPath(
       const base::FilePath& profile_path) OVERRIDE;
   virtual content::WebContents* GetStartPageContents() OVERRIDE;
+  virtual const Users& GetUsers() const OVERRIDE;
 
   // Overridden from content::NotificationObserver:
   virtual void Observe(int type,
@@ -99,6 +100,7 @@ class AppListViewDelegate : public app_list::AppListViewDelegate,
   scoped_ptr<app_list::SearchController> search_controller_;
   scoped_ptr<AppListControllerDelegate> controller_;
   Profile* profile_;
+  Users users_;
   app_list::AppListModel* model_;  // Weak. Owned by AppListView.
 
   content::NotificationRegistrar registrar_;

ui/app_list/app_list.gyp

@@ -41,6 +41,7 @@
         'app_list_model.cc',
         'app_list_model.h',
         'app_list_model_observer.h',
+        'app_list_view_delegate.cc',
         'app_list_view_delegate.h',
         'cocoa/app_list_pager_view.h',
         'cocoa/app_list_pager_view.mm',

ui/app_list/app_list_menu.cc

@@ -13,11 +13,10 @@
 
 namespace app_list {
 
-AppListMenu::AppListMenu(AppListViewDelegate* delegate,
-                         const AppListModel::Users& users)
+AppListMenu::AppListMenu(AppListViewDelegate* delegate)
     : menu_model_(this),
       delegate_(delegate),
-      users_(users) {
+      users_(delegate->GetUsers()) {
   InitMenu();
 }
 

ui/app_list/app_list_menu.h

@@ -5,13 +5,11 @@
 #ifndef UI_APP_LIST_APP_LIST_MENU_H_
 #define UI_APP_LIST_APP_LIST_MENU_H_
 
-#include "ui/app_list/app_list_model.h"
+#include "ui/app_list/app_list_view_delegate.h"
 #include "ui/base/models/simple_menu_model.h"
 
 namespace app_list {
 
-class AppListViewDelegate;
-
 // Menu for the app list. This is shown in the top right hand corner of the
 // app list.
 // TODO(benwells): We should consider moving this into Chrome.
@@ -26,9 +24,7 @@ class AppListMenu : public ui::SimpleMenuModel::Delegate {
     SELECT_PROFILE,
   };
 
-  AppListMenu(
-      AppListViewDelegate* delegate,
-      const AppListModel::Users& users);
+  explicit AppListMenu(AppListViewDelegate* delegate);
   virtual ~AppListMenu();
 
   ui::SimpleMenuModel* menu_model() { return &menu_model_; }
@@ -45,7 +41,7 @@ class AppListMenu : public ui::SimpleMenuModel::Delegate {
 
   ui::SimpleMenuModel menu_model_;
   AppListViewDelegate* delegate_;
-  AppListModel::Users users_;
+  AppListViewDelegate::Users users_;
 
   DISALLOW_COPY_AND_ASSIGN(AppListMenu);
 };

ui/app_list/app_list_model.cc

@@ -11,10 +11,6 @@
 
 namespace app_list {
 
-AppListModel::User::User() : active(false) {}
-
-AppListModel::User::~User() {}
-
 AppListModel::AppListModel()
     : item_list_(new AppListItemList),
       search_box_(new SearchBoxModel),
@@ -44,13 +40,6 @@ void AppListModel::SetStatus(Status status) {
                     OnAppListModelStatusChanged());
 }
 
-void AppListModel::SetUsers(const Users& users) {
-  users_ = users;
-  FOR_EACH_OBSERVER(AppListModelObserver,
-                    observers_,
-                    OnAppListModelUsersChanged());
-}
-
 void AppListModel::SetSignedIn(bool signed_in) {
   if (signed_in_ == signed_in)
     return;

ui/app_list/app_list_model.h

@@ -5,13 +5,9 @@
 #ifndef UI_APP_LIST_APP_LIST_MODEL_H_
 #define UI_APP_LIST_APP_LIST_MODEL_H_
 
-#include <vector>
-
 #include "base/basictypes.h"
-#include "base/files/file_path.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/observer_list.h"
-#include "base/strings/string16.h"
 #include "ui/app_list/app_list_export.h"
 #include "ui/app_list/app_list_item_list.h"
 #include "ui/base/models/list_model.h"
@@ -30,31 +26,12 @@ class SearchResult;
 // the model for SearchBoxView. SearchResults owns a list of SearchResult.
 class APP_LIST_EXPORT AppListModel {
  public:
-  // A user of the app list.
-  struct APP_LIST_EXPORT User {
-    User();
-    ~User();
-
-    // Whether or not this user is the current user of the app list.
-    bool active;
-
-    // The name of this user.
-    base::string16 name;
-
-    // The email address of this user.
-    base::string16 email;
-
-    // The path to this user's profile directory.
-    base::FilePath profile_path;
-  };
-
   enum Status {
     STATUS_NORMAL,
     STATUS_SYNCING,  // Syncing apps or installing synced apps.
   };
 
   typedef ui::ListModel<SearchResult> SearchResults;
-  typedef std::vector<User> Users;
 
   AppListModel();
   ~AppListModel();
@@ -63,7 +40,6 @@ class APP_LIST_EXPORT AppListModel {
   void RemoveObserver(AppListModelObserver* observer);
 
   void SetStatus(Status status);
-  void SetUsers(const Users& profile_menu_items);
   void SetSignedIn(bool signed_in);
 
   AppListItemList* item_list() { return item_list_.get(); }
@@ -72,17 +48,11 @@ class APP_LIST_EXPORT AppListModel {
   Status status() const { return status_; }
   bool signed_in() const { return signed_in_; }
 
-  const Users& users() const {
-    return users_;
-  }
-
  private:
   scoped_ptr<AppListItemList> item_list_;
   scoped_ptr<SearchBoxModel> search_box_;
   scoped_ptr<SearchResults> results_;
 
-  Users users_;
-
   bool signed_in_;
   Status status_;
   ObserverList<AppListModelObserver> observers_;

ui/app_list/app_list_model_observer.h

@@ -14,9 +14,6 @@ class APP_LIST_EXPORT AppListModelObserver {
   // Invoked when AppListModel's status has changed.
   virtual void OnAppListModelStatusChanged() {}
 
-  // Invoked when AppListModel's profile menu items have changed.
-  virtual void OnAppListModelUsersChanged() {}
-
   // Invoked when AppListModel's current user's signin status has changed.
   virtual void OnAppListModelSigninStatusChanged() {}
 

ui/app_list/app_list_model_unittest.cc

@@ -23,7 +23,6 @@ class TestObserver : public AppListModelObserver,
  public:
   TestObserver()
       : status_changed_count_(0),
-        users_changed_count_(0),
         signin_changed_count_(0),
         items_added_(0),
         items_removed_(0),
@@ -37,10 +36,6 @@ class TestObserver : public AppListModelObserver,
     ++status_changed_count_;
   }
 
-  virtual void OnAppListModelUsersChanged() OVERRIDE {
-    ++users_changed_count_;
-  }
-
   virtual void OnAppListModelSigninStatusChanged() OVERRIDE {
     ++signin_changed_count_;
   }
@@ -62,15 +57,13 @@ class TestObserver : public AppListModelObserver,
   }
 
   int status_changed_count() const { return status_changed_count_; }
-  int users_changed_count() const { return users_changed_count_; }
   int signin_changed_count() const { return signin_changed_count_; }
   size_t items_added() { return items_added_; }
   size_t items_removed() { return items_removed_; }
   size_t items_moved() { return items_moved_; }
 
   void ResetCounts() {
     status_changed_count_ = 0;
-    users_changed_count_ = 0;
     signin_changed_count_ = 0;
     items_added_ = 0;
     items_removed_ = 0;
@@ -79,7 +72,6 @@ class TestObserver : public AppListModelObserver,
 
  private:
   int status_changed_count_;
-  int users_changed_count_;
   int signin_changed_count_;
   size_t items_added_;
   size_t items_removed_;
@@ -130,17 +122,6 @@ TEST_F(AppListModelTest, SetStatus) {
   EXPECT_EQ(2, observer_.status_changed_count());
 }
 
-TEST_F(AppListModelTest, SetUsers) {
-  EXPECT_EQ(0u, model_.users().size());
-  AppListModel::Users users;
-  users.push_back(AppListModel::User());
-  users[0].name = UTF8ToUTF16("test");
-  model_.SetUsers(users);
-  EXPECT_EQ(1, observer_.users_changed_count());
-  ASSERT_EQ(1u, model_.users().size());
-  EXPECT_EQ(UTF8ToUTF16("test"), model_.users()[0].name);
-}
-
 TEST_F(AppListModelTest, SetSignedIn) {
   EXPECT_TRUE(model_.signed_in());
   model_.SetSignedIn(false);

ui/app_list/app_list_view_delegate.cc

@@ -0,0 +1,15 @@
+// Copyright 2013 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 "ui/app_list/app_list_view_delegate.h"
+
+namespace app_list {
+
+AppListViewDelegate::User::User() : active(false) {
+}
+
+AppListViewDelegate::User::~User() {
+}
+
+}  // app_list

ui/app_list/app_list_view_delegate.h

@@ -5,7 +5,11 @@
 #ifndef UI_APP_LIST_APP_LIST_VIEW_DELEGATE_H_
 #define UI_APP_LIST_APP_LIST_VIEW_DELEGATE_H_
 
+#include <vector>
+
 #include "base/callback_forward.h"
+#include "base/files/file_path.h"
+#include "base/strings/string16.h"
 #include "base/strings/string16.h"
 #include "ui/app_list/app_list_export.h"
 
@@ -30,6 +34,25 @@ class SigninDelegate;
 
 class APP_LIST_EXPORT AppListViewDelegate {
  public:
+  // A user of the app list.
+  struct APP_LIST_EXPORT User {
+    User();
+    ~User();
+
+    // Whether or not this user is the current user of the app list.
+    bool active;
+
+    // The name of this user.
+    base::string16 name;
+
+    // The email address of this user.
+    base::string16 email;
+
+    // The path to this user's profile directory.
+    base::FilePath profile_path;
+  };
+  typedef std::vector<User> Users;
+
   // AppListView owns the delegate.
   virtual ~AppListViewDelegate() {}
 
@@ -97,6 +120,9 @@ class APP_LIST_EXPORT AppListViewDelegate {
 
   // Get the start page web contents. Owned by the AppListViewDelegate.
   virtual content::WebContents* GetStartPageContents() = 0;
+
+  // Returns the list of users (for AppListMenu).
+  virtual const Users& GetUsers() const = 0;
 };
 
 }  // namespace app_list

ui/app_list/cocoa/app_list_view_controller.mm

@@ -88,7 +88,6 @@ - (void)revealSearchResults:(BOOL)show;
 
  private:
   // Overridden from app_list::AppListModelObserver:
-  virtual void OnAppListModelUsersChanged() OVERRIDE;
   virtual void OnAppListModelSigninStatusChanged() OVERRIDE;
 
   AppListViewController* parent_;  // Weak. Owns us.
@@ -106,10 +105,6 @@ - (void)revealSearchResults:(BOOL)show;
   [[parent_ appsGridController] model]->RemoveObserver(this);
 }
 
-void AppListModelObserverBridge::OnAppListModelUsersChanged() {
-  [parent_ onSigninStatusChanged];
-}
-
 void AppListModelObserverBridge::OnAppListModelSigninStatusChanged() {
   [parent_ onSigninStatusChanged];
 }

ui/app_list/cocoa/apps_search_box_controller.mm

@@ -161,8 +161,7 @@ - (void)rebuildMenu {
 
   menuController_.reset();
   appListMenu_.reset(
-      new app_list::AppListMenu([delegate_ appListDelegate],
-                                [delegate_ appListModel]->users()));
+      new app_list::AppListMenu([delegate_ appListDelegate]));
   menuController_.reset([[AppListMenuController alloc]
       initWithSearchBoxController:self]);
   [menuButton_ setMenu:[menuController_ menu]];  // Menu will populate here.