Disable profile avatar menu shortcut when there's only one profile.

BUG=108367
TEST=Shortcut doesn't launch menu when there's a single profile.
Adding/deleting profiles propertly changes the state.

Review URL: https://chromiumcodereview.appspot.com/11574010

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

6 files changed, 124 insertions, 31 deletions

diff --git a/chrome/browser/ui/browser.cc b/chrome/browser/ui/browser.cc
index be3caa3..f1d1d7d 100644
--- a/chrome/browser/ui/browser.cc
+++ b/chrome/browser/ui/browser.cc
@@ -372,7 +372,8 @@ Browser::Browser(const CreateParams& params)
               new BrowserSyncedWindowDelegate(this))),
       bookmark_bar_state_(BookmarkBar::HIDDEN),
       ALLOW_THIS_IN_INITIALIZER_LIST(
-          command_controller_(new chrome::BrowserCommandController(this))),
+          command_controller_(new chrome::BrowserCommandController(
+              this, g_browser_process->profile_manager()))),
       window_has_shown_(false) {
   if (!app_name_.empty())
     chrome::RegisterAppPrefs(app_name_, profile_);
@@ -496,6 +497,10 @@ Browser::~Browser() {
   search_model_->RemoveObserver(this);
   tab_strip_model_->RemoveObserver(this);
 
+  // Destroy the BrowserCommandController before removing the browser, so that
+  // it doesn't act on any notifications that are sent as a result of removing
+  // the browser.
+  command_controller_.reset();
   BrowserList::RemoveBrowser(this);
 
   SessionService* session_service =
@@ -527,8 +532,6 @@ Browser::~Browser() {
 
   encoding_auto_detect_.Destroy();
 
-  command_controller_.reset();
-
   if (profile_->IsOffTheRecord() &&
       !BrowserList::IsOffTheRecordSessionActiveForProfile(profile_)) {
     // An incognito profile is no longer needed, this indirectly frees
diff --git a/chrome/browser/ui/browser.h b/chrome/browser/ui/browser.h
index 4e219f8..735fd96 100644
--- a/chrome/browser/ui/browser.h
+++ b/chrome/browser/ui/browser.h
@@ -499,13 +499,13 @@ class Browser : public TabStripModelObserver,
   friend class FullscreenControllerInteractiveTest;
   friend class FullscreenControllerTest;
   FRIEND_TEST_ALL_PREFIXES(AppModeTest, EnableAppModeTest);
+  FRIEND_TEST_ALL_PREFIXES(BrowserCommandControllerTest,
+                           IsReservedCommandOrKeyIsApp);
+  FRIEND_TEST_ALL_PREFIXES(BrowserCommandControllerTest, AppFullScreen);
   FRIEND_TEST_ALL_PREFIXES(BrowserTest, NoTabsInPopups);
   FRIEND_TEST_ALL_PREFIXES(BrowserTest, ConvertTabToAppShortcut);
   FRIEND_TEST_ALL_PREFIXES(BrowserTest, OpenAppWindowLikeNtp);
   FRIEND_TEST_ALL_PREFIXES(BrowserTest, AppIdSwitch);
-  FRIEND_TEST_ALL_PREFIXES(BrowserWithTestWindowTest,
-                           IsReservedCommandOrKeyIsApp);
-  FRIEND_TEST_ALL_PREFIXES(BrowserWithTestWindowTest, AppFullScreen);
   FRIEND_TEST_ALL_PREFIXES(FullscreenControllerTest,
                            TabEntersPresentationModeFromWindowed);
   FRIEND_TEST_ALL_PREFIXES(FullscreenExitBubbleControllerTest,
diff --git a/chrome/browser/ui/browser_command_controller.cc b/chrome/browser/ui/browser_command_controller.cc
index b137307..7231d5a 100644
--- a/chrome/browser/ui/browser_command_controller.cc
+++ b/chrome/browser/ui/browser_command_controller.cc
@@ -147,12 +147,17 @@ namespace chrome {
 ///////////////////////////////////////////////////////////////////////////////
 // BrowserCommandController, public:
 
-BrowserCommandController::BrowserCommandController(Browser* browser)
+BrowserCommandController::BrowserCommandController(
+    Browser* browser,
+    ProfileManager* profile_manager)
     : browser_(browser),
+      profile_manager_(profile_manager),
       ALLOW_THIS_IN_INITIALIZER_LIST(command_updater_(this)),
       block_command_execution_(false),
       last_blocked_command_id_(-1),
       last_blocked_command_disposition_(CURRENT_TAB) {
+  if (profile_manager_)
+    profile_manager_->GetProfileInfoCache().AddObserver(this);
   browser_->tab_strip_model()->AddObserver(this);
   PrefService* local_state = g_browser_process->local_state();
   if (local_state) {
@@ -222,6 +227,8 @@ BrowserCommandController::~BrowserCommandController() {
   profile_pref_registrar_.RemoveAll();
   local_pref_registrar_.RemoveAll();
   browser_->tab_strip_model()->RemoveObserver(this);
+  if (profile_manager_)
+    profile_manager_->GetProfileInfoCache().RemoveObserver(this);
 }
 
 bool BrowserCommandController::IsReservedCommandOrKey(
@@ -705,6 +712,47 @@ void BrowserCommandController::Observe(
   }
 }
 
+
+////////////////////////////////////////////////////////////////////////////////
+// BrowserCommandController, ProfileInfoCacheObserver implementation:
+
+void BrowserCommandController::OnProfileAdded(const FilePath& profile_path) {
+  UpdateCommandsForMultipleProfiles();
+}
+
+void BrowserCommandController::OnProfileWasRemoved(
+    const FilePath& profile_path,
+    const string16& profile_name) {
+  UpdateCommandsForMultipleProfiles();
+}
+
+void BrowserCommandController::OnProfileWillBeRemoved(
+    const FilePath& profile_path) {
+}
+
+void BrowserCommandController::OnProfileNameChanged(
+    const FilePath& profile_path,
+    const string16& old_profile_name) {
+}
+
+void BrowserCommandController::OnProfileAvatarChanged(
+    const FilePath& profile_path) {
+}
+
+////////////////////////////////////////////////////////////////////////////////
+// BrowserCommandController, ProfileSyncServiceObserver implementation:
+
+void BrowserCommandController::OnStateChanged() {
+  DCHECK(ProfileSyncServiceFactory::GetInstance()->HasProfileSyncService(
+      profile()));
+  // For unit tests, we don't have a window.
+  if (!window())
+    return;
+  const bool show_main_ui = IsShowingMainUI(window()->IsFullscreen());
+  command_updater_.UpdateCommandEnabled(IDC_SHOW_SYNC_SETUP,
+      show_main_ui && profile()->GetOriginalProfile()->IsSyncAccessible());
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // BrowserCommandController, TabStripModelObserver implementation:
 
@@ -749,20 +797,6 @@ void BrowserCommandController::TabRestoreServiceDestroyed(
 }
 
 ////////////////////////////////////////////////////////////////////////////////
-// BrowserCommandController, ProfileSyncServiceObserver implementation:
-
-void BrowserCommandController::OnStateChanged() {
-  DCHECK(ProfileSyncServiceFactory::GetInstance()->HasProfileSyncService(
-      profile()));
-  // For unit tests, we don't have a window.
-  if (!window())
-    return;
-  const bool show_main_ui = IsShowingMainUI(window()->IsFullscreen());
-  command_updater_.UpdateCommandEnabled(IDC_SHOW_SYNC_SETUP,
-      show_main_ui && profile()->GetOriginalProfile()->IsSyncAccessible());
-}
-
-////////////////////////////////////////////////////////////////////////////////
 // BrowserCommandController, private:
 
 bool BrowserCommandController::IsShowingMainUI(bool is_fullscreen) {
@@ -1126,10 +1160,13 @@ void BrowserCommandController::UpdateCommandsForFullscreenMode(
 
 void BrowserCommandController::UpdateCommandsForMultipleProfiles() {
   bool show_main_ui = IsShowingMainUI(window() && window()->IsFullscreen());
+  bool has_multiple_profiles = profile_manager_ &&
+                               profile_manager_->GetNumberOfProfiles() > 1;
   command_updater_.UpdateCommandEnabled(IDC_SHOW_AVATAR_MENU,
       show_main_ui &&
       !profile()->IsOffTheRecord() &&
-      ProfileManager::IsMultipleProfilesEnabled());
+      ProfileManager::IsMultipleProfilesEnabled() &&
+      has_multiple_profiles);
 }
 
 void BrowserCommandController::UpdatePrintingState() {
diff --git a/chrome/browser/ui/browser_command_controller.h b/chrome/browser/ui/browser_command_controller.h
index 3f76188..31bfe7b 100644
--- a/chrome/browser/ui/browser_command_controller.h
+++ b/chrome/browser/ui/browser_command_controller.h
@@ -9,6 +9,7 @@
 #include "chrome/browser/api/sync/profile_sync_service_observer.h"
 #include "chrome/browser/command_updater.h"
 #include "chrome/browser/command_updater_delegate.h"
+#include "chrome/browser/profiles/profile_info_cache_observer.h"
 #include "chrome/browser/sessions/tab_restore_service_observer.h"
 #include "chrome/browser/ui/tabs/tab_strip_model_observer.h"
 #include "content/public/browser/notification_observer.h"
@@ -18,6 +19,7 @@
 class Browser;
 class BrowserWindow;
 class Profile;
+class ProfileManager;
 
 namespace content {
 struct NativeWebKeyboardEvent;
@@ -27,11 +29,12 @@ namespace chrome {
 
 class BrowserCommandController : public CommandUpdaterDelegate,
                                  public content::NotificationObserver,
+                                 public ProfileInfoCacheObserver,
+                                 public ProfileSyncServiceObserver,
                                  public TabStripModelObserver,
-                                 public TabRestoreServiceObserver,
-                                 public ProfileSyncServiceObserver {
+                                 public TabRestoreServiceObserver {
  public:
-  explicit BrowserCommandController(Browser* browser);
+  BrowserCommandController(Browser* browser, ProfileManager* profile_manager);
   virtual ~BrowserCommandController();
 
   CommandUpdater* command_updater() { return &command_updater_; }
@@ -86,6 +89,18 @@ class BrowserCommandController : public CommandUpdaterDelegate,
                        const content::NotificationSource& source,
                        const content::NotificationDetails& details) OVERRIDE;
 
+  // Overridden from ProfileInfoCacheObserver:
+  virtual void OnProfileAdded(const FilePath& profile_path) OVERRIDE;
+  virtual void OnProfileWillBeRemoved(const FilePath& profile_path) OVERRIDE;
+  virtual void OnProfileWasRemoved(const FilePath& profile_path,
+                                   const string16& profile_name) OVERRIDE;
+  virtual void OnProfileNameChanged(const FilePath& profile_path,
+                                    const string16& old_profile_name) OVERRIDE;
+  virtual void OnProfileAvatarChanged(const FilePath& profile_path) OVERRIDE;
+
+  // Overridden from ProfileSyncServiceObserver:
+  virtual void OnStateChanged() OVERRIDE;
+
   // Overridden from TabStripModelObserver:
   virtual void TabInsertedAt(content::WebContents* contents,
                              int index,
@@ -103,9 +118,6 @@ class BrowserCommandController : public CommandUpdaterDelegate,
   virtual void TabRestoreServiceChanged(TabRestoreService* service) OVERRIDE;
   virtual void TabRestoreServiceDestroyed(TabRestoreService* service) OVERRIDE;
 
-  // Overridden from ProfileSyncServiceObserver:
-  virtual void OnStateChanged() OVERRIDE;
-
   // Returns true if the regular Chrome UI (not the fullscreen one and
   // not the single-tab one) is shown. Used for updating window command states
   // only. Consider using SupportsWindowFeature if you need the mentioned
@@ -172,6 +184,8 @@ class BrowserCommandController : public CommandUpdaterDelegate,
 
   Browser* browser_;
 
+  ProfileManager* profile_manager_;
+
   // The CommandUpdater that manages the browser window commands.
   CommandUpdater command_updater_;
 
diff --git a/chrome/browser/ui/browser_command_controller_unittest.cc b/chrome/browser/ui/browser_command_controller_unittest.cc
index 4e66090..447d2bd 100644
--- a/chrome/browser/ui/browser_command_controller_unittest.cc
+++ b/chrome/browser/ui/browser_command_controller_unittest.cc
@@ -5,14 +5,20 @@
 #include "chrome/browser/ui/browser_command_controller.h"
 
 #include "chrome/app/chrome_command_ids.h"
+#include "chrome/browser/browser_process.h"
+#include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_commands.h"
 #include "chrome/browser/ui/browser_window_state.h"
 #include "chrome/test/base/browser_with_test_window_test.h"
+#include "chrome/test/base/testing_browser_process.h"
+#include "chrome/test/base/testing_profile_manager.h"
 #include "content/public/browser/native_web_keyboard_event.h"
 #include "ui/base/keycodes/keyboard_codes.h"
 
-TEST_F(BrowserWithTestWindowTest, IsReservedCommandOrKey) {
+typedef BrowserWithTestWindowTest BrowserCommandControllerTest;
+
+TEST_F(BrowserCommandControllerTest, IsReservedCommandOrKey) {
 #if defined(OS_CHROMEOS)
   // F1-3 keys are reserved Chrome accelerators on Chrome OS.
   EXPECT_TRUE(browser()->command_controller()->IsReservedCommandOrKey(
@@ -84,7 +90,7 @@ TEST_F(BrowserWithTestWindowTest, IsReservedCommandOrKey) {
 #endif  // USE_AURA
 }
 
-TEST_F(BrowserWithTestWindowTest, IsReservedCommandOrKeyIsApp) {
+TEST_F(BrowserCommandControllerTest, IsReservedCommandOrKeyIsApp) {
   browser()->app_name_ = "app";
   ASSERT_TRUE(browser()->is_app());
 
@@ -119,7 +125,7 @@ TEST_F(BrowserWithTestWindowTest, IsReservedCommandOrKeyIsApp) {
 #endif  // USE_AURA
 }
 
-TEST_F(BrowserWithTestWindowTest, AppFullScreen) {
+TEST_F(BrowserCommandControllerTest, AppFullScreen) {
   // Enable for tabbed browser.
   EXPECT_TRUE(chrome::IsCommandEnabled(browser(), IDC_FULLSCREEN));
 
@@ -143,3 +149,31 @@ TEST_F(BrowserWithTestWindowTest, AppFullScreen) {
   panel_browser.command_controller()->FullscreenStateChanged();
   EXPECT_FALSE(chrome::IsCommandEnabled(&panel_browser, IDC_FULLSCREEN));
 }
+
+TEST_F(BrowserCommandControllerTest, AvatarMenuDisabledWhenOnlyOneProfile) {
+  if (!ProfileManager::IsMultipleProfilesEnabled())
+    return;
+
+  TestingProfileManager testing_profile_manager(
+      static_cast<TestingBrowserProcess*>(g_browser_process));
+  ASSERT_TRUE(testing_profile_manager.SetUp());
+  ProfileManager* profile_manager = testing_profile_manager.profile_manager();
+
+  chrome::BrowserCommandController command_controller(browser(),
+                                                      profile_manager);
+  const CommandUpdater* command_updater = command_controller.command_updater();
+
+  testing_profile_manager.CreateTestingProfile("p1");
+  ASSERT_EQ(1U, profile_manager->GetNumberOfProfiles());
+  EXPECT_FALSE(command_updater->IsCommandEnabled(IDC_SHOW_AVATAR_MENU));
+
+  testing_profile_manager.CreateTestingProfile("p2");
+  ASSERT_EQ(2U, profile_manager->GetNumberOfProfiles());
+  EXPECT_TRUE(command_updater->IsCommandEnabled(IDC_SHOW_AVATAR_MENU));
+
+  testing_profile_manager.DeleteTestingProfile("p1");
+  ASSERT_EQ(1U, profile_manager->GetNumberOfProfiles());
+  EXPECT_FALSE(command_updater->IsCommandEnabled(IDC_SHOW_AVATAR_MENU));
+
+  testing_profile_manager.DeleteTestingProfile("p2");
+}
diff --git a/chrome/browser/ui/cocoa/browser/avatar_button_controller_unittest.mm b/chrome/browser/ui/cocoa/browser/avatar_button_controller_unittest.mm
index 7de2968..4db468f 100644
--- a/chrome/browser/ui/cocoa/browser/avatar_button_controller_unittest.mm
+++ b/chrome/browser/ui/cocoa/browser/avatar_button_controller_unittest.mm
@@ -53,6 +53,9 @@ TEST_F(AvatarButtonControllerTest, AddRemoveProfiles) {
 }
 
 TEST_F(AvatarButtonControllerTest, DoubleOpen) {
+  // Create a second profile to enable the avatar menu.
+  testing_profile_manager()->CreateTestingProfile("p2");
+
   EXPECT_FALSE([controller() menuController]);
 
   [button() performClick:button()];
@@ -67,4 +70,6 @@ TEST_F(AvatarButtonControllerTest, DoubleOpen) {
   static_cast<InfoBubbleWindow*>(menu.window).delayOnClose = NO;
   [menu close];
   EXPECT_FALSE([controller() menuController]);
+
+  testing_profile_manager()->DeleteTestingProfile("p2");
 }