Prepare ExternalPolicyDataUpdater and ResourceCache for blocking pool

This CL allows the ExternalPolicyDataUpdater and ResourceCache classes to
be used in the blocking pool. It is a prerequisite for CL 22562003 which
will add code to downloading and caching of external policy data that runs
in the blocking pool and uses these classes.

BUG=256635
TEST=Updated unit tests

Review URL: https://chromiumcodereview.appspot.com/23868021

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

chrome/browser/chromeos/policy/user_cloud_policy_manager_factory_chromeos.cc

@@ -7,6 +7,8 @@
 #include "base/command_line.h"
 #include "base/files/file_path.h"
 #include "base/logging.h"
+#include "base/memory/ref_counted.h"
+#include "base/message_loop/message_loop_proxy.h"
 #include "base/path_service.h"
 #include "base/time/time.h"
 #include "chrome/browser/browser_process.h"
@@ -24,6 +26,7 @@
 #include "chromeos/chromeos_switches.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "components/browser_context_keyed_service/browser_context_dependency_manager.h"
+#include "content/public/browser/browser_thread.h"
 #include "net/url_request/url_request_context_getter.h"
 
 namespace policy {
@@ -147,8 +150,12 @@ scoped_ptr<UserCloudPolicyManagerChromeOS>
     store->LoadImmediately();
 
   scoped_ptr<ResourceCache> resource_cache;
-  if (command_line->HasSwitch(switches::kEnableComponentCloudPolicy))
-    resource_cache.reset(new ResourceCache(resource_cache_dir));
+  if (command_line->HasSwitch(switches::kEnableComponentCloudPolicy)) {
+    resource_cache.reset(new ResourceCache(
+        resource_cache_dir,
+        content::BrowserThread::GetMessageLoopProxyForThread(
+            content::BrowserThread::FILE)));
+  }
 
   scoped_ptr<UserCloudPolicyManagerChromeOS> manager(
       new UserCloudPolicyManagerChromeOS(

chrome/browser/policy/cloud/cloud_external_data_manager_base_unittest.cc

@@ -144,7 +144,8 @@ CouldExternalDataManagerBaseTest::CouldExternalDataManagerBaseTest()
 
 void CouldExternalDataManagerBaseTest::SetUp() {
   ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
-  resource_cache_.reset(new ResourceCache(temp_dir_.path()));
+  resource_cache_.reset(new ResourceCache(temp_dir_.path(),
+                                          base::MessageLoopProxy::current()));
   SetUpExternalDataManager();
 
   // Set |kStringPolicy| to a string value.

chrome/browser/policy/cloud/cloud_external_data_store_unittest.cc

@@ -6,8 +6,10 @@
 
 #include "base/compiler_specific.h"
 #include "base/files/scoped_temp_dir.h"
+#include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/sha1.h"
+#include "base/test/test_simple_task_runner.h"
 #include "chrome/browser/policy/cloud/resource_cache.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -49,7 +51,9 @@ CouldExternalDataStoreTest::CouldExternalDataStoreTest()
 
 void CouldExternalDataStoreTest::SetUp() {
   ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
-  resource_cache_.reset(new ResourceCache(temp_dir_.path()));
+  resource_cache_.reset(new ResourceCache(
+      temp_dir_.path(),
+      make_scoped_refptr(new base::TestSimpleTaskRunner)));
 }
 
 TEST_F(CouldExternalDataStoreTest, StoreAndLoad) {

chrome/browser/policy/cloud/component_cloud_policy_service_unittest.cc

@@ -107,7 +107,7 @@ class ComponentCloudPolicyServiceTest : public testing::Test {
 
   virtual void SetUp() OVERRIDE {
     ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
-    cache_ = new ResourceCache(temp_dir_.path());
+    cache_ = new ResourceCache(temp_dir_.path(), loop_.message_loop_proxy());
     service_.reset(new ComponentCloudPolicyService(
         &delegate_,
         &store_,

chrome/browser/policy/cloud/component_cloud_policy_store_unittest.cc

@@ -11,8 +11,9 @@
 #include "base/basictypes.h"
 #include "base/callback.h"
 #include "base/files/scoped_temp_dir.h"
-#include "base/memory/scoped_ptr.h"
+#include "base/memory/ref_counted.h"
 #include "base/sha1.h"
+#include "base/test/test_simple_task_runner.h"
 #include "chrome/browser/policy/cloud/cloud_policy_constants.h"
 #include "chrome/browser/policy/cloud/policy_builder.h"
 #include "chrome/browser/policy/cloud/resource_cache.h"
@@ -61,7 +62,9 @@ class ComponentCloudPolicyStoreTest : public testing::Test {
  protected:
   virtual void SetUp() OVERRIDE {
     ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
-    cache_.reset(new ResourceCache(temp_dir_.path()));
+    cache_.reset(new ResourceCache(
+        temp_dir_.path(),
+        make_scoped_refptr(new base::TestSimpleTaskRunner)));
     store_.reset(new ComponentCloudPolicyStore(&store_delegate_, cache_.get()));
     store_->SetCredentials(ComponentPolicyBuilder::kFakeUsername,
                            ComponentPolicyBuilder::kFakeToken);

chrome/browser/policy/cloud/component_cloud_policy_updater_unittest.cc

@@ -73,12 +73,12 @@ class ComponentCloudPolicyUpdaterTest : public testing::Test {
 
   scoped_ptr<em::PolicyFetchResponse> CreateResponse();
 
+  scoped_refptr<base::TestSimpleTaskRunner> task_runner_;
   base::ScopedTempDir temp_dir_;
   scoped_ptr<ResourceCache> cache_;
   scoped_ptr<ComponentCloudPolicyStore> store_;
   MockComponentCloudPolicyStoreDelegate store_delegate_;
   net::TestURLFetcherFactory fetcher_factory_;
-  scoped_refptr<base::TestSimpleTaskRunner> task_runner_;
   scoped_ptr<ExternalPolicyDataFetcherBackend> fetcher_backend_;
   scoped_ptr<ComponentCloudPolicyUpdater> updater_;
   ComponentPolicyBuilder builder_;
@@ -87,12 +87,12 @@ class ComponentCloudPolicyUpdaterTest : public testing::Test {
 
 void ComponentCloudPolicyUpdaterTest::SetUp() {
   ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
-  cache_.reset(new ResourceCache(temp_dir_.path()));
+  task_runner_ = new base::TestSimpleTaskRunner();
+  cache_.reset(new ResourceCache(temp_dir_.path(), task_runner_));
   store_.reset(new ComponentCloudPolicyStore(&store_delegate_, cache_.get()));
   store_->SetCredentials(ComponentPolicyBuilder::kFakeUsername,
                          ComponentPolicyBuilder::kFakeToken);
   fetcher_factory_.set_remove_fetcher_on_delete(true);
-  task_runner_ = new base::TestSimpleTaskRunner();
   fetcher_backend_.reset(new ExternalPolicyDataFetcherBackend(
       task_runner_,
       scoped_refptr<net::URLRequestContextGetter>()));

chrome/browser/policy/cloud/external_policy_data_updater.cc

@@ -304,7 +304,7 @@ ExternalPolicyDataUpdater::ExternalPolicyDataUpdater(
 }
 
 ExternalPolicyDataUpdater::~ExternalPolicyDataUpdater() {
-  DCHECK(CalledOnValidThread());
+  DCHECK(task_runner_->RunsTasksOnCurrentThread());
   shutting_down_ = true;
   STLDeleteValues(&job_map_);
 }
@@ -313,7 +313,7 @@ void ExternalPolicyDataUpdater::FetchExternalData(
     const std::string key,
     const Request& request,
     const FetchSuccessCallback& callback) {
-  DCHECK(CalledOnValidThread());
+  DCHECK(task_runner_->RunsTasksOnCurrentThread());
 
   // Check whether a job exists for this |key| already.
   FetchJob* job = job_map_[key];
@@ -338,7 +338,7 @@ void ExternalPolicyDataUpdater::FetchExternalData(
 
 void ExternalPolicyDataUpdater::CancelExternalDataFetch(
     const std::string& key) {
-  DCHECK(CalledOnValidThread());
+  DCHECK(task_runner_->RunsTasksOnCurrentThread());
 
   // If a |job| exists for this |key|, delete it. If the |job| is on the queue,
   // its WeakPtr will be invalidated and skipped by StartNextJobs(). If |job| is

chrome/browser/policy/cloud/external_policy_data_updater.h

@@ -14,7 +14,6 @@
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
-#include "base/threading/non_thread_safe.h"
 
 namespace base {
 class SequencedTaskRunner;
@@ -32,7 +31,7 @@ class ExternalPolicyDataFetcher;
 // with exponential backoff.
 // The actual fetching is handled by an ExternalPolicyDataFetcher, allowing this
 // class to run on a background thread where network I/O is not possible.
-class ExternalPolicyDataUpdater : public base::NonThreadSafe {
+class ExternalPolicyDataUpdater {
  public:
   struct Request {
    public:

chrome/browser/policy/cloud/resource_cache.cc

@@ -8,6 +8,7 @@
 #include "base/file_util.h"
 #include "base/files/file_enumerator.h"
 #include "base/logging.h"
+#include "base/sequenced_task_runner.h"
 #include "base/strings/string_util.h"
 
 namespace policy {
@@ -53,21 +54,21 @@ bool Base64Decode(const std::string& encoded, std::string* value) {
 
 }  // namespace
 
-ResourceCache::ResourceCache(const base::FilePath& cache_dir)
-    : cache_dir_(cache_dir) {
-  // Allow the cache to be created in a different thread than the thread that is
-  // going to use it.
-  DetachFromThread();
+ResourceCache::ResourceCache(
+    const base::FilePath& cache_dir,
+    scoped_refptr<base::SequencedTaskRunner> task_runner)
+    : cache_dir_(cache_dir),
+      task_runner_(task_runner) {
 }
 
 ResourceCache::~ResourceCache() {
-  DCHECK(CalledOnValidThread());
+  DCHECK(task_runner_->RunsTasksOnCurrentThread());
 }
 
 bool ResourceCache::Store(const std::string& key,
                           const std::string& subkey,
                           const std::string& data) {
-  DCHECK(CalledOnValidThread());
+  DCHECK(task_runner_->RunsTasksOnCurrentThread());
   base::FilePath subkey_path;
   // Delete the file before writing to it. This ensures that the write does not
   // follow a symlink planted at |subkey_path|, clobbering a file outside the
@@ -86,7 +87,7 @@ bool ResourceCache::Store(const std::string& key,
 bool ResourceCache::Load(const std::string& key,
                          const std::string& subkey,
                          std::string* data) {
-  DCHECK(CalledOnValidThread());
+  DCHECK(task_runner_->RunsTasksOnCurrentThread());
   base::FilePath subkey_path;
   // Only read from |subkey_path| if it is not a symlink.
   if (!VerifyKeyPathAndGetSubkeyPath(key, false, subkey, &subkey_path) ||
@@ -100,7 +101,7 @@ bool ResourceCache::Load(const std::string& key,
 void ResourceCache::LoadAllSubkeys(
     const std::string& key,
     std::map<std::string, std::string>* contents) {
-  DCHECK(CalledOnValidThread());
+  DCHECK(task_runner_->RunsTasksOnCurrentThread());
   contents->clear();
   base::FilePath key_path;
   if (!VerifyKeyPath(key, false, &key_path))
@@ -123,7 +124,7 @@ void ResourceCache::LoadAllSubkeys(
 }
 
 void ResourceCache::Delete(const std::string& key, const std::string& subkey) {
-  DCHECK(CalledOnValidThread());
+  DCHECK(task_runner_->RunsTasksOnCurrentThread());
   base::FilePath subkey_path;
   if (VerifyKeyPathAndGetSubkeyPath(key, false, subkey, &subkey_path))
     base::DeleteFile(subkey_path, false);
@@ -134,7 +135,7 @@ void ResourceCache::Delete(const std::string& key, const std::string& subkey) {
 }
 
 void ResourceCache::PurgeOtherKeys(const std::set<std::string>& keys_to_keep) {
-  DCHECK(CalledOnValidThread());
+  DCHECK(task_runner_->RunsTasksOnCurrentThread());
   std::set<std::string> encoded_keys_to_keep;
   if (!Base64Encode(keys_to_keep, &encoded_keys_to_keep))
     return;
@@ -152,7 +153,7 @@ void ResourceCache::PurgeOtherKeys(const std::set<std::string>& keys_to_keep) {
 void ResourceCache::PurgeOtherSubkeys(
     const std::string& key,
     const std::set<std::string>& subkeys_to_keep) {
-  DCHECK(CalledOnValidThread());
+  DCHECK(task_runner_->RunsTasksOnCurrentThread());
   base::FilePath key_path;
   if (!VerifyKeyPath(key, false, &key_path))
     return;

chrome/browser/policy/cloud/resource_cache.h

@@ -11,7 +11,11 @@
 
 #include "base/basictypes.h"
 #include "base/files/file_path.h"
-#include "base/threading/non_thread_safe.h"
+#include "base/memory/ref_counted.h"
+
+namespace base {
+class SequencedTaskRunner;
+}
 
 namespace policy {
 
@@ -19,11 +23,12 @@ namespace policy {
 // a subkey, and can be queried by (key, subkey) or (key) lookups.
 // The contents of the cache have to be manually cleared using Delete() or
 // PurgeOtherSubkeys().
-// Instances of this class can be created on any thread, but from then on must
-// be always used from the same thread, and it must support file I/O.
-class ResourceCache : public base::NonThreadSafe {
+// The class can be instantiated on any thread but from then on, it must be
+// accessed via the |task_runner| only. The |task_runner| must support file I/O.
+class ResourceCache {
  public:
-  explicit ResourceCache(const base::FilePath& cache_path);
+  explicit ResourceCache(const base::FilePath& cache_path,
+                         scoped_refptr<base::SequencedTaskRunner> task_runner);
   virtual ~ResourceCache();
 
   // Stores |data| under (key, subkey). Returns true if the store suceeded, and
@@ -72,6 +77,9 @@ class ResourceCache : public base::NonThreadSafe {
 
   base::FilePath cache_dir_;
 
+  // Task runner that |this| runs on.
+  scoped_refptr<base::SequencedTaskRunner> task_runner_;
+
   DISALLOW_COPY_AND_ASSIGN(ResourceCache);
 };
 

chrome/browser/policy/cloud/resource_cache_unittest.cc

@@ -6,6 +6,7 @@
 
 #include "base/basictypes.h"
 #include "base/files/scoped_temp_dir.h"
+#include "base/test/test_simple_task_runner.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace policy {
@@ -29,7 +30,8 @@ const char kData1[] = "{}";
 TEST(ResourceCacheTest, StoreAndLoad) {
   base::ScopedTempDir temp_dir;
   ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
-  ResourceCache cache(temp_dir.path());
+  ResourceCache cache(temp_dir.path(),
+                      make_scoped_refptr(new base::TestSimpleTaskRunner));
 
   // No data initially.
   std::string data;