Adds async interface method, which is used by JsonPrefStore. Also see the bug description for more info.
BUG=chromium-os:14289
TEST=JsonPrefStoreTest.*
Review URL: http://codereview.chromium.org/6894020
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@84962 0039d316-1c4b-4281-b951-d872f2087c98
diff --git a/chrome/browser/browser_main.cc b/chrome/browser/browser_main.cc
index 68da6cb4..f80a813 100644
--- a/chrome/browser/browser_main.cc
+++ b/chrome/browser/browser_main.cc
@@ -793,7 +793,7 @@
FilePath parent_profile =
parsed_command_line.GetSwitchValuePath(switches::kParentProfile);
scoped_ptr<PrefService> parent_local_state(
- PrefService::CreatePrefService(parent_profile, NULL, NULL));
+ PrefService::CreatePrefService(parent_profile, NULL, NULL, false));
parent_local_state->RegisterStringPref(prefs::kApplicationLocale,
std::string());
// Right now, we only inherit the locale setting from the parent profile.
diff --git a/chrome/browser/browser_process_impl.cc b/chrome/browser/browser_process_impl.cc
index f2c276e..d1e43d6 100644
--- a/chrome/browser/browser_process_impl.cc
+++ b/chrome/browser/browser_process_impl.cc
@@ -881,7 +881,7 @@
FilePath local_state_path;
PathService::Get(chrome::FILE_LOCAL_STATE, &local_state_path);
local_state_.reset(
- PrefService::CreatePrefService(local_state_path, NULL, NULL));
+ PrefService::CreatePrefService(local_state_path, NULL, NULL, false));
// Initialize the prefs of the local state.
browser::RegisterLocalState(local_state_.get());
diff --git a/chrome/browser/chromeos/login/signed_settings_temp_storage_unittest.cc b/chrome/browser/chromeos/login/signed_settings_temp_storage_unittest.cc
index e196ea6..b6273eb 100644
--- a/chrome/browser/chromeos/login/signed_settings_temp_storage_unittest.cc
+++ b/chrome/browser/chromeos/login/signed_settings_temp_storage_unittest.cc
@@ -30,7 +30,8 @@
FilePath temp_file;
ASSERT_TRUE(
file_util::CreateTemporaryFileInDir(temp_dir_.path(), &temp_file));
- local_state_.reset(PrefService::CreatePrefService(temp_file, NULL, NULL));
+ local_state_.reset(
+ PrefService::CreatePrefService(temp_file, NULL, NULL, false));
ASSERT_TRUE(NULL != local_state_.get());
SignedSettingsTempStorage::RegisterPrefs(local_state_.get());
}
diff --git a/chrome/browser/content_settings/content_settings_pref_provider_unittest.cc b/chrome/browser/content_settings/content_settings_pref_provider_unittest.cc
index 105cb96..c6f7f2d 100644
--- a/chrome/browser/content_settings/content_settings_pref_provider_unittest.cc
+++ b/chrome/browser/content_settings/content_settings_pref_provider_unittest.cc
@@ -36,7 +36,7 @@
: PrefService(
managed_platform_prefs, managed_cloud_prefs, extension_prefs,
command_line_prefs, user_prefs, recommended_platform_prefs,
- recommended_cloud_prefs, default_store, NULL) {}
+ recommended_cloud_prefs, default_store, false) {}
virtual ~ContentSettingsPrefService() {}
};
}
diff --git a/chrome/browser/policy/configuration_policy_pref_store.cc b/chrome/browser/policy/configuration_policy_pref_store.cc
index 68fe0d0..96dfb2e 100644
--- a/chrome/browser/policy/configuration_policy_pref_store.cc
+++ b/chrome/browser/policy/configuration_policy_pref_store.cc
@@ -1039,7 +1039,7 @@
provider_->IsInitializationComplete()) {
initialization_complete_ = true;
FOR_EACH_OBSERVER(PrefStore::Observer, observers_,
- OnInitializationCompleted());
+ OnInitializationCompleted(true));
}
}
diff --git a/chrome/browser/policy/configuration_policy_pref_store_unittest.cc b/chrome/browser/policy/configuration_policy_pref_store_unittest.cc
index 07eed39..478c0cce 100644
--- a/chrome/browser/policy/configuration_policy_pref_store_unittest.cc
+++ b/chrome/browser/policy/configuration_policy_pref_store_unittest.cc
@@ -819,7 +819,7 @@
TEST_F(ConfigurationPolicyPrefStoreRefreshTest, Initialization) {
EXPECT_FALSE(store_->IsInitializationComplete());
- EXPECT_CALL(observer_, OnInitializationCompleted()).Times(1);
+ EXPECT_CALL(observer_, OnInitializationCompleted(true)).Times(1);
provider_.SetInitializationComplete(true);
EXPECT_FALSE(store_->IsInitializationComplete());
diff --git a/chrome/browser/prefs/overlay_persistent_pref_store.cc b/chrome/browser/prefs/overlay_persistent_pref_store.cc
index e75fbc66..13c51bb3 100644
--- a/chrome/browser/prefs/overlay_persistent_pref_store.cc
+++ b/chrome/browser/prefs/overlay_persistent_pref_store.cc
@@ -4,6 +4,7 @@
#include "chrome/browser/prefs/overlay_persistent_pref_store.h"
+#include "base/memory/scoped_ptr.h"
#include "base/values.h"
OverlayPersistentPrefStore::OverlayPersistentPrefStore(
@@ -80,9 +81,17 @@
PersistentPrefStore::PrefReadError OverlayPersistentPrefStore::ReadPrefs() {
// We do not read intentionally.
+ OnInitializationCompleted(true);
return PersistentPrefStore::PREF_READ_ERROR_NONE;
}
+void OverlayPersistentPrefStore::ReadPrefsAsync(
+ ReadErrorDelegate* error_delegate_raw) {
+ scoped_ptr<ReadErrorDelegate> error_delegate(error_delegate_raw);
+ // We do not read intentionally.
+ OnInitializationCompleted(true);
+}
+
bool OverlayPersistentPrefStore::WritePrefs() {
// We do not write intentionally.
return true;
@@ -105,7 +114,7 @@
FOR_EACH_OBSERVER(PrefStore::Observer, observers_, OnPrefValueChanged(key));
}
-void OverlayPersistentPrefStore::OnInitializationCompleted() {
+void OverlayPersistentPrefStore::OnInitializationCompleted(bool succeeded) {
FOR_EACH_OBSERVER(PrefStore::Observer, observers_,
- OnInitializationCompleted());
+ OnInitializationCompleted(succeeded));
}
diff --git a/chrome/browser/prefs/overlay_persistent_pref_store.h b/chrome/browser/prefs/overlay_persistent_pref_store.h
index bbb85c3..57ffe8d7 100644
--- a/chrome/browser/prefs/overlay_persistent_pref_store.h
+++ b/chrome/browser/prefs/overlay_persistent_pref_store.h
@@ -43,6 +43,7 @@
virtual void RemoveValue(const std::string& key);
virtual bool ReadOnly() const;
virtual PrefReadError ReadPrefs();
+ virtual void ReadPrefsAsync(ReadErrorDelegate* delegate);
virtual bool WritePrefs();
virtual void ScheduleWritePrefs();
virtual void CommitPendingWrite();
@@ -51,7 +52,7 @@
private:
// Methods of PrefStore::Observer.
virtual void OnPrefValueChanged(const std::string& key);
- virtual void OnInitializationCompleted();
+ virtual void OnInitializationCompleted(bool succeeded);
ObserverList<PrefStore::Observer, true> observers_;
PrefValueMap overlay_;
diff --git a/chrome/browser/prefs/pref_notifier.h b/chrome/browser/prefs/pref_notifier.h
index f2d9c52..f4d3a5a 100644
--- a/chrome/browser/prefs/pref_notifier.h
+++ b/chrome/browser/prefs/pref_notifier.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
@@ -21,7 +21,7 @@
virtual void OnPreferenceChanged(const std::string& pref_name) = 0;
// Broadcasts the intialization completed notification.
- virtual void OnInitializationCompleted() = 0;
+ virtual void OnInitializationCompleted(bool succeeded) = 0;
};
#endif // CHROME_BROWSER_PREFS_PREF_NOTIFIER_H_
diff --git a/chrome/browser/prefs/pref_notifier_impl.cc b/chrome/browser/prefs/pref_notifier_impl.cc
index d5ad24c..6d1dd2a 100644
--- a/chrome/browser/prefs/pref_notifier_impl.cc
+++ b/chrome/browser/prefs/pref_notifier_impl.cc
@@ -74,13 +74,13 @@
FireObservers(path);
}
-void PrefNotifierImpl::OnInitializationCompleted() {
+void PrefNotifierImpl::OnInitializationCompleted(bool succeeded) {
DCHECK(CalledOnValidThread());
NotificationService::current()->Notify(
NotificationType::PREF_INITIALIZATION_COMPLETED,
Source<PrefService>(pref_service_),
- NotificationService::NoDetails());
+ Details<bool>(&succeeded));
}
void PrefNotifierImpl::FireObservers(const std::string& path) {
diff --git a/chrome/browser/prefs/pref_notifier_impl.h b/chrome/browser/prefs/pref_notifier_impl.h
index 085e547..8323fcbe 100644
--- a/chrome/browser/prefs/pref_notifier_impl.h
+++ b/chrome/browser/prefs/pref_notifier_impl.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
@@ -30,7 +30,7 @@
// PrefNotifier overrides.
virtual void OnPreferenceChanged(const std::string& pref_name);
- virtual void OnInitializationCompleted();
+ virtual void OnInitializationCompleted(bool succeeded);
protected:
// A map from pref names to a list of observers. Observers get fired in the
diff --git a/chrome/browser/prefs/pref_notifier_impl_unittest.cc b/chrome/browser/prefs/pref_notifier_impl_unittest.cc
index 096b625..c413e32 100644
--- a/chrome/browser/prefs/pref_notifier_impl_unittest.cc
+++ b/chrome/browser/prefs/pref_notifier_impl_unittest.cc
@@ -86,8 +86,8 @@
Field(&NotificationType::value,
NotificationType::PREF_INITIALIZATION_COMPLETED),
Source<PrefService>(&pref_service_),
- NotificationService::NoDetails()));
- notifier.OnInitializationCompleted();
+ Property(&Details<bool>::ptr, testing::Pointee(true))));
+ notifier.OnInitializationCompleted(true);
}
TEST_F(PrefNotifierTest, AddAndRemovePrefObservers) {
diff --git a/chrome/browser/prefs/pref_service.cc b/chrome/browser/prefs/pref_service.cc
index 5096d1d7..5614bbf 100644
--- a/chrome/browser/prefs/pref_service.cc
+++ b/chrome/browser/prefs/pref_service.cc
@@ -79,25 +79,41 @@
// Forwards a notification after a PostMessage so that we can wait for the
// MessageLoop to run.
-void NotifyReadError(PrefService* pref, int message_id) {
+void NotifyReadError(int message_id) {
ShowProfileErrorDialog(message_id);
}
+// Shows notifications which correspond to PersistentPrefStore's reading errors.
+class ReadErrorHandler : public PersistentPrefStore::ReadErrorDelegate {
+ public:
+ virtual void OnError(PersistentPrefStore::PrefReadError error) {
+ if (error != PersistentPrefStore::PREF_READ_ERROR_NONE) {
+ // Failing to load prefs on startup is a bad thing(TM). See bug 38352 for
+ // an example problem that this can cause.
+ // Do some diagnosis and try to avoid losing data.
+ int message_id = 0;
+ if (error <= PersistentPrefStore::PREF_READ_ERROR_JSON_TYPE) {
+ message_id = IDS_PREFERENCES_CORRUPT_ERROR;
+ } else if (error != PersistentPrefStore::PREF_READ_ERROR_NO_FILE) {
+ message_id = IDS_PREFERENCES_UNREADABLE_ERROR;
+ }
+
+ if (message_id) {
+ BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
+ NewRunnableFunction(&NotifyReadError, message_id));
+ }
+ UMA_HISTOGRAM_ENUMERATION("PrefService.ReadError", error, 20);
+ }
+ }
+};
+
} // namespace
// static
PrefService* PrefService::CreatePrefService(const FilePath& pref_filename,
PrefStore* extension_prefs,
- Profile* profile) {
- return CreatePrefServiceAsync(pref_filename, extension_prefs, profile, NULL);
-}
-
-// static
-PrefService* PrefService::CreatePrefServiceAsync(
- const FilePath& pref_filename,
- PrefStore* extension_prefs,
- Profile* profile,
- PrefServiceDelegate* delegate) {
+ Profile* profile,
+ bool async) {
using policy::ConfigurationPolicyPrefStore;
#if defined(OS_LINUX)
@@ -129,9 +145,10 @@
profile);
DefaultPrefStore* default_pref_store = new DefaultPrefStore();
- return new PrefService(managed_platform, managed_cloud, extension_prefs,
- command_line, user, recommended_platform,
- recommended_cloud, default_pref_store, delegate);
+ return new PrefService(
+ managed_platform, managed_cloud, extension_prefs,
+ command_line, user, recommended_platform,
+ recommended_cloud, default_pref_store, async);
}
PrefService* PrefService::CreateIncognitoPrefService(
@@ -147,10 +164,9 @@
PrefStore* recommended_platform_prefs,
PrefStore* recommended_cloud_prefs,
DefaultPrefStore* default_store,
- PrefServiceDelegate* delegate)
+ bool async)
: user_pref_store_(user_prefs),
- default_store_(default_store),
- delegate_(delegate) {
+ default_store_(default_store) {
pref_sync_associator_.reset(new PrefModelAssociator(this));
pref_notifier_.reset(new PrefNotifierImpl(this));
pref_value_store_.reset(
@@ -164,15 +180,14 @@
default_store,
pref_sync_associator_.get(),
pref_notifier_.get()));
- InitFromStorage();
+ InitFromStorage(async);
}
PrefService::PrefService(const PrefService& original,
PrefStore* incognito_extension_prefs)
: user_pref_store_(
new OverlayPersistentPrefStore(original.user_pref_store_.get())),
- default_store_(original.default_store_.get()),
- delegate_(NULL) {
+ default_store_(original.default_store_.get()) {
// Incognito mode doesn't sync, so no need to create PrefModelAssociator.
pref_notifier_.reset(new PrefNotifierImpl(this));
pref_value_store_.reset(original.pref_value_store_->CloneAndSpecialize(
@@ -186,7 +201,6 @@
default_store_.get(),
NULL, // pref_sync_associator_
pref_notifier_.get()));
- InitFromStorage();
}
PrefService::~PrefService() {
@@ -203,47 +217,17 @@
pref_sync_associator_.reset();
}
-void PrefService::OnPrefsRead(PersistentPrefStore::PrefReadError error,
- bool no_dir) {
- if (no_dir) {
- // 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.
- CHECK(delegate_);
- delegate_->OnPrefsLoaded(this, false);
- return;
- }
-
- if (error != PersistentPrefStore::PREF_READ_ERROR_NONE) {
- // Failing to load prefs on startup is a bad thing(TM). See bug 38352 for
- // an example problem that this can cause.
- // Do some diagnosis and try to avoid losing data.
- int message_id = 0;
- if (error <= PersistentPrefStore::PREF_READ_ERROR_JSON_TYPE) {
- message_id = IDS_PREFERENCES_CORRUPT_ERROR;
- } else if (error != PersistentPrefStore::PREF_READ_ERROR_NO_FILE) {
- message_id = IDS_PREFERENCES_UNREADABLE_ERROR;
- }
-
- if (message_id) {
- BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
- NewRunnableFunction(&NotifyReadError, this, message_id));
- }
- UMA_HISTOGRAM_ENUMERATION("PrefService.ReadError", error, 20);
- }
-
- if (delegate_)
- delegate_->OnPrefsLoaded(this, true);
-}
-
-void PrefService::InitFromStorage() {
- if (!delegate_) {
- const PersistentPrefStore::PrefReadError error =
- user_pref_store_->ReadPrefs();
- OnPrefsRead(error, false);
+void PrefService::InitFromStorage(bool async) {
+ if (!async) {
+ ReadErrorHandler error_handler;
+ error_handler.OnError(user_pref_store_->ReadPrefs());
} else {
- // todo(altimofeev): move this method to PersistentPrefStore interface.
- (static_cast<JsonPrefStore*>(user_pref_store_.get()))->ReadPrefs(this);
+ // Guarantee that initialization happens after this function returned.
+ MessageLoop::current()->PostTask(
+ FROM_HERE,
+ NewRunnableMethod(user_pref_store_.get(),
+ &PersistentPrefStore::ReadPrefsAsync,
+ new ReadErrorHandler()));
}
}
diff --git a/chrome/browser/prefs/pref_service.h b/chrome/browser/prefs/pref_service.h
index 3990d5a..0a98c4c 100644
--- a/chrome/browser/prefs/pref_service.h
+++ b/chrome/browser/prefs/pref_service.h
@@ -36,13 +36,7 @@
class PrefService;
-class PrefServiceDelegate {
- public:
- virtual void OnPrefsLoaded(PrefService* prefs, bool success) = 0;
-};
-
-class PrefService : public base::NonThreadSafe,
- public JsonPrefStore::Delegate {
+class PrefService : public base::NonThreadSafe {
public:
// Enum used when registering preferences to determine if it should be synced
// or not. This is only used for profile prefs, not local state prefs.
@@ -129,26 +123,21 @@
DISALLOW_COPY_AND_ASSIGN(Preference);
};
- // JsonPrefStore::Delegate implementaion.
- virtual void OnPrefsRead(PersistentPrefStore::PrefReadError error,
- bool no_dir);
-
// Factory method that creates a new instance of a PrefService with the
// applicable PrefStores. The |pref_filename| points to the user preference
// file. The |profile| is the one to which these preferences apply; it may be
// NULL if we're dealing with the local state. This is the usual way to create
// a new PrefService. |extension_pref_store| is used as the source for
// extension-controlled preferences and may be NULL. The PrefService takes
- // ownership of |extension_pref_store|.
+ // ownership of |extension_pref_store|. If |async| is true, asynchronous
+ // version is used. Notifies using PREF_INITIALIZATION_COMPLETED in the end.
+ // Details is set to the created PrefService or NULL if creation has failed.
+ // Note, it is guaranteed that in asynchronous version initialization happens
+ // after this function returned.
static PrefService* CreatePrefService(const FilePath& pref_filename,
PrefStore* extension_pref_store,
- Profile* profile);
-
- // Same as above, but with async initialization.
- static PrefService* CreatePrefServiceAsync(const FilePath& pref_filename,
- PrefStore* extension_pref_store,
- Profile* profile,
- PrefServiceDelegate* delegate);
+ Profile* profile,
+ bool async);
// Creates an incognito copy of the pref service that shares most pref stores
// but uses a fresh non-persistent overlay for the user pref store and an
@@ -316,7 +305,7 @@
PrefStore* recommended_platform_prefs,
PrefStore* recommended_cloud_prefs,
DefaultPrefStore* default_store,
- PrefServiceDelegate* delegate);
+ bool async);
// The PrefNotifier handles registering and notifying preference observers.
// It is created and owned by this PrefService. Subclasses may access it for
@@ -376,7 +365,7 @@
// Load preferences from storage, attempting to diagnose and handle errors.
// This should only be called from the constructor.
- void InitFromStorage();
+ void InitFromStorage(bool async);
// Used to set the value of dictionary or list values in the user pref store.
// This will create a dictionary or list if one does not exist in the user
@@ -400,10 +389,6 @@
// of registered preferences are.
mutable PreferenceSet prefs_;
- // Holds delegator to be called after initialization, if async version
- // is used.
- PrefServiceDelegate* delegate_;
-
// The model associator that maintains the links with the sync db.
scoped_ptr<PrefModelAssociator> pref_sync_associator_;
diff --git a/chrome/browser/prefs/pref_service_mock_builder.cc b/chrome/browser/prefs/pref_service_mock_builder.cc
index 1de0403..66e3e99 100644
--- a/chrome/browser/prefs/pref_service_mock_builder.cc
+++ b/chrome/browser/prefs/pref_service_mock_builder.cc
@@ -114,7 +114,7 @@
recommended_platform_prefs_.get(),
recommended_cloud_prefs_.get(),
new DefaultPrefStore(),
- NULL);
+ false);
managed_platform_prefs_ = NULL;
managed_cloud_prefs_ = NULL;
extension_prefs_ = NULL;
diff --git a/chrome/browser/prefs/pref_value_store.cc b/chrome/browser/prefs/pref_value_store.cc
index dbb29aebd..44b526f 100644
--- a/chrome/browser/prefs/pref_value_store.cc
+++ b/chrome/browser/prefs/pref_value_store.cc
@@ -38,8 +38,9 @@
pref_value_store_->OnPrefValueChanged(type_, key);
}
-void PrefValueStore::PrefStoreKeeper::OnInitializationCompleted() {
- pref_value_store_->OnInitializationCompleted(type_);
+void PrefValueStore::PrefStoreKeeper::OnInitializationCompleted(
+ bool succeeded) {
+ pref_value_store_->OnInitializationCompleted(type_, succeeded);
}
PrefValueStore::PrefValueStore(PrefStore* managed_platform_prefs,
@@ -53,7 +54,8 @@
PrefModelAssociator* pref_sync_associator,
PrefNotifier* pref_notifier)
: pref_sync_associator_(pref_sync_associator),
- pref_notifier_(pref_notifier) {
+ pref_notifier_(pref_notifier),
+ initialization_failed_(false) {
InitPrefStore(MANAGED_PLATFORM_STORE, managed_platform_prefs);
InitPrefStore(MANAGED_CLOUD_STORE, managed_cloud_prefs);
InitPrefStore(EXTENSION_STORE, extension_prefs);
@@ -245,7 +247,14 @@
}
void PrefValueStore::OnInitializationCompleted(
- PrefValueStore::PrefStoreType type) {
+ PrefValueStore::PrefStoreType type, bool succeeded) {
+ if (initialization_failed_)
+ return;
+ if (!succeeded) {
+ initialization_failed_ = true;
+ pref_notifier_->OnInitializationCompleted(false);
+ return;
+ }
CheckInitializationCompleted();
}
@@ -255,11 +264,13 @@
}
void PrefValueStore::CheckInitializationCompleted() {
+ if (initialization_failed_)
+ return;
for (size_t i = 0; i <= PREF_STORE_TYPE_MAX; ++i) {
scoped_refptr<PrefStore> store =
GetPrefStore(static_cast<PrefStoreType>(i));
if (store && !store->IsInitializationComplete())
return;
}
- pref_notifier_->OnInitializationCompleted();
+ pref_notifier_->OnInitializationCompleted(true);
}
diff --git a/chrome/browser/prefs/pref_value_store.h b/chrome/browser/prefs/pref_value_store.h
index 07a5b61..5d3d5bb7 100644
--- a/chrome/browser/prefs/pref_value_store.h
+++ b/chrome/browser/prefs/pref_value_store.h
@@ -155,7 +155,7 @@
private:
// PrefStore::Observer implementation.
virtual void OnPrefValueChanged(const std::string& key);
- virtual void OnInitializationCompleted();
+ virtual void OnInitializationCompleted(bool succeeded);
// PrefValueStore this keeper is part of.
PrefValueStore* pref_value_store_;
@@ -214,7 +214,7 @@
void OnPrefValueChanged(PrefStoreType type, const std::string& key);
// Handle the event that the store for |type| has completed initialization.
- void OnInitializationCompleted(PrefStoreType type);
+ void OnInitializationCompleted(PrefStoreType type, bool succeeded);
// Initializes a pref store keeper. Sets up a PrefStoreKeeper that will take
// ownership of the passed |pref_store|.
@@ -247,6 +247,9 @@
// A mapping of preference names to their registered types.
PrefTypeMap pref_types_;
+ // True if not all of the PrefStores were initialized successfully.
+ bool initialization_failed_;
+
DISALLOW_COPY_AND_ASSIGN(PrefValueStore);
};
diff --git a/chrome/browser/prefs/pref_value_store_unittest.cc b/chrome/browser/prefs/pref_value_store_unittest.cc
index 17d939c..b5eaff0 100644
--- a/chrome/browser/prefs/pref_value_store_unittest.cc
+++ b/chrome/browser/prefs/pref_value_store_unittest.cc
@@ -29,7 +29,7 @@
class MockPrefNotifier : public PrefNotifier {
public:
MOCK_METHOD1(OnPreferenceChanged, void(const std::string&));
- MOCK_METHOD0(OnInitializationCompleted, void());
+ MOCK_METHOD1(OnInitializationCompleted, void(bool));
};
// Allows to capture sync model associator interaction.
@@ -494,7 +494,7 @@
}
TEST_F(PrefValueStoreTest, OnInitializationCompleted) {
- EXPECT_CALL(pref_notifier_, OnInitializationCompleted()).Times(0);
+ EXPECT_CALL(pref_notifier_, OnInitializationCompleted(true)).Times(0);
managed_platform_pref_store_->SetInitializationCompleted();
managed_cloud_pref_store_->SetInitializationCompleted();
extension_pref_store_->SetInitializationCompleted();
@@ -505,7 +505,7 @@
Mock::VerifyAndClearExpectations(&pref_notifier_);
// The notification should only be triggered after the last store is done.
- EXPECT_CALL(pref_notifier_, OnInitializationCompleted()).Times(1);
+ EXPECT_CALL(pref_notifier_, OnInitializationCompleted(true)).Times(1);
user_pref_store_->SetInitializationCompleted();
Mock::VerifyAndClearExpectations(&pref_notifier_);
}
diff --git a/chrome/browser/prefs/testing_pref_store.cc b/chrome/browser/prefs/testing_pref_store.cc
index e85a836..d080169 100644
--- a/chrome/browser/prefs/testing_pref_store.cc
+++ b/chrome/browser/prefs/testing_pref_store.cc
@@ -55,9 +55,16 @@
PersistentPrefStore::PrefReadError TestingPrefStore::ReadPrefs() {
prefs_.Clear();
+ NotifyInitializationCompleted();
return PersistentPrefStore::PREF_READ_ERROR_NONE;
}
+void TestingPrefStore::ReadPrefsAsync(ReadErrorDelegate* error_delegate_raw) {
+ scoped_ptr<ReadErrorDelegate> error_delegate(error_delegate_raw);
+ prefs_.Clear();
+ NotifyInitializationCompleted();
+}
+
bool TestingPrefStore::WritePrefs() {
prefs_written_ = true;
return prefs_written_;
@@ -73,7 +80,7 @@
}
void TestingPrefStore::NotifyInitializationCompleted() {
- FOR_EACH_OBSERVER(Observer, observers_, OnInitializationCompleted());
+ FOR_EACH_OBSERVER(Observer, observers_, OnInitializationCompleted(true));
}
void TestingPrefStore::ReportValueChanged(const std::string& key) {
diff --git a/chrome/browser/prefs/testing_pref_store.h b/chrome/browser/prefs/testing_pref_store.h
index 048dcc7..ff6dc00 100644
--- a/chrome/browser/prefs/testing_pref_store.h
+++ b/chrome/browser/prefs/testing_pref_store.h
@@ -37,6 +37,7 @@
virtual void RemoveValue(const std::string& key);
virtual bool ReadOnly() const;
virtual PersistentPrefStore::PrefReadError ReadPrefs();
+ virtual void ReadPrefsAsync(ReadErrorDelegate* error_delegate);
virtual bool WritePrefs();
virtual void ScheduleWritePrefs() {}
virtual void CommitPendingWrite() {}
diff --git a/chrome/browser/prefs/value_map_pref_store.cc b/chrome/browser/prefs/value_map_pref_store.cc
index 4d2c8ca5..39ee3a1 100644
--- a/chrome/browser/prefs/value_map_pref_store.cc
+++ b/chrome/browser/prefs/value_map_pref_store.cc
@@ -37,7 +37,7 @@
}
void ValueMapPrefStore::NotifyInitializationCompleted() {
- FOR_EACH_OBSERVER(Observer, observers_, OnInitializationCompleted());
+ FOR_EACH_OBSERVER(Observer, observers_, OnInitializationCompleted(true));
}
ValueMapPrefStore::iterator ValueMapPrefStore::begin() {
diff --git a/chrome/browser/profiles/profile_impl.cc b/chrome/browser/profiles/profile_impl.cc
index e168882..7b73fe5 100644
--- a/chrome/browser/profiles/profile_impl.cc
+++ b/chrome/browser/profiles/profile_impl.cc
@@ -292,18 +292,23 @@
&ProfileImpl::EnsureSessionServiceCreated);
if (delegate_) {
- prefs_.reset(PrefService::CreatePrefServiceAsync(
+ prefs_.reset(PrefService::CreatePrefService(
GetPrefFilePath(),
new ExtensionPrefStore(GetExtensionPrefValueMap(), false),
GetOriginalProfile(),
- this)); // Ask to notify us in the end.
+ true));
+ // Wait for the notifcation that prefs has been loaded (successfully or
+ // not).
+ registrar_.Add(this, NotificationType::PREF_INITIALIZATION_COMPLETED,
+ Source<PrefService>(prefs_.get()));
} else {
// Load prefs synchronously.
prefs_.reset(PrefService::CreatePrefService(
GetPrefFilePath(),
new ExtensionPrefStore(GetExtensionPrefValueMap(), false),
- GetOriginalProfile()));
- OnPrefsLoaded(prefs_.get(), true);
+ GetOriginalProfile(),
+ false));
+ OnPrefsLoaded(true);
}
}
@@ -803,9 +808,7 @@
return transport_security_state_.get();
}
-void ProfileImpl::OnPrefsLoaded(PrefService* prefs, bool success) {
- DCHECK(prefs == prefs_.get());
-
+void ProfileImpl::OnPrefsLoaded(bool success) {
if (!success) {
DCHECK(delegate_);
delegate_->OnProfileCreated(this, false);
@@ -1319,39 +1322,55 @@
void ProfileImpl::Observe(NotificationType type,
const NotificationSource& source,
const NotificationDetails& details) {
- if (NotificationType::PREF_CHANGED == type) {
- std::string* pref_name_in = Details<std::string>(details).ptr();
- PrefService* prefs = Source<PrefService>(source).ptr();
- DCHECK(pref_name_in && prefs);
- if (*pref_name_in == prefs::kSpellCheckDictionary ||
- *pref_name_in == prefs::kEnableSpellCheck) {
- ReinitializeSpellCheckHost(true);
- } else if (*pref_name_in == prefs::kEnableAutoSpellCorrect) {
- bool enabled = prefs->GetBoolean(prefs::kEnableAutoSpellCorrect);
- for (RenderProcessHost::iterator i(RenderProcessHost::AllHostsIterator());
- !i.IsAtEnd(); i.Advance()) {
- RenderProcessHost* process = i.GetCurrentValue();
- process->Send(new SpellCheckMsg_EnableAutoSpellCorrect(enabled));
- }
- } else if (*pref_name_in == prefs::kClearSiteDataOnExit) {
- clear_local_state_on_exit_ =
- prefs->GetBoolean(prefs::kClearSiteDataOnExit);
- if (webkit_context_) {
- webkit_context_->set_clear_local_state_on_exit(
- clear_local_state_on_exit_);
- }
- if (appcache_service_) {
- appcache_service_->SetClearLocalStateOnExit(
- clear_local_state_on_exit_);
- }
- } else if (*pref_name_in == prefs::kGoogleServicesUsername) {
- ProfileManager* profile_manager = g_browser_process->profile_manager();
- profile_manager->RegisterProfileName(this);
+ switch (type.value) {
+ case NotificationType::PREF_INITIALIZATION_COMPLETED: {
+ bool* succeeded = Details<bool>(details).ptr();
+ PrefService *prefs = Source<PrefService>(source).ptr();
+ DCHECK(prefs == prefs_.get());
+ registrar_.Remove(this, NotificationType::PREF_INITIALIZATION_COMPLETED,
+ Source<PrefService>(prefs));
+ OnPrefsLoaded(*succeeded);
+ break;
}
- } else if (NotificationType::BOOKMARK_MODEL_LOADED == type) {
- GetProfileSyncService(); // Causes lazy-load if sync is enabled.
- registrar_.Remove(this, NotificationType::BOOKMARK_MODEL_LOADED,
- Source<Profile>(this));
+ case NotificationType::PREF_CHANGED: {
+ std::string* pref_name_in = Details<std::string>(details).ptr();
+ PrefService* prefs = Source<PrefService>(source).ptr();
+ DCHECK(pref_name_in && prefs);
+ if (*pref_name_in == prefs::kSpellCheckDictionary ||
+ *pref_name_in == prefs::kEnableSpellCheck) {
+ ReinitializeSpellCheckHost(true);
+ } else if (*pref_name_in == prefs::kEnableAutoSpellCorrect) {
+ bool enabled = prefs->GetBoolean(prefs::kEnableAutoSpellCorrect);
+ for (RenderProcessHost::iterator
+ i(RenderProcessHost::AllHostsIterator());
+ !i.IsAtEnd(); i.Advance()) {
+ RenderProcessHost* process = i.GetCurrentValue();
+ process->Send(new SpellCheckMsg_EnableAutoSpellCorrect(enabled));
+ }
+ } else if (*pref_name_in == prefs::kClearSiteDataOnExit) {
+ clear_local_state_on_exit_ =
+ prefs->GetBoolean(prefs::kClearSiteDataOnExit);
+ if (webkit_context_) {
+ webkit_context_->set_clear_local_state_on_exit(
+ clear_local_state_on_exit_);
+ }
+ if (appcache_service_) {
+ appcache_service_->SetClearLocalStateOnExit(
+ clear_local_state_on_exit_);
+ }
+ } else if (*pref_name_in == prefs::kGoogleServicesUsername) {
+ ProfileManager* profile_manager = g_browser_process->profile_manager();
+ profile_manager->RegisterProfileName(this);
+ }
+ break;
+ }
+ case NotificationType::BOOKMARK_MODEL_LOADED:
+ GetProfileSyncService(); // Causes lazy-load if sync is enabled.
+ registrar_.Remove(this, NotificationType::BOOKMARK_MODEL_LOADED,
+ Source<Profile>(this));
+ break;
+ default:
+ NOTREACHED();
}
}
diff --git a/chrome/browser/profiles/profile_impl.h b/chrome/browser/profiles/profile_impl.h
index 0809482..f5eb627 100644
--- a/chrome/browser/profiles/profile_impl.h
+++ b/chrome/browser/profiles/profile_impl.h
@@ -36,8 +36,7 @@
// The default profile implementation.
class ProfileImpl : public Profile,
public SpellCheckHostObserver,
- public NotificationObserver,
- public PrefServiceDelegate {
+ public NotificationObserver {
public:
virtual ~ProfileImpl();
@@ -158,9 +157,8 @@
// Does final initialization. Should be called after prefs were loaded.
void DoFinalInit();
- // PrefServiceDelegate implementation. Does final prefs initialization and
- // calls Init().
- virtual void OnPrefsLoaded(PrefService* prefs, bool success);
+ // Does final prefs initialization and calls Init().
+ void OnPrefsLoaded(bool success);
void CreateWebDataService();
FilePath GetPrefFilePath();
diff --git a/chrome/common/json_pref_store.cc b/chrome/common/json_pref_store.cc
index f8b054f..f391225 100644
--- a/chrome/common/json_pref_store.cc
+++ b/chrome/common/json_pref_store.cc
@@ -10,8 +10,8 @@
#include "base/callback.h"
#include "base/file_util.h"
#include "base/memory/ref_counted.h"
+#include "base/message_loop_proxy.h"
#include "base/values.h"
-#include "content/browser/browser_thread.h"
#include "content/common/json_value_serializer.h"
namespace {
@@ -19,47 +19,54 @@
// Some extensions we'll tack on to copies of the Preferences files.
const FilePath::CharType* kBadExtension = FILE_PATH_LITERAL("bad");
-// Differentiates file loading between UI and FILE threads.
+// Differentiates file loading between origin thread and passed
+// (aka file) thread.
class FileThreadDeserializer
: public base::RefCountedThreadSafe<FileThreadDeserializer> {
public:
- explicit FileThreadDeserializer(JsonPrefStore* delegate)
- : delegate_(delegate) {
+ explicit FileThreadDeserializer(JsonPrefStore* delegate,
+ base::MessageLoopProxy* file_loop_proxy)
+ : delegate_(delegate),
+ file_loop_proxy_(file_loop_proxy),
+ origin_loop_proxy_(base::MessageLoopProxy::CreateForCurrentThread()) {
}
void Start(const FilePath& path) {
- DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
- BrowserThread::PostTask(
- BrowserThread::FILE,
+ DCHECK(origin_loop_proxy_->BelongsToCurrentThread());
+ file_loop_proxy_->PostTask(
FROM_HERE,
NewRunnableMethod(this,
&FileThreadDeserializer::ReadFileAndReport,
path));
}
- // Deserializes JSON on the FILE thread.
+ // Deserializes JSON on the file thread.
void ReadFileAndReport(const FilePath& path) {
- DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
+ DCHECK(file_loop_proxy_->BelongsToCurrentThread());
+ value_.reset(DoReading(path, &error_, &no_dir_));
+
+ origin_loop_proxy_->PostTask(
+ FROM_HERE,
+ NewRunnableMethod(this, &FileThreadDeserializer::ReportOnOriginThread));
+ }
+
+ // Reports deserialization result on the origin thread.
+ void ReportOnOriginThread() {
+ DCHECK(origin_loop_proxy_->BelongsToCurrentThread());
+ delegate_->OnFileRead(value_.release(), error_, no_dir_);
+ }
+
+ static Value* DoReading(const FilePath& path,
+ PersistentPrefStore::PrefReadError* error,
+ bool* no_dir) {
int error_code;
std::string error_msg;
JSONFileValueSerializer serializer(path);
- value_.reset(serializer.Deserialize(&error_code, &error_msg));
-
- HandleErrors(value_.get(), path, error_code, error_msg, &error_);
-
- no_dir_ = !file_util::PathExists(path.DirName());
-
- BrowserThread::PostTask(
- BrowserThread::UI,
- FROM_HERE,
- NewRunnableMethod(this, &FileThreadDeserializer::ReportOnUIThread));
- }
-
- // Reports deserialization result on the UI thread.
- void ReportOnUIThread() {
- DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
- delegate_->OnFileRead(value_.release(), error_, no_dir_);
+ Value* value = serializer.Deserialize(&error_code, &error_msg);
+ HandleErrors(value, path, error_code, error_msg, error);
+ *no_dir = !file_util::PathExists(path.DirName());
+ return value;
}
static void HandleErrors(const Value* value,
@@ -75,6 +82,8 @@
PersistentPrefStore::PrefReadError error_;
scoped_ptr<Value> value_;
scoped_refptr<JsonPrefStore> delegate_;
+ scoped_refptr<base::MessageLoopProxy> file_loop_proxy_;
+ scoped_refptr<base::MessageLoopProxy> origin_loop_proxy_;
};
// static
@@ -128,9 +137,12 @@
JsonPrefStore::JsonPrefStore(const FilePath& filename,
base::MessageLoopProxy* file_message_loop_proxy)
: path_(filename),
+ file_message_loop_proxy_(file_message_loop_proxy),
prefs_(new DictionaryValue()),
read_only_(false),
- writer_(filename, file_message_loop_proxy) {
+ writer_(filename, file_message_loop_proxy),
+ error_delegate_(NULL),
+ initialized_(false) {
}
JsonPrefStore::~JsonPrefStore() {
@@ -155,6 +167,10 @@
observers_.RemoveObserver(observer);
}
+bool JsonPrefStore::IsInitializationComplete() const {
+ return initialized_;
+}
+
PrefStore::ReadResult JsonPrefStore::GetMutableValue(const std::string& key,
Value** result) {
return prefs_->Get(key, result) ? READ_OK : READ_NO_VALUE;
@@ -194,11 +210,21 @@
PersistentPrefStore::PrefReadError error,
bool no_dir) {
scoped_ptr<Value> value(value_owned);
+ initialized_ = true;
+
+ if (no_dir) {
+ FOR_EACH_OBSERVER(PrefStore::Observer,
+ observers_,
+ OnInitializationCompleted(false));
+ return;
+ }
+
switch (error) {
case PREF_READ_ERROR_ACCESS_DENIED:
case PREF_READ_ERROR_FILE_OTHER:
case PREF_READ_ERROR_FILE_LOCKED:
case PREF_READ_ERROR_JSON_TYPE:
+ case PREF_READ_ERROR_FILE_NOT_SPECIFIED:
read_only_ = true;
break;
case PREF_READ_ERROR_NONE:
@@ -216,53 +242,39 @@
NOTREACHED() << "Unknown error: " << error;
}
- if (delegate_)
- delegate_->OnPrefsRead(error, no_dir);
+ if (error_delegate_.get() && error != PREF_READ_ERROR_NONE)
+ error_delegate_->OnError(error);
+
+ FOR_EACH_OBSERVER(PrefStore::Observer,
+ observers_,
+ OnInitializationCompleted(true));
}
-void JsonPrefStore::ReadPrefs(Delegate* delegate) {
- DCHECK(delegate);
- delegate_ = delegate;
-
+void JsonPrefStore::ReadPrefsAsync(ReadErrorDelegate *error_delegate) {
+ initialized_ = false;
+ error_delegate_.reset(error_delegate);
if (path_.empty()) {
- read_only_ = true;
- delegate_->OnPrefsRead(PREF_READ_ERROR_FILE_NOT_SPECIFIED, false);
+ OnFileRead(NULL, PREF_READ_ERROR_FILE_NOT_SPECIFIED, false);
return;
}
- // This guarantees that class will not be deleted while JSON is readed.
- DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-
// Start async reading of the preferences file. It will delete itself
// in the end.
scoped_refptr<FileThreadDeserializer> deserializer(
- new FileThreadDeserializer(this));
+ new FileThreadDeserializer(this, file_message_loop_proxy_.get()));
deserializer->Start(path_);
}
PersistentPrefStore::PrefReadError JsonPrefStore::ReadPrefs() {
- delegate_ = NULL;
-
if (path_.empty()) {
- read_only_ = true;
+ OnFileRead(NULL, PREF_READ_ERROR_FILE_NOT_SPECIFIED, false);
return PREF_READ_ERROR_FILE_NOT_SPECIFIED;
}
- int error_code = 0;
- std::string error_msg;
-
- JSONFileValueSerializer serializer(path_);
- scoped_ptr<Value> value(serializer.Deserialize(&error_code, &error_msg));
-
- PersistentPrefStore::PrefReadError error;
- FileThreadDeserializer::HandleErrors(value.get(),
- path_,
- error_code,
- error_msg,
- &error);
-
- OnFileRead(value.release(), error, false);
-
+ PrefReadError error;
+ bool no_dir;
+ Value* value = FileThreadDeserializer::DoReading(path_, &error, &no_dir);
+ OnFileRead(value, error, no_dir);
return error;
}
diff --git a/chrome/common/json_pref_store.h b/chrome/common/json_pref_store.h
index d8da37d0..8291a02 100644
--- a/chrome/common/json_pref_store.h
+++ b/chrome/common/json_pref_store.h
@@ -27,11 +27,6 @@
class JsonPrefStore : public PersistentPrefStore,
public ImportantFileWriter::DataSerializer {
public:
- class Delegate {
- public:
- virtual void OnPrefsRead(PrefReadError error, bool no_dir) = 0;
- };
-
// |file_message_loop_proxy| is the MessageLoopProxy for a thread on which
// file I/O can be done.
JsonPrefStore(const FilePath& pref_filename,
@@ -43,6 +38,7 @@
const Value** result) const;
virtual void AddObserver(PrefStore::Observer* observer);
virtual void RemoveObserver(PrefStore::Observer* observer);
+ virtual bool IsInitializationComplete() const;
// PersistentPrefStore overrides:
virtual ReadResult GetMutableValue(const std::string& key, Value** result);
@@ -51,15 +47,16 @@
virtual void RemoveValue(const std::string& key);
virtual bool ReadOnly() const;
virtual PrefReadError ReadPrefs();
- // todo(altimofeev): move it to the PersistentPrefStore inteface.
- void ReadPrefs(Delegate* delegate);
+ virtual void ReadPrefsAsync(ReadErrorDelegate* error_delegate);
virtual bool WritePrefs();
virtual void ScheduleWritePrefs();
virtual void CommitPendingWrite();
virtual void ReportValueChanged(const std::string& key);
// This method is called after JSON file has been read. Method takes
- // ownership of the |value| pointer.
+ // ownership of the |value| pointer. Note, this method is used with
+ // asynchronous file reading, so class exposes it only for the internal needs.
+ // (read: do not call it manually).
void OnFileRead(Value* value_owned, PrefReadError error, bool no_dir);
private:
@@ -67,6 +64,7 @@
virtual bool SerializeData(std::string* output);
FilePath path_;
+ scoped_refptr<base::MessageLoopProxy> file_message_loop_proxy_;
scoped_ptr<DictionaryValue> prefs_;
@@ -77,7 +75,9 @@
ObserverList<PrefStore::Observer, true> observers_;
- Delegate* delegate_;
+ scoped_ptr<ReadErrorDelegate> error_delegate_;
+
+ bool initialized_;
DISALLOW_COPY_AND_ASSIGN(JsonPrefStore);
};
diff --git a/chrome/common/json_pref_store_unittest.cc b/chrome/common/json_pref_store_unittest.cc
index e7f34f52..d46f4328 100644
--- a/chrome/common/json_pref_store_unittest.cc
+++ b/chrome/common/json_pref_store_unittest.cc
@@ -5,6 +5,7 @@
#include "base/file_util.h"
#include "base/memory/ref_counted.h"
#include "base/memory/scoped_ptr.h"
+#include "base/memory/scoped_temp_dir.h"
#include "base/message_loop.h"
#include "base/message_loop_proxy.h"
#include "base/path_service.h"
@@ -13,12 +14,27 @@
#include "base/threading/thread.h"
#include "base/utf_string_conversions.h"
#include "base/values.h"
-#include "base/memory/scoped_temp_dir.h"
-#include "chrome/common/json_pref_store.h"
#include "chrome/common/chrome_paths.h"
+#include "chrome/common/json_pref_store.h"
#include "chrome/common/pref_names.h"
+#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
+namespace {
+
+class MockPrefStoreObserver : public PrefStore::Observer {
+ public:
+ MOCK_METHOD1(OnPrefValueChanged, void (const std::string&));
+ MOCK_METHOD1(OnInitializationCompleted, void (bool));
+};
+
+class MockReadErrorDelegate : public PersistentPrefStore::ReadErrorDelegate {
+ public:
+ MOCK_METHOD1(OnError, void(PersistentPrefStore::PrefReadError));
+};
+
+} // namespace
+
class JsonPrefStoreTest : public testing::Test {
protected:
virtual void SetUp() {
@@ -70,28 +86,11 @@
moved_aside));
}
-TEST_F(JsonPrefStoreTest, Basic) {
- ASSERT_TRUE(file_util::CopyFile(data_dir_.AppendASCII("read.json"),
- temp_dir_.path().AppendASCII("write.json")));
-
- // Test that the persistent value can be loaded.
- 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_proxy_.get());
- ASSERT_EQ(PersistentPrefStore::PREF_READ_ERROR_NONE, pref_store->ReadPrefs());
- ASSERT_FALSE(pref_store->ReadOnly());
-
- // The JSON file looks like this:
- // {
- // "homepage": "http://www.cnn.com",
- // "some_directory": "/usr/local/",
- // "tabs": {
- // "new_windows_in_tabs": true,
- // "max_tabs": 20
- // }
- // }
-
+// This function is used to avoid code duplication while testing synchronous and
+// asynchronous version of the JsonPrefStore loading.
+void RunBasicJsonPrefStoreTest(JsonPrefStore *pref_store,
+ const FilePath& output_file,
+ const FilePath& golden_output_file) {
const char kNewWindowsInTabs[] = "tabs.new_windows_in_tabs";
const char kMaxTabs[] = "tabs.max_tabs";
const char kLongIntPref[] = "long_int.pref";
@@ -152,11 +151,96 @@
EXPECT_EQ(214748364842LL, value);
// Serialize and compare to expected output.
- FilePath output_file = input_file;
- FilePath golden_output_file = data_dir_.AppendASCII("write.golden.json");
ASSERT_TRUE(file_util::PathExists(golden_output_file));
ASSERT_TRUE(pref_store->WritePrefs());
MessageLoop::current()->RunAllPending();
EXPECT_TRUE(file_util::TextContentsEqual(golden_output_file, output_file));
ASSERT_TRUE(file_util::Delete(output_file, false));
}
+
+TEST_F(JsonPrefStoreTest, Basic) {
+ ASSERT_TRUE(file_util::CopyFile(data_dir_.AppendASCII("read.json"),
+ temp_dir_.path().AppendASCII("write.json")));
+
+ // Test that the persistent value can be loaded.
+ 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_proxy_.get());
+ ASSERT_EQ(PersistentPrefStore::PREF_READ_ERROR_NONE, pref_store->ReadPrefs());
+ ASSERT_FALSE(pref_store->ReadOnly());
+
+ // The JSON file looks like this:
+ // {
+ // "homepage": "http://www.cnn.com",
+ // "some_directory": "/usr/local/",
+ // "tabs": {
+ // "new_windows_in_tabs": true,
+ // "max_tabs": 20
+ // }
+ // }
+
+ RunBasicJsonPrefStoreTest(pref_store,
+ input_file,
+ data_dir_.AppendASCII("write.golden.json"));
+}
+
+TEST_F(JsonPrefStoreTest, BasicAsync) {
+ ASSERT_TRUE(file_util::CopyFile(data_dir_.AppendASCII("read.json"),
+ temp_dir_.path().AppendASCII("write.json")));
+
+ // Test that the persistent value can be loaded.
+ 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_proxy_.get());
+
+ MockPrefStoreObserver mock_observer;
+ pref_store->AddObserver(&mock_observer);
+
+ 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_.RunAllPending();
+ pref_store->RemoveObserver(&mock_observer);
+
+ ASSERT_FALSE(pref_store->ReadOnly());
+
+ // The JSON file looks like this:
+ // {
+ // "homepage": "http://www.cnn.com",
+ // "some_directory": "/usr/local/",
+ // "tabs": {
+ // "new_windows_in_tabs": true,
+ // "max_tabs": 20
+ // }
+ // }
+
+ RunBasicJsonPrefStoreTest(pref_store,
+ input_file,
+ data_dir_.AppendASCII("write.golden.json"));
+}
+
+// Tests asynchronous reading of the file when there is no file.
+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_proxy_.get());
+ MockPrefStoreObserver mock_observer;
+ pref_store->AddObserver(&mock_observer);
+
+ 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_NO_FILE)).Times(1);
+ message_loop_.RunAllPending();
+ pref_store->RemoveObserver(&mock_observer);
+
+ EXPECT_FALSE(pref_store->ReadOnly());
+}
diff --git a/chrome/common/persistent_pref_store.h b/chrome/common/persistent_pref_store.h
index 620fee8..0f007c5a 100644
--- a/chrome/common/persistent_pref_store.h
+++ b/chrome/common/persistent_pref_store.h
@@ -34,6 +34,13 @@
PREF_READ_ERROR_FILE_NOT_SPECIFIED
};
+ class ReadErrorDelegate {
+ public:
+ virtual ~ReadErrorDelegate() {}
+
+ virtual void OnError(PrefReadError error) = 0;
+ };
+
// Equivalent to PrefStore::GetValue but returns a mutable value.
virtual ReadResult GetMutableValue(const std::string& key,
Value** result) = 0;
@@ -62,9 +69,16 @@
// read errors during startup.
virtual bool ReadOnly() const = 0;
- // Reads the preferences from disk.
+ // Reads the preferences from disk. Notifies observers via
+ // "PrefStore::OnInitializationCompleted" when done.
virtual PrefReadError ReadPrefs() = 0;
+ // Reads the preferences from disk asynchronously. Notifies observers via
+ // "PrefStore::OnInitializationCompleted" when done. Also it fires
+ // |error_delegate| if it is not NULL and reading error has occurred.
+ // Owns |error_delegate|.
+ virtual void ReadPrefsAsync(ReadErrorDelegate* error_delegate) = 0;
+
// Writes the preferences to disk immediately.
virtual bool WritePrefs() = 0;
diff --git a/chrome/common/pref_store.h b/chrome/common/pref_store.h
index 48b51a3..eec74e9 100644
--- a/chrome/common/pref_store.h
+++ b/chrome/common/pref_store.h
@@ -30,7 +30,7 @@
// Called when the value for the given |key| in the store changes.
virtual void OnPrefValueChanged(const std::string& key) = 0;
// Notification about the PrefStore being fully initialized.
- virtual void OnInitializationCompleted() = 0;
+ virtual void OnInitializationCompleted(bool succeeded) = 0;
};
// Return values for GetValue().
diff --git a/chrome/common/pref_store_observer_mock.h b/chrome/common/pref_store_observer_mock.h
index 3aadf342..0218d339 100644
--- a/chrome/common/pref_store_observer_mock.h
+++ b/chrome/common/pref_store_observer_mock.h
@@ -17,7 +17,7 @@
virtual ~PrefStoreObserverMock();
MOCK_METHOD1(OnPrefValueChanged, void(const std::string&));
- MOCK_METHOD0(OnInitializationCompleted, void());
+ MOCK_METHOD1(OnInitializationCompleted, void(bool));
private:
DISALLOW_COPY_AND_ASSIGN(PrefStoreObserverMock);
diff --git a/chrome/test/testing_pref_service.cc b/chrome/test/testing_pref_service.cc
index 6d792b9..79e98cc 100644
--- a/chrome/test/testing_pref_service.cc
+++ b/chrome/test/testing_pref_service.cc
@@ -26,7 +26,7 @@
recommended_platform_prefs,
NULL,
new DefaultPrefStore(),
- NULL),
+ false),
managed_platform_prefs_(managed_platform_prefs),
user_prefs_(user_prefs),
recommended_platform_prefs_(recommended_platform_prefs) {
@@ -111,4 +111,3 @@
EXPECT_EQ(&local_state_, browser_process_->local_state());
browser_process_->SetLocalState(NULL);
}
-
