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 19 matching lines @@
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_types.h"
 #include "grit/chromium_strings.h"
 #include "grit/generated_resources.h"
 #include "grit/theme_resources.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/resource/resource_bundle.h"
 
 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() {
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 //  BackgroundModeManager::BackgroundModeData, ui::SimpleMenuModel overrides
 bool BackgroundModeManager::BackgroundModeData::IsCommandIdChecked(
     int command_id) const {
   DCHECK(command_id == IDC_STATUS_TRAY_KEEP_CHROME_RUNNING_IN_BACKGROUND);
@@ skipping 55 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.
   EndBackgroundMode();
 }
 
 // static
 void BackgroundModeManager::RegisterPrefs(PrefService* prefs) {
   prefs->RegisterBooleanPref(prefs::kUserCreatedLoginItem, false);
   prefs->RegisterBooleanPref(prefs::kBackgroundModeEnabled, true);
 }
 
 
 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.
   registrar_.Add(this, chrome::NOTIFICATION_EXTENSION_LOADED,
                  content::Source<Profile>(profile));
   registrar_.Add(this, chrome::NOTIFICATION_EXTENSION_PERMISSIONS_UPDATED,
                  content::Source<Profile>(profile));
 
 
   // Check for the presence of background apps after all extensions have been
@@ skipping 15 matching lines @@
     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);
 }
 
+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(
     int type,
     const content::NotificationSource& source,
     const content::NotificationDetails& details) {
   switch (type) {
     case chrome::NOTIFICATION_PREF_CHANGED:
       DCHECK(*content::Details<std::string>(details).ptr() ==
              prefs::kBackgroundModeEnabled);
@@ skipping 33 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 44 matching lines @@
 #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().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 {
   DCHECK(command_id == IDC_STATUS_TRAY_KEEP_CHROME_RUNNING_IN_BACKGROUND);
   return true;
 }
 
 bool BackgroundModeManager::IsCommandIdEnabled(
     int command_id) const {
   return command_id != IDC_MinimumLabelValue;
@@ skipping 105 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::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) {
 
   // 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);
 }