Revert 165062 - We believe this change broke startup_tests

Original description:

Moved JsonPrefStore to use SequencedWorkerPool instead of FILE thread. The pool also ensures that the same file requests are written in order received and that they block on shutdown.

BUG=153367
TEST=existing unit/browser tests

Review URL: https://codereview.chromium.org/11027070

TBR=zelidrag@chromium.org
Review URL: https://codereview.chromium.org/11312046

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

3 files changed, 40 insertions, 63 deletions

diff --git a/base/prefs/json_pref_store.cc b/base/prefs/json_pref_store.cc
index 98810d5..18dc0b7 100644
--- a/base/prefs/json_pref_store.cc
+++ b/base/prefs/json_pref_store.cc
@@ -13,8 +13,6 @@
 #include "base/json/json_string_value_serializer.h"
 #include "base/memory/ref_counted.h"
 #include "base/message_loop_proxy.h"
-#include "base/sequenced_task_runner.h"
-#include "base/threading/sequenced_worker_pool.h"
 #include "base/values.h"
 
 namespace {
@@ -28,25 +26,25 @@ class FileThreadDeserializer
     : public base::RefCountedThreadSafe<FileThreadDeserializer> {
  public:
   FileThreadDeserializer(JsonPrefStore* delegate,
-                         base::SequencedTaskRunner* sequenced_task_runner)
+                         base::MessageLoopProxy* file_loop_proxy)
       : no_dir_(false),
         error_(PersistentPrefStore::PREF_READ_ERROR_NONE),
         delegate_(delegate),
-        sequenced_task_runner_(sequenced_task_runner),
+        file_loop_proxy_(file_loop_proxy),
         origin_loop_proxy_(base::MessageLoopProxy::current()) {
   }
 
   void Start(const FilePath& path) {
     DCHECK(origin_loop_proxy_->BelongsToCurrentThread());
-    sequenced_task_runner_->PostTask(
+    file_loop_proxy_->PostTask(
         FROM_HERE,
         base::Bind(&FileThreadDeserializer::ReadFileAndReport,
                    this, path));
   }
 
-  // Deserializes JSON on the sequenced task runner.
+  // Deserializes JSON on the file thread.
   void ReadFileAndReport(const FilePath& path) {
-    DCHECK(sequenced_task_runner_->RunsTasksOnCurrentThread());
+    DCHECK(file_loop_proxy_->BelongsToCurrentThread());
 
     value_.reset(DoReading(path, &error_, &no_dir_));
 
@@ -86,9 +84,9 @@ class FileThreadDeserializer
   bool no_dir_;
   PersistentPrefStore::PrefReadError error_;
   scoped_ptr<Value> value_;
-  const scoped_refptr<JsonPrefStore> delegate_;
-  const scoped_refptr<base::SequencedTaskRunner> sequenced_task_runner_;
-  const scoped_refptr<base::MessageLoopProxy> origin_loop_proxy_;
+  scoped_refptr<JsonPrefStore> delegate_;
+  scoped_refptr<base::MessageLoopProxy> file_loop_proxy_;
+  scoped_refptr<base::MessageLoopProxy> origin_loop_proxy_;
 };
 
 // static
@@ -139,23 +137,13 @@ void FileThreadDeserializer::HandleErrors(
 
 }  // namespace
 
-scoped_refptr<base::SequencedTaskRunner> JsonPrefStore::GetTaskRunnerForFile(
-    const FilePath& filename,
-    base::SequencedWorkerPool* worker_pool) {
-  std::string token("json_pref_store-");
-  token.append(filename.AsUTF8Unsafe());
-  return worker_pool->GetSequencedTaskRunnerWithShutdownBehavior(
-      worker_pool->GetNamedSequenceToken(token),
-      base::SequencedWorkerPool::BLOCK_SHUTDOWN);
-}
-
 JsonPrefStore::JsonPrefStore(const FilePath& filename,
-                             base::SequencedTaskRunner* sequenced_task_runner)
+                             base::MessageLoopProxy* file_message_loop_proxy)
     : path_(filename),
-      sequenced_task_runner_(sequenced_task_runner),
+      file_message_loop_proxy_(file_message_loop_proxy),
       prefs_(new DictionaryValue()),
       read_only_(false),
-      writer_(filename, sequenced_task_runner),
+      writer_(filename, file_message_loop_proxy),
       error_delegate_(NULL),
       initialized_(false),
       read_error_(PREF_READ_ERROR_OTHER) {
@@ -257,7 +245,7 @@ void JsonPrefStore::ReadPrefsAsync(ReadErrorDelegate *error_delegate) {
   // Start async reading of the preferences file. It will delete itself
   // in the end.
   scoped_refptr<FileThreadDeserializer> deserializer(
-      new FileThreadDeserializer(this, sequenced_task_runner_.get()));
+      new FileThreadDeserializer(this, file_message_loop_proxy_.get()));
   deserializer->Start(path_);
 }
 
diff --git a/base/prefs/json_pref_store.h b/base/prefs/json_pref_store.h
index 7a75d4c..5ae1ca0 100644
--- a/base/prefs/json_pref_store.h
+++ b/base/prefs/json_pref_store.h
@@ -20,8 +20,7 @@
 
 namespace base {
 class DictionaryValue;
-class SequencedWorkerPool;
-class SequencedTaskRunner;
+class MessageLoopProxy;
 class Value;
 }
 
@@ -32,16 +31,10 @@ class BASE_PREFS_EXPORT JsonPrefStore
     : public PersistentPrefStore,
       public base::ImportantFileWriter::DataSerializer {
  public:
-  // Returns instance of SequencedTaskRunner which guarantees that file
-  // operations on the same file will be executed in sequenced order.
-  static scoped_refptr<base::SequencedTaskRunner> GetTaskRunnerForFile(
-      const FilePath& pref_filename,
-      base::SequencedWorkerPool* worker_pool);
-
-  // |sequenced_task_runner| is must be a shutdown-blocking task runner, ideally
-  // created by GetTaskRunnerForFile() method above.
+  // |file_message_loop_proxy| is the MessageLoopProxy for a thread on which
+  // file I/O can be done.
   JsonPrefStore(const FilePath& pref_filename,
-                base::SequencedTaskRunner* sequenced_task_runner);
+                base::MessageLoopProxy* file_message_loop_proxy);
 
   // PrefStore overrides:
   virtual ReadResult GetValue(const std::string& key,
@@ -79,7 +72,7 @@ class BASE_PREFS_EXPORT JsonPrefStore
   virtual bool SerializeData(std::string* output) OVERRIDE;
 
   FilePath path_;
-  const scoped_refptr<base::SequencedTaskRunner> sequenced_task_runner_;
+  scoped_refptr<base::MessageLoopProxy> file_message_loop_proxy_;
 
   scoped_ptr<base::DictionaryValue> prefs_;
 
diff --git a/base/prefs/json_pref_store_unittest.cc b/base/prefs/json_pref_store_unittest.cc
index e7239bc..7661acd 100644
--- a/base/prefs/json_pref_store_unittest.cc
+++ b/base/prefs/json_pref_store_unittest.cc
@@ -5,12 +5,13 @@
 #include "base/file_util.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/message_loop.h"
+#include "base/message_loop_proxy.h"
 #include "base/path_service.h"
 #include "base/prefs/json_pref_store.h"
 #include "base/scoped_temp_dir.h"
 #include "base/string_number_conversions.h"
 #include "base/string_util.h"
-#include "base/threading/sequenced_worker_pool.h"
 #include "base/threading/thread.h"
 #include "base/utf_string_conversions.h"
 #include "base/values.h"
@@ -36,7 +37,9 @@ class MockReadErrorDelegate : public PersistentPrefStore::ReadErrorDelegate {
 
 class JsonPrefStoreTest : public testing::Test {
  protected:
-  virtual void SetUp() OVERRIDE {
+  virtual void SetUp() {
+    message_loop_proxy_ = base::MessageLoopProxy::current();
+
     ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
 
     ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &data_dir_));
@@ -50,6 +53,7 @@ class JsonPrefStoreTest : public testing::Test {
   FilePath data_dir_;
   // A message loop that we can use as the file thread message loop.
   MessageLoop message_loop_;
+  scoped_refptr<base::MessageLoopProxy> message_loop_proxy_;
 };
 
 // Test fallback behavior for a nonexistent file.
@@ -57,8 +61,7 @@ TEST_F(JsonPrefStoreTest, NonExistentFile) {
   FilePath bogus_input_file = data_dir_.AppendASCII("read.txt");
   ASSERT_FALSE(file_util::PathExists(bogus_input_file));
   scoped_refptr<JsonPrefStore> pref_store =
-      new JsonPrefStore(
-          bogus_input_file, message_loop_.message_loop_proxy());
+      new JsonPrefStore(bogus_input_file, message_loop_proxy_.get());
   EXPECT_EQ(PersistentPrefStore::PREF_READ_ERROR_NO_FILE,
             pref_store->ReadPrefs());
   EXPECT_FALSE(pref_store->ReadOnly());
@@ -70,8 +73,7 @@ TEST_F(JsonPrefStoreTest, InvalidFile) {
   FilePath invalid_file = temp_dir_.path().AppendASCII("invalid.json");
   ASSERT_TRUE(file_util::CopyFile(invalid_file_original, invalid_file));
   scoped_refptr<JsonPrefStore> pref_store =
-      new JsonPrefStore(
-          invalid_file, message_loop_.message_loop_proxy());
+      new JsonPrefStore(invalid_file, message_loop_proxy_.get());
   EXPECT_EQ(PersistentPrefStore::PREF_READ_ERROR_JSON_PARSE,
             pref_store->ReadPrefs());
   EXPECT_FALSE(pref_store->ReadOnly());
@@ -86,7 +88,7 @@ TEST_F(JsonPrefStoreTest, InvalidFile) {
 
 // This function is used to avoid code duplication while testing synchronous and
 // asynchronous version of the JsonPrefStore loading.
-void RunBasicJsonPrefStoreTest(JsonPrefStore* pref_store,
+void RunBasicJsonPrefStoreTest(JsonPrefStore *pref_store,
                                const FilePath& output_file,
                                const FilePath& golden_output_file) {
   const char kNewWindowsInTabs[] = "tabs.new_windows_in_tabs";
@@ -164,8 +166,7 @@ TEST_F(JsonPrefStoreTest, Basic) {
   FilePath input_file = temp_dir_.path().AppendASCII("write.json");
   ASSERT_TRUE(file_util::PathExists(input_file));
   scoped_refptr<JsonPrefStore> pref_store =
-      new JsonPrefStore(
-          input_file, message_loop_.message_loop_proxy());
+      new JsonPrefStore(input_file, message_loop_proxy_.get());
   ASSERT_EQ(PersistentPrefStore::PREF_READ_ERROR_NONE, pref_store->ReadPrefs());
   ASSERT_FALSE(pref_store->ReadOnly());
 
@@ -192,24 +193,21 @@ TEST_F(JsonPrefStoreTest, BasicAsync) {
   FilePath input_file = temp_dir_.path().AppendASCII("write.json");
   ASSERT_TRUE(file_util::PathExists(input_file));
   scoped_refptr<JsonPrefStore> pref_store =
-      new JsonPrefStore(
-          input_file, message_loop_.message_loop_proxy());
+      new JsonPrefStore(input_file, message_loop_proxy_.get());
 
-  {
-    MockPrefStoreObserver mock_observer;
-    pref_store->AddObserver(&mock_observer);
+  MockPrefStoreObserver mock_observer;
+  pref_store->AddObserver(&mock_observer);
 
-    MockReadErrorDelegate* mock_error_delegate = new MockReadErrorDelegate;
-    pref_store->ReadPrefsAsync(mock_error_delegate);
+  MockReadErrorDelegate *mock_error_delegate = new MockReadErrorDelegate;
+  pref_store->ReadPrefsAsync(mock_error_delegate);
 
-    EXPECT_CALL(mock_observer, OnInitializationCompleted(true)).Times(1);
-    EXPECT_CALL(*mock_error_delegate,
-                OnError(PersistentPrefStore::PREF_READ_ERROR_NONE)).Times(0);
-    message_loop_.RunUntilIdle();
-    pref_store->RemoveObserver(&mock_observer);
+  EXPECT_CALL(mock_observer, OnInitializationCompleted(true)).Times(1);
+  EXPECT_CALL(*mock_error_delegate,
+              OnError(PersistentPrefStore::PREF_READ_ERROR_NONE)).Times(0);
+  message_loop_.RunUntilIdle();
+  pref_store->RemoveObserver(&mock_observer);
 
-    ASSERT_FALSE(pref_store->ReadOnly());
-  }
+  ASSERT_FALSE(pref_store->ReadOnly());
 
   // The JSON file looks like this:
   // {
@@ -231,8 +229,7 @@ TEST_F(JsonPrefStoreTest, AsyncNonExistingFile) {
   FilePath bogus_input_file = data_dir_.AppendASCII("read.txt");
   ASSERT_FALSE(file_util::PathExists(bogus_input_file));
   scoped_refptr<JsonPrefStore> pref_store =
-      new JsonPrefStore(
-          bogus_input_file, message_loop_.message_loop_proxy());
+      new JsonPrefStore(bogus_input_file, message_loop_proxy_.get());
   MockPrefStoreObserver mock_observer;
   pref_store->AddObserver(&mock_observer);
 
@@ -258,8 +255,7 @@ TEST_F(JsonPrefStoreTest, NeedsEmptyValue) {
   // Test that the persistent value can be loaded.
   ASSERT_TRUE(file_util::PathExists(pref_file));
   scoped_refptr<JsonPrefStore> pref_store =
-      new JsonPrefStore(
-          pref_file, message_loop_.message_loop_proxy());
+      new JsonPrefStore(pref_file, message_loop_proxy_.get());
   ASSERT_EQ(PersistentPrefStore::PREF_READ_ERROR_NONE, pref_store->ReadPrefs());
   ASSERT_FALSE(pref_store->ReadOnly());