summaryrefslogtreecommitdiffstats
path: root/chrome/browser/prefs
diff options
context:
space:
mode:
authorxiyuan@chromium.org <xiyuan@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2012-04-12 03:41:37 +0000
committerxiyuan@chromium.org <xiyuan@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2012-04-12 03:41:37 +0000
commitea3e497b1949405ae04caa8b1dc8400672b1dfa1 (patch)
tree597971f904ab59e7e47fbe108c241e66640c5d4e /chrome/browser/prefs
parent92a794994111f442e9c7ba1792a5418a77c2ca74 (diff)
downloadchromium_src-ea3e497b1949405ae04caa8b1dc8400672b1dfa1.zip
chromium_src-ea3e497b1949405ae04caa8b1dc8400672b1dfa1.tar.gz
chromium_src-ea3e497b1949405ae04caa8b1dc8400672b1dfa1.tar.bz2
Keep emtpy List/Dictionary pref value with non-empty default.
- Add a MarkNeedsEmptyValue method to JsonPrefStore; - In PrefService::RegisterPreference, mark ListValue and DictionaryValue pref with non-empty default as needing empty value in |user_pref_store_|; - Update ChromeLauncherDelegate to put back default pinned apps and add migration logic for M19 users; - Add unit tests; BUG=122679 TEST=Verify fix for issue 122679. Review URL: http://codereview.chromium.org/10055003 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@131919 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome/browser/prefs')
-rw-r--r--chrome/browser/prefs/overlay_user_pref_store.cc5
-rw-r--r--chrome/browser/prefs/overlay_user_pref_store.h1
-rw-r--r--chrome/browser/prefs/pref_service.cc17
-rw-r--r--chrome/browser/prefs/pref_service_mock_builder.cc14
-rw-r--r--chrome/browser/prefs/pref_service_mock_builder.h7
-rw-r--r--chrome/browser/prefs/pref_service_unittest.cc81
-rw-r--r--chrome/browser/prefs/testing_pref_store.cc3
-rw-r--r--chrome/browser/prefs/testing_pref_store.h1
8 files changed, 125 insertions, 4 deletions
diff --git a/chrome/browser/prefs/overlay_user_pref_store.cc b/chrome/browser/prefs/overlay_user_pref_store.cc
index ca4f585..8d04ec3 100644
--- a/chrome/browser/prefs/overlay_user_pref_store.cc
+++ b/chrome/browser/prefs/overlay_user_pref_store.cc
@@ -101,6 +101,11 @@ void OverlayUserPrefStore::RemoveValue(const std::string& key) {
ReportValueChanged(key);
}
+void OverlayUserPrefStore::MarkNeedsEmptyValue(const std::string& key) {
+ if (!ShallBeStoredInOverlay(key))
+ underlay_->MarkNeedsEmptyValue(key);
+}
+
bool OverlayUserPrefStore::ReadOnly() const {
return false;
}
diff --git a/chrome/browser/prefs/overlay_user_pref_store.h b/chrome/browser/prefs/overlay_user_pref_store.h
index f0a1f4b..105dc67 100644
--- a/chrome/browser/prefs/overlay_user_pref_store.h
+++ b/chrome/browser/prefs/overlay_user_pref_store.h
@@ -45,6 +45,7 @@ class OverlayUserPrefStore : public PersistentPrefStore,
virtual void SetValueSilently(const std::string& key,
base::Value* value) OVERRIDE;
virtual void RemoveValue(const std::string& key) OVERRIDE;
+ virtual void MarkNeedsEmptyValue(const std::string& key) OVERRIDE;
virtual bool ReadOnly() const OVERRIDE;
virtual PrefReadError GetReadError() const OVERRIDE;
virtual PrefReadError ReadPrefs() OVERRIDE;
diff --git a/chrome/browser/prefs/pref_service.cc b/chrome/browser/prefs/pref_service.cc
index cf9f22e..de7f406 100644
--- a/chrome/browser/prefs/pref_service.cc
+++ b/chrome/browser/prefs/pref_service.cc
@@ -727,6 +727,23 @@ void PrefService::RegisterPreference(const char* path,
DCHECK(orig_type != Value::TYPE_NULL && orig_type != Value::TYPE_BINARY) <<
"invalid preference type: " << orig_type;
+ // For ListValue and DictionaryValue with non empty default, empty value
+ // for |path| needs to be persisted in |user_pref_store_|. So that
+ // non empty default is not used when user sets an empty ListValue or
+ // DictionaryValue.
+ bool needs_empty_value = false;
+ if (orig_type == base::Value::TYPE_LIST) {
+ const base::ListValue* list = NULL;
+ if (default_value->GetAsList(&list) && !list->empty())
+ needs_empty_value = true;
+ } else if (orig_type == base::Value::TYPE_DICTIONARY) {
+ const base::DictionaryValue* dict = NULL;
+ if (default_value->GetAsDictionary(&dict) && !dict->empty())
+ needs_empty_value = true;
+ }
+ if (needs_empty_value)
+ user_pref_store_->MarkNeedsEmptyValue(path);
+
// Hand off ownership.
default_store_->SetDefaultValue(path, scoped_value.release());
diff --git a/chrome/browser/prefs/pref_service_mock_builder.cc b/chrome/browser/prefs/pref_service_mock_builder.cc
index 5b4ba24..edfe696 100644
--- a/chrome/browser/prefs/pref_service_mock_builder.cc
+++ b/chrome/browser/prefs/pref_service_mock_builder.cc
@@ -77,10 +77,16 @@ PrefServiceMockBuilder::WithCommandLine(CommandLine* command_line) {
PrefServiceMockBuilder&
PrefServiceMockBuilder::WithUserFilePrefs(const FilePath& prefs_file) {
- user_prefs_ =
- new JsonPrefStore(prefs_file,
- BrowserThread::GetMessageLoopProxyForThread(
- BrowserThread::FILE));
+ return WithUserFilePrefs(prefs_file,
+ BrowserThread::GetMessageLoopProxyForThread(
+ BrowserThread::FILE));
+}
+
+PrefServiceMockBuilder&
+PrefServiceMockBuilder::WithUserFilePrefs(
+ const FilePath& prefs_file,
+ base::MessageLoopProxy* message_loop_proxy) {
+ user_prefs_ = new JsonPrefStore(prefs_file, message_loop_proxy);
return *this;
}
diff --git a/chrome/browser/prefs/pref_service_mock_builder.h b/chrome/browser/prefs/pref_service_mock_builder.h
index 5ea5ace..2dd764f 100644
--- a/chrome/browser/prefs/pref_service_mock_builder.h
+++ b/chrome/browser/prefs/pref_service_mock_builder.h
@@ -15,6 +15,10 @@ class CommandLine;
class FilePath;
class PrefService;
+namespace base {
+class MessageLoopProxy;
+}
+
namespace policy {
class PolicyService;
}
@@ -46,6 +50,9 @@ class PrefServiceMockBuilder {
// Specifies to use an actual file-backed user pref store.
PrefServiceMockBuilder& WithUserFilePrefs(const FilePath& prefs_file);
+ PrefServiceMockBuilder& WithUserFilePrefs(
+ const FilePath& prefs_file,
+ base::MessageLoopProxy* message_loop_proxy);
// Creates the PrefService, invalidating the entire builder configuration.
PrefService* Create();
diff --git a/chrome/browser/prefs/pref_service_unittest.cc b/chrome/browser/prefs/pref_service_unittest.cc
index 8a7069c..38d6ea96 100644
--- a/chrome/browser/prefs/pref_service_unittest.cc
+++ b/chrome/browser/prefs/pref_service_unittest.cc
@@ -5,7 +5,10 @@
#include <string>
#include "base/command_line.h"
+#include "base/file_util.h"
#include "base/memory/scoped_ptr.h"
+#include "base/path_service.h"
+#include "base/scoped_temp_dir.h"
#include "base/utf_string_conversions.h"
#include "base/values.h"
#include "chrome/browser/policy/configuration_policy_pref_store.h"
@@ -16,9 +19,11 @@
#include "chrome/browser/prefs/pref_observer_mock.h"
#include "chrome/browser/prefs/pref_service_mock_builder.h"
#include "chrome/browser/prefs/pref_value_store.h"
+#include "chrome/browser/prefs/scoped_user_pref_update.h"
#include "chrome/browser/prefs/testing_pref_store.h"
#include "chrome/common/chrome_paths.h"
#include "chrome/common/chrome_switches.h"
+#include "chrome/common/json_pref_store.h"
#include "chrome/common/pref_names.h"
#include "chrome/test/base/chrome_render_view_host_test_harness.h"
#include "chrome/test/base/testing_pref_service.h"
@@ -182,6 +187,82 @@ TEST(PrefServiceTest, UpdateCommandLinePrefStore) {
EXPECT_TRUE(actual_bool_value);
}
+class PrefServiceUserFilePrefsTest : public testing::Test {
+ protected:
+ virtual void SetUp() {
+ ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
+
+ ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &data_dir_));
+ data_dir_ = data_dir_.AppendASCII("pref_service");
+ ASSERT_TRUE(file_util::PathExists(data_dir_));
+ }
+
+ void ClearListValue(PrefService* prefs, const char* key) {
+ ListPrefUpdate updater(prefs, key);
+ updater->Clear();
+ }
+
+ void ClearDictionaryValue(PrefService* prefs, const char* key) {
+ DictionaryPrefUpdate updater(prefs, key);
+ updater->Clear();
+ }
+
+ // The path to temporary directory used to contain the test operations.
+ ScopedTempDir temp_dir_;
+ // The path to the directory where the test data is stored.
+ FilePath data_dir_;
+ // A message loop that we can use as the file thread message loop.
+ MessageLoop message_loop_;
+};
+
+// Verifies that ListValue and DictionaryValue pref with non emtpy default
+// preserves its empty value.
+TEST_F(PrefServiceUserFilePrefsTest, PreserveEmptyValue) {
+ FilePath pref_file = temp_dir_.path().AppendASCII("write.json");
+
+ ASSERT_TRUE(file_util::CopyFile(
+ data_dir_.AppendASCII("read.need_empty_value.json"),
+ pref_file));
+
+ PrefServiceMockBuilder builder;
+ builder.WithUserFilePrefs(pref_file, base::MessageLoopProxy::current());
+ scoped_ptr<PrefService> prefs(builder.Create());
+
+ // Register testing prefs.
+ prefs->RegisterListPref("list",
+ PrefService::UNSYNCABLE_PREF);
+ prefs->RegisterDictionaryPref("dict",
+ PrefService::UNSYNCABLE_PREF);
+
+ base::ListValue* non_empty_list = new base::ListValue;
+ non_empty_list->Append(base::Value::CreateStringValue("test"));
+ prefs->RegisterListPref("list_needs_empty_value",
+ non_empty_list,
+ PrefService::UNSYNCABLE_PREF);
+
+ base::DictionaryValue* non_empty_dict = new base::DictionaryValue;
+ non_empty_dict->SetString("dummy", "whatever");
+ prefs->RegisterDictionaryPref("dict_needs_empty_value",
+ non_empty_dict,
+ PrefService::UNSYNCABLE_PREF);
+
+ // Set all testing prefs to empty.
+ ClearListValue(prefs.get(), "list");
+ ClearListValue(prefs.get(), "list_needs_empty_value");
+ ClearDictionaryValue(prefs.get(), "dict");
+ ClearDictionaryValue(prefs.get(), "dict_needs_empty_value");
+
+ // Write to file.
+ prefs->CommitPendingWrite();
+ MessageLoop::current()->RunAllPending();
+
+ // Compare to expected output.
+ FilePath golden_output_file =
+ data_dir_.AppendASCII("write.golden.need_empty_value.json");
+ ASSERT_TRUE(file_util::PathExists(golden_output_file));
+ EXPECT_TRUE(file_util::TextContentsEqual(golden_output_file, pref_file));
+}
+
class PrefServiceSetValueTest : public testing::Test {
protected:
static const char kName[];
diff --git a/chrome/browser/prefs/testing_pref_store.cc b/chrome/browser/prefs/testing_pref_store.cc
index 8948598..f7ce0f4 100644
--- a/chrome/browser/prefs/testing_pref_store.cc
+++ b/chrome/browser/prefs/testing_pref_store.cc
@@ -53,6 +53,9 @@ void TestingPrefStore::RemoveValue(const std::string& key) {
NotifyPrefValueChanged(key);
}
+void TestingPrefStore::MarkNeedsEmptyValue(const std::string& key) {
+}
+
bool TestingPrefStore::ReadOnly() const {
return read_only_;
}
diff --git a/chrome/browser/prefs/testing_pref_store.h b/chrome/browser/prefs/testing_pref_store.h
index 85965da..9cda821 100644
--- a/chrome/browser/prefs/testing_pref_store.h
+++ b/chrome/browser/prefs/testing_pref_store.h
@@ -38,6 +38,7 @@ class TestingPrefStore : public PersistentPrefStore {
virtual void SetValueSilently(const std::string& key,
base::Value* value) OVERRIDE;
virtual void RemoveValue(const std::string& key) OVERRIDE;
+ virtual void MarkNeedsEmptyValue(const std::string& key) OVERRIDE;
virtual bool ReadOnly() const OVERRIDE;
virtual PrefReadError GetReadError() const OVERRIDE;
virtual PersistentPrefStore::PrefReadError ReadPrefs() OVERRIDE;
generated by cgit v1.1 at 2025-03-18 06:27:59 +0000