Fix racing condition that blocks profiles menu showing up

Chrome should show profiles menu when "enable-udd-profiles" is supplied
on command line. This is broken while refactoring app menu code from
ToolbarView into AppMenuModel. Previously, the profile submenu model is
kept in ToolbarView and profiles menu would show up on 2nd time app menu
showing up. After the refactoring, profile menu is moved into
AppMenuModel, which is re-created everytime before we show app menu. In
AppMenuModel, it requests profiles data from GetProfilesHelper which
will later invoke OnGetProfilesDone. However, app menu is created on the
UI thread and blocks the callback until it shows up. This makes the
profiles mneu empty.
The fix is to leverage an existing profile list in BrowserProcess. When
"enable-udd-profiles" is on, BrowserInit will get the initial profile
list and NewProfileDialog will refresh it if user creates new profile.
And AppMenuModel just use the list to populate the profile menu.

BUG=30417
TEST=Verify profiles menu exists when "enable-udd-profiles" is on command line for issue 30417.

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

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

6 files changed, 70 insertions, 18 deletions

diff --git a/chrome/browser/app_menu_model.cc b/chrome/browser/app_menu_model.cc
index 5cd69f3..020e1b1 100644
--- a/chrome/browser/app_menu_model.cc
+++ b/chrome/browser/app_menu_model.cc
@@ -18,14 +18,11 @@
 AppMenuModel::AppMenuModel(menus::SimpleMenuModel::Delegate* delegate,
                            Browser* browser)
     : menus::SimpleMenuModel(delegate),
-      ALLOW_THIS_IN_INITIALIZER_LIST(
-          profiles_helper_(new GetProfilesHelper(this))),
       browser_(browser) {
   Build();
 }
 
 AppMenuModel::~AppMenuModel() {
-  profiles_helper_->OnDelegateDeleted();
 }
 
 void AppMenuModel::Build() {
@@ -37,9 +34,12 @@ void AppMenuModel::Build() {
   // done.  See OnGetProfilesDone().
   const CommandLine& command_line = *CommandLine::ForCurrentProcess();
   if (command_line.HasSwitch(switches::kEnableUserDataDirProfiles)) {
+    // Triggers profile list refresh in case it's changed.
+    UserDataManager::Get()->RefreshUserDataDirProfiles();
+
     if (!profiles_menu_contents_.get()) {
-      profiles_helper_->GetProfiles(NULL);
       profiles_menu_contents_.reset(new menus::SimpleMenuModel(delegate()));
+      BuildProfileSubMenu();
     }
     AddSubMenuWithStringId(IDS_PROFILE_MENU, profiles_menu_contents_.get());
   }
@@ -92,14 +92,14 @@ void AppMenuModel::Build() {
   }
 }
 
-void AppMenuModel::OnGetProfilesDone(
-    const std::vector<std::wstring>& profiles) {
+void AppMenuModel::BuildProfileSubMenu() {
   // Nothing to do if the menu has gone away.
   if (!profiles_menu_contents_.get())
     return;
 
-  // Store the latest list of profiles in the browser.
-  browser_->set_user_data_dir_profiles(profiles);
+  // Use the list of profiles in the browser.
+  const std::vector<std::wstring>& profiles =
+      browser_->user_data_dir_profiles();
 
   // Add direct submenu items for profiles.
   std::vector<std::wstring>::const_iterator iter = profiles.begin();
diff --git a/chrome/browser/app_menu_model.h b/chrome/browser/app_menu_model.h
index d298a86..41343d4 100644
--- a/chrome/browser/app_menu_model.h
+++ b/chrome/browser/app_menu_model.h
@@ -14,26 +14,19 @@ class Browser;
 
 // A menu model that builds the contents of the app menu. This menu has only
 // one level (no submenus).
-class AppMenuModel : public menus::SimpleMenuModel,
-                     public GetProfilesHelper::Delegate {
+class AppMenuModel : public menus::SimpleMenuModel {
  public:
   explicit AppMenuModel(menus::SimpleMenuModel::Delegate* delegate,
                         Browser* browser);
   virtual ~AppMenuModel();
 
-  // Overridden from GetProfilesHelper::Delegate
-  virtual void OnGetProfilesDone(
-      const std::vector<std::wstring>& profiles);
-
  private:
   void Build();
+  void BuildProfileSubMenu();
 
   // Contents of the profiles menu to populate with profile names.
   scoped_ptr<menus::SimpleMenuModel> profiles_menu_contents_;
 
-  // Helper class to enumerate profiles information on the file thread.
-  scoped_refptr<GetProfilesHelper> profiles_helper_;
-
   Browser* browser_;  // weak
 
   DISALLOW_COPY_AND_ASSIGN(AppMenuModel);
diff --git a/chrome/browser/browser_init.cc b/chrome/browser/browser_init.cc
index 3ccb82f..568d0fd 100644
--- a/chrome/browser/browser_init.cc
+++ b/chrome/browser/browser_init.cc
@@ -30,6 +30,7 @@
 #include "chrome/browser/tab_contents/tab_contents.h"
 #include "chrome/browser/tab_contents/tab_contents_view.h"
 #include "chrome/browser/net/url_fixer_upper.h"
+#include "chrome/browser/user_data_manager.h"
 #include "chrome/common/chrome_constants.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_switches.h"
@@ -837,6 +838,14 @@ bool BrowserInit::ProcessCmdLineImpl(const CommandLine& command_line,
     net::SetExplicitlyAllowedPorts(allowed_ports);
   }
 
+  if (command_line.HasSwitch(switches::kEnableUserDataDirProfiles)) {
+    // Update user data dir profiles when kEnableUserDataDirProfiles is enabled.
+    // Profile enumeration would be scheduled on file thread and when
+    // enumeration is done, the profile list in BrowserProcess would be
+    // updated on ui thread.
+    UserDataManager::Get()->RefreshUserDataDirProfiles();
+  }
+
   // If we don't want to launch a new browser window or tab (in the case
   // of an automation request), we are done here.
   if (!silent_launch) {
diff --git a/chrome/browser/user_data_manager.cc b/chrome/browser/user_data_manager.cc
index 588f699..e7d71c2 100644
--- a/chrome/browser/user_data_manager.cc
+++ b/chrome/browser/user_data_manager.cc
@@ -15,6 +15,7 @@
 #include "base/path_service.h"
 #include "base/process_util.h"
 #include "base/string_util.h"
+#include "chrome/browser/browser_process.h"
 #include "chrome/browser/chrome_thread.h"
 #include "chrome/common/chrome_constants.h"
 #include "chrome/common/chrome_paths.h"
@@ -64,7 +65,23 @@ class LaunchChromeForProfileIndexHelper : GetProfilesHelper::Delegate {
   DISALLOW_COPY_AND_ASSIGN(LaunchChromeForProfileIndexHelper);
 };
 
-}  // namespace
+// Helper to update global user data dir profiles list.
+class UserDataDirProfilesUpdater : GetProfilesHelper::Delegate {
+ public:
+  UserDataDirProfilesUpdater();
+  virtual ~UserDataDirProfilesUpdater();
+
+  // Request profiles via GetProfilesHelper.
+  void Run();
+
+  // GetProfilesHelper::Delegate method.
+  void OnGetProfilesDone(const std::vector<std::wstring>& profiles);
+
+ private:
+  scoped_refptr<GetProfilesHelper> profiles_helper_;
+
+  DISALLOW_COPY_AND_ASSIGN(UserDataDirProfilesUpdater);
+};
 
 LaunchChromeForProfileIndexHelper::LaunchChromeForProfileIndexHelper(
     const UserDataManager* manager,
@@ -93,6 +110,29 @@ void LaunchChromeForProfileIndexHelper::OnGetProfilesDone(
   delete this;
 }
 
+UserDataDirProfilesUpdater::UserDataDirProfilesUpdater()
+    : ALLOW_THIS_IN_INITIALIZER_LIST(
+          profiles_helper_(new GetProfilesHelper(this))) {
+}
+
+UserDataDirProfilesUpdater::~UserDataDirProfilesUpdater() {
+  profiles_helper_->OnDelegateDeleted();
+}
+
+void UserDataDirProfilesUpdater::Run() {
+  profiles_helper_->GetProfiles(NULL);
+}
+
+void UserDataDirProfilesUpdater::OnGetProfilesDone(
+    const std::vector<std::wstring>& profiles) {
+  g_browser_process->user_data_dir_profiles() = profiles;
+
+  // We are done, delete ourselves.
+  delete this;
+}
+
+}  // namespace
+
 // Separator used in folder names between the prefix and the profile name.
 // For e.g. a folder for the profile "Joe" would be named "User Data-Joe".
 static const wchar_t kProfileFolderSeparator[] = L"-";
@@ -266,6 +306,12 @@ bool UserDataManager::CreateDesktopShortcutForProfile(
 #endif
 }
 
+void UserDataManager::RefreshUserDataDirProfiles() const {
+  // UserDataDirProfilesUpdater will delete itself when done.
+  UserDataDirProfilesUpdater* updater = new UserDataDirProfilesUpdater();
+  updater->Run();
+}
+
 GetProfilesHelper::GetProfilesHelper(Delegate* delegate)
     : delegate_(delegate),
       message_loop_(NULL) {
diff --git a/chrome/browser/user_data_manager.h b/chrome/browser/user_data_manager.h
index 14b9faa..f7dfe17 100644
--- a/chrome/browser/user_data_manager.h
+++ b/chrome/browser/user_data_manager.h
@@ -65,6 +65,9 @@ class UserDataManager {
   // on a separate thread.
   void LaunchChromeForProfile(int index) const;
 
+  // Updates global user data dir profile list stored in g_browser_process.
+  void RefreshUserDataDirProfiles() const;
+
  private:
   // Gets the name of the profile from the name of the folder.
   // Returns false if the folder does not correspond to a profile folder, true
diff --git a/chrome/browser/views/new_profile_dialog.cc b/chrome/browser/views/new_profile_dialog.cc
index 54ac154..f580bb0 100644
--- a/chrome/browser/views/new_profile_dialog.cc
+++ b/chrome/browser/views/new_profile_dialog.cc
@@ -104,6 +104,7 @@ bool NewProfileDialog::Accept() {
         profile_name);
 
   UserDataManager::Get()->LaunchChromeForProfile(profile_name);
+  UserDataManager::Get()->RefreshUserDataDirProfiles();
   return true;
 }