Skip to content

Commit

Permalink
Initialize chrome::DIR_USER_DATA early on for service processes, etc.
Browse files Browse the repository at this point in the history
http://crrev.com/251126 broke service/helper processes.
(--user-data-dir command-line args weren't respected)

Restore the early PathService DIR_USER_DATA init/override.
(done as before in ChromeMainDelegate::PreSandboxStartup)

Move init code to a file-local InitializeUserDataDir helper.
Append the fallback to the commandline for other processes.
(otherwise child/service processes re-use the bad dir)

Simplify ChromeBrowserMainParts::PreCreateThreadsImpl.
(just get DIR_USER_DATA, don't re-attempt init/override)
Show a warning messagebox here if user-data-dir was invalid.
Move warning UI helper to c/b/ui/startup/bad_flags_prompt.h

Add [Get|Set]InvalidSpecifiedUserDataDir for warning UI.
(add dynamic_annotations dependency for LazyInstance usage)

Remove the ChromeMainUserDataDirTest.GetUserDataDir test.
(now ineffective, it can't hook before PreSandboxStartup)

BUG=345025,345582,318999
TEST=The --user-data-dir command-line argument works as expected for browser, child, and service processes (cloud print connector, chrome logging, etc.). Users are still warned with a dialog when they supply invalid or restricted directory paths for the browser.
R=sky@chromium.org,vitalybuka@chromium.org

Review URL: https://codereview.chromium.org/174253002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@253803 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
msw@chromium.org committed Feb 27, 2014
1 parent 6c77d64 commit a5827b9
Show file tree
Hide file tree
Showing 12 changed files with 129 additions and 223 deletions.
58 changes: 58 additions & 0 deletions chrome/app/chrome_main_delegate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,20 @@
#include "base/path_service.h"
#include "base/process/memory.h"
#include "base/process/process_handle.h"
#include "base/strings/string_util.h"
#include "build/build_config.h"
#include "chrome/browser/chrome_content_browser_client.h"
#include "chrome/browser/defaults.h"
#include "chrome/common/chrome_constants.h"
#include "chrome/common/chrome_content_client.h"
#include "chrome/common/chrome_paths.h"
#include "chrome/common/chrome_paths_internal.h"
#include "chrome/common/chrome_switches.h"
#include "chrome/common/chrome_version_info.h"
#include "chrome/common/crash_keys.h"
#include "chrome/common/logging_chrome.h"
#include "chrome/common/profiling.h"
#include "chrome/common/switch_utils.h"
#include "chrome/common/url_constants.h"
#include "chrome/plugin/chrome_content_plugin_client.h"
#include "chrome/renderer/chrome_content_renderer_client.h"
Expand Down Expand Up @@ -96,6 +99,14 @@
#include "components/breakpad/app/breakpad_linux.h"
#endif

#if defined(OS_LINUX)
#include "base/environment.h"
#endif

#if defined(OS_MACOSX) || defined(OS_WIN)
#include "chrome/browser/policy/policy_path_parser.h"
#endif

#if !defined(CHROME_MULTIPLE_DLL_CHILD)
base::LazyInstance<chrome::ChromeContentBrowserClient>
g_chrome_content_browser_client = LAZY_INSTANCE_INITIALIZER;
Expand Down Expand Up @@ -310,6 +321,50 @@ struct MainFunction {
int (*function)(const content::MainFunctionParams&);
};

// Initializes the user data dir. Must be called before InitializeLocalState().
void InitializeUserDataDir() {
CommandLine* command_line = CommandLine::ForCurrentProcess();
base::FilePath user_data_dir =
command_line->GetSwitchValuePath(switches::kUserDataDir);
std::string process_type =
command_line->GetSwitchValueASCII(switches::kProcessType);

#if defined(OS_LINUX)
// On Linux, Chrome does not support running multiple copies under different
// DISPLAYs, so the profile directory can be specified in the environment to
// support the virtual desktop use-case.
if (user_data_dir.empty()) {
std::string user_data_dir_string;
scoped_ptr<base::Environment> environment(base::Environment::Create());
if (environment->GetVar("CHROME_USER_DATA_DIR", &user_data_dir_string) &&
IsStringUTF8(user_data_dir_string)) {
user_data_dir = base::FilePath::FromUTF8Unsafe(user_data_dir_string);
}
}
#endif
#if defined(OS_MACOSX) || defined(OS_WIN)
policy::path_parser::CheckUserDataDirPolicy(&user_data_dir);
#endif

const bool specified_directory_was_invalid = !user_data_dir.empty() &&
!PathService::OverrideAndCreateIfNeeded(chrome::DIR_USER_DATA,
user_data_dir, chrome::ProcessNeedsProfileDir(process_type));
// Save inaccessible or invalid paths so the user may be prompted later.
if (specified_directory_was_invalid)
chrome::SetInvalidSpecifiedUserDataDir(user_data_dir);

// Getting the user data directory can fail if the directory isn't creatable.
// ProcessSingleton needs a real user data directory 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!";

// Append the fallback user data directory to the commandline. Otherwise,
// child or service processes will attempt to use the invalid directory.
if (specified_directory_was_invalid)
command_line->AppendSwitchPath(switches::kUserDataDir, user_data_dir);
}

} // namespace

ChromeMainDelegate::ChromeMainDelegate() {
Expand Down Expand Up @@ -578,6 +633,9 @@ void ChromeMainDelegate::PreSandboxStartup() {
child_process_logging::Init();
#endif

// Initialize the user data dir for service processes, logging, etc.
InitializeUserDataDir();

stats_counter_timer_.reset(new base::StatsCounterTimer("Chrome.Init"));
startup_timer_.reset(new base::StatsScope<base::StatsCounterTimer>
(*stats_counter_timer_));
Expand Down
11 changes: 5 additions & 6 deletions chrome/browser/chrome_browser_main.cc
Original file line number Diff line number Diff line change
Expand Up @@ -95,11 +95,11 @@
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/browser_finder.h"
#include "chrome/browser/ui/host_desktop.h"
#include "chrome/browser/ui/startup/bad_flags_prompt.h"
#include "chrome/browser/ui/startup/default_browser_prompt.h"
#include "chrome/browser/ui/startup/startup_browser_creator.h"
#include "chrome/browser/ui/uma_browsing_activity_observer.h"
#include "chrome/browser/ui/webui/chrome_web_ui_controller_factory.h"
#include "chrome/browser/user_data_dir_extractor.h"
#include "chrome/common/chrome_constants.h"
#include "chrome/common/chrome_paths.h"
#include "chrome/common/chrome_result_codes.h"
Expand Down Expand Up @@ -785,11 +785,10 @@ int ChromeBrowserMainParts::PreCreateThreads() {
int ChromeBrowserMainParts::PreCreateThreadsImpl() {
TRACE_EVENT0("startup", "ChromeBrowserMainParts::PreCreateThreadsImpl")
run_message_loop_ = false;
{
TRACE_EVENT0("startup",
"ChromeBrowserMainParts::PreCreateThreadsImpl:GetUserDataDir");
user_data_dir_ = chrome::GetUserDataDir(parameters());
}
CHECK(PathService::Get(chrome::DIR_USER_DATA, &user_data_dir_));
#if !defined(OS_ANDROID)
chrome::MaybeShowInvalidUserDataDirWarningDialog();
#endif

// Force MediaCaptureDevicesDispatcher to be created on UI thread.
MediaCaptureDevicesDispatcher::GetInstance();
Expand Down
38 changes: 38 additions & 0 deletions chrome/browser/ui/startup/bad_flags_prompt.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,23 @@
#include "chrome/browser/ui/startup/bad_flags_prompt.h"

#include "base/command_line.h"
#include "base/files/file_path.h"
#include "base/strings/utf_string_conversions.h"
#include "chrome/browser/infobars/infobar_service.h"
#include "chrome/browser/infobars/simple_alert_infobar_delegate.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/simple_message_box.h"
#include "chrome/browser/ui/tabs/tab_strip_model.h"
#include "chrome/common/chrome_paths.h"
#include "chrome/common/chrome_switches.h"
#include "chrome/common/switch_utils.h"
#include "components/startup_metric_utils/startup_metric_utils.h"
#include "components/translate/core/common/translate_switches.h"
#include "extensions/common/switches.h"
#include "grit/chromium_strings.h"
#include "grit/generated_resources.h"
#include "ui/base/l10n/l10n_util.h"
#include "ui/base/resource/resource_bundle.h"

namespace chrome {

Expand Down Expand Up @@ -59,4 +66,35 @@ void ShowBadFlagsPrompt(Browser* browser) {
}
}

void MaybeShowInvalidUserDataDirWarningDialog() {
const base::FilePath& user_data_dir = GetInvalidSpecifiedUserDataDir();
if (user_data_dir.empty())
return;

startup_metric_utils::SetNonBrowserUIDisplayed();

// Ensure the ResourceBundle is initialized for string resource access.
bool cleanup_resource_bundle = false;
if (!ResourceBundle::HasSharedInstance()) {
cleanup_resource_bundle = true;
std::string locale = l10n_util::GetApplicationLocale(std::string());
const char kUserDataDirDialogFallbackLocale[] = "en-US";
if (locale.empty())
locale = kUserDataDirDialogFallbackLocale;
ResourceBundle::InitSharedInstanceWithLocale(locale, NULL);
}

const base::string16& title =
l10n_util::GetStringUTF16(IDS_CANT_WRITE_USER_DIRECTORY_TITLE);
const base::string16& message =
l10n_util::GetStringFUTF16(IDS_CANT_WRITE_USER_DIRECTORY_SUMMARY,
user_data_dir.LossyDisplayName());

if (cleanup_resource_bundle)
ResourceBundle::CleanupSharedInstance();

// More complex dialogs cannot be shown before the earliest calls here.
ShowMessageBox(NULL, title, message, chrome::MESSAGE_BOX_TYPE_WARNING);
}

} // namespace chrome
3 changes: 3 additions & 0 deletions chrome/browser/ui/startup/bad_flags_prompt.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ namespace chrome {
// command line flags.
void ShowBadFlagsPrompt(Browser* browser);

// Shows a warning dialog if the originally specified user data dir was invalid.
void MaybeShowInvalidUserDataDirWarningDialog();

} // namespace chrome

#endif // CHROME_BROWSER_UI_STARTUP_BAD_FLAGS_PROMPT_H_
111 changes: 0 additions & 111 deletions chrome/browser/user_data_dir_extractor.cc

This file was deleted.

31 changes: 0 additions & 31 deletions chrome/browser/user_data_dir_extractor.h

This file was deleted.

Loading

0 comments on commit a5827b9

Please sign in to comment.