summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--base/file_util_win.cc9
-rw-r--r--chrome/browser/extensions/convert_user_script.cc10
-rw-r--r--chrome/browser/extensions/convert_web_app.cc10
-rw-r--r--chrome/browser/extensions/crx_installer.cc11
-rw-r--r--chrome/browser/extensions/sandboxed_extension_unpacker.cc42
-rw-r--r--chrome/browser/extensions/sandboxed_extension_unpacker.h10
-rw-r--r--chrome/browser/extensions/sandboxed_extension_unpacker_unittest.cc16
-rw-r--r--chrome/common/chrome_paths.cc3
-rw-r--r--chrome/common/chrome_paths.h5
-rw-r--r--chrome/common/extensions/extension_file_util.cc72
-rw-r--r--chrome/common/extensions/extension_file_util.h8
11 files changed, 151 insertions, 45 deletions
diff --git a/base/file_util_win.cc b/base/file_util_win.cc
index 7476b53..51bcb4e 100644
--- a/base/file_util_win.cc
+++ b/base/file_util_win.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 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.
@@ -667,9 +667,10 @@ bool CreateDirectory(const FilePath& full_path) {
if (!::CreateDirectory(full_path_str, NULL)) {
DWORD error_code = ::GetLastError();
if (error_code == ERROR_ALREADY_EXISTS && DirectoryExists(full_path)) {
- // This error code doesn't indicate whether we were racing with someone
- // creating the same directory, or a file with the same path, therefore
- // we check.
+ // This error code ERROR_ALREADY_EXISTS doesn't indicate whether we
+ // were racing with someone creating the same directory, or a file
+ // with the same path. If DirectoryExists() returns true, we lost the
+ // race to create the same directory.
return true;
} else {
LOG(WARNING) << "Failed to create directory " << full_path_str
diff --git a/chrome/browser/extensions/convert_user_script.cc b/chrome/browser/extensions/convert_user_script.cc
index 7322c70..0b45da8 100644
--- a/chrome/browser/extensions/convert_user_script.cc
+++ b/chrome/browser/extensions/convert_user_script.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2009 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.
@@ -18,6 +18,7 @@
#include "chrome/common/chrome_paths.h"
#include "chrome/common/extensions/extension.h"
#include "chrome/common/extensions/extension_constants.h"
+#include "chrome/common/extensions/extension_file_util.h"
#include "chrome/common/extensions/user_script.h"
#include "chrome/common/json_value_serializer.h"
#include "googleurl/src/gurl.h"
@@ -45,8 +46,11 @@ scoped_refptr<Extension> ConvertUserScriptToExtension(
return NULL;
}
- FilePath user_data_temp_dir;
- CHECK(PathService::Get(chrome::DIR_USER_DATA_TEMP, &user_data_temp_dir));
+ FilePath user_data_temp_dir = extension_file_util::GetUserDataTempDir();
+ if (user_data_temp_dir.empty()) {
+ *error = "Could not get path to profile temporary directory.";
+ return NULL;
+ }
ScopedTempDir temp_dir;
if (!temp_dir.CreateUniqueTempDirUnderPath(user_data_temp_dir)) {
diff --git a/chrome/browser/extensions/convert_web_app.cc b/chrome/browser/extensions/convert_web_app.cc
index e1c8c92..d410f2a 100644
--- a/chrome/browser/extensions/convert_web_app.cc
+++ b/chrome/browser/extensions/convert_web_app.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 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.
@@ -22,6 +22,7 @@
#include "chrome/common/chrome_paths.h"
#include "chrome/common/extensions/extension.h"
#include "chrome/common/extensions/extension_constants.h"
+#include "chrome/common/extensions/extension_file_util.h"
#include "chrome/common/json_value_serializer.h"
#include "chrome/common/web_apps.h"
#include "gfx/codec/png_codec.h"
@@ -84,8 +85,11 @@ std::string ConvertTimeToExtensionVersion(const Time& create_time) {
scoped_refptr<Extension> ConvertWebAppToExtension(
const WebApplicationInfo& web_app,
const Time& create_time) {
- FilePath user_data_temp_dir;
- CHECK(PathService::Get(chrome::DIR_USER_DATA_TEMP, &user_data_temp_dir));
+ FilePath user_data_temp_dir = extension_file_util::GetUserDataTempDir();
+ if (user_data_temp_dir.empty()) {
+ LOG(ERROR) << "Could not get path to profile temporary directory.";
+ return NULL;
+ }
ScopedTempDir temp_dir;
if (!temp_dir.CreateUniqueTempDirUnderPath(user_data_temp_dir)) {
diff --git a/chrome/browser/extensions/crx_installer.cc b/chrome/browser/extensions/crx_installer.cc
index 1593aea..f62c983 100644
--- a/chrome/browser/extensions/crx_installer.cc
+++ b/chrome/browser/extensions/crx_installer.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 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.
@@ -117,18 +117,9 @@ CrxInstaller::~CrxInstaller() {
void CrxInstaller::InstallCrx(const FilePath& source_file) {
source_file_ = source_file;
- FilePath user_data_temp_dir;
- {
- // We shouldn't be doing disk IO on the UI thread.
- // http://code.google.com/p/chromium/issues/detail?id=60634
- base::ThreadRestrictions::ScopedAllowIO allow_io;
- CHECK(PathService::Get(chrome::DIR_USER_DATA_TEMP, &user_data_temp_dir));
- }
-
scoped_refptr<SandboxedExtensionUnpacker> unpacker(
new SandboxedExtensionUnpacker(
source_file,
- user_data_temp_dir,
g_browser_process->resource_dispatcher_host(),
this));
diff --git a/chrome/browser/extensions/sandboxed_extension_unpacker.cc b/chrome/browser/extensions/sandboxed_extension_unpacker.cc
index 6ae14a2..c43b83d 100644
--- a/chrome/browser/extensions/sandboxed_extension_unpacker.cc
+++ b/chrome/browser/extensions/sandboxed_extension_unpacker.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 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.
@@ -12,12 +12,15 @@
#include "base/file_util.h"
#include "base/file_util_proxy.h"
#include "base/message_loop.h"
+#include "base/metrics/histogram.h"
+#include "base/path_service.h"
#include "base/scoped_handle.h"
#include "base/task.h"
#include "base/utf_string_conversions.h" // TODO(viettrungluu): delete me.
#include "chrome/browser/browser_thread.h"
#include "chrome/browser/extensions/extension_service.h"
#include "chrome/browser/renderer_host/resource_dispatcher_host.h"
+#include "chrome/common/chrome_paths.h"
#include "chrome/common/chrome_switches.h"
#include "chrome/common/extensions/extension.h"
#include "chrome/common/extensions/extension_constants.h"
@@ -33,28 +36,44 @@ const char SandboxedExtensionUnpacker::kExtensionHeaderMagic[] = "Cr24";
SandboxedExtensionUnpacker::SandboxedExtensionUnpacker(
const FilePath& crx_path,
- const FilePath& temp_path,
ResourceDispatcherHost* rdh,
SandboxedExtensionUnpackerClient* client)
- : crx_path_(crx_path), temp_path_(temp_path),
+ : crx_path_(crx_path),
thread_identifier_(BrowserThread::ID_COUNT),
rdh_(rdh), client_(client), got_response_(false) {
}
-void SandboxedExtensionUnpacker::Start() {
- // We assume that we are started on the thread that the client wants us to do
- // file IO on.
+bool SandboxedExtensionUnpacker::CreateTempDirectory() {
CHECK(BrowserThread::GetCurrentThreadIdentifier(&thread_identifier_));
- // Create a temporary directory to work in.
- if (!temp_dir_.CreateUniqueTempDirUnderPath(temp_path_)) {
- // Could not create temporary directory.
+ FilePath user_data_temp_dir = extension_file_util::GetUserDataTempDir();
+ if (user_data_temp_dir.empty()) {
+ // TODO(skerner): This should have its own string.
+ // Using an existing string so that the change can be merged.
ReportFailure(l10n_util::GetStringFUTF8(
IDS_EXTENSION_PACKAGE_INSTALL_ERROR,
ASCIIToUTF16("COULD_NOT_CREATE_TEMP_DIRECTORY")));
- return;
+ return false;
+ }
+
+ if (!temp_dir_.CreateUniqueTempDirUnderPath(user_data_temp_dir)) {
+ ReportFailure(l10n_util::GetStringFUTF8(
+ IDS_EXTENSION_PACKAGE_INSTALL_ERROR,
+ ASCIIToUTF16("COULD_NOT_CREATE_TEMP_DIRECTORY")));
+ return false;
}
+ return true;
+}
+
+void SandboxedExtensionUnpacker::Start() {
+ // We assume that we are started on the thread that the client wants us to do
+ // file IO on.
+ CHECK(BrowserThread::GetCurrentThreadIdentifier(&thread_identifier_));
+
+ if (!CreateTempDirectory())
+ return; // ReportFailure() already called.
+
// Initialize the path that will eventually contain the unpacked extension.
extension_root_ = temp_dir_.path().AppendASCII(
extension_filenames::kTempExtensionName);
@@ -311,10 +330,13 @@ bool SandboxedExtensionUnpacker::ValidateSignature() {
}
void SandboxedExtensionUnpacker::ReportFailure(const std::string& error) {
+ UMA_HISTOGRAM_COUNTS("Extensions.SandboxUnpackFailure", 1);
client_->OnUnpackFailure(error);
}
void SandboxedExtensionUnpacker::ReportSuccess() {
+ UMA_HISTOGRAM_COUNTS("Extensions.SandboxUnpackSuccess", 1);
+
// Client takes ownership of temporary directory and extension.
client_->OnUnpackSuccess(temp_dir_.Take(), extension_root_, extension_);
extension_ = NULL;
diff --git a/chrome/browser/extensions/sandboxed_extension_unpacker.h b/chrome/browser/extensions/sandboxed_extension_unpacker.h
index e47b26c..455e3f6 100644
--- a/chrome/browser/extensions/sandboxed_extension_unpacker.h
+++ b/chrome/browser/extensions/sandboxed_extension_unpacker.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 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.
@@ -93,7 +93,6 @@ class SandboxedExtensionUnpacker : public UtilityProcessHost::Client {
// |client| with the result. If |rdh| is provided, unpacking is done in a
// sandboxed subprocess. Otherwise, it is done in-process.
SandboxedExtensionUnpacker(const FilePath& crx_path,
- const FilePath& temp_path,
ResourceDispatcherHost* rdh,
SandboxedExtensionUnpackerClient* cilent);
@@ -107,6 +106,10 @@ class SandboxedExtensionUnpacker : public UtilityProcessHost::Client {
virtual ~SandboxedExtensionUnpacker();
+ // Set |temp_dir_| as a temporary directory to unpack the extension in.
+ // Return true on success.
+ virtual bool CreateTempDirectory();
+
// Validates the signature of the extension and extract the key to
// |public_key_|. Returns true if the signature validates, false otherwise.
//
@@ -141,9 +144,6 @@ class SandboxedExtensionUnpacker : public UtilityProcessHost::Client {
// The path to the CRX to unpack.
FilePath crx_path_;
- // A path to a temp dir to unpack in.
- FilePath temp_path_;
-
// Our client's thread. This is the thread we respond on.
BrowserThread::ID thread_identifier_;
diff --git a/chrome/browser/extensions/sandboxed_extension_unpacker_unittest.cc b/chrome/browser/extensions/sandboxed_extension_unpacker_unittest.cc
index 6246a6f..ecc521c 100644
--- a/chrome/browser/extensions/sandboxed_extension_unpacker_unittest.cc
+++ b/chrome/browser/extensions/sandboxed_extension_unpacker_unittest.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2009 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.
@@ -22,12 +22,16 @@ namespace keys = extension_manifest_keys;
using testing::_;
using testing::Invoke;
+namespace {
+
void OnUnpackSuccess(const FilePath& temp_dir,
const FilePath& extension_root,
const Extension* extension) {
// Don't delete temp_dir here, we need to do some post op checking.
}
+} // namespace
+
class MockSandboxedExtensionUnpackerClient
: public SandboxedExtensionUnpackerClient {
public:
@@ -78,7 +82,7 @@ class SandboxedExtensionUnpackerTest : public testing::Test {
// a temp folder to play in.
ASSERT_TRUE(PathService::Get(base::DIR_TEMP, &install_dir_));
install_dir_ =
- install_dir_.AppendASCII("sandboxed_extension_unpacker_test");
+ install_dir_.AppendASCII("sandboxed_extension_unpacker_test");
file_util::Delete(install_dir_, true);
file_util::CreateDirectory(install_dir_);
@@ -89,14 +93,14 @@ class SandboxedExtensionUnpackerTest : public testing::Test {
unpacker_.reset(new ExtensionUnpacker(crx_path));
-
// Build a temp area where the extension will be unpacked.
ASSERT_TRUE(PathService::Get(base::DIR_TEMP, &temp_dir_));
temp_dir_ = temp_dir_.AppendASCII("sandboxed_extension_unpacker_test_Temp");
- file_util::CreateDirectory(temp_dir_);
+ ASSERT_TRUE(file_util::CreateDirectory(temp_dir_));
sandboxed_unpacker_ =
- new SandboxedExtensionUnpacker(crx_path, temp_dir_, NULL, client_);
+ new SandboxedExtensionUnpacker(crx_path, NULL, client_);
+
// Hack since SandboxedExtensionUnpacker gets its background thread id from
// the Start call, but we don't call it here.
sandboxed_unpacker_->thread_identifier_ = BrowserThread::FILE;
@@ -159,6 +163,7 @@ class SandboxedExtensionUnpackerTest : public testing::Test {
TEST_F(SandboxedExtensionUnpackerTest, NoCatalogsSuccess) {
EXPECT_CALL(*client_, OnUnpackSuccess(_, _, _));
+ EXPECT_CALL(*client_, OnUnpackFailure(_)).Times(0);
SetupUnpacker("no_l10n.crx");
ASSERT_TRUE(unpacker_->Run());
@@ -180,6 +185,7 @@ TEST_F(SandboxedExtensionUnpackerTest, NoCatalogsSuccess) {
TEST_F(SandboxedExtensionUnpackerTest, WithCatalogsSuccess) {
EXPECT_CALL(*client_, OnUnpackSuccess(_, _, _));
+ EXPECT_CALL(*client_, OnUnpackFailure(_)).Times(0);
SetupUnpacker("good_l10n.crx");
ASSERT_TRUE(unpacker_->Run());
diff --git a/chrome/common/chrome_paths.cc b/chrome/common/chrome_paths.cc
index 344db91..51aa019 100644
--- a/chrome/common/chrome_paths.cc
+++ b/chrome/common/chrome_paths.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2006-2010 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.
@@ -182,7 +182,6 @@ bool PathProvider(int key, FilePath* result) {
if (!PathService::Get(chrome::DIR_USER_DATA, &cur))
return false;
cur = cur.Append(FILE_PATH_LITERAL("Temp"));
- create_dir = true;
break;
case chrome::DIR_INTERNAL_PLUGINS:
if (!GetInternalPluginsDirectory(&cur))
diff --git a/chrome/common/chrome_paths.h b/chrome/common/chrome_paths.h
index 21867bd..09fe1b6 100644
--- a/chrome/common/chrome_paths.h
+++ b/chrome/common/chrome_paths.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2006-2010 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.
@@ -35,6 +35,9 @@ enum {
// this when a temporary file or directory will
// be moved into the profile, to avoid issues
// moving across volumes. See crbug.com/13044 .
+ // Getting this path does not create it. Users
+ // should check that the path exists before
+ // using it.
DIR_INTERNAL_PLUGINS, // Directory where internal plugins reside.
#if !defined(OS_MACOSX) && defined(OS_POSIX)
DIR_POLICY_FILES, // Directory for system-wide read-only
diff --git a/chrome/common/extensions/extension_file_util.cc b/chrome/common/extensions/extension_file_util.cc
index e8636be..9472f03 100644
--- a/chrome/common/extensions/extension_file_util.cc
+++ b/chrome/common/extensions/extension_file_util.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 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.
@@ -10,8 +10,12 @@
#include "app/l10n_util.h"
#include "base/file_util.h"
#include "base/logging.h"
+#include "base/metrics/histogram.h"
+#include "base/path_service.h"
#include "base/scoped_temp_dir.h"
+#include "base/threading/thread_restrictions.h"
#include "base/utf_string_conversions.h"
+#include "chrome/common/chrome_paths.h"
#include "chrome/common/extensions/extension.h"
#include "chrome/common/extensions/extension_action.h"
#include "chrome/common/extensions/extension_l10n_util.h"
@@ -517,4 +521,70 @@ FilePath ExtensionURLToRelativeFilePath(const GURL& url) {
return path;
}
+FilePath GetUserDataTempDir() {
+ // We do file IO in this function, but only when the current profile's
+ // Temp directory has never been used before, or in a rare error case.
+ // Developers are not likely to see these situations often, so do an
+ // explicit thread check.
+ base::ThreadRestrictions::AssertIOAllowed();
+
+ // Getting chrome::DIR_USER_DATA_TEMP is failing. Use histogram to see why.
+ // TODO(skerner): Fix the problem, and remove this code. crbug.com/70056
+ enum DirectoryCreationResult {
+ SUCCESS = 0,
+
+ CANT_GET_PARENT_PATH,
+ CANT_GET_UDT_PATH,
+ NOT_A_DIRECTORY,
+ CANT_CREATE_DIR,
+ CANT_WRITE_TO_PATH,
+
+ UNSET,
+ NUM_DIRECTORY_CREATION_RESULTS
+ };
+
+ // All paths should set |result|.
+ DirectoryCreationResult result = UNSET;
+
+ FilePath temp_path;
+ if (!PathService::Get(chrome::DIR_USER_DATA_TEMP, &temp_path)) {
+ FilePath parent_path;
+ if (!PathService::Get(chrome::DIR_USER_DATA, &parent_path))
+ result = CANT_GET_PARENT_PATH;
+ else
+ result = CANT_GET_UDT_PATH;
+
+ } else if (file_util::PathExists(temp_path)) {
+
+ // Path exists. Check that it is a directory we can write to.
+ if (!file_util::DirectoryExists(temp_path)) {
+ result = NOT_A_DIRECTORY;
+
+ } else if (!file_util::PathIsWritable(temp_path)) {
+ result = CANT_WRITE_TO_PATH;
+
+ } else {
+ // Temp is a writable directory.
+ result = SUCCESS;
+ }
+
+ } else if (!file_util::CreateDirectory(temp_path)) {
+ // Path doesn't exist, and we failed to create it.
+ result = CANT_CREATE_DIR;
+
+ } else {
+ // Successfully created the Temp directory.
+ result = SUCCESS;
+ }
+
+ UMA_HISTOGRAM_ENUMERATION("Extensions.GetUserDataTempDir",
+ result,
+ NUM_DIRECTORY_CREATION_RESULTS);
+
+ if (result == SUCCESS)
+ return temp_path;
+
+ return FilePath();
+}
+
} // namespace extension_file_util
diff --git a/chrome/common/extensions/extension_file_util.h b/chrome/common/extensions/extension_file_util.h
index f6a72bc..b26b564 100644
--- a/chrome/common/extensions/extension_file_util.h
+++ b/chrome/common/extensions/extension_file_util.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 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.
@@ -76,6 +76,12 @@ bool CheckForIllegalFilenames(const FilePath& extension_path,
// Get a relative file path from a chrome-extension:// URL.
FilePath ExtensionURLToRelativeFilePath(const GURL& url);
+// Get a path to a temp directory for unpacking an extension.
+// This is essentially PathService::Get(chrome::DIR_USER_DATA_TEMP, ...),
+// with a histogram that allows us to understand why it is failing.
+// Return an empty file path on failure.
+FilePath GetUserDataTempDir();
+
} // namespace extension_file_util
#endif // CHROME_COMMON_EXTENSIONS_EXTENSION_FILE_UTIL_H_