Thread IO safety: annotate file_util, and block IO thread from doing IO

- Mark functions in file_util_posix as requiring permission to perform
  disk actions.
- Mark the IO thread as disallowed from performing disk actions.
- Temporarily work around the protections in places where we currently
  have bugs.

BUG=59847,59849,60207,60211,60394
TEST=no dchecks in debug builds

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@63636 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 43 insertions, 3 deletions

diff --git a/chrome/browser/extensions/autoupdate_interceptor.cc b/chrome/browser/extensions/autoupdate_interceptor.cc
index 2acd0a0..ef0091f 100644
--- a/chrome/browser/extensions/autoupdate_interceptor.cc
+++ b/chrome/browser/extensions/autoupdate_interceptor.cc
@@ -5,6 +5,7 @@
 #include "chrome/browser/extensions/autoupdate_interceptor.h"
 
 #include "base/file_util.h"
+#include "base/thread_restrictions.h"
 #include "chrome/browser/browser_thread.h"
 #include "net/url_request/url_request_test_job.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -40,6 +41,10 @@ URLRequestJob* AutoUpdateInterceptor::MaybeIntercept(URLRequest* request) {
       return NULL;
   }
 
+  // It's ok to do a blocking disk access on this thread; this class
+  // is just used for tests.
+  base::ThreadRestrictions::ScopedAllowIO allow_io;
+
   // Search for this request's url, ignoring any query parameters.
   GURL url = request->url();
   if (url.has_query()) {
@@ -61,6 +66,9 @@ URLRequestJob* AutoUpdateInterceptor::MaybeIntercept(URLRequest* request) {
 void AutoUpdateInterceptor::SetResponse(const std::string url,
                                         const FilePath& path) {
   EXPECT_TRUE(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  // It's ok to do a blocking disk access on this thread; this class
+  // is just used for tests.
+  base::ThreadRestrictions::ScopedAllowIO allow_io;
   GURL gurl(url);
   EXPECT_EQ("http", gurl.scheme());
   EXPECT_EQ("localhost", gurl.host());
diff --git a/chrome/browser/io_thread.cc b/chrome/browser/io_thread.cc
index 4dc520a..8772ffd 100644
--- a/chrome/browser/io_thread.cc
+++ b/chrome/browser/io_thread.cc
@@ -12,6 +12,7 @@
 #include "base/string_number_conversions.h"
 #include "base/string_split.h"
 #include "base/string_util.h"
+#include "base/thread_restrictions.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/browser_thread.h"
 #include "chrome/browser/gpu_process_host.h"
@@ -257,6 +258,13 @@ net::ProxyScriptFetcher* IOThread::CreateAndRegisterProxyScriptFetcher(
 }
 
 void IOThread::Init() {
+#if defined(OS_LINUX)
+  // TODO(evan): test and enable this on all platforms.
+  // Though this thread is called the "IO" thread, it actually just routes
+  // messages around; it shouldn't be allowed to perform any blocking disk I/O.
+  base::ThreadRestrictions::SetIOAllowed(false);
+#endif
+
   BrowserProcessSubThread::Init();
 
   DCHECK_EQ(MessageLoop::TYPE_IO, message_loop()->type());
diff --git a/chrome/browser/net/url_request_mock_http_job.cc b/chrome/browser/net/url_request_mock_http_job.cc
index a35cd1e..4029605 100644
--- a/chrome/browser/net/url_request_mock_http_job.cc
+++ b/chrome/browser/net/url_request_mock_http_job.cc
@@ -7,6 +7,7 @@
 #include "base/file_util.h"
 #include "base/message_loop.h"
 #include "base/string_util.h"
+#include "base/thread_restrictions.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/common/url_constants.h"
 #include "net/base/net_util.h"
@@ -87,6 +88,10 @@ bool URLRequestMockHTTPJob::IsRedirectResponse(GURL* location,
 // Private const version.
 void URLRequestMockHTTPJob::GetResponseInfoConst(
     net::HttpResponseInfo* info) const {
+  // We have to load our headers from disk, but we only use this class
+  // from tests, so allow these IO operations to happen on any thread.
+  base::ThreadRestrictions::ScopedAllowIO allow_io;
+
   FilePath header_file = FilePath(file_path_.value() + kMockHeaderFileSuffix);
   std::string raw_headers;
   if (!file_util::ReadFileToString(header_file, &raw_headers))
diff --git a/chrome/browser/net/url_request_mock_util.cc b/chrome/browser/net/url_request_mock_util.cc
index e6531f8..b3ac398 100644
--- a/chrome/browser/net/url_request_mock_util.cc
+++ b/chrome/browser/net/url_request_mock_util.cc
@@ -7,6 +7,7 @@
 #include <string>
 
 #include "base/path_service.h"
+#include "base/thread_restrictions.h"
 #include "chrome/browser/browser_thread.h"
 #include "chrome/browser/net/url_request_failed_dns_job.h"
 #include "chrome/browser/net/url_request_mock_http_job.h"
@@ -24,6 +25,11 @@ void SetUrlRequestMocksEnabled(bool enabled) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
 
   if (enabled) {
+    // We have to look around for our helper files, but we only use
+    // this from tests, so allow these IO operations to happen
+    // anywhere.
+    base::ThreadRestrictions::ScopedAllowIO allow_io;
+
     URLRequestFilter::GetInstance()->ClearHandlers();
 
     URLRequestFailedDnsJob::AddUrlHandler();
diff --git a/chrome/browser/printing/print_dialog_cloud_uitest.cc b/chrome/browser/printing/print_dialog_cloud_uitest.cc
index 1cc446d..3b57961 100644
--- a/chrome/browser/printing/print_dialog_cloud_uitest.cc
+++ b/chrome/browser/printing/print_dialog_cloud_uitest.cc
@@ -11,6 +11,7 @@
 #include "base/file_util.h"
 #include "base/path_service.h"
 #include "base/singleton.h"
+#include "base/thread_restrictions.h"
 #include "base/values.h"
 #include "chrome/browser/browser_list.h"
 #include "chrome/browser/browser_thread.h"
@@ -32,7 +33,11 @@ class TestData {
  public:
   TestData() {}
 
-  char* GetTestData() {
+  const char* GetTestData() {
+    // Fetching this data blocks the IO thread, but we don't really care because
+    // this is a test.
+    base::ThreadRestrictions::ScopedAllowIO allow_io;
+
     if (test_data_.empty()) {
       FilePath test_data_directory;
       PathService::Get(chrome::DIR_TEST_DATA, &test_data_directory);
@@ -40,7 +45,7 @@ class TestData {
           test_data_directory.AppendASCII("printing/cloud_print_uitest.html");
       file_util::ReadFileToString(test_file, &test_data_);
     }
-    return &test_data_[0];
+    return test_data_.c_str();
   }
  private:
   std::string test_data_;
diff --git a/chrome/browser/service/service_process_control.cc b/chrome/browser/service/service_process_control.cc
index 534cc5a..6a41395 100644
--- a/chrome/browser/service/service_process_control.cc
+++ b/chrome/browser/service/service_process_control.cc
@@ -9,6 +9,7 @@
 #include "base/process_util.h"
 #include "base/stl_util-inl.h"
 #include "base/thread.h"
+#include "base/thread_restrictions.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/browser_thread.h"
 #include "chrome/browser/io_thread.h"
@@ -55,7 +56,14 @@ class ServiceProcessControl::Launcher
   void DoDetectLaunched(Task* task) {
     const uint32 kMaxLaunchDetectRetries = 10;
 
-    launched_ = CheckServiceProcessReady();
+    {
+      // We should not be doing blocking disk IO from this thread!
+      // Temporarily allowed until we fix
+      //   http://code.google.com/p/chromium/issues/detail?id=60207
+      base::ThreadRestrictions::ScopedAllowIO allow_io;
+      launched_ = CheckServiceProcessReady();
+    }
+
     if (launched_ || (retry_count_ >= kMaxLaunchDetectRetries)) {
       BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
           NewRunnableMethod(this, &Launcher::Notify, task));