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)
@@ -43,7 +43,10 @@
       command_id_(command_id),
       profile_(profile),
       background_mode_manager_(background_mode_manager) {
-  name_ = UTF8ToUTF16(profile_->GetProfileName());
+  ProfileInfoCache* cache = background_mode_manager_->profile_cache();
+  size_t index = cache->GetIndexOfProfileWithPath(profile_->GetPath());
+  if (index != std::string::npos)

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

It seems weird to use std::string::npos here, which seems like it's specifically
for string indices? Is this the standard idiom we're using with
GetIndexOfProfileWithPath()?

[rpetterson, 2011/12/06 05:52:34]

I agree that it seems odd. I was originally checking if index was less than the
total number of profiles. But GetIndexOfProfileWithPath returns
std::string::npos if the path isn't found so I thought it best to use that.

+    name_ = cache->GetNameOfProfileAtIndex(index);

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

Also, it's somewhat odd to have this split in responsibilities between
BackgroundModeManager and BackgroundModeManagerData - it's not clear who is
responsible for setting the name (if the name is available at constructor time,
then BMD is responsible, otherwise BMM is responsible).

I'd suggest moving all of the "set the name" logic/responsibilities into one or
the other class - either have BMD register as an observer on ProfileInfoCache
and notify BackgroundModeManager when its name has changed, or else remove this
logic from the BMD constructor, and just have BackgroundModeManager always
explicitly call SetName() after creating BackgroundModeData in
RegisterProfile().

[rpetterson, 2011/12/06 05:52:34]

I moved everything to the BMM. I would have preferred the BMD, but due to
removing from the list in OnProfileRemoved, it couldn't be in the BMD.

   if (name_.empty())
     name_ = l10n_util::GetStringUTF16(IDS_PROFILES_DEFAULT_NAME);
 }
@@ -121,6 +124,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 +163,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 +199,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) {
@@ -234,6 +250,10 @@
     UpdateStatusTrayIconContextMenu();
 }
 
+ProfileInfoCache* BackgroundModeManager::profile_cache() {
+  return profile_cache_;
+}
+
 // static
 void BackgroundModeManager::LaunchBackgroundApplication(
     Profile* profile,
@@ -299,7 +319,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) {
@@ -362,7 +382,45 @@
     UpdateStatusTrayIconContextMenu();
   }
 }
+///////////////////////////////////////////////////////////////////////////////
+//  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() == FilePath(profile_base_dir)) {
+      it->second->SetName(profile_name);
+      UpdateStatusTrayIconContextMenu();
+    }
+  }
+}
 
+void BackgroundModeManager::OnProfileRemoved(const string16& profile_name) {
+  // Remove the profile from our map of profiles
+  BackgroundModeInfoMap::const_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);
+  DCHECK(it != background_mode_data_.end());
+  it->second->SetName(new_profile_name);
+  UpdateStatusTrayIconContextMenu();
+}
+
 ///////////////////////////////////////////////////////////////////////////////
 //  BackgroundModeManager::BackgroundModeData, ui::SimpleMenuModel overrides
 bool BackgroundModeManager::IsCommandIdChecked(
@@ -489,7 +547,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 +646,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 +690,22 @@
   return background_mode_data_.find(profile)->second.get();
 }
 
+BackgroundModeManager::BackgroundModeInfoMap::const_iterator
+BackgroundModeManager::GetBackgroundModeIterator(
+    const string16& profile_name) const {
+  BackgroundModeInfoMap::const_iterator profile_it =
+      background_mode_data_.end();
+  for (BackgroundModeInfoMap::const_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) {