Show user data dialog earlier on Windows.

On Windows if the user data directory does not exist, we must show the user
data dir picker dialog prior to creating the local state to avoid a CHECK
failure. After obtaining the new dir we will restart chrome with the switch
--user-data-dir appended to the command line.

BUG=196301
TEST=CQ


Review URL: https://chromiumcodereview.appspot.com/12662033

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

13 files changed, 234 insertions, 36 deletions

diff --git a/chrome/browser/browser_process.h b/chrome/browser/browser_process.h
index c8286bc..a8be8f3 100644
--- a/chrome/browser/browser_process.h
+++ b/chrome/browser/browser_process.h
@@ -225,6 +225,8 @@ class BrowserProcess {
   virtual void PlatformSpecificCommandLineProcessing(
       const CommandLine& command_line) = 0;
 
+  virtual bool created_local_state() const = 0;
+
  private:
   DISALLOW_COPY_AND_ASSIGN(BrowserProcess);
 };
diff --git a/chrome/browser/browser_process_impl.cc b/chrome/browser/browser_process_impl.cc
index 00f01b8eb..0f6b4c0 100644
--- a/chrome/browser/browser_process_impl.cc
+++ b/chrome/browser/browser_process_impl.cc
@@ -638,6 +638,10 @@ void BrowserProcessImpl::PlatformSpecificCommandLineProcessing(
 }
 #endif
 
+bool BrowserProcessImpl::created_local_state() const {
+    return created_local_state_;
+}
+
 // static
 void BrowserProcessImpl::RegisterPrefs(PrefRegistrySimple* registry) {
   registry->RegisterBooleanPref(prefs::kDefaultBrowserSettingEnabled,
diff --git a/chrome/browser/browser_process_impl.h b/chrome/browser/browser_process_impl.h
index 0213a72..0d2199f 100644
--- a/chrome/browser/browser_process_impl.h
+++ b/chrome/browser/browser_process_impl.h
@@ -137,6 +137,7 @@ class BrowserProcessImpl : public BrowserProcess,
       media_file_system_registry() OVERRIDE;
   virtual void PlatformSpecificCommandLineProcessing(
       const CommandLine& command_line) OVERRIDE;
+  virtual bool created_local_state() const OVERRIDE;
 
   static void RegisterPrefs(PrefRegistrySimple* registry);
 
diff --git a/chrome/browser/chrome_browser_main.cc b/chrome/browser/chrome_browser_main.cc
index 4df00b0..7a5747a 100644
--- a/chrome/browser/chrome_browser_main.cc
+++ b/chrome/browser/chrome_browser_main.cc
@@ -103,6 +103,7 @@
 #include "chrome/browser/ui/uma_browsing_activity_observer.h"
 #include "chrome/browser/ui/user_data_dir_dialog.h"
 #include "chrome/browser/ui/webui/chrome_web_ui_controller_factory.h"
+#include "chrome/browser/user_data_dir_extractor.h"
 #include "chrome/common/child_process_logging.h"
 #include "chrome/common/chrome_constants.h"
 #include "chrome/common/chrome_paths.h"
@@ -386,35 +387,10 @@ Profile* CreateProfile(const content::MainFunctionParams& parameters,
   if (profile)
     return profile;
 
-#if defined(OS_WIN)
-#if defined(USE_AURA)
-  // TODO(beng):
-  NOTIMPLEMENTED();
-#else
-  // Ideally, we should be able to run w/o access to disk.  For now, we
-  // prompt the user to pick a different user-data-dir and restart chrome
-  // with the new dir.
-  // http://code.google.com/p/chromium/issues/detail?id=11510
-  base::FilePath new_user_data_dir =
-      chrome::ShowUserDataDirDialog(user_data_dir);
-
-  if (!new_user_data_dir.empty()) {
-    // Because of the way CommandLine parses, it's sufficient to append a new
-    // --user-data-dir switch.  The last flag of the same name wins.
-    // TODO(tc): It would be nice to remove the flag we don't want, but that
-    // sounds risky if we parse differently than CommandLineToArgvW.
-    CommandLine new_command_line = parameters.command_line;
-    new_command_line.AppendSwitchPath(switches::kUserDataDir,
-                                      new_user_data_dir);
-    base::LaunchProcess(new_command_line, base::LaunchOptions(), NULL);
-  }
-#endif
-#else
   // TODO(port): fix this.  See comments near the definition of
   // user_data_dir.  It is better to CHECK-fail here than it is to
   // silently exit because of missing code in the above test.
   CHECK(profile) << "Cannot get default profile.";
-#endif
 
   return NULL;
 }
@@ -806,17 +782,7 @@ int ChromeBrowserMainParts::PreCreateThreads() {
 
 int ChromeBrowserMainParts::PreCreateThreadsImpl() {
   run_message_loop_ = false;
-#if defined(OS_WIN)
-  PathService::Get(chrome::DIR_USER_DATA, &user_data_dir_);
-#else
-  // Getting the user data dir can fail if the directory isn't
-  // creatable, for example; on Windows in code below we bring up a
-  // dialog prompting the user to pick a different directory.
-  // However, ProcessSingleton needs a real user_data_dir on Mac/Linux,
-  // so it's better to fail here than fail mysteriously elsewhere.
-  CHECK(PathService::Get(chrome::DIR_USER_DATA, &user_data_dir_))
-      << "Must be able to get user data directory!";
-#endif
+  user_data_dir_ = chrome::GetUserDataDir(parameters());
 
   // Whether this is First Run. |do_first_run_tasks_| should be prefered to this
   // unless the desire is actually to know whether this is really First Run
diff --git a/chrome/browser/user_data_dir_extractor.cc b/chrome/browser/user_data_dir_extractor.cc
new file mode 100644
index 0000000..45084c3
--- /dev/null
+++ b/chrome/browser/user_data_dir_extractor.cc
@@ -0,0 +1,26 @@
+// Copyright (c) 2013 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 "chrome/browser/user_data_dir_extractor.h"
+
+#include "base/files/file_path.h"
+#include "base/logging.h"
+#include "base/path_service.h"
+#include "chrome/common/chrome_paths.h"
+
+namespace chrome {
+
+base::FilePath GetUserDataDir(const content::MainFunctionParams& parameters) {
+  base::FilePath user_data_dir;
+
+  // Getting the user data dir can fail if the directory isn't creatable, for
+  // example: on Windows we bring up a dialog prompting the user to pick a
+  // different directory. However, ProcessSingleton needs a real user_data_dir
+  // on Mac/Linux, so it's better to fail here than fail mysteriously elsewhere.
+  CHECK(PathService::Get(chrome::DIR_USER_DATA, &user_data_dir))
+      << "Must be able to get user data directory!";
+  return user_data_dir;
+}
+
+}  // namespace chrome
diff --git a/chrome/browser/user_data_dir_extractor.h b/chrome/browser/user_data_dir_extractor.h
new file mode 100644
index 0000000..5e8af0a
--- /dev/null
+++ b/chrome/browser/user_data_dir_extractor.h
@@ -0,0 +1,23 @@
+// Copyright (c) 2013 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.
+
+#ifndef CHROME_BROWSER_USER_DATA_DIR_EXTRACTOR_H_
+#define CHROME_BROWSER_USER_DATA_DIR_EXTRACTOR_H_
+
+namespace base {
+class FilePath;
+}
+
+namespace content{
+struct MainFunctionParams;
+}
+
+namespace chrome {
+
+// Returns the user data dir. Must be called prior to InitializeLocalState().
+base::FilePath GetUserDataDir(const content::MainFunctionParams& parameters);
+
+}  // namespace chrome
+
+#endif  // CHROME_BROWSER_USER_DATA_DIR_EXTRACTOR_H_
diff --git a/chrome/browser/user_data_dir_extractor_win.cc b/chrome/browser/user_data_dir_extractor_win.cc
new file mode 100644
index 0000000..5c4b941
--- /dev/null
+++ b/chrome/browser/user_data_dir_extractor_win.cc
@@ -0,0 +1,68 @@
+// Copyright (c) 2013 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 "chrome/browser/user_data_dir_extractor_win.h"
+
+#include "base/command_line.h"
+#include "base/file_util.h"
+#include "base/logging.h"
+#include "base/path_service.h"
+#include "base/process_util.h"
+#include "chrome/browser/ui/user_data_dir_dialog.h"
+#include "chrome/browser/user_data_dir_extractor.h"
+#include "chrome/common/chrome_paths.h"
+#include "chrome/common/chrome_switches.h"
+#include "content/public/common/main_function_params.h"
+
+namespace chrome {
+
+namespace {
+
+GetUserDataDirCallback* custom_get_user_data_dir_callback = NULL;
+
+}  // namespace
+
+void InstallCustomGetUserDataDirCallbackForTest(
+    GetUserDataDirCallback* callback) {
+  custom_get_user_data_dir_callback = callback;
+}
+
+base::FilePath GetUserDataDir(const content::MainFunctionParams& parameters) {
+  // If tests have installed a custom callback for GetUserDataDir(), invoke the
+  // callback and return.
+  if (custom_get_user_data_dir_callback)
+    return custom_get_user_data_dir_callback->Run();
+
+  base::FilePath user_data_dir;
+  PathService::Get(chrome::DIR_USER_DATA, &user_data_dir);
+
+  // On Windows, if we fail to get the user data dir, bring up a dialog to
+  // prompt the user to pick a different directory, and restart chrome with
+  // the new dir.
+  // http://code.google.com/p/chromium/issues/detail?id=11510
+  if (!file_util::PathExists(user_data_dir)) {
+#if defined(USE_AURA)
+    // TODO(beng):
+    NOTIMPLEMENTED();
+#else
+    base::FilePath new_user_data_dir =
+        chrome::ShowUserDataDirDialog(user_data_dir);
+
+    if (!new_user_data_dir.empty()) {
+      // Because of the way CommandLine parses, it's sufficient to append a new
+      // --user-data-dir switch.  The last flag of the same name wins.
+      // TODO(tc): It would be nice to remove the flag we don't want, but that
+      // sounds risky if we parse differently than CommandLineToArgvW.
+      CommandLine new_command_line = parameters.command_line;
+      new_command_line.AppendSwitchPath(switches::kUserDataDir,
+                                        new_user_data_dir);
+      base::LaunchProcess(new_command_line, base::LaunchOptions(), NULL);
+    }
+#endif
+    NOTREACHED() << "Failed to get user data directory!";
+  }
+  return user_data_dir;
+}
+
+}  // namespace chrome
diff --git a/chrome/browser/user_data_dir_extractor_win.h b/chrome/browser/user_data_dir_extractor_win.h
new file mode 100644
index 0000000..bff11a7
--- /dev/null
+++ b/chrome/browser/user_data_dir_extractor_win.h
@@ -0,0 +1,22 @@
+// Copyright (c) 2013 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.
+
+#ifndef CHROME_BROWSER_USER_DATA_DIR_EXTRACTOR_WIN_H_
+#define CHROME_BROWSER_USER_DATA_DIR_EXTRACTOR_WIN_H_
+
+#include "base/callback.h"
+#include "base/files/file_path.h"
+
+namespace chrome {
+
+typedef base::Callback<base::FilePath()> GetUserDataDirCallback;
+
+// Tests can call this to install a custom callback to be called when
+// GetUserDataDir() is invoked.
+void InstallCustomGetUserDataDirCallbackForTest(
+    GetUserDataDirCallback* callback);
+
+}  // namespace chrome
+
+#endif  // CHROME_BROWSER_USER_DATA_DIR_EXTRACTOR_WIN_H_
diff --git a/chrome/browser/user_data_dir_extractor_win_browsertest.cc b/chrome/browser/user_data_dir_extractor_win_browsertest.cc
new file mode 100644
index 0000000..fa8483c
--- /dev/null
+++ b/chrome/browser/user_data_dir_extractor_win_browsertest.cc
@@ -0,0 +1,74 @@
+// Copyright (c) 2012 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 "chrome/test/ui/ui_test.h"
+
+#include "base/command_line.h"
+#include "base/file_util.h"
+#include "base/path_service.h"
+#include "base/process_util.h"
+#include "chrome/browser/browser_process.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_commands.h"
+#include "chrome/browser/user_data_dir_extractor_win.h"
+#include "chrome/common/chrome_notification_types.h"
+#include "chrome/common/chrome_paths.h"
+#include "chrome/common/chrome_switches.h"
+#include "chrome/test/base/in_process_browser_test.h"
+#include "chrome/test/base/ui_test_utils.h"
+
+// These tests only apply to the Windows version; see chrome_browser_main.cc:
+// function GetUserDataDir() for details.
+class ChromeMainUserDataDirTest : public InProcessBrowserTest {
+ public:
+  ChromeMainUserDataDirTest()
+      : did_get_user_data_dir_(false),
+        did_create_local_state_before_get_user_data_dir_(false) {}
+
+  bool did_get_user_data_dir() const { return did_get_user_data_dir_; }
+  bool did_create_local_state_before_get_user_data_dir() const {
+    return did_create_local_state_before_get_user_data_dir_;
+  }
+
+ protected:
+  virtual void SetUp() OVERRIDE {
+    // Install custom GetUserDataDir callback.
+    callback_ = base::Bind(&ChromeMainUserDataDirTest::GetUserDataDirCallback,
+                           this);
+    chrome::InstallCustomGetUserDataDirCallbackForTest(&callback_);
+
+    InProcessBrowserTest::SetUp();
+  }
+
+ private:
+  base::FilePath GetUserDataDirCallback() {
+    did_get_user_data_dir_ = true;
+    did_create_local_state_before_get_user_data_dir_ =
+        g_browser_process && g_browser_process->created_local_state();
+
+    base::FilePath user_data_dir;
+    PathService::Get(chrome::DIR_USER_DATA, &user_data_dir);
+    return user_data_dir;
+  }
+
+  chrome::GetUserDataDirCallback callback_;
+  bool did_get_user_data_dir_;
+  bool did_create_local_state_before_get_user_data_dir_;
+
+  DISALLOW_COPY_AND_ASSIGN(ChromeMainUserDataDirTest);
+};
+
+IN_PROC_BROWSER_TEST_F(ChromeMainUserDataDirTest, GetUserDataDir) {
+  // Verify that GetUserDataDir() has been invoked.
+  ASSERT_TRUE(did_get_user_data_dir());
+
+  // Verify that local state was not created before invoking GetUserDataDir().
+  ASSERT_FALSE(did_create_local_state_before_get_user_data_dir());
+
+  // Verify that we have a valid browser process.
+  ASSERT_TRUE(g_browser_process);
+
+  // Verify that local state is created now.
+  ASSERT_TRUE(g_browser_process->created_local_state());
+}
diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi
index 59a9b2d..188b80f 100644
--- a/chrome/chrome_browser.gypi
+++ b/chrome/chrome_browser.gypi
@@ -2061,6 +2061,10 @@
         'browser/usb/usb_service.h',
         'browser/usb/usb_service_factory.cc',
         'browser/usb/usb_service_factory.h',
+        'browser/user_data_dir_extractor.cc',
+        'browser/user_data_dir_extractor.h',
+        'browser/user_data_dir_extractor_win.cc',
+        'browser/user_data_dir_extractor_win.h',
         'browser/user_style_sheet_watcher.cc',
         'browser/user_style_sheet_watcher.h',
         'browser/user_style_sheet_watcher_factory.cc',
@@ -2852,6 +2856,7 @@
             'browser/importer/nss_decryptor_system_nss.h',
             'browser/lifetime/application_lifetime_stub.cc',
             'browser/profiles/profile_shortcut_manager_stub.cc',
+            'browser/user_data_dir_extractor.cc',
           ],
           'conditions': [
             ['win_use_allocator_shim==1', {
diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi
index abceb29..8ca321c 100644
--- a/chrome/chrome_tests.gypi
+++ b/chrome/chrome_tests.gypi
@@ -1532,6 +1532,7 @@
         'browser/ui/webui/sync_setup_browsertest.js',
         'browser/ui/webui/web_ui_test_handler.cc',
         'browser/ui/webui/web_ui_test_handler.h',
+        'browser/user_data_dir_extractor_win_browsertest.cc',
         'browser/unload_browsertest.cc',
         'common/mac/mock_launchd.cc',
         'common/mac/mock_launchd.h',
@@ -1833,6 +1834,7 @@
             # TODO(port): http://crbug.com/45770
             'browser/printing/printing_layout_browsertest.cc',
             'browser/ui/views/app_list/app_list_controller_win_browsertest.cc',
+            'browser/user_data_dir_extractor_win_browsertest.cc',
           ],
         }],
         ['toolkit_uses_gtk == 1', {
diff --git a/chrome/test/base/testing_browser_process.cc b/chrome/test/base/testing_browser_process.cc
index 5670db1..a14834d 100644
--- a/chrome/test/base/testing_browser_process.cc
+++ b/chrome/test/base/testing_browser_process.cc
@@ -330,6 +330,10 @@ void TestingBrowserProcess::PlatformSpecificCommandLineProcessing(
     const CommandLine& command_line) {
 }
 
+bool TestingBrowserProcess::created_local_state() const {
+    return (local_state_ != NULL);
+}
+
 void TestingBrowserProcess::SetBookmarkPromptController(
     BookmarkPromptController* controller) {
 #if !defined(OS_IOS)
diff --git a/chrome/test/base/testing_browser_process.h b/chrome/test/base/testing_browser_process.h
index e583020..0d40a9c 100644
--- a/chrome/test/base/testing_browser_process.h
+++ b/chrome/test/base/testing_browser_process.h
@@ -111,6 +111,7 @@ class TestingBrowserProcess : public BrowserProcess {
       media_file_system_registry() OVERRIDE;
   virtual void PlatformSpecificCommandLineProcessing(
       const CommandLine& command_line) OVERRIDE;
+  virtual bool created_local_state() const OVERRIDE;
 
   // Set the local state for tests. Consumer is responsible for cleaning it up
   // afterwards (using ScopedTestingLocalState, for example).