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

chrome/browser/background/background_mode_manager.cc

Index: chrome/browser/background/background_mode_manager.cc
===================================================================
--- chrome/browser/background/background_mode_manager.cc	(revision 111700)
+++ chrome/browser/background/background_mode_manager.cc	(working copy)
@@ -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 @@
     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 @@
   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() {
   // 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 @@
   // 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,15 @@
                            NEW_FOREGROUND_TAB);
 }
 
+int BackgroundModeManager::NumberOfBackgroundModeData() {
+  return background_mode_data_.size();
+}
+
+string16 BackgroundModeManager::BackgroundModeDataName(Profile* profile) {
+  return background_mode_data_[profile]->name();

[Andrew T Wilson (Slow), 2011/12/06 06:46:02]

Perhaps we should add a DCHECK(background_mode_data_.count(profile) > 0) to make
sure the passed profile exists in the map?

[rpetterson, 2011/12/06 20:09:05]

Function removed.

+}
+
+
 ///////////////////////////////////////////////////////////////////////////////
 //  BackgroundModeManager, content::NotificationObserver overrides
 void BackgroundModeManager::Observe(
@@ -299,7 +323,7 @@
       // 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) {
@@ -364,6 +388,49 @@
 }
 
 ///////////////////////////////////////////////////////////////////////////////
+//  BackgroundModeManager, ProfileInfoCacheObserver overrides
+void BackgroundModeManager::OnProfileAdded(const string16& profile_name,
+                                           const string16& profile_base_dir) {
+  // 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().BaseName().LossyDisplayName() ==
+          profile_base_dir) {
+      it->second->SetName(profile_name);
+      UpdateStatusTrayIconContextMenu();

[Andrew T Wilson (Slow), 2011/12/06 06:46:02]

Should we break out of the loop here/early return?

[rpetterson, 2011/12/06 20:09:05]

Done.

+    }
+  }
+}
+
+void BackgroundModeManager::OnProfileRemoved(const string16& profile_name) {
+  // Remove the profile from our map of profiles

[Andrew T Wilson (Slow), 2011/12/06 06:46:02]

nit: period at the end of the comment.

[rpetterson, 2011/12/06 20:09:05]

Done.

+  BackgroundModeInfoMap::iterator it =
+      GetBackgroundModeIterator(profile_name);
+  // If a profile isn't running a background app, it may not be in the map

[Andrew T Wilson (Slow), 2011/12/06 06:46:02]

nit: period at end of comment

[rpetterson, 2011/12/06 20:09:05]

Done.

+  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 for if due to unittests, but really this should only be called on

[Andrew T Wilson (Slow), 2011/12/06 06:46:02]

nit: "We check for if" - did you omit a word here?

[rpetterson, 2011/12/06 20:09:05]

Done.

+  // profiles already known by the background mode manager.
+  if (it != background_mode_data_.end()) {
+    it->second->SetName(new_profile_name);
+    UpdateStatusTrayIconContextMenu();
+  }
+}
+
+///////////////////////////////////////////////////////////////////////////////
 //  BackgroundModeManager::BackgroundModeData, ui::SimpleMenuModel overrides
 bool BackgroundModeManager::IsCommandIdChecked(
     int command_id) const {
@@ -489,7 +556,7 @@
 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 +655,7 @@
 
   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 +699,22 @@
   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) {