Adds async interface method to PersistentPrefStore.

chrome/browser/prefs/pref_service.cc

Index: chrome/browser/prefs/pref_service.cc
diff --git a/chrome/browser/prefs/pref_service.cc b/chrome/browser/prefs/pref_service.cc
index 9071f24898abe56a4e2f85186565ff74c218e880..beb4841d8619659b1e9fd1072420cdfed3d9d030 100644
--- a/chrome/browser/prefs/pref_service.cc
+++ b/chrome/browser/prefs/pref_service.cc
@@ -194,9 +194,15 @@ PrefService::~PrefService() {
   default_store_ = NULL;
 }
 
-void PrefService::OnPrefsRead(PersistentPrefStore::PrefReadError error,
-                              bool no_dir) {
-  if (no_dir) {
+void PrefService::OnInitializationCompleted() {

[Mattias Nissler (ping if slow), 2011/04/26 09:04:26]

If we use the PREF_INITIALIZATION_COMPLETED notification instead, this can go
away and the error handling here can really be moved either down to
JsonPrefStore, or over to PrefNotifier, which is the designated home for all
signaling to the outer world.

[altimofeev, 2011/04/27 10:32:08]

I have leaved "PrefStore::Observer" here, because "PrefStore::Observer" and
"PrefNotifier" both have OnInitializationCompleted() method, so
"PrefNotiferImpl" can't simply inherit from both of this interfaces.

[Mattias Nissler (ping if slow), 2011/04/27 12:16:30]

Yes, PrefNotifier already has OnInitializationCompleted(). Signaling the outer
world is a PrefNotifier responsibility, which is why I'd like to see the
signaling code moved there. You can change the interface between PrefValueStore
and PrefNotifier to what you need, it's internal to the pref system.

Here is more details on how I imagine this to work:

1. PrefService::CreatePrefService() gets called and PrefStores get constructed
2. ReadPrefs or ReadPrefsAsync() gets called
3. All the pref stores (including JsonPrefStore) notify PrefValueStore about
their initialization status by calling OnInitializationCompleted().
4. Once PrefValueStore has heard back from all of them (or decided there is an
error), it delegates to the PrefNotifier to tell the outside world.
5. PrefNotifierImpl sends the notification.

+  // This is the only message PrefService is waiting for, so remove us from the
+  // observers list.
+  user_pref_store_->RemoveObserver(this);
+
+  PersistentPrefStore::PrefReadError error;
+  bool is_fatal;
+  user_pref_store_->GetErrors(&error, &is_fatal);

[Bernhard Bauer, 2011/04/25 15:54:10]

Could you pass these values directly to OnInitializationCompleted? Then you
wouldn't need to store them in the user pref store, making reading the
preferences stateless.

[altimofeev, 2011/04/27 10:32:08]

The problem is that "OnInitializationCompleted" is part of the generic
"PrefStore::Observer" interface, which has other users as well.

[Bernhard Bauer, 2011/04/27 10:52:16]

I think passing (PREF_READ_ERROR_NONE, false) at the other call sites should be
fine. Mattias, WDYT?

Another idea would be to add an OnInitializationError method which gets these
values. The contract would then be that eventually one of these two methods is
called.

[Mattias Nissler (ping if slow), 2011/04/27 12:16:30]

I think passing the parameters makes sense. At the moment, your only use for the
GetErrors() function is so you can call it when you get notified. As long as
there's no reason to call GetErrors() in other context, it doesn't seem useful
to have it separately from the callback.

+  if (is_fatal) {
     // Bad news. When profile is created, the process that creates the directory
     // is explicitly started. So if directory is missing it probably means that
     // Chromium hasn't sufficient privileges.
@@ -228,13 +234,11 @@ void PrefService::OnPrefsRead(PersistentPrefStore::PrefReadError error,
 }
 
 void PrefService::InitFromStorage() {
+  user_pref_store_->AddObserver(this);
   if (!delegate_) {
-    const PersistentPrefStore::PrefReadError error =
-        user_pref_store_->ReadPrefs();
-    OnPrefsRead(error, false);
+    user_pref_store_->ReadPrefs();
   } else {
-    // todo(altimofeev): move this method to PersistentPrefStore interface.
-    (static_cast<JsonPrefStore*>(user_pref_store_.get()))->ReadPrefs(this);
+    user_pref_store_.get()->ReadPrefsAsync();
   }
 }