summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorfinnur@chromium.org <finnur@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2010-03-25 03:00:49 +0000
committerfinnur@chromium.org <finnur@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2010-03-25 03:00:49 +0000
commit27fbcccda73e7bd257c20f0bbf4b4c31af15c9c2 (patch)
tree1733e5af0ff85042f99f9165c93015e7df6b40c0
parenta25fa87a6cf2d10414454d8b8ad75ffeb13fb2cd (diff)
downloadchromium_src-27fbcccda73e7bd257c20f0bbf4b4c31af15c9c2.zip
chromium_src-27fbcccda73e7bd257c20f0bbf4b4c31af15c9c2.tar.gz
chromium_src-27fbcccda73e7bd257c20f0bbf4b4c31af15c9c2.tar.bz2
Ensure all callers to GetFilePath are on the File thread
by DCHECKing based on the thread type. TEST=Extensions and automated tests should work as before BUG=http://crbug.com/38521 Review URL: http://codereview.chromium.org/1293001 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@42578 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat
-rw-r--r--chrome/browser/browser_process_impl.cc7
-rw-r--r--chrome/browser/extensions/extension_dom_ui.cc2
-rw-r--r--chrome/browser/extensions/extension_protocols.cc5
-rw-r--r--chrome/common/extensions/extension_resource.cc27
-rw-r--r--chrome/common/extensions/extension_resource.h41
5 files changed, 66 insertions, 16 deletions
diff --git a/chrome/browser/browser_process_impl.cc b/chrome/browser/browser_process_impl.cc
index 30f0b24..afaf666 100644
--- a/chrome/browser/browser_process_impl.cc
+++ b/chrome/browser/browser_process_impl.cc
@@ -42,6 +42,7 @@
#include "chrome/common/chrome_constants.h"
#include "chrome/common/chrome_paths.h"
#include "chrome/common/chrome_switches.h"
+#include "chrome/common/extensions/extension_resource.h"
#include "chrome/common/extensions/extension_l10n_util.h"
#include "chrome/common/notification_service.h"
#include "chrome/common/pref_names.h"
@@ -335,6 +336,12 @@ void BrowserProcessImpl::CreateFileThread() {
if (!thread->StartWithOptions(options))
return;
file_thread_.swap(thread);
+
+ // ExtensionResource is in chrome/common, so it cannot depend on
+ // chrome/browser, which means it cannot lookup what the File thread is.
+ // We therefore store the thread ID from here so we can validate the proper
+ // thread usage in the ExtensionResource class.
+ ExtensionResource::set_file_thread_id(file_thread_->thread_id());
}
void BrowserProcessImpl::CreateDBThread() {
diff --git a/chrome/browser/extensions/extension_dom_ui.cc b/chrome/browser/extensions/extension_dom_ui.cc
index 0a7e5d1..7ee7339 100644
--- a/chrome/browser/extensions/extension_dom_ui.cc
+++ b/chrome/browser/extensions/extension_dom_ui.cc
@@ -322,5 +322,5 @@ RefCountedMemory* ExtensionDOMUI::GetFaviconResourceBytes(Profile* profile,
// TODO(arv): Move this off of the UI thread and onto the File thread. If
// possible to do this asynchronously, use ImageLoadingTracker.
return ReadFileData(extension->GetIconPath(
- Extension::EXTENSION_ICON_BITTY).GetFilePath());
+ Extension::EXTENSION_ICON_BITTY).GetFilePathOnAnyThreadHack());
}
diff --git a/chrome/browser/extensions/extension_protocols.cc b/chrome/browser/extensions/extension_protocols.cc
index b6e66f6..0dd4e7e 100644
--- a/chrome/browser/extensions/extension_protocols.cc
+++ b/chrome/browser/extensions/extension_protocols.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
+// Copyright (c) 2010 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.
@@ -44,7 +44,8 @@ static URLRequestJob* CreateExtensionURLRequestJob(URLRequest* request,
ExtensionResource resource(directory_path,
extension_file_util::ExtensionURLToRelativeFilePath(request->url()));
- return new URLRequestFileJob(request, resource.GetFilePath());
+ return new URLRequestFileJob(request,
+ resource.GetFilePathOnAnyThreadHack());
}
// Factory registered with URLRequest to create URLRequestJobs for
diff --git a/chrome/common/extensions/extension_resource.cc b/chrome/common/extensions/extension_resource.cc
index 8eed2e9..eda2d26 100644
--- a/chrome/common/extensions/extension_resource.cc
+++ b/chrome/common/extensions/extension_resource.cc
@@ -7,6 +7,10 @@
#include "base/file_util.h"
#include "base/logging.h"
+PlatformThreadId ExtensionResource::file_thread_id_ = 0;
+
+bool ExtensionResource::check_for_file_thread_ = false;
+
ExtensionResource::ExtensionResource() {
}
@@ -16,7 +20,7 @@ ExtensionResource::ExtensionResource(const FilePath& extension_root,
relative_path_(relative_path) {
}
-const FilePath& ExtensionResource::GetFilePath() const {
+const FilePath& ExtensionResource::GetFilePathOnAnyThreadHack() const {
if (extension_root_.empty() || relative_path_.empty()) {
DCHECK(full_resource_path_.empty());
return full_resource_path_;
@@ -26,13 +30,20 @@ const FilePath& ExtensionResource::GetFilePath() const {
if (!full_resource_path_.empty())
return full_resource_path_;
- full_resource_path_ = GetFilePath(extension_root_, relative_path_);
+ full_resource_path_ =
+ GetFilePathOnAnyThreadHack(extension_root_, relative_path_);
return full_resource_path_;
}
+const FilePath& ExtensionResource::GetFilePath() const {
+ DCHECK(!check_for_file_thread_ ||
+ ExtensionResource::file_thread_id_ == PlatformThread::CurrentId());
+ return GetFilePathOnAnyThreadHack();
+}
+
// static
-FilePath ExtensionResource::GetFilePath(const FilePath& extension_root,
- const FilePath& relative_path) {
+FilePath ExtensionResource::GetFilePathOnAnyThreadHack(
+ const FilePath& extension_root, const FilePath& relative_path) {
// We need to resolve the parent references in the extension_root
// path on its own because IsParent doesn't like parent references.
FilePath clean_extension_root(extension_root);
@@ -57,6 +68,14 @@ FilePath ExtensionResource::GetFilePath(const FilePath& extension_root,
return FilePath();
}
+// static
+FilePath ExtensionResource::GetFilePath(
+ const FilePath& extension_root, const FilePath& relative_path) {
+ DCHECK(!check_for_file_thread_ ||
+ ExtensionResource::file_thread_id_ == PlatformThread::CurrentId());
+ return GetFilePathOnAnyThreadHack(extension_root, relative_path);
+}
+
// Unit-testing helpers.
FilePath::StringType ExtensionResource::NormalizeSeperators(
FilePath::StringType path) const {
diff --git a/chrome/common/extensions/extension_resource.h b/chrome/common/extensions/extension_resource.h
index e87787d..755c959 100644
--- a/chrome/common/extensions/extension_resource.h
+++ b/chrome/common/extensions/extension_resource.h
@@ -6,6 +6,7 @@
#define CHROME_COMMON_EXTENSIONS_EXTENSION_RESOURCE_H_
#include "base/file_path.h"
+#include "base/platform_thread.h"
// Represents a resource inside an extension. For example, an image, or a
// JavaScript file. This is more complicated than just a simple FilePath
@@ -18,20 +19,33 @@ class ExtensionResource {
ExtensionResource(const FilePath& extension_root,
const FilePath& relative_path);
- // Returns actual path to the resource (default or locale specific).
- // *** MIGHT HIT FILESYSTEM. Do not call on UI thread! ***
- // NOTE: If you need to access ExtensionResources, such as bitmaps, on the UI
- // thread, please use the ImageLoadingTracker, which uses the File thread.
+ // Returns actual path to the resource (default or locale specific). In the
+ // browser process, this will DCHECK if not called on the file thread. To
+ // easily load extension images on the UI thread, see ImageLoadingTracker.
const FilePath& GetFilePath() const;
- // Static version to avoid creating an instance of ExtensionResource
- // when all we want is the localization code.
- // *** MIGHT HIT FILESYSTEM. Do not call on UI thread! ***
- // NOTE: If you need to access ExtensionResources, such as bitmaps, on the UI
- // thread, please use the ImageLoadingTracker, which uses the File thread.
+ // Gets the physical file path for the extension resource, taking into account
+ // localization. In the browser process, this will DCHECK if not called on the
+ // file thread. To easily load extension images on the UI thread, see
+ // ImageLoadingTracker.
static FilePath GetFilePath(const FilePath& extension_root,
const FilePath& relative_path);
+ // Gets the file path on any thread. Unlike GetFilePath(), these can be called
+ // from any thread without DCHECKing.
+ //
+ // In the browser process, calling this method is almost always wrong. Use
+ // GetFilePath() on the file thread instead.
+ const FilePath& GetFilePathOnAnyThreadHack() const;
+ static FilePath GetFilePathOnAnyThreadHack(const FilePath& extension_root,
+ const FilePath& relative_path);
+
+ // Setter for the proper thread to run file tasks on.
+ static void set_file_thread_id(PlatformThreadId thread_id) {
+ file_thread_id_ = thread_id;
+ check_for_file_thread_ = true;
+ }
+
// Getters
const FilePath& extension_root() const { return extension_root_; }
const FilePath& relative_path() const { return relative_path_; }
@@ -49,6 +63,15 @@ class ExtensionResource {
// Full path to extension resource. Starts empty.
mutable FilePath full_resource_path_;
+
+ // The thread id for the file thread. If set, GetFilePath() and related
+ // methods will DCHECK that they are on this thread when called.
+ static PlatformThreadId file_thread_id_;
+
+ // Whether to check for file thread. See |file_thread_id_|. If set,
+ // GetFilePath() and related methods will DCHECK that they are on this thread
+ // when called.
+ static bool check_for_file_thread_;
};
#endif // CHROME_COMMON_EXTENSIONS_EXTENSION_RESOURCE_H_
generated by cgit v1.1 at 2025-04-04 22:58:27 +0000