Skip to content

Commit

Permalink
Revert of Add PrefStore::GetValues (patchset chromium#8 id:140001 of h…
Browse files Browse the repository at this point in the history
…ttps://codereview.chromium.org/2692203007/ )

Reason for revert:
Breaks compile on Cronet proxy.

BUG=694814

Original issue's description:
> Add PrefStore::GetValues
>
> Currently there's no way to proxy a PrefStore, which we need to
> in the new prefs service, because there's no way to get its
> initial state. The existing observer interface can only be used
> to get changed values and GetValue requires that you know all the
> keys in advance.
>
> GetValues involve a deep copy, but the intended consumer needs to
> take ownership to send the value over the wire anyway.
>
> Design doc: https://docs.google.com/document/d/1Fj013SXClTzk4Yfq2eoL9OkKfN0h-GLXPAokCXFkcTY/edit?usp=sharing
>
> BUG=654988
>
> Review-Url: https://codereview.chromium.org/2692203007
> Cr-Commit-Position: refs/heads/master@{#451691}
> Committed: https://chromium.googlesource.com/chromium/src/+/311d4a192239299e42e66b1319b19ee58a5a4b06

TBR=sammc@chromium.org,emaxx@chromium.org,bauerb@chromium.org,tibell@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=654988

Review-Url: https://codereview.chromium.org/2704133006
Cr-Commit-Position: refs/heads/master@{#451856}
  • Loading branch information
alexelias authored and Commit bot committed Feb 21, 2017
1 parent 680838d commit 0373fe1
Show file tree
Hide file tree
Showing 23 changed files with 0 additions and 163 deletions.
5 changes: 0 additions & 5 deletions chrome/browser/supervised_user/supervised_user_pref_store.cc
Original file line number Diff line number Diff line change
Expand Up @@ -87,11 +87,6 @@ bool SupervisedUserPrefStore::GetValue(const std::string& key,
return prefs_->GetValue(key, value);
}

std::unique_ptr<base::DictionaryValue> SupervisedUserPrefStore::GetValues()
const {
return prefs_->AsDictionaryValue();
}

void SupervisedUserPrefStore::AddObserver(PrefStore::Observer* observer) {
observers_.AddObserver(observer);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ class SupervisedUserPrefStore : public PrefStore,
// PrefStore overrides:
bool GetValue(const std::string& key,
const base::Value** value) const override;
std::unique_ptr<base::DictionaryValue> GetValues() const override;
void AddObserver(PrefStore::Observer* observer) override;
void RemoveObserver(PrefStore::Observer* observer) override;
bool HasObservers() const override;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
#include "base/bind.h"
#include "base/location.h"
#include "base/logging.h"
#include "base/memory/ptr_util.h"
#include "base/single_thread_task_runner.h"
#include "base/strings/string16.h"
#include "base/strings/utf_string_conversions.h"
Expand Down Expand Up @@ -80,13 +79,6 @@ bool ConfigurationPolicyPrefStore::GetValue(const std::string& key,
return true;
}

std::unique_ptr<base::DictionaryValue> ConfigurationPolicyPrefStore::GetValues()
const {
if (!prefs_)
return base::MakeUnique<base::DictionaryValue>();
return prefs_->AsDictionaryValue();
}

void ConfigurationPolicyPrefStore::OnPolicyUpdated(
const PolicyNamespace& ns,
const PolicyMap& previous,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ class POLICY_EXPORT ConfigurationPolicyPrefStore
bool IsInitializationComplete() const override;
bool GetValue(const std::string& key,
const base::Value** result) const override;
std::unique_ptr<base::DictionaryValue> GetValues() const override;

// PolicyService::Observer methods:
void OnPolicyUpdated(const PolicyNamespace& ns,
Expand Down
4 changes: 0 additions & 4 deletions components/prefs/default_pref_store.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,6 @@ bool DefaultPrefStore::GetValue(const std::string& key,
return prefs_.GetValue(key, result);
}

std::unique_ptr<base::DictionaryValue> DefaultPrefStore::GetValues() const {
return prefs_.AsDictionaryValue();
}

void DefaultPrefStore::AddObserver(PrefStore::Observer* observer) {
observers_.AddObserver(observer);
}
Expand Down
2 changes: 0 additions & 2 deletions components/prefs/default_pref_store.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
#ifndef COMPONENTS_PREFS_DEFAULT_PREF_STORE_H_
#define COMPONENTS_PREFS_DEFAULT_PREF_STORE_H_

#include <memory>
#include <string>

#include "base/macros.h"
Expand All @@ -25,7 +24,6 @@ class COMPONENTS_PREFS_EXPORT DefaultPrefStore : public PrefStore {
// PrefStore implementation:
bool GetValue(const std::string& key,
const base::Value** result) const override;
std::unique_ptr<base::DictionaryValue> GetValues() const override;
void AddObserver(PrefStore::Observer* observer) override;
void RemoveObserver(PrefStore::Observer* observer) override;
bool HasObservers() const override;
Expand Down
4 changes: 0 additions & 4 deletions components/prefs/in_memory_pref_store.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,6 @@ bool InMemoryPrefStore::GetValue(const std::string& key,
return prefs_.GetValue(key, value);
}

std::unique_ptr<base::DictionaryValue> InMemoryPrefStore::GetValues() const {
return prefs_.AsDictionaryValue();
}

bool InMemoryPrefStore::GetMutableValue(const std::string& key,
base::Value** value) {
return prefs_.GetValue(key, value);
Expand Down
1 change: 0 additions & 1 deletion components/prefs/in_memory_pref_store.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ class COMPONENTS_PREFS_EXPORT InMemoryPrefStore : public PersistentPrefStore {
// PrefStore implementation.
bool GetValue(const std::string& key,
const base::Value** result) const override;
std::unique_ptr<base::DictionaryValue> GetValues() const override;
void AddObserver(PrefStore::Observer* observer) override;
void RemoveObserver(PrefStore::Observer* observer) override;
bool HasObservers() const override;
Expand Down
4 changes: 0 additions & 4 deletions components/prefs/json_pref_store.cc
Original file line number Diff line number Diff line change
Expand Up @@ -190,10 +190,6 @@ bool JsonPrefStore::GetValue(const std::string& key,
return true;
}

std::unique_ptr<base::DictionaryValue> JsonPrefStore::GetValues() const {
return prefs_->CreateDeepCopy();
}

void JsonPrefStore::AddObserver(PrefStore::Observer* observer) {
DCHECK(CalledOnValidThread());

Expand Down
1 change: 0 additions & 1 deletion components/prefs/json_pref_store.h
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,6 @@ class COMPONENTS_PREFS_EXPORT JsonPrefStore
// PrefStore overrides:
bool GetValue(const std::string& key,
const base::Value** result) const override;
std::unique_ptr<base::DictionaryValue> GetValues() const override;
void AddObserver(PrefStore::Observer* observer) override;
void RemoveObserver(PrefStore::Observer* observer) override;
bool HasObservers() const override;
Expand Down
18 changes: 0 additions & 18 deletions components/prefs/overlay_user_pref_store.cc
Original file line number Diff line number Diff line change
Expand Up @@ -47,24 +47,6 @@ bool OverlayUserPrefStore::GetValue(const std::string& key,
return underlay_->GetValue(GetUnderlayKey(key), result);
}

std::unique_ptr<base::DictionaryValue> OverlayUserPrefStore::GetValues() const {
auto values = underlay_->GetValues();
auto overlay_values = overlay_.AsDictionaryValue();
for (const auto& overlay_mapping : overlay_to_underlay_names_map_) {
const std::string& overlay_key = overlay_mapping.first;
const std::string& underlay_key = overlay_mapping.second;
std::unique_ptr<base::Value> out_value;
if (overlay_key != underlay_key) {
values->Remove(underlay_key, &out_value);
}
overlay_values->Remove(overlay_key, &out_value);
if (out_value) {
values->Set(overlay_key, std::move(out_value));
}
}
return values;
}

bool OverlayUserPrefStore::GetMutableValue(const std::string& key,
base::Value** result) {
if (!ShallBeStoredInOverlay(key))
Expand Down
1 change: 0 additions & 1 deletion components/prefs/overlay_user_pref_store.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ class COMPONENTS_PREFS_EXPORT OverlayUserPrefStore : public PersistentPrefStore,
bool IsInitializationComplete() const override;
bool GetValue(const std::string& key,
const base::Value** result) const override;
std::unique_ptr<base::DictionaryValue> GetValues() const override;

// Methods of PersistentPrefStore.
bool GetMutableValue(const std::string& key, base::Value** result) override;
Expand Down
41 changes: 0 additions & 41 deletions components/prefs/overlay_user_pref_store_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,9 @@ namespace {

const char kBrowserWindowPlacement[] = "browser.window_placement";
const char kShowBookmarkBar[] = "bookmark_bar.show_on_all_tabs";
const char kSharedKey[] = "sync_promo.show_on_first_run_allowed";

const char* const overlay_key = kBrowserWindowPlacement;
const char* const regular_key = kShowBookmarkBar;
const char* const shared_key = kSharedKey;
// With the removal of the kWebKitGlobalXXX prefs, we'll no longer have real
// prefs using the overlay pref store, so make up keys here.
const char mapped_overlay_key[] = "test.per_tab.javascript_enabled";
Expand All @@ -37,7 +35,6 @@ class OverlayUserPrefStoreTest : public testing::Test {
: underlay_(new TestingPrefStore()),
overlay_(new OverlayUserPrefStore(underlay_.get())) {
overlay_->RegisterOverlayPref(overlay_key);
overlay_->RegisterOverlayPref(shared_key);
overlay_->RegisterOverlayPref(mapped_overlay_key, mapped_underlay_key);
}

Expand Down Expand Up @@ -312,42 +309,4 @@ TEST_F(OverlayUserPrefStoreTest, ClearMutableValues) {
EXPECT_TRUE(base::FundamentalValue(42).Equals(value));
}

TEST_F(OverlayUserPrefStoreTest, GetValues) {
// To check merge behavior, create underlay and overlay so each has a key the
// other doesn't have and they have one key in common.
underlay_->SetValue(regular_key, base::WrapUnique(new FundamentalValue(42)),
WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS);
overlay_->SetValue(overlay_key, base::WrapUnique(new FundamentalValue(43)),
WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS);
underlay_->SetValue(shared_key, base::WrapUnique(new FundamentalValue(42)),
WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS);
overlay_->SetValue(shared_key, base::WrapUnique(new FundamentalValue(43)),
WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS);
underlay_->SetValue(mapped_underlay_key,
base::WrapUnique(new FundamentalValue(42)),
WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS);
overlay_->SetValue(mapped_overlay_key,
base::WrapUnique(new FundamentalValue(43)),
WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS);

auto values = overlay_->GetValues();
const Value* value = nullptr;
// Check that an overlay preference is returned.
ASSERT_TRUE(values->Get(overlay_key, &value));
EXPECT_TRUE(base::FundamentalValue(43).Equals(value));

// Check that an underlay preference is returned.
ASSERT_TRUE(values->Get(regular_key, &value));
EXPECT_TRUE(base::FundamentalValue(42).Equals(value));

// Check that the overlay is preferred.
ASSERT_TRUE(values->Get(shared_key, &value));
EXPECT_TRUE(base::FundamentalValue(43).Equals(value));

// Check that mapping works.
ASSERT_TRUE(values->Get(mapped_overlay_key, &value));
EXPECT_TRUE(base::FundamentalValue(43).Equals(value));
EXPECT_FALSE(values->Get(mapped_underlay_key, &value));
}

} // namespace base
5 changes: 0 additions & 5 deletions components/prefs/pref_store.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,13 @@
#ifndef COMPONENTS_PREFS_PREF_STORE_H_
#define COMPONENTS_PREFS_PREF_STORE_H_

#include <memory>
#include <string>

#include "base/macros.h"
#include "base/memory/ref_counted.h"
#include "components/prefs/base_prefs_export.h"

namespace base {
class DictionaryValue;
class Value;
}

Expand Down Expand Up @@ -54,9 +52,6 @@ class COMPONENTS_PREFS_EXPORT PrefStore : public base::RefCounted<PrefStore> {
virtual bool GetValue(const std::string& key,
const base::Value** result) const = 0;

// Get all the values. Never returns a null pointer.
virtual std::unique_ptr<base::DictionaryValue> GetValues() const = 0;

protected:
friend class base::RefCounted<PrefStore>;
virtual ~PrefStore() {}
Expand Down
8 changes: 0 additions & 8 deletions components/prefs/pref_value_map.cc
Original file line number Diff line number Diff line change
Expand Up @@ -157,11 +157,3 @@ void PrefValueMap::GetDifferingKeys(
for ( ; other_pref != other_prefs.end(); ++other_pref)
differing_keys->push_back(other_pref->first);
}

std::unique_ptr<base::DictionaryValue> PrefValueMap::AsDictionaryValue() const {
auto dictionary = base::MakeUnique<base::DictionaryValue>();
for (const auto& value : prefs_) {
dictionary->Set(value.first, value.second->CreateDeepCopy());
}
return dictionary;
}
4 changes: 0 additions & 4 deletions components/prefs/pref_value_map.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
#include "components/prefs/base_prefs_export.h"

namespace base {
class DictionaryValue;
class Value;
}

Expand Down Expand Up @@ -84,9 +83,6 @@ class COMPONENTS_PREFS_EXPORT PrefValueMap {
void GetDifferingKeys(const PrefValueMap* other,
std::vector<std::string>* differing_keys) const;

// Copies the map into a dictionary value.
std::unique_ptr<base::DictionaryValue> AsDictionaryValue() const;

private:
Map prefs_;

Expand Down
4 changes: 0 additions & 4 deletions components/prefs/testing_pref_store.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,6 @@ bool TestingPrefStore::GetValue(const std::string& key,
return prefs_.GetValue(key, value);
}

std::unique_ptr<base::DictionaryValue> TestingPrefStore::GetValues() const {
return prefs_.AsDictionaryValue();
}

bool TestingPrefStore::GetMutableValue(const std::string& key,
base::Value** value) {
return prefs_.GetValue(key, value);
Expand Down
1 change: 0 additions & 1 deletion components/prefs/testing_pref_store.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ class TestingPrefStore : public PersistentPrefStore {
// Overriden from PrefStore.
bool GetValue(const std::string& key,
const base::Value** result) const override;
std::unique_ptr<base::DictionaryValue> GetValues() const override;
void AddObserver(PrefStore::Observer* observer) override;
void RemoveObserver(PrefStore::Observer* observer) override;
bool HasObservers() const override;
Expand Down
4 changes: 0 additions & 4 deletions components/prefs/value_map_pref_store.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,6 @@ bool ValueMapPrefStore::GetValue(const std::string& key,
return prefs_.GetValue(key, value);
}

std::unique_ptr<base::DictionaryValue> ValueMapPrefStore::GetValues() const {
return prefs_.AsDictionaryValue();
}

void ValueMapPrefStore::AddObserver(PrefStore::Observer* observer) {
observers_.AddObserver(observer);
}
Expand Down
1 change: 0 additions & 1 deletion components/prefs/value_map_pref_store.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ class COMPONENTS_PREFS_EXPORT ValueMapPrefStore : public WriteablePrefStore {
// PrefStore overrides:
bool GetValue(const std::string& key,
const base::Value** value) const override;
std::unique_ptr<base::DictionaryValue> GetValues() const override;
void AddObserver(PrefStore::Observer* observer) override;
void RemoveObserver(PrefStore::Observer* observer) override;
bool HasObservers() const override;
Expand Down
14 changes: 0 additions & 14 deletions components/user_prefs/tracked/segregated_pref_store.cc
Original file line number Diff line number Diff line change
Expand Up @@ -83,20 +83,6 @@ bool SegregatedPrefStore::GetValue(const std::string& key,
return StoreForKey(key)->GetValue(key, result);
}

std::unique_ptr<base::DictionaryValue> SegregatedPrefStore::GetValues() const {
auto values = default_pref_store_->GetValues();
auto selected_pref_store_values = selected_pref_store_->GetValues();
for (const auto& key : selected_preference_names_) {
const base::Value* value = nullptr;
if (selected_pref_store_values->Get(key, &value)) {
values->Set(key, value->CreateDeepCopy());
} else {
values->Remove(key, nullptr);
}
}
return values;
}

void SegregatedPrefStore::SetValue(const std::string& key,
std::unique_ptr<base::Value> value,
uint32_t flags) {
Expand Down
1 change: 0 additions & 1 deletion components/user_prefs/tracked/segregated_pref_store.h
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ class SegregatedPrefStore : public PersistentPrefStore {
bool IsInitializationComplete() const override;
bool GetValue(const std::string& key,
const base::Value** result) const override;
std::unique_ptr<base::DictionaryValue> GetValues() const override;

// WriteablePrefStore implementation
void SetValue(const std::string& key,
Expand Down
30 changes: 0 additions & 30 deletions components/user_prefs/tracked/segregated_pref_store_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ namespace {

const char kSelectedPref[] = "selected_pref";
const char kUnselectedPref[] = "unselected_pref";
const char kSharedPref[] = "shared_pref";

const char kValue1[] = "value1";
const char kValue2[] = "value2";
Expand Down Expand Up @@ -71,7 +70,6 @@ class SegregatedPrefStoreTest : public testing::Test {

std::set<std::string> selected_pref_names;
selected_pref_names.insert(kSelectedPref);
selected_pref_names.insert(kSharedPref);

segregated_store_ = new SegregatedPrefStore(default_store_, selected_store_,
selected_pref_names);
Expand Down Expand Up @@ -276,31 +274,3 @@ TEST_F(SegregatedPrefStoreTest, IsInitializationCompleteAsync) {
default_store_->SetBlockAsyncRead(false);
EXPECT_TRUE(segregated_store_->IsInitializationComplete());
}

TEST_F(SegregatedPrefStoreTest, GetValues) {
// To check merge behavior, create selected and default stores so each has a
// key the other doesn't have and they have one key in common.
selected_store_->SetValue(kSelectedPref,
base::MakeUnique<base::StringValue>(kValue1),
WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS);
default_store_->SetValue(kUnselectedPref,
base::MakeUnique<base::StringValue>(kValue2),
WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS);
selected_store_->SetValue(kSharedPref,
base::MakeUnique<base::StringValue>(kValue1),
WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS);

auto values = segregated_store_->GetValues();
const base::Value* value = nullptr;
// Check that a selected preference is returned.
ASSERT_TRUE(values->Get(kSelectedPref, &value));
EXPECT_TRUE(base::FundamentalValue(kValue1).Equals(value));

// Check that a a default preference is returned.
ASSERT_TRUE(values->Get(kUnselectedPref, &value));
EXPECT_TRUE(base::FundamentalValue(kValue2).Equals(value));

// Check that the selected preference is preferred.
ASSERT_TRUE(values->Get(kSharedPref, &value));
EXPECT_TRUE(base::FundamentalValue(kValue1).Equals(value));
}

0 comments on commit 0373fe1

Please sign in to comment.