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

chrome/browser/background/background_mode_manager.cc

 // Copyright (c) 2011 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 <string>
 
 #include "base/base_paths.h"
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/logging.h"
@@ skipping 25 matching lines @@
 #include "ui/base/resource/resource_bundle.h"
 
 BackgroundModeManager::BackgroundModeData::BackgroundModeData(
     int command_id,
     Profile* profile,
     BackgroundModeManager* background_mode_manager)
     : applications_(new BackgroundApplicationListModel(profile)),
       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);
 }
 
 BackgroundModeManager::BackgroundModeData::~BackgroundModeData() {
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 //  BackgroundModeManager::BackgroundModeData, ui::SimpleMenuModel overrides
 bool BackgroundModeManager::BackgroundModeData::IsCommandIdChecked(
@@ skipping 57 matching lines @@
       const std::string& name = (*cursor)->name();
       menu->AddItem(position, UTF8ToUTF16(name));
       if (icon)
         menu->SetIcon(menu->GetItemCount() - 1, *icon);
     }
   }
   if (containing_menu)
     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,
     const BackgroundModeData* bmd2) {
   return bmd1->name_ < bmd2->name_;
 }
 
 
 ///////////////////////////////////////////////////////////////////////////////
 //  BackgroundModeManager, public
@@ skipping 10 matching lines @@
       current_command_id_(0) {
   // We should never start up if there is no browser process or if we are
   // currently quitting.
   CHECK(g_browser_process != NULL);
   CHECK(!browser_shutdown::IsTryingToQuit());
   // If background mode is currently disabled, just exit - don't listen for any
   // notifications.
   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());
     pref_registrar_.Add(prefs::kBackgroundModeEnabled, this);
   }
 
   // Keep the browser alive until extensions are done loading - this is needed
   // by the --no-startup-window flag. We want to stay alive until we load
   // extensions, at which point we should either run in background mode (if
   // there are background apps) or exit if there are none.
@@ skipping 12 matching lines @@
 
   // Listen for the application shutting down so we can decrement our KeepAlive
   // count.
   registrar_.Add(this, content::NOTIFICATION_APP_TERMINATING,
                  content::NotificationService::AllSources());
 }
 
 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) {
     it->second->applications_->RemoveObserver(this);
   }
 
   // We're going away, so exit background mode (does nothing if we aren't in
   // background mode currently). This is primarily needed for unit tests,
   // because in an actual running system we'd get an APP_TERMINATING
   // notification before being destroyed.
@@ skipping 30 matching lines @@
                  content::Source<Profile>(profile));
 
   bmd->applications_->AddObserver(this);
 
   // If we're adding a new profile and running in multi-profile mode, this new
   // profile should be added to the status icon if one currently exists.
   if (in_background_mode_ && status_icon_)
     UpdateStatusTrayIconContextMenu();
 }
 
+ProfileInfoCache* BackgroundModeManager::profile_cache() {
+  return profile_cache_;
+}
+
 // static
 void BackgroundModeManager::LaunchBackgroundApplication(
     Profile* profile,
     const Extension* extension) {
   ExtensionService* service = profile->GetExtensionService();
   extension_misc::LaunchContainer launch_container =
       service->extension_prefs()->GetLaunchContainer(
           extension, ExtensionPrefs::LAUNCH_REGULAR);
   Browser::OpenApplication(profile, extension, launch_container, GURL(),
                            NEW_FOREGROUND_TAB);
@@ skipping 45 matching lines @@
     case content::NOTIFICATION_APP_TERMINATING:
       // Make sure we aren't still keeping the app alive (only happens if we
       // don't receive an EXTENSIONS_READY notification for some reason).
       EndKeepAliveForStartup();
       // Performing an explicit shutdown, so exit background mode (does nothing
       // if we aren't in background mode currently).
       EndBackgroundMode();
       // 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) {
         it->second->applications_->RemoveObserver(this);
       }
       break;
     default:
       NOTREACHED();
       break;
   }
@@ skipping 42 matching lines @@
 #if !defined(OS_MACOSX)
       EnableLaunchOnStartup(true);
 #endif
 
       StartBackgroundMode();
     }
     // List of applications changed so update the UI.
     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(
     int command_id) const {
   DCHECK(command_id == IDC_STATUS_TRAY_KEEP_CHROME_RUNNING_IN_BACKGROUND);
   return true;
 }
 
 bool BackgroundModeManager::IsCommandIdEnabled(
@@ skipping 107 matching lines @@
   // If background mode is currently enabled, turn it off.
   if (in_background_mode_) {
     EndBackgroundMode();
     EnableLaunchOnStartup(false);
   }
 }
 
 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) {
     count += it->second->GetBackgroundAppCount();
   }
   DCHECK(count >= 0);
   return count;
 }
 
 int BackgroundModeManager::GetBackgroundAppCountForProfile(
@@ skipping 78 matching lines @@
   // Create a context menu item for Chrome.
   ui::SimpleMenuModel* menu = new ui::SimpleMenuModel(this);
   // Add About item
   menu->AddItem(IDC_ABOUT, l10n_util::GetStringFUTF16(IDS_ABOUT,
       l10n_util::GetStringUTF16(IDS_PRODUCT_NAME)));
   menu->AddItemWithStringId(IDC_TASK_MANAGER, IDS_TASK_MANAGER);
   menu->AddSeparator();
 
   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) {
        bmd_vector.push_back(it->second.get());
     }
     std::sort(bmd_vector.begin(), bmd_vector.end(),
               &BackgroundModeData::BackgroundModeDataCompare);
     for (std::vector<BackgroundModeData*>::const_iterator bmd_it =
          bmd_vector.begin();
          bmd_it != bmd_vector.end();
@@ skipping 23 matching lines @@
   status_icon_ = NULL;
   context_menu_ = NULL;
 }
 
 BackgroundModeManager::BackgroundModeData*
 BackgroundModeManager::GetBackgroundModeData(Profile* const profile) const {
   DCHECK(background_mode_data_.find(profile) != background_mode_data_.end());
   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) {
 
   // Background mode is disabled if the appropriate flag is passed, or if
   // extensions are disabled, or if the associated preference is unset. It's
   // always disabled on chromeos since chrome is always running on that
   // platform, making it superfluous.
 #if defined(OS_CHROMEOS)
   if (command_line->HasSwitch(switches::kKeepAliveForTest))
       return false;
   return true;
 #else
   bool background_mode_disabled =
       command_line->HasSwitch(switches::kDisableBackgroundMode) ||
       command_line->HasSwitch(switches::kDisableExtensions);
   return background_mode_disabled;
 #endif
 }
 
 bool BackgroundModeManager::IsBackgroundModePrefEnabled() const {
   PrefService* service = g_browser_process->local_state();
   DCHECK(service);
   return service->GetBoolean(prefs::kBackgroundModeEnabled);
 }