summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorskerner@chromium.org <skerner@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2010-07-30 15:33:28 +0000
committerskerner@chromium.org <skerner@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2010-07-30 15:33:28 +0000
commitbd781d897ea507f9abc028462d04224ca273875a (patch)
treebf14c6237ec30d3e1815e3978807d1943ad22f9f
parent5021691cce9e56fe0bb778df3ad92e41c55c6ed0 (diff)
downloadchromium_src-bd781d897ea507f9abc028462d04224ca273875a.zip
chromium_src-bd781d897ea507f9abc028462d04224ca273875a.tar.gz
chromium_src-bd781d897ea507f9abc028462d04224ca273875a.tar.bz2
Remove some debug switches:
--issue35198-crxdir-browser --issue35198-permission They were used to understand issue 35198. They are no longer needed. There is one more switch, --issue35198-logging, that is not being removed in this CL. The logging it adds may still be useful. BUG=50604 TEST=manual Review URL: http://codereview.chromium.org/3052023 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@54323 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r--base/file_util.h5
-rw-r--r--base/file_util_posix.cc8
-rw-r--r--base/file_util_unittest.cc1
-rw-r--r--base/file_util_win.cc71
-rw-r--r--base/scoped_temp_dir.cc9
-rw-r--r--base/scoped_temp_dir.h3
-rw-r--r--base/scoped_temp_dir_unittest.cc2
-rw-r--r--chrome/browser/extensions/convert_user_script.cc2
-rw-r--r--chrome/browser/extensions/sandboxed_extension_unpacker.cc21
-rw-r--r--chrome/common/chrome_switches.cc7
-rw-r--r--chrome/common/chrome_switches.h2
11 files changed, 10 insertions, 121 deletions
diff --git a/base/file_util.h b/base/file_util.h
index 66e450a..bd973b23 100644
--- a/base/file_util.h
+++ b/base/file_util.h
@@ -273,13 +273,8 @@ bool CreateNewTempDirectory(const FilePath::StringType& prefix,
// Create a directory within another directory.
// Extra characters will be appended to |prefix| to ensure that the
// new directory does not have the same name as an existing directory.
-// If |loosen_permissions| is true, the new directory will be readable
-// and writable to all users on windows. It is ignored on other platforms.
-// |loosen_permissions| exists to allow debugging of crbug/35198, and will
-// be removed when the issue is understood.
bool CreateTemporaryDirInDir(const FilePath& base_dir,
const FilePath::StringType& prefix,
- bool loosen_permissions,
FilePath* new_dir);
// Creates a directory, as well as creating any parent directories, if they
diff --git a/base/file_util_posix.cc b/base/file_util_posix.cc
index 4d9cf42..13798d3 100644
--- a/base/file_util_posix.cc
+++ b/base/file_util_posix.cc
@@ -430,15 +430,7 @@ static bool CreateTemporaryDirInDirImpl(const FilePath& base_dir,
bool CreateTemporaryDirInDir(const FilePath& base_dir,
const FilePath::StringType& prefix,
- bool loosen_permissions,
FilePath* new_dir) {
- // To understand crbug/35198, the ability to call this
- // this function on windows while giving loose permissions
- // to the resulting directory has been temporarily added.
- // It should not be possible to call this function with
- // loosen_permissions == true on non-windows platforms.
- DCHECK(!loosen_permissions);
-
FilePath::StringType mkdtemp_template = prefix;
mkdtemp_template.append(FILE_PATH_LITERAL("XXXXXX"));
return CreateTemporaryDirInDirImpl(base_dir, mkdtemp_template, new_dir);
diff --git a/base/file_util_unittest.cc b/base/file_util_unittest.cc
index 7860111..4a1417a 100644
--- a/base/file_util_unittest.cc
+++ b/base/file_util_unittest.cc
@@ -1577,7 +1577,6 @@ TEST_F(FileUtilTest, CreateNewTemporaryDirInDirTest) {
ASSERT_TRUE(file_util::CreateTemporaryDirInDir(
test_dir_,
FILE_PATH_LITERAL("CreateNewTemporaryDirInDirTest"),
- false,
&new_dir));
EXPECT_TRUE(file_util::PathExists(new_dir));
EXPECT_TRUE(test_dir_.IsParent(new_dir));
diff --git a/base/file_util_win.cc b/base/file_util_win.cc
index 26955f2..b6a741b 100644
--- a/base/file_util_win.cc
+++ b/base/file_util_win.cc
@@ -64,56 +64,6 @@ bool DevicePathToDriveLetterPath(const FilePath& device_path,
return true;
}
-// Build a security descriptor with the weakest possible file permissions.
-bool InitLooseSecurityDescriptor(SECURITY_ATTRIBUTES *sa,
- SECURITY_DESCRIPTOR *sd) {
- DWORD last_error;
-
- if (!InitializeSecurityDescriptor(sd, SECURITY_DESCRIPTOR_REVISION)) {
- last_error = GetLastError();
- LOG(ERROR) << "InitializeSecurityDescriptor failed: GetLastError() = "
- << last_error;
- return false;
- }
-
- if (!SetSecurityDescriptorDacl(sd,
- TRUE, // bDaclPresent: Add one to |sd|.
- NULL, // pDacl: NULL means allow all access.
- FALSE // bDaclDefaulted: Not defaulted.
- )) {
- last_error = GetLastError();
- LOG(ERROR) << "SetSecurityDescriptorDacl() failed: GetLastError() = "
- << last_error;
- return false;
- }
-
- if (!SetSecurityDescriptorGroup(sd,
- NULL, // pGroup: No no primary group.
- FALSE // bGroupDefaulted: Not defaulted.
- )) {
- last_error = GetLastError();
- LOG(ERROR) << "SetSecurityDescriptorGroup() failed: GetLastError() = "
- << last_error;
- return false;
- }
-
- if (!SetSecurityDescriptorSacl(sd,
- FALSE, // bSaclPresent: No SACL.
- NULL,
- FALSE
- )) {
- last_error = GetLastError();
- LOG(ERROR) << "SetSecurityDescriptorSacl() failed: GetLastError() = "
- << last_error;
- return false;
- }
-
- sa->nLength = sizeof(SECURITY_ATTRIBUTES);
- sa->lpSecurityDescriptor = sd;
- sa->bInheritHandle = TRUE;
- return true;
-}
-
} // namespace
std::wstring GetDirectoryFromPath(const std::wstring& path) {
@@ -601,19 +551,7 @@ bool CreateTemporaryFileInDir(const FilePath& dir,
bool CreateTemporaryDirInDir(const FilePath& base_dir,
const FilePath::StringType& prefix,
- bool loosen_permissions,
FilePath* new_dir) {
- SECURITY_ATTRIBUTES sa;
- SECURITY_DESCRIPTOR sd;
-
- LPSECURITY_ATTRIBUTES directory_security_attributes = NULL;
- if (loosen_permissions) {
- if (InitLooseSecurityDescriptor(&sa, &sd))
- directory_security_attributes = &sa;
- else
- LOG(ERROR) << "Failed to init security attributes, fall back to NULL.";
- }
-
FilePath path_to_create;
srand(static_cast<uint32>(time(NULL)));
@@ -628,8 +566,7 @@ bool CreateTemporaryDirInDir(const FilePath& base_dir,
new_dir_name.append(IntToWString(rand() % kint16max));
path_to_create = path_to_create.Append(new_dir_name);
- if (::CreateDirectory(path_to_create.value().c_str(),
- directory_security_attributes))
+ if (::CreateDirectory(path_to_create.value().c_str(), NULL))
break;
count++;
}
@@ -639,7 +576,6 @@ bool CreateTemporaryDirInDir(const FilePath& base_dir,
}
*new_dir = path_to_create;
-
return true;
}
@@ -649,10 +585,7 @@ bool CreateNewTempDirectory(const FilePath::StringType& prefix,
if (!GetTempDir(&system_temp_dir))
return false;
- return CreateTemporaryDirInDir(system_temp_dir,
- prefix,
- false,
- new_temp_path);
+ return CreateTemporaryDirInDir(system_temp_dir, prefix, new_temp_path);
}
bool CreateDirectory(const FilePath& full_path) {
diff --git a/base/scoped_temp_dir.cc b/base/scoped_temp_dir.cc
index 28f59bf..53452f1 100644
--- a/base/scoped_temp_dir.cc
+++ b/base/scoped_temp_dir.cc
@@ -4,7 +4,6 @@
#include "base/scoped_temp_dir.h"
-#include "base/command_line.h"
#include "base/file_util.h"
#include "base/logging.h"
#include "base/string_util.h"
@@ -27,19 +26,15 @@ bool ScopedTempDir::CreateUniqueTempDir() {
return true;
}
-bool ScopedTempDir::CreateUniqueTempDirUnderPath(const FilePath& base_path,
- bool loose_permissions) {
+bool ScopedTempDir::CreateUniqueTempDirUnderPath(const FilePath& base_path) {
// If |path| does not exist, create it.
- if (!file_util::CreateDirectory(base_path)) {
- LOG(ERROR) << "Failed to create base directory " << base_path.value();
+ if (!file_util::CreateDirectory(base_path))
return false;
- }
// Create a new, uniquely named directory under |base_path|.
if (!file_util::CreateTemporaryDirInDir(
base_path,
FILE_PATH_LITERAL("scoped_dir_"),
- loose_permissions,
&path_)) {
return false;
}
diff --git a/base/scoped_temp_dir.h b/base/scoped_temp_dir.h
index c24891d..702c2bc 100644
--- a/base/scoped_temp_dir.h
+++ b/base/scoped_temp_dir.h
@@ -27,8 +27,7 @@ class ScopedTempDir {
bool CreateUniqueTempDir();
// Creates a unique directory under a given path, and takes ownership of it.
- bool CreateUniqueTempDirUnderPath(const FilePath& path,
- bool loose_permissions);
+ bool CreateUniqueTempDirUnderPath(const FilePath& path);
// Takes ownership of directory at |path|, creating it if necessary.
// Don't call multiple times unless Take() has been called first.
diff --git a/base/scoped_temp_dir_unittest.cc b/base/scoped_temp_dir_unittest.cc
index e180119..4be0d07 100644
--- a/base/scoped_temp_dir_unittest.cc
+++ b/base/scoped_temp_dir_unittest.cc
@@ -65,7 +65,7 @@ TEST(ScopedTempDir, UniqueTempDirUnderPath) {
FilePath test_path;
{
ScopedTempDir dir;
- EXPECT_TRUE(dir.CreateUniqueTempDirUnderPath(base_path, false));
+ EXPECT_TRUE(dir.CreateUniqueTempDirUnderPath(base_path));
test_path = dir.path();
EXPECT_TRUE(file_util::DirectoryExists(test_path));
EXPECT_TRUE(base_path.IsParent(test_path));
diff --git a/chrome/browser/extensions/convert_user_script.cc b/chrome/browser/extensions/convert_user_script.cc
index b06553a..d50bec1 100644
--- a/chrome/browser/extensions/convert_user_script.cc
+++ b/chrome/browser/extensions/convert_user_script.cc
@@ -45,7 +45,7 @@ Extension* ConvertUserScriptToExtension(const FilePath& user_script_path,
CHECK(PathService::Get(chrome::DIR_USER_DATA_TEMP, &user_data_temp_dir));
ScopedTempDir temp_dir;
- if (!temp_dir.CreateUniqueTempDirUnderPath(user_data_temp_dir, false)) {
+ if (!temp_dir.CreateUniqueTempDirUnderPath(user_data_temp_dir)) {
*error = "Could not create temporary directory.";
return NULL;
}
diff --git a/chrome/browser/extensions/sandboxed_extension_unpacker.cc b/chrome/browser/extensions/sandboxed_extension_unpacker.cc
index c087649..2de1c09 100644
--- a/chrome/browser/extensions/sandboxed_extension_unpacker.cc
+++ b/chrome/browser/extensions/sandboxed_extension_unpacker.cc
@@ -42,18 +42,8 @@ void SandboxedExtensionUnpacker::Start() {
// file IO on.
CHECK(ChromeThread::GetCurrentThreadIdentifier(&thread_identifier_));
- // To understand crbug/35198, allow users who can reproduce the bug
- // to loosen permissions on the scoped directory.
- bool loosen_permissions = false;
-#if defined (OS_WIN)
- loosen_permissions = CommandLine::ForCurrentProcess()->HasSwitch(
- switches::kIssue35198Permission);
- LOG(INFO) << "loosen_permissions = " << loosen_permissions;
-#endif
-
// Create a temporary directory to work in.
- if (!temp_dir_.CreateUniqueTempDirUnderPath(temp_path_,
- loosen_permissions)) {
+ if (!temp_dir_.CreateUniqueTempDirUnderPath(temp_path_)) {
ReportFailure("Could not create temporary directory.");
return;
}
@@ -62,15 +52,6 @@ void SandboxedExtensionUnpacker::Start() {
extension_root_ = temp_dir_.path().AppendASCII(
extension_filenames::kTempExtensionName);
- // To understand crbug/35198, allow users who can reproduce the bug to
- // create the unpack directory in the browser process.
- bool crxdir_in_browser = CommandLine::ForCurrentProcess()->HasSwitch(
- switches::kIssue35198CrxDirBrowser);
- LOG(INFO) << "crxdir_in_browser = " << crxdir_in_browser;
- if (crxdir_in_browser && !file_util::CreateDirectory(extension_root_)) {
- LOG(ERROR) << "Failed to create directory " << extension_root_.value();
- }
-
// Extract the public key and validate the package.
if (!ValidateSignature())
return; // ValidateSignature() already reported the error.
diff --git a/chrome/common/chrome_switches.cc b/chrome/common/chrome_switches.cc
index 6016cda..d31eb6d 100644
--- a/chrome/common/chrome_switches.cc
+++ b/chrome/common/chrome_switches.cc
@@ -566,12 +566,9 @@ const char kInternalNaCl[] = "internal-nacl";
// Runs a trusted Pepper plugin inside the renderer process.
const char kInternalPepper[] = "internal-pepper";
-// The following flags allow users who can reproduce crbug/35198
-// to enable extra logging and behaviors. They will be removed once
-// the issue is fixed.
-const char kIssue35198CrxDirBrowser[] = "issue35198-crxdir-browser";
+// The following flag allows users who can reproduce crbug/35198
+// to enable extra logging. It will be removed once the issue is fixed.
const char kIssue35198ExtraLogging[] = "issue35198-logging";
-const char kIssue35198Permission[] = "issue35198-permission";
// Specifies the flags passed to JS engine
const char kJavaScriptFlags[] = "js-flags";
diff --git a/chrome/common/chrome_switches.h b/chrome/common/chrome_switches.h
index e822cb5..3d42124 100644
--- a/chrome/common/chrome_switches.h
+++ b/chrome/common/chrome_switches.h
@@ -171,9 +171,7 @@ extern const char kInstallerTestClean[];
extern const char kInstallerTestForce[];
extern const char kInternalNaCl[];
extern const char kInternalPepper[];
-extern const char kIssue35198CrxDirBrowser[];
extern const char kIssue35198ExtraLogging[];
-extern const char kIssue35198Permission[];
extern const char kJavaScriptFlags[];
extern const char kKeepAliveForTest[];
extern const char kLoadExtension[];