Skip to content

Commit

Permalink
Revert 135321 - Make sure only the main browser process and service p…
Browse files Browse the repository at this point in the history
…roceses are allowed to create the profile directory.

This patch lets Chrome start with profile located on a network share on
Windows Vista and newer.

BUG=120388
TEST=Start Chrome with --user-data-dir pointing to a network share location and try to navigate to a web page. This should not lead to a hang of the renderer.


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

TBR=pastarmovj@chromium.org
Review URL: https://chromiumcodereview.appspot.com/10382012

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@135347 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
pastarmovj@chromium.org committed May 4, 2012
1 parent 1c47238 commit 72f5829
Show file tree
Hide file tree
Showing 10 changed files with 11 additions and 73 deletions.
25 changes: 7 additions & 18 deletions base/path_service.cc
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) 2012 The Chromium Authors. All rights reserved.
// 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.

Expand Down Expand Up @@ -222,29 +222,18 @@ bool PathService::Get(int key, FilePath* result) {
}

bool PathService::Override(int key, const FilePath& path) {
// Just call the full function with true for the value of |create|.
return OverrideAndCreateIfNeeded(key, path, true);
}

bool PathService::OverrideAndCreateIfNeeded(int key,
const FilePath& path,
bool create) {
PathData* path_data = GetPathData();
DCHECK(path_data);
DCHECK_GT(key, base::DIR_CURRENT) << "invalid path key";

FilePath file_path = path;

// For some locations this will fail if called from inside the sandbox there-
// fore we protect this call with a flag.
if (create) {
// Make sure the directory exists. We need to do this before we translate
// this to the absolute path because on POSIX, AbsolutePath fails if called
// on a non-existent path.
if (!file_util::PathExists(file_path) &&
!file_util::CreateDirectory(file_path))
return false;
}
// Make sure the directory exists. We need to do this before we translate
// this to the absolute path because on POSIX, AbsolutePath fails if called
// on a non-existent path.
if (!file_util::PathExists(file_path) &&
!file_util::CreateDirectory(file_path))
return false;

// We need to have an absolute path, as extensions and plugins don't like
// relative paths, and will gladly crash the browser in CHECK()s if they get a
Expand Down
9 changes: 1 addition & 8 deletions base/path_service.h
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) 2012 The Chromium Authors. All rights reserved.
// 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.

Expand Down Expand Up @@ -40,13 +40,6 @@ class BASE_EXPORT PathService {
// over the lifetime of the app, so this method should be used with caution.
static bool Override(int key, const FilePath& path);

// This function does the same as PathService::Override but it takes an extra
// parameter |create| which guides whether the directory to be overriden must
// be created in case it doesn't exist already.
static bool OverrideAndCreateIfNeeded(int key,
const FilePath& path,
bool create);

// To extend the set of supported keys, you can register a path provider,
// which is just a function mirroring PathService::Get. The ProviderFunc
// returns false if it cannot provide a non-empty path for the given key.
Expand Down
8 changes: 2 additions & 6 deletions chrome/app/chrome_main_delegate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -567,12 +567,8 @@ void ChromeMainDelegate::PreSandboxStartup() {
#if defined(OS_MACOSX) || defined(OS_WIN)
CheckUserDataDirPolicy(&user_data_dir);
#endif
if (!user_data_dir.empty()) {
CHECK(PathService::OverrideAndCreateIfNeeded(
chrome::DIR_USER_DATA,
user_data_dir,
chrome::ProcessNeedsProfileDir(process_type)));
}
if (!user_data_dir.empty())
CHECK(PathService::Override(chrome::DIR_USER_DATA, user_data_dir));

startup_timer_.reset(new base::StatsScope<base::StatsCounterTimer>
(content::Counters::chrome_main()));
Expand Down
4 changes: 0 additions & 4 deletions chrome/chrome.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -1055,10 +1055,6 @@
'tools/crash_service/crash_service.cc',
'tools/crash_service/crash_service.h',
'tools/crash_service/main.cc',
'../content/public/common/content_switches.cc',
],
'defines': [
'COMPILE_CONTENT_STATICALLY',
],
'msvs_settings': {
'VCLinkerTool': {
Expand Down
4 changes: 0 additions & 4 deletions chrome/common/chrome_paths.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
#include "chrome/common/chrome_constants.h"
#include "chrome/common/chrome_paths_internal.h"
#include "chrome/common/chrome_switches.h"
#include "content/public/common/content_switches.h"

#if defined(OS_MACOSX)
#include "base/mac/mac_util.h"
Expand Down Expand Up @@ -145,9 +144,6 @@ bool PathProvider(int key, FilePath* result) {
FilePath cur;
switch (key) {
case chrome::DIR_USER_DATA:
CHECK(ProcessNeedsProfileDir(
CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
switches::kProcessType)));
if (!GetDefaultUserDataDirectory(&cur)) {
NOTREACHED();
return false;
Expand Down
5 changes: 0 additions & 5 deletions chrome/common/chrome_paths_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@
#define CHROME_COMMON_CHROME_PATHS_INTERNAL_H_
#pragma once

#include <string>

#include "build/build_config.h"

#if defined(OS_MACOSX)
Expand Down Expand Up @@ -87,9 +85,6 @@ NSBundle* OuterAppBundle();

#endif // OS_MACOSX

// Checks if the |process_type| has the rights to access the profile.
bool ProcessNeedsProfileDir(const std::string& process_type);

} // namespace chrome

#endif // CHROME_COMMON_CHROME_PATHS_INTERNAL_H_
7 changes: 0 additions & 7 deletions chrome/common/chrome_paths_linux.cc
Original file line number Diff line number Diff line change
Expand Up @@ -103,11 +103,4 @@ bool GetUserDesktop(FilePath* result) {
return true;
}

bool ProcessNeedsProfileDir(const std::string& process_type) {
// For now we have no reason to forbid this on Linux as we don't
// have the roaming profile troubles there. Moreover the Linux breakpad needs
// profile dir access in all process if enabled on Linux.
return true;
}

} // namespace chrome
8 changes: 1 addition & 7 deletions chrome/common/chrome_paths_mac.mm
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) 2012 The Chromium Authors. All rights reserved.
// 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.

Expand Down Expand Up @@ -199,10 +199,4 @@ bool GetLocalLibraryDirectory(FilePath* result) {
return bundle;
}

bool ProcessNeedsProfileDir(const std::string& process_type) {
// For now we have no reason to forbid this on other MacOS as we don't
// have the roaming profile troubles there.
return true;
}

} // namespace chrome
13 changes: 0 additions & 13 deletions chrome/common/chrome_paths_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
#include "base/win/scoped_co_mem.h"
#include "chrome/common/chrome_constants.h"
#include "chrome/installer/util/browser_distribution.h"
#include "content/public/common/content_switches.h"

namespace chrome {

Expand Down Expand Up @@ -98,16 +97,4 @@ bool GetUserDesktop(FilePath* result) {
return true;
}

bool ProcessNeedsProfileDir(const std::string& process_type) {
// On windows we don't want subprocesses other than the browser process and
// service processes to be able to use the profile directory because if it
// lies on a network share the sandbox will prevent us from accessing it.
// TODO(pastarmovj): For no gpu processes are whitelisted too because they do
// use the profile dir in some way but this must be investigated and fixed if
// possible.
return process_type.empty() ||
process_type == switches::kServiceProcess ||
process_type == switches::kGpuProcess;
}

} // namespace chrome
1 change: 0 additions & 1 deletion chrome/common_constants.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,6 @@
],
'defines': [
'<@(nacl_win64_defines)',
'COMPILE_CONTENT_STATICALLY',
],
'configurations': {
'Common_Base': {
Expand Down

0 comments on commit 72f5829

Please sign in to comment.