Fixing background mode manager to correctly display the name of the profile in...

Fixing background mode manager to correctly display the name of the profile in the status icon. Previously it was not being updated when Chrome first started up and it was also using the incorrect name. Now it gets the correct name from the profile info cache and also updates based on changes to the profile info cache.

BUG=101837
TEST=start chrome with background apps and the status icon will have names populated correctly, also BackgroundModeManagerTest::ProfileInfoCacheObserver

Review URL: http://codereview.chromium.org/8802013

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

chrome/browser/background/background_mode_manager.cc

@@ -37,15 +37,10 @@
 
 BackgroundModeManager::BackgroundModeData::BackgroundModeData(
     int command_id,
-    Profile* profile,
-    BackgroundModeManager* background_mode_manager)
+    Profile* profile)
     : applications_(new BackgroundApplicationListModel(profile)),
       command_id_(command_id),
-      profile_(profile),
-      background_mode_manager_(background_mode_manager) {
-  name_ = UTF8ToUTF16(profile_->GetProfileName());
-  if (name_.empty())
-    name_ = l10n_util::GetStringUTF16(IDS_PROFILES_DEFAULT_NAME);
+      profile_(profile) {
 }
 
 BackgroundModeManager::BackgroundModeData::~BackgroundModeData() {
@@ -121,6 +116,15 @@ void BackgroundModeManager::BackgroundModeData::BuildProfileMenu(
     containing_menu->AddSubMenu(command_id_, name_, menu);
 }
 
+void BackgroundModeManager::BackgroundModeData::SetName(
+    const string16& new_profile_name) {
+  name_ = new_profile_name;
+}
+
+string16 BackgroundModeManager::BackgroundModeData::name() {
+  return name_;
+}
+
 // static
 bool BackgroundModeManager::BackgroundModeData::BackgroundModeDataCompare(
     const BackgroundModeData* bmd1,
@@ -151,6 +155,10 @@ BackgroundModeManager::BackgroundModeManager(
   if (IsBackgroundModePermanentlyDisabled(command_line))
     return;
 
+  // Add self as an observer for the profile info cache so we know when profiles
+  // are deleted and their names change.
+  profile_cache_->AddObserver(this);
+
   // Listen for the background mode preference changing.
   if (g_browser_process->local_state()) {  // Skip for unit tests
     pref_registrar_.Init(g_browser_process->local_state());
@@ -183,7 +191,7 @@ BackgroundModeManager::BackgroundModeManager(
 BackgroundModeManager::~BackgroundModeManager() {
   // Remove ourselves from the application observer list (only needed by unit
   // tests since APP_TERMINATING is what does this in a real running system).
-  for (std::map<Profile*, BackgroundModeInfo>::iterator it =
+  for (BackgroundModeInfoMap::iterator it =
        background_mode_data_.begin();
        it != background_mode_data_.end();
        ++it) {
@@ -208,9 +216,16 @@ void BackgroundModeManager::RegisterProfile(Profile* profile) {
   // We don't want to register multiple times for one profile.
   DCHECK(background_mode_data_.find(profile) == background_mode_data_.end());
   BackgroundModeInfo bmd(new BackgroundModeData(current_command_id_++,
-                                                profile, this));
+                                                profile));
   background_mode_data_[profile] = bmd;
 
+  // Initially set the name for this background mode data.
+  size_t index = profile_cache_->GetIndexOfProfileWithPath(profile->GetPath());
+  string16 name = l10n_util::GetStringUTF16(IDS_PROFILES_DEFAULT_NAME);
+  if (index != std::string::npos)
+    name = profile_cache_->GetNameOfProfileAtIndex(index);
+  bmd->SetName(name);
+
   // Listen for when extensions are loaded or add the background permission so
   // we can display a "background app installed" notification and enter
   // "launch on login" mode on the Mac.
@@ -246,6 +261,10 @@ void BackgroundModeManager::LaunchBackgroundApplication(
                            NEW_FOREGROUND_TAB);
 }
 
+int BackgroundModeManager::NumberOfBackgroundModeData() {
+  return background_mode_data_.size();
+}
+
 ///////////////////////////////////////////////////////////////////////////////
 //  BackgroundModeManager, content::NotificationObserver overrides
 void BackgroundModeManager::Observe(
@@ -299,7 +318,7 @@ void BackgroundModeManager::Observe(
       // Shutting down, so don't listen for any more notifications so we don't
       // try to re-enter/exit background mode again.
       registrar_.RemoveAll();
-      for (std::map<Profile*, BackgroundModeInfo>::iterator it =
+      for (BackgroundModeInfoMap::iterator it =
                background_mode_data_.begin();
            it != background_mode_data_.end();
            ++it) {
@@ -363,6 +382,59 @@ void BackgroundModeManager::OnApplicationListChanged(Profile* profile) {
   }
 }
 
+///////////////////////////////////////////////////////////////////////////////
+//  BackgroundModeManager, ProfileInfoCacheObserver overrides
+void BackgroundModeManager::OnProfileAdded(const string16& profile_name,
+                                           const string16& profile_base_dir,
+                                           const FilePath& profile_path,
+                                           const gfx::Image* avatar_image) {
+  // At this point, the profile should be registered with the background mode
+  // manager, but when it's actually added to the cache is when its name is
+  // set so we need up to update that with the background_mode_data.
+  for (BackgroundModeInfoMap::const_iterator it =
+       background_mode_data_.begin();
+       it != background_mode_data_.end();
+       ++it) {
+    if (it->first->GetPath() == profile_path) {
+      it->second->SetName(profile_name);
+      UpdateStatusTrayIconContextMenu();
+      return;
+    }
+  }
+}
+
+void BackgroundModeManager::OnProfileRemoved(const string16& profile_name) {
+  // Remove the profile from our map of profiles.
+  BackgroundModeInfoMap::iterator it =
+      GetBackgroundModeIterator(profile_name);
+  // If a profile isn't running a background app, it may not be in the map.
+  if (it != background_mode_data_.end()) {
+    background_mode_data_.erase(it);
+    UpdateStatusTrayIconContextMenu();
+  }
+}
+
+void BackgroundModeManager::OnProfileNameChanged(
+    const string16& old_profile_name,
+    const string16& new_profile_name) {
+  BackgroundModeInfoMap::const_iterator it =
+      GetBackgroundModeIterator(old_profile_name);
+  // We check that the returned iterator is valid due to unittests, but really
+  // this should only be called on profiles already known by the background
+  // mode manager.
+  if (it != background_mode_data_.end()) {
+    it->second->SetName(new_profile_name);
+    UpdateStatusTrayIconContextMenu();
+  }
+}
+
+void BackgroundModeManager::OnProfileAvatarChanged(
+    const string16& profile_name,
+    const string16& profile_base_dir,
+    const FilePath& profile_path,
+    const gfx::Image* avatar_image) {
+
+}
 ///////////////////////////////////////////////////////////////////////////////
 //  BackgroundModeManager::BackgroundModeData, ui::SimpleMenuModel overrides
 bool BackgroundModeManager::IsCommandIdChecked(
@@ -489,7 +561,7 @@ void BackgroundModeManager::DisableBackgroundMode() {
 int BackgroundModeManager::GetBackgroundAppCount() const {
   int count = 0;
   // Walk the BackgroundModeData for all profiles and count the number of apps.
-  for (std::map<Profile*, BackgroundModeInfo>::const_iterator it =
+  for (BackgroundModeInfoMap::const_iterator it =
        background_mode_data_.begin();
        it != background_mode_data_.end();
        ++it) {
@@ -588,7 +660,7 @@ void BackgroundModeManager::UpdateStatusTrayIconContextMenu() {
 
   if (background_mode_data_.size() > 1) {
     std::vector<BackgroundModeData*> bmd_vector;
-    for (std::map<Profile*, BackgroundModeInfo>::iterator it =
+    for (BackgroundModeInfoMap::iterator it =
          background_mode_data_.begin();
          it != background_mode_data_.end();
          ++it) {
@@ -632,6 +704,22 @@ BackgroundModeManager::GetBackgroundModeData(Profile* const profile) const {
   return background_mode_data_.find(profile)->second.get();
 }
 
+BackgroundModeManager::BackgroundModeInfoMap::iterator
+BackgroundModeManager::GetBackgroundModeIterator(
+    const string16& profile_name) {
+  BackgroundModeInfoMap::iterator profile_it =
+      background_mode_data_.end();
+  for (BackgroundModeInfoMap::iterator it =
+       background_mode_data_.begin();
+       it != background_mode_data_.end();
+       ++it) {
+    if (it->second->name() == profile_name) {
+      profile_it = it;
+    }
+  }
+  return profile_it;
+}
+
 // static
 bool BackgroundModeManager::IsBackgroundModePermanentlyDisabled(
     const CommandLine* command_line) {

chrome/browser/background/background_mode_manager.h

@@ -11,6 +11,7 @@
 #include "base/gtest_prod_util.h"
 #include "chrome/browser/background/background_application_list_model.h"
 #include "chrome/browser/prefs/pref_change_registrar.h"
+#include "chrome/browser/profiles/profile_info_cache_observer.h"
 #include "chrome/browser/profiles/profile_keyed_service.h"
 #include "chrome/browser/status_icons/status_icon.h"
 #include "content/public/browser/notification_observer.h"
@@ -43,6 +44,7 @@ class StatusTray;
 class BackgroundModeManager
     : public content::NotificationObserver,
       public BackgroundApplicationListModel::Observer,
+      public ProfileInfoCacheObserver,
       public ProfileKeyedService,
       public ui::SimpleMenuModel::Delegate {
  public:
@@ -62,6 +64,9 @@ class BackgroundModeManager
   static void LaunchBackgroundApplication(Profile* profile,
                                           const Extension* extension);
 
+  // For testing purposes.
+  int NumberOfBackgroundModeData();
+
  private:
   friend class TestBackgroundModeManager;
   friend class BackgroundModeManagerTest;
@@ -77,13 +82,14 @@ class BackgroundModeManager
                            MultiProfile);
   FRIEND_TEST_ALL_PREFIXES(BackgroundModeManagerTest,
                            ProfileInfoCacheStorage);
+  FRIEND_TEST_ALL_PREFIXES(BackgroundModeManagerTest,
+                           ProfileInfoCacheObserver);
 
   class BackgroundModeData : public ui::SimpleMenuModel::Delegate {
    public:
     explicit BackgroundModeData(
         int command_id,
-        Profile* profile,
-        BackgroundModeManager* background_mode_manager);
+        Profile* profile);
     virtual ~BackgroundModeData();
 
     // The cached list of BackgroundApplications.
@@ -112,6 +118,14 @@ class BackgroundModeManager
     void BuildProfileMenu(ui::SimpleMenuModel* menu,
                           ui::SimpleMenuModel* containing_menu);
 
+    // Set the name associated with this background mode data for displaying in
+    // the status tray.
+    void SetName(const string16& new_profile_name);
+
+    // The name associated with this background mode data. This should match
+    // the name in the ProfileInfoCache for this profile.
+    string16 name();
+
     // Used for sorting BackgroundModeData*s.
     static bool BackgroundModeDataCompare(const BackgroundModeData* bmd1,
                                           const BackgroundModeData* bmd2);
@@ -125,9 +139,6 @@ class BackgroundModeManager
 
     // The profile associated with this background app data.
     Profile* profile_;
-
-    // The background mode manager which owns this BackgroundModeData.
-    BackgroundModeManager* background_mode_manager_;
   };
 
   // Ideally we would want our BackgroundModeData to be scoped_ptrs,
@@ -139,6 +150,8 @@ class BackgroundModeManager
   // which is similar to a shared_ptr.
   typedef linked_ptr<BackgroundModeData> BackgroundModeInfo;
 
+  typedef std::map<Profile*, BackgroundModeInfo> BackgroundModeInfoMap;
+
   // content::NotificationObserver implementation.
   virtual void Observe(int type,
                        const content::NotificationSource& source,
@@ -149,6 +162,19 @@ class BackgroundModeManager
                                         Profile* profile) OVERRIDE;
   virtual void OnApplicationListChanged(Profile* profile) OVERRIDE;
 
+  // Overrides from ProfileInfoCacheObserver
+  virtual void OnProfileAdded(const string16& profile_name,
+                              const string16& profile_base_dir,
+                              const FilePath& profile_path,
+                              const gfx::Image* avatar_image) OVERRIDE;
+  virtual void OnProfileRemoved(const string16& profile_name) OVERRIDE;
+  virtual void OnProfileNameChanged(const string16& old_profile_name,
+                                    const string16& new_profile_name) OVERRIDE;
+  virtual void OnProfileAvatarChanged(const string16& profile_name,
+                                      const string16& profile_base_dir,
+                                      const FilePath& profile_path,
+                                      const gfx::Image* avatar_image) OVERRIDE;
+
   // Overrides from SimpleMenuModel::Delegate implementation.
   virtual bool IsCommandIdChecked(int command_id) const OVERRIDE;
   virtual bool IsCommandIdEnabled(int command_id) const OVERRIDE;
@@ -211,6 +237,12 @@ class BackgroundModeManager
   BackgroundModeManager::BackgroundModeData* GetBackgroundModeData(
       Profile* const profile) const;
 
+  // Returns the iterator associated with a particular profile name.
+  // This should not be used to iterate over the background mode data. It is
+  // used to efficiently delete an item from the background mode data map.
+  BackgroundModeInfoMap::iterator GetBackgroundModeIterator(
+      const string16& profile_name);
+
   // Returns true if the "Let chrome run in the background" pref is checked.
   // (virtual to allow overriding in tests).
   virtual bool IsBackgroundModePrefEnabled() const;
@@ -237,7 +269,7 @@ class BackgroundModeManager
   PrefChangeRegistrar pref_registrar_;
 
   // The profile-keyed data for this background mode manager. Keyed on profile.
-  std::map<Profile*, BackgroundModeInfo> background_mode_data_;
+  BackgroundModeInfoMap background_mode_data_;
 
   // Reference to our status tray. If null, the platform doesn't support status
   // icons.

chrome/browser/background/background_mode_manager_unittest.cc

@@ -4,6 +4,7 @@
 
 #include "base/command_line.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/utf_string_conversions.h"
 #include "chrome/browser/background/background_mode_manager.h"
 #include "chrome/browser/profiles/profile_info_cache.h"
 #include "chrome/browser/ui/browser_list.h"
@@ -12,6 +13,8 @@
 #include "chrome/test/base/testing_profile.h"
 #include "chrome/test/base/testing_profile_manager.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "ui/gfx/image/image.h"
+#include "ui/gfx/image/image_unittest_util.h"
 
 class BackgroundModeManagerTest : public testing::Test {
  public:
@@ -248,3 +251,43 @@ TEST_F(BackgroundModeManagerTest, ProfileInfoCacheStorage) {
   // profiles in the cache.
   EXPECT_EQ(2u, cache->GetNumberOfProfiles());
 }
+TEST_F(BackgroundModeManagerTest, ProfileInfoCacheObserver) {
+  TestingProfile* profile1 = profile_manager_.CreateTestingProfile("p1");
+  TestBackgroundModeManager manager(
+      command_line_.get(), profile_manager_.profile_info_cache());
+  manager.RegisterProfile(profile1);
+  EXPECT_FALSE(BrowserList::WillKeepAlive());
+
+  ProfileInfoCache* cache = profile_manager_.profile_info_cache();
+
+  // Install app, should show status tray icon.
+  manager.OnBackgroundAppInstalled(NULL);
+  manager.SetBackgroundAppCount(1);
+  manager.SetBackgroundAppCountForProfile(1);
+  manager.OnApplicationListChanged(profile1);
+
+  string16 p1name = cache->GetNameOfProfileAtIndex(0);
+  manager.OnProfileNameChanged(p1name, UTF8ToUTF16("p1new"));
+
+  EXPECT_EQ(UTF8ToUTF16("p1new"),
+            manager.GetBackgroundModeData(profile1)->name());
+
+  TestingProfile* profile2 = profile_manager_.CreateTestingProfile("p2");
+  manager.RegisterProfile(profile2);
+  EXPECT_EQ(2, manager.NumberOfBackgroundModeData());
+
+  gfx::Image gaia_image(gfx::test::CreateImage());
+  manager.OnProfileAdded(UTF8ToUTF16("p2new"),
+                         profile2->GetPath().BaseName().LossyDisplayName(),
+                         profile2->GetPath(),
+                         &gaia_image);
+  EXPECT_EQ(UTF8ToUTF16("p2new"),
+            manager.GetBackgroundModeData(profile2)->name());
+
+  manager.OnProfileRemoved(UTF8ToUTF16("p2new"));
+  EXPECT_EQ(1, manager.NumberOfBackgroundModeData());
+
+  // Check that the background mode data we think is in the map actually is.
+  EXPECT_EQ(UTF8ToUTF16("p1new"),
+            manager.GetBackgroundModeData(profile1)->name());
+}