Moved JsonPrefStore to use SequencedWorkerPool

chrome/common/json_pref_store.h

Index: chrome/common/json_pref_store.h
diff --git a/chrome/common/json_pref_store.h b/chrome/common/json_pref_store.h
index 7ceb7dad2ac0c472e51a1405e58861e6fd6ad93a..20ccb775f2d863f19336e9f7e3a027b934050b11 100644
--- a/chrome/common/json_pref_store.h
+++ b/chrome/common/json_pref_store.h
@@ -18,7 +18,8 @@
 
 namespace base {
 class DictionaryValue;
-class MessageLoopProxy;
+class SequencedWorkerPool;
+class SequencedTaskRunner;
 class Value;
 }
 
@@ -28,10 +29,17 @@ class FilePath;
 class JsonPrefStore : public PersistentPrefStore,
                       public ImportantFileWriter::DataSerializer {
  public:
-  // |file_message_loop_proxy| is the MessageLoopProxy for a thread on which
-  // file I/O can be done.
-  JsonPrefStore(const FilePath& pref_filename,
-                base::MessageLoopProxy* file_message_loop_proxy);
+  // 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(

[akalin, 2012/10/19 02:00:51]

have this return a raw pointer like Create() below. (It's the recommended way,
counter-intuitively)

[zel, 2012/10/19 18:45:07]

Really? SequencedWorkerPool::GetSequencedTaskRunnerWithShutdownBehavior returns
scoped_refptr<SequencedTaskRunner> and I wanted to stay consistent with that.

Wouldn't we nuke the instance of SequencedTaskRunner object if the returned
scoped_refptr<SequencedTaskRunner> from
SequencedWorkerPool::GetSequencedTaskRunnerWithShutdownBehavior goes out of
scope at the end of this function?

+      const FilePath& pref_filename,
+      base::SequencedWorkerPool* worker_pool);
+
+  // Creates an instance JsonPrefStore for a given |pref_filename| that will
+  // perform all file I/O operations on |sequenced_task_runner|.

[akalin, 2012/10/19 02:00:51]

comment here that |sequenced_task_runner| must be a shutdown-blocking task
runner, and that it should (almost?) always be one created by
GetTaskRunnerForFile above.

[zel, 2012/10/19 18:45:07]

Done.

+  static JsonPrefStore* Create(
+      const FilePath& pref_filename,
+      base::SequencedTaskRunner* sequenced_task_runner);
 
   // PrefStore overrides:
   virtual ReadResult GetValue(const std::string& key,
@@ -63,13 +71,16 @@ class JsonPrefStore : public PersistentPrefStore,
   void OnFileRead(base::Value* value_owned, PrefReadError error, bool no_dir);
 
  private:
+  // |task_runner| is the TaskRunner for a thread on which file I/O can be done.

[akalin, 2012/10/19 02:00:51]

update comment:

task_runner -> sequenced_task_runner
TaskRunner for a thread -> SequencedTaskRunner

[zel, 2012/10/19 18:45:07]

Done.

+  JsonPrefStore(const FilePath& pref_filename,
+                base::SequencedTaskRunner* sequenced_task_runner);
   virtual ~JsonPrefStore();
 
   // ImportantFileWriter::DataSerializer overrides:
   virtual bool SerializeData(std::string* output) OVERRIDE;
 
   FilePath path_;
-  scoped_refptr<base::MessageLoopProxy> file_message_loop_proxy_;
+  scoped_refptr<base::SequencedTaskRunner> sequenced_task_runner_;

[akalin, 2012/10/19 02:00:51]

const scoped_refptr

[zel, 2012/10/19 18:45:07]

Done.

 
   scoped_ptr<base::DictionaryValue> prefs_;