diff options
| -rw-r--r-- | base/values.cc | 10 | ||||
| -rw-r--r-- | base/values.h | 17 | ||||
| -rw-r--r-- | chrome/browser/prefs/incognito_user_pref_store.cc | 3 | ||||
| -rw-r--r-- | chrome/browser/prefs/incognito_user_pref_store.h | 25 | ||||
| -rw-r--r-- | chrome/browser/prefs/pref_service.cc | 14 | ||||
| -rw-r--r-- | chrome/browser/prefs/pref_service.h | 5 | ||||
| -rw-r--r-- | chrome/browser/prefs/testing_pref_store.h | 24 | ||||
| -rw-r--r-- | chrome/browser/ui/browser.cc | 5 | ||||
| -rw-r--r-- | chrome/browser/ui/views/chrome_views_delegate.cc | 16 | ||||
| -rw-r--r-- | chrome/common/json_pref_store.cc | 81 | ||||
| -rw-r--r-- | chrome/common/json_pref_store.h | 39 | ||||
| -rw-r--r-- | chrome/common/persistent_pref_store.h | 4 |
12 files changed, 55 insertions, 188 deletions
diff --git a/base/values.cc b/base/values.cc index 799e100..17aba16 100644 --- a/base/values.cc +++ b/base/values.cc @@ -65,9 +65,6 @@ namespace base { ///////////////////// Value //////////////////// Value::~Value() { -#if !defined(OS_CHROMEOS) - CHECK(!check_on_delete_); -#endif } // static @@ -149,12 +146,7 @@ bool Value::Equals(const Value* a, const Value* b) { return a->Equals(b); } -Value::Value(Type type) - : type_(type) -#if !defined(OS_CHROMEOS) - , check_on_delete_(false) -#endif -{ +Value::Value(Type type) : type_(type) { } ///////////////////// FundamentalValue //////////////////// diff --git a/base/values.h b/base/values.h index d33d031..a30791b 100644 --- a/base/values.h +++ b/base/values.h @@ -113,28 +113,15 @@ class BASE_EXPORT Value { // NULLs are considered equal but different from Value::CreateNullValue(). static bool Equals(const Value* a, const Value* b); -#if !defined(OS_CHROMEOS) - // NOTE: We pass Value from libcros tp Chrome, so changing the size of Value - // breaks it. TODO(stevenjb): Eliminate that dependency (crosbug.com/19576). - // TODO(sky) bug 91396: remove this when we figure out 91396. - // If true crash when deleted. - void set_check_on_delete(bool value) { check_on_delete_ = value; } -#else - void set_check_on_delete(bool value) {} -#endif - protected: // This isn't safe for end-users (they should use the Create*Value() // static methods above), but it's useful for subclasses. explicit Value(Type type); private: - Type type_; + Value(); -#if !defined(OS_CHROMEOS) - // See description above setter. - bool check_on_delete_; -#endif + Type type_; DISALLOW_COPY_AND_ASSIGN(Value); }; diff --git a/chrome/browser/prefs/incognito_user_pref_store.cc b/chrome/browser/prefs/incognito_user_pref_store.cc index d1240fd..87bcb2c 100644 --- a/chrome/browser/prefs/incognito_user_pref_store.cc +++ b/chrome/browser/prefs/incognito_user_pref_store.cc @@ -134,9 +134,6 @@ void IncognitoUserPrefStore::ReportValueChanged(const std::string& key) { FOR_EACH_OBSERVER(PrefStore::Observer, observers_, OnPrefValueChanged(key)); } -void IncognitoUserPrefStore::CheckIfValueDestroyed(const std::string& key) { -} - void IncognitoUserPrefStore::OnPrefValueChanged(const std::string& key) { if (!overlay_.GetValue(key, NULL)) ReportValueChanged(key); diff --git a/chrome/browser/prefs/incognito_user_pref_store.h b/chrome/browser/prefs/incognito_user_pref_store.h index 38a7a59..b5be17e 100644 --- a/chrome/browser/prefs/incognito_user_pref_store.h +++ b/chrome/browser/prefs/incognito_user_pref_store.h @@ -9,7 +9,6 @@ #include <string> #include "base/basictypes.h" -#include "base/compiler_specific.h" #include "base/memory/ref_counted.h" #include "base/observer_list.h" #include "chrome/browser/prefs/pref_value_map.h" @@ -39,19 +38,17 @@ class IncognitoUserPrefStore : public PersistentPrefStore, // Methods of PersistentPrefStore. virtual ReadResult GetMutableValue(const std::string& key, - base::Value** result) OVERRIDE; - virtual void SetValue(const std::string& key, base::Value* value) OVERRIDE; - virtual void SetValueSilently(const std::string& key, - base::Value* value) OVERRIDE; - virtual void RemoveValue(const std::string& key) OVERRIDE; - virtual bool ReadOnly() const OVERRIDE; - virtual PrefReadError ReadPrefs() OVERRIDE; - virtual void ReadPrefsAsync(ReadErrorDelegate* delegate) OVERRIDE; - virtual bool WritePrefs() OVERRIDE; - virtual void ScheduleWritePrefs() OVERRIDE; - virtual void CommitPendingWrite() OVERRIDE; - virtual void ReportValueChanged(const std::string& key) OVERRIDE; - virtual void CheckIfValueDestroyed(const std::string& key) OVERRIDE; + base::Value** result); + virtual void SetValue(const std::string& key, base::Value* value); + virtual void SetValueSilently(const std::string& key, base::Value* value); + 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(); + virtual void ReportValueChanged(const std::string& key); private: // Methods of PrefStore::Observer. diff --git a/chrome/browser/prefs/pref_service.cc b/chrome/browser/prefs/pref_service.cc index 3f76755..6cb9836 100644 --- a/chrome/browser/prefs/pref_service.cc +++ b/chrome/browser/prefs/pref_service.cc @@ -838,20 +838,6 @@ SyncableService* PrefService::GetSyncableService() { return pref_sync_associator_.get(); } -void PrefService::CheckIfValueDestroyed(const char* path, - base::Value::Type type) { - // Make sure the value exists. - Value* value = GetMutableUserPref(path, type); - if (!value) { - const Preference* pref = FindPreference(path); - CHECK(pref); - CHECK_EQ(type, pref->GetType()); - CHECK(false); - } - - user_pref_store_->CheckIfValueDestroyed(path); -} - /////////////////////////////////////////////////////////////////////////////// // PrefService::Preference diff --git a/chrome/browser/prefs/pref_service.h b/chrome/browser/prefs/pref_service.h index a357a6e..5361928 100644 --- a/chrome/browser/prefs/pref_service.h +++ b/chrome/browser/prefs/pref_service.h @@ -293,11 +293,6 @@ class PrefService : public base::NonThreadSafe { // TODO(zea): Have PrefService implement SyncableService directly. SyncableService* GetSyncableService(); - // TODO(sky): remove this. It's used in tracking 91396. - // Used for debugging. Crashes if the value at |path| is deleted out from - // under us. - void CheckIfValueDestroyed(const char* path, base::Value::Type type); - protected: // Construct a new pref service, specifying the pref sources as explicit // PrefStore pointers. This constructor is what CreatePrefService() ends up diff --git a/chrome/browser/prefs/testing_pref_store.h b/chrome/browser/prefs/testing_pref_store.h index 94fbe2f..106b836 100644 --- a/chrome/browser/prefs/testing_pref_store.h +++ b/chrome/browser/prefs/testing_pref_store.h @@ -29,19 +29,17 @@ class TestingPrefStore : public PersistentPrefStore { // PersistentPrefStore overrides: virtual ReadResult GetMutableValue(const std::string& key, - base::Value** result) OVERRIDE; - virtual void ReportValueChanged(const std::string& key) OVERRIDE; - virtual void SetValue(const std::string& key, base::Value* value) OVERRIDE; - virtual void SetValueSilently(const std::string& key, - base::Value* value) OVERRIDE; - virtual void RemoveValue(const std::string& key) OVERRIDE; - virtual bool ReadOnly() const OVERRIDE; - virtual PersistentPrefStore::PrefReadError ReadPrefs() OVERRIDE; - virtual void ReadPrefsAsync(ReadErrorDelegate* error_delegate) OVERRIDE; - virtual bool WritePrefs() OVERRIDE; - virtual void ScheduleWritePrefs() OVERRIDE {} - virtual void CommitPendingWrite() OVERRIDE {} - virtual void CheckIfValueDestroyed(const std::string& key) OVERRIDE {} + base::Value** result); + virtual void ReportValueChanged(const std::string& key); + virtual void SetValue(const std::string& key, base::Value* value); + virtual void SetValueSilently(const std::string& key, base::Value* value); + 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() {} // Marks the store as having completed initialization. void SetInitializationCompleted(); diff --git a/chrome/browser/ui/browser.cc b/chrome/browser/ui/browser.cc index 098179b..12ad687 100644 --- a/chrome/browser/ui/browser.cc +++ b/chrome/browser/ui/browser.cc @@ -2275,11 +2275,6 @@ void Browser::RegisterUserPrefs(PrefService* prefs) { PrefService::UNSYNCABLE_PREF); prefs->RegisterDictionaryPref(prefs::kBrowserWindowPlacement, PrefService::UNSYNCABLE_PREF); - // TODO(sky): remove this. It's used in tracking 91396. -#if defined(OS_WIN) - prefs->CheckIfValueDestroyed(prefs::kBrowserWindowPlacement, - base::Value::TYPE_DICTIONARY); -#endif prefs->RegisterDictionaryPref(prefs::kPreferencesWindowPlacement, PrefService::UNSYNCABLE_PREF); prefs->RegisterBooleanPref(prefs::kImportBookmarks, diff --git a/chrome/browser/ui/views/chrome_views_delegate.cc b/chrome/browser/ui/views/chrome_views_delegate.cc index 4a734c9..a471e6f 100644 --- a/chrome/browser/ui/views/chrome_views_delegate.cc +++ b/chrome/browser/ui/views/chrome_views_delegate.cc @@ -8,7 +8,6 @@ #include "base/string_util.h" #include "base/utf_string_conversions.h" #include "chrome/browser/browser_process.h" -#include "chrome/browser/browser_shutdown.h" #include "chrome/browser/prefs/pref_service.h" #include "chrome/browser/prefs/scoped_user_pref_update.h" #include "chrome/browser/profiles/profile_manager.h" @@ -33,17 +32,13 @@ namespace { // been initialized. // TODO(mirandac): This function will also separate windows by profile in a // multi-profile environment. -// TODO(sky): remove is_incognito. Used in tracking 91396. -PrefService* GetPrefsForWindow(const views::Widget* window, - bool* is_incognito) { +PrefService* GetPrefsForWindow(const views::Widget* window) { Profile* profile = reinterpret_cast<Profile*>( window->GetNativeWindowProperty(Profile::kProfileKey)); - *is_incognito = false; if (!profile) { // Use local state for windows that have no explicit profile. return g_browser_process->local_state(); } - *is_incognito = profile->IsOffTheRecord(); return profile->GetPrefs(); } @@ -67,18 +62,13 @@ void ChromeViewsDelegate::SaveWindowPlacement(const views::Widget* window, const std::wstring& window_name, const gfx::Rect& bounds, bool maximized) { - bool is_incognito = false; - PrefService* prefs = GetPrefsForWindow(window, &is_incognito); + PrefService* prefs = GetPrefsForWindow(window); if (!prefs) return; - volatile browser_shutdown::ShutdownType shutdown_type = - browser_shutdown::GetShutdownType(); - CHECK(prefs->FindPreference(WideToUTF8(window_name).c_str())) << - is_incognito << " " << shutdown_type; + DCHECK(prefs->FindPreference(WideToUTF8(window_name).c_str())); DictionaryPrefUpdate update(prefs, WideToUTF8(window_name).c_str()); DictionaryValue* window_preferences = update.Get(); - CHECK(window_preferences) << is_incognito << " " << shutdown_type; window_preferences->SetInteger("left", bounds.x()); window_preferences->SetInteger("top", bounds.y()); window_preferences->SetInteger("right", bounds.right()); diff --git a/chrome/common/json_pref_store.cc b/chrome/common/json_pref_store.cc index 8767fc5..f3bd776 100644 --- a/chrome/common/json_pref_store.cc +++ b/chrome/common/json_pref_store.cc @@ -148,7 +148,6 @@ JsonPrefStore::JsonPrefStore(const FilePath& filename, } JsonPrefStore::~JsonPrefStore() { - ResetCheckIfValueDestroyed(prefs_.get()); CommitPendingWrite(); } @@ -181,20 +180,29 @@ PrefStore::ReadResult JsonPrefStore::GetMutableValue(const std::string& key, } void JsonPrefStore::SetValue(const std::string& key, Value* value) { - SetValueImpl(key, value, SET_VALUE_NOTIFY); + DCHECK(value); + scoped_ptr<Value> new_value(value); + Value* old_value = NULL; + prefs_->Get(key, &old_value); + if (!old_value || !value->Equals(old_value)) { + prefs_->Set(key, new_value.release()); + FOR_EACH_OBSERVER(PrefStore::Observer, observers_, OnPrefValueChanged(key)); + } } void JsonPrefStore::SetValueSilently(const std::string& key, Value* value) { - SetValueImpl(key, value, SET_VALUE_DONT_NOTIFY); + DCHECK(value); + scoped_ptr<Value> new_value(value); + Value* old_value = NULL; + prefs_->Get(key, &old_value); + if (!old_value || !value->Equals(old_value)) + prefs_->Set(key, new_value.release()); } void JsonPrefStore::RemoveValue(const std::string& key) { - Value* value = NULL; - bool path_existed = prefs_->Remove(key, &value); - scoped_ptr<Value> owned_value(value); - ResetCheckIfValueDestroyed(value); - if (path_existed) + if (prefs_->Remove(key, NULL)) { FOR_EACH_OBSERVER(PrefStore::Observer, observers_, OnPrefValueChanged(key)); + } } bool JsonPrefStore::ReadOnly() const { @@ -224,7 +232,6 @@ void JsonPrefStore::OnFileRead(Value* value_owned, break; case PREF_READ_ERROR_NONE: DCHECK(value.get()); - ResetCheckIfValueDestroyed(prefs_.get()); prefs_.reset(static_cast<DictionaryValue*>(value.release())); break; case PREF_READ_ERROR_NO_FILE: @@ -303,13 +310,6 @@ void JsonPrefStore::ReportValueChanged(const std::string& key) { FOR_EACH_OBSERVER(PrefStore::Observer, observers_, OnPrefValueChanged(key)); } -void JsonPrefStore::CheckIfValueDestroyed(const std::string& key) { - Value* value = NULL; - CHECK_EQ(READ_OK, GetMutableValue(key, &value)); - CHECK(value); - value->set_check_on_delete(true); -} - bool JsonPrefStore::SerializeData(std::string* output) { // TODO(tc): Do we want to prune webkit preferences that match the default // value? @@ -318,52 +318,3 @@ bool JsonPrefStore::SerializeData(std::string* output) { scoped_ptr<DictionaryValue> copy(prefs_->DeepCopyWithoutEmptyChildren()); return serializer.Serialize(*(copy.get())); } - -void JsonPrefStore::SetValueImpl(const std::string& key, - Value* value, - SetValueType type) { - DCHECK(value); - scoped_ptr<Value> new_value(value); - Value* old_value = NULL; - prefs_->Get(key, &old_value); - ResetCheckIfValueDestroyed(old_value); - if (!old_value || !value->Equals(old_value)) { - prefs_->Set(key, new_value.release()); - if (type == SET_VALUE_NOTIFY) - ReportValueChanged(key); - } -} - -// static -void JsonPrefStore::ResetCheckIfValueDestroyed(Value* value) { - if (!value) - return; - - value->set_check_on_delete(false); - - switch (value->GetType()) { - case Value::TYPE_DICTIONARY: { - DictionaryValue* dictionary = static_cast<DictionaryValue*>(value); - for (DictionaryValue::key_iterator i = dictionary->begin_keys(); - i != dictionary->end_keys(); ++i) { - Value* child_value = NULL; - dictionary->GetWithoutPathExpansion(*i, &child_value); - ResetCheckIfValueDestroyed(child_value); - } - break; - } - - case Value::TYPE_LIST: { - ListValue* list = static_cast<ListValue*>(value); - for (size_t i = 0; i < list->GetSize(); ++i) { - Value* child_value = NULL; - list->Get(i, &child_value); - ResetCheckIfValueDestroyed(child_value); - } - break; - } - - default: - break; - } -} diff --git a/chrome/common/json_pref_store.h b/chrome/common/json_pref_store.h index 3cbd352..b94030d 100644 --- a/chrome/common/json_pref_store.h +++ b/chrome/common/json_pref_store.h @@ -42,19 +42,17 @@ class JsonPrefStore : public PersistentPrefStore, // PersistentPrefStore overrides: virtual ReadResult GetMutableValue(const std::string& key, - base::Value** result) OVERRIDE; - virtual void SetValue(const std::string& key, base::Value* value) OVERRIDE; - virtual void SetValueSilently(const std::string& key, - base::Value* value) OVERRIDE; - virtual void RemoveValue(const std::string& key) OVERRIDE; - virtual bool ReadOnly() const OVERRIDE; - virtual PrefReadError ReadPrefs() OVERRIDE; - virtual void ReadPrefsAsync(ReadErrorDelegate* error_delegate) OVERRIDE; - virtual bool WritePrefs() OVERRIDE; - virtual void ScheduleWritePrefs() OVERRIDE; - virtual void CommitPendingWrite() OVERRIDE; - virtual void ReportValueChanged(const std::string& key) OVERRIDE; - virtual void CheckIfValueDestroyed(const std::string& key) OVERRIDE; + base::Value** result); + virtual void SetValue(const std::string& key, base::Value* value); + virtual void SetValueSilently(const std::string& key, base::Value* value); + virtual void RemoveValue(const std::string& key); + virtual bool ReadOnly() const; + virtual PrefReadError ReadPrefs(); + 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. Note, this method is used with @@ -63,24 +61,9 @@ class JsonPrefStore : public PersistentPrefStore, void OnFileRead(base::Value* value_owned, PrefReadError error, bool no_dir); private: - enum SetValueType { - // Indicates observers should be notified of the change. - SET_VALUE_NOTIFY, - - // Indicates observers should not be notified of the change. - SET_VALUE_DONT_NOTIFY - }; - // ImportantFileWriter::DataSerializer overrides: virtual bool SerializeData(std::string* output); - // Implementation of SetValue. - void SetValueImpl(const std::string& key, - base::Value* value, - SetValueType type); - - static void ResetCheckIfValueDestroyed(base::Value* value); - FilePath path_; scoped_refptr<base::MessageLoopProxy> file_message_loop_proxy_; diff --git a/chrome/common/persistent_pref_store.h b/chrome/common/persistent_pref_store.h index 5a6c71a..b209716 100644 --- a/chrome/common/persistent_pref_store.h +++ b/chrome/common/persistent_pref_store.h @@ -87,10 +87,6 @@ class PersistentPrefStore : public PrefStore { // Lands any pending writes to disk. virtual void CommitPendingWrite() = 0; - - // TODO(sky): remove this. It's used in tracking 91396. - // Used for debugging. Crashes if the value at |key| is unexpected deleted. - virtual void CheckIfValueDestroyed(const std::string& key) = 0; }; #endif // CHROME_COMMON_PERSISTENT_PREF_STORE_H_ |