Refactor: Migrate all prefs to string_view
There should be no behavioral changes, just how the parameters are
taken.
What's wrong with std::string:
1. Data is heap allocated. Most usecase of pref involves look up
some value with a compile time constant string. For example,
|prefs.GetKey(kPrefsManifestBytes)|. When this code is executed,
what it's doing is create a std::instance with kPrefsManifestBytes,
which is a const char *. The program must first determine the length
of |kPrefsManifestBytes|, allocate sufficient heap memory to store
it, then copy all contents over. After prefs.GetKey() is called, the
allocated string must be deallocated. Everytime we call GetKey()
with a const char *, we execute the same
strlen()-allocate()-copy()-deallocate() sequence. Which is not
efficient.
2. Often requires passing by reference, which introduces 1 more
pointer indirection
Why/How std::string_view fixes these problems
1. std::string_view does not own the underlying data. When you do
std::string_view{kPrefsManifestBytes}, it merely takes the pointer
in, store it, then compute the size. No heap allocation happens.
However, since std::string_view doesn't own the underlying data,
lifetime of std::string_view cannot exceed that of const char *.
This is fine for our usecase, because constants like
kPrefsManifestBytes are static constants and are valid thorugh out
the entire program execution
2. Since std::string_view is virtuall a tuple<const char *data,
size_t size>, no need to pass it by reference. It can be efficiently
passed around by value, reducing pointer indirection.
Side note:
std::string_view is essentially the C++ equivalence of go/java-tips/012
Test: th
Change-Id: I4c3f1d88a0587e36ac5eca43d553448da1e2e878
diff --git a/common/fake_prefs.cc b/common/fake_prefs.cc
index ea6ea60..e87e0ec 100644
--- a/common/fake_prefs.cc
+++ b/common/fake_prefs.cc
@@ -28,7 +28,7 @@
namespace {
-void CheckNotNull(const string& key, void* ptr) {
+void CheckNotNull(std::string_view key, void* ptr) {
EXPECT_NE(nullptr, ptr) << "Called Get*() for key \"" << key
<< "\" with a null parameter.";
}
@@ -63,41 +63,41 @@
bool FakePrefs::PrefValue::*const FakePrefs::PrefConsts<bool>::member =
&FakePrefs::PrefValue::as_bool;
-bool FakePrefs::GetString(const string& key, string* value) const {
+bool FakePrefs::GetString(std::string_view key, string* value) const {
return GetValue(key, value);
}
-bool FakePrefs::SetString(const string& key, std::string_view value) {
+bool FakePrefs::SetString(std::string_view key, std::string_view value) {
SetValue(key, std::string(value));
return true;
}
-bool FakePrefs::GetInt64(const string& key, int64_t* value) const {
+bool FakePrefs::GetInt64(std::string_view key, int64_t* value) const {
return GetValue(key, value);
}
-bool FakePrefs::SetInt64(const string& key, const int64_t value) {
+bool FakePrefs::SetInt64(std::string_view key, const int64_t value) {
SetValue(key, value);
return true;
}
-bool FakePrefs::GetBoolean(const string& key, bool* value) const {
+bool FakePrefs::GetBoolean(std::string_view key, bool* value) const {
return GetValue(key, value);
}
-bool FakePrefs::SetBoolean(const string& key, const bool value) {
+bool FakePrefs::SetBoolean(std::string_view key, const bool value) {
SetValue(key, value);
return true;
}
-bool FakePrefs::Exists(const string& key) const {
+bool FakePrefs::Exists(std::string_view key) const {
return values_.find(key) != values_.end();
}
-bool FakePrefs::Delete(const string& key) {
+bool FakePrefs::Delete(std::string_view key) {
if (values_.find(key) == values_.end())
return false;
- values_.erase(key);
+ values_.erase(std::string{key});
const auto observers_for_key = observers_.find(key);
if (observers_for_key != observers_.end()) {
std::vector<ObserverInterface*> copy_observers(observers_for_key->second);
@@ -107,7 +107,7 @@
return true;
}
-bool FakePrefs::Delete(const string& key, const vector<string>& nss) {
+bool FakePrefs::Delete(std::string_view key, const vector<string>& nss) {
bool success = Delete(key);
for (const auto& ns : nss) {
vector<string> ns_keys;
@@ -123,7 +123,7 @@
return success;
}
-bool FakePrefs::GetSubKeys(const string& ns, vector<string>* keys) const {
+bool FakePrefs::GetSubKeys(std::string_view ns, vector<string>* keys) const {
for (const auto& pr : values_)
if (pr.first.compare(0, ns.length(), ns) == 0)
keys->push_back(pr.first);
@@ -142,7 +142,7 @@
return "Unknown";
}
-void FakePrefs::CheckKeyType(const string& key, PrefType type) const {
+void FakePrefs::CheckKeyType(std::string_view key, PrefType type) const {
auto it = values_.find(key);
EXPECT_TRUE(it == values_.end() || it->second.type == type)
<< "Key \"" << key << "\" if defined as " << GetTypeName(it->second.type)
@@ -150,10 +150,11 @@
}
template <typename T>
-void FakePrefs::SetValue(const string& key, T value) {
+void FakePrefs::SetValue(std::string_view key, T value) {
+ std::string str_key{key};
CheckKeyType(key, PrefConsts<T>::type);
- values_[key].type = PrefConsts<T>::type;
- values_[key].value.*(PrefConsts<T>::member) = std::move(value);
+ values_[str_key].type = PrefConsts<T>::type;
+ values_[str_key].value.*(PrefConsts<T>::member) = std::move(value);
const auto observers_for_key = observers_.find(key);
if (observers_for_key != observers_.end()) {
std::vector<ObserverInterface*> copy_observers(observers_for_key->second);
@@ -163,7 +164,7 @@
}
template <typename T>
-bool FakePrefs::GetValue(const string& key, T* value) const {
+bool FakePrefs::GetValue(std::string_view key, T* value) const {
CheckKeyType(key, PrefConsts<T>::type);
auto it = values_.find(key);
if (it == values_.end())
@@ -173,12 +174,14 @@
return true;
}
-void FakePrefs::AddObserver(const string& key, ObserverInterface* observer) {
- observers_[key].push_back(observer);
+void FakePrefs::AddObserver(std::string_view key, ObserverInterface* observer) {
+ observers_[string{key}].push_back(observer);
}
-void FakePrefs::RemoveObserver(const string& key, ObserverInterface* observer) {
- std::vector<ObserverInterface*>& observers_for_key = observers_[key];
+void FakePrefs::RemoveObserver(std::string_view key,
+ ObserverInterface* observer) {
+ string str_key{key};
+ std::vector<ObserverInterface*>& observers_for_key = observers_[str_key];
auto observer_it =
std::find(observers_for_key.begin(), observers_for_key.end(), observer);
EXPECT_NE(observer_it, observers_for_key.end())
@@ -186,7 +189,7 @@
if (observer_it != observers_for_key.end())
observers_for_key.erase(observer_it);
if (observers_for_key.empty())
- observers_.erase(key);
+ observers_.erase(str_key);
}
} // namespace chromeos_update_engine
diff --git a/common/fake_prefs.h b/common/fake_prefs.h
index 430c291..7ae9fb9 100644
--- a/common/fake_prefs.h
+++ b/common/fake_prefs.h
@@ -17,6 +17,7 @@
#ifndef UPDATE_ENGINE_COMMON_FAKE_PREFS_H_
#define UPDATE_ENGINE_COMMON_FAKE_PREFS_H_
+#include <functional>
#include <map>
#include <string>
#include <string_view>
@@ -40,24 +41,23 @@
~FakePrefs();
// PrefsInterface methods.
- bool GetString(const std::string& key, std::string* value) const override;
- bool SetString(const std::string& key, std::string_view value) override;
- bool GetInt64(const std::string& key, int64_t* value) const override;
- bool SetInt64(const std::string& key, const int64_t value) override;
- bool GetBoolean(const std::string& key, bool* value) const override;
- bool SetBoolean(const std::string& key, const bool value) override;
+ bool GetString(std::string_view key, std::string* value) const override;
+ bool SetString(std::string_view key, std::string_view value) override;
+ bool GetInt64(std::string_view key, int64_t* value) const override;
+ bool SetInt64(std::string_view key, const int64_t value) override;
+ bool GetBoolean(std::string_view key, bool* value) const override;
+ bool SetBoolean(std::string_view key, const bool value) override;
- bool Exists(const std::string& key) const override;
- bool Delete(const std::string& key) override;
- bool Delete(const std::string& key,
+ bool Exists(std::string_view key) const override;
+ bool Delete(std::string_view key) override;
+ bool Delete(std::string_view key,
const std::vector<std::string>& nss) override;
- bool GetSubKeys(const std::string& ns,
+ bool GetSubKeys(std::string_view ns,
std::vector<std::string>* keys) const override;
- void AddObserver(const std::string& key,
- ObserverInterface* observer) override;
- void RemoveObserver(const std::string& key,
+ void AddObserver(std::string_view key, ObserverInterface* observer) override;
+ void RemoveObserver(std::string_view key,
ObserverInterface* observer) override;
private:
@@ -92,24 +92,25 @@
static std::string GetTypeName(PrefType type);
// Checks that the |key| is either not present or has the given |type|.
- void CheckKeyType(const std::string& key, PrefType type) const;
+ void CheckKeyType(std::string_view key, PrefType type) const;
// Helper function to set a value of the passed |key|. It sets the type based
// on the template parameter T.
template <typename T>
- void SetValue(const std::string& key, T value);
+ void SetValue(std::string_view key, T value);
// Helper function to get a value from the map checking for invalid calls.
// The function fails the test if you attempt to read a value defined as a
// different type. Returns whether the get succeeded.
template <typename T>
- bool GetValue(const std::string& key, T* value) const;
+ bool GetValue(std::string_view key, T* value) const;
// Container for all the key/value pairs.
- std::map<std::string, PrefTypeValue> values_;
+ std::map<std::string, PrefTypeValue, std::less<>> values_;
// The registered observers watching for changes.
- std::map<std::string, std::vector<ObserverInterface*>> observers_;
+ std::map<std::string, std::vector<ObserverInterface*>, std::less<>>
+ observers_;
DISALLOW_COPY_AND_ASSIGN(FakePrefs);
};
diff --git a/common/mock_prefs.h b/common/mock_prefs.h
index 49431fb..f308074 100644
--- a/common/mock_prefs.h
+++ b/common/mock_prefs.h
@@ -29,27 +29,24 @@
class MockPrefs : public PrefsInterface {
public:
- MOCK_CONST_METHOD2(GetString,
- bool(const std::string& key, std::string* value));
- MOCK_METHOD2(SetString, bool(const std::string& key, std::string_view value));
- MOCK_CONST_METHOD2(GetInt64, bool(const std::string& key, int64_t* value));
- MOCK_METHOD2(SetInt64, bool(const std::string& key, const int64_t value));
+ MOCK_CONST_METHOD2(GetString, bool(std::string_view key, std::string* value));
+ MOCK_METHOD2(SetString, bool(std::string_view key, std::string_view value));
+ MOCK_CONST_METHOD2(GetInt64, bool(std::string_view key, int64_t* value));
+ MOCK_METHOD2(SetInt64, bool(std::string_view key, const int64_t value));
- MOCK_CONST_METHOD2(GetBoolean, bool(const std::string& key, bool* value));
- MOCK_METHOD2(SetBoolean, bool(const std::string& key, const bool value));
+ MOCK_CONST_METHOD2(GetBoolean, bool(std::string_view key, bool* value));
+ MOCK_METHOD2(SetBoolean, bool(std::string_view key, const bool value));
- MOCK_CONST_METHOD1(Exists, bool(const std::string& key));
- MOCK_METHOD1(Delete, bool(const std::string& key));
+ MOCK_CONST_METHOD1(Exists, bool(std::string_view key));
+ MOCK_METHOD1(Delete, bool(std::string_view key));
MOCK_METHOD2(Delete,
- bool(const std::string& key,
- const std::vector<std::string>& nss));
+ bool(std::string_view key, const std::vector<std::string>& nss));
MOCK_CONST_METHOD2(GetSubKeys,
- bool(const std::string&, std::vector<std::string>*));
+ bool(std::string_view, std::vector<std::string>*));
- MOCK_METHOD2(AddObserver, void(const std::string& key, ObserverInterface*));
- MOCK_METHOD2(RemoveObserver,
- void(const std::string& key, ObserverInterface*));
+ MOCK_METHOD2(AddObserver, void(std::string_view key, ObserverInterface*));
+ MOCK_METHOD2(RemoveObserver, void(std::string_view key, ObserverInterface*));
};
} // namespace chromeos_update_engine
diff --git a/common/prefs.cc b/common/prefs.cc
index 1e06be4..f33a8a9 100644
--- a/common/prefs.cc
+++ b/common/prefs.cc
@@ -51,11 +51,11 @@
} // namespace
-bool PrefsBase::GetString(const string& key, string* value) const {
+bool PrefsBase::GetString(const std::string_view key, string* value) const {
return storage_->GetKey(key, value);
}
-bool PrefsBase::SetString(const string& key, std::string_view value) {
+bool PrefsBase::SetString(std::string_view key, std::string_view value) {
TEST_AND_RETURN_FALSE(storage_->SetKey(key, value));
const auto observers_for_key = observers_.find(key);
if (observers_for_key != observers_.end()) {
@@ -66,7 +66,7 @@
return true;
}
-bool PrefsBase::GetInt64(const string& key, int64_t* value) const {
+bool PrefsBase::GetInt64(const std::string_view key, int64_t* value) const {
string str_value;
if (!GetString(key, &str_value))
return false;
@@ -75,11 +75,11 @@
return true;
}
-bool PrefsBase::SetInt64(const string& key, const int64_t value) {
+bool PrefsBase::SetInt64(std::string_view key, const int64_t value) {
return SetString(key, base::NumberToString(value));
}
-bool PrefsBase::GetBoolean(const string& key, bool* value) const {
+bool PrefsBase::GetBoolean(std::string_view key, bool* value) const {
string str_value;
if (!GetString(key, &str_value))
return false;
@@ -95,15 +95,15 @@
return false;
}
-bool PrefsBase::SetBoolean(const string& key, const bool value) {
+bool PrefsBase::SetBoolean(std::string_view key, const bool value) {
return SetString(key, value ? "true" : "false");
}
-bool PrefsBase::Exists(const string& key) const {
+bool PrefsBase::Exists(std::string_view key) const {
return storage_->KeyExists(key);
}
-bool PrefsBase::Delete(const string& key) {
+bool PrefsBase::Delete(std::string_view key) {
TEST_AND_RETURN_FALSE(storage_->DeleteKey(key));
const auto observers_for_key = observers_.find(key);
if (observers_for_key != observers_.end()) {
@@ -114,7 +114,7 @@
return true;
}
-bool PrefsBase::Delete(const string& pref_key, const vector<string>& nss) {
+bool PrefsBase::Delete(std::string_view pref_key, const vector<string>& nss) {
// Delete pref key for platform.
bool success = Delete(pref_key);
// Delete pref key in each namespace.
@@ -132,16 +132,18 @@
return success;
}
-bool PrefsBase::GetSubKeys(const string& ns, vector<string>* keys) const {
+bool PrefsBase::GetSubKeys(std::string_view ns, vector<string>* keys) const {
return storage_->GetSubKeys(ns, keys);
}
-void PrefsBase::AddObserver(const string& key, ObserverInterface* observer) {
- observers_[key].push_back(observer);
+void PrefsBase::AddObserver(std::string_view key, ObserverInterface* observer) {
+ observers_[std::string{key}].push_back(observer);
}
-void PrefsBase::RemoveObserver(const string& key, ObserverInterface* observer) {
- std::vector<ObserverInterface*>& observers_for_key = observers_[key];
+void PrefsBase::RemoveObserver(std::string_view key,
+ ObserverInterface* observer) {
+ std::vector<ObserverInterface*>& observers_for_key =
+ observers_[std::string{key}];
auto observer_it =
std::find(observers_for_key.begin(), observers_for_key.end(), observer);
if (observer_it != observers_for_key.end())
@@ -165,7 +167,7 @@
return true;
}
-bool Prefs::FileStorage::GetKey(const string& key, string* value) const {
+bool Prefs::FileStorage::GetKey(std::string_view key, string* value) const {
base::FilePath filename;
TEST_AND_RETURN_FALSE(GetFileNameForKey(key, &filename));
if (!base::ReadFileToString(filename, value)) {
@@ -174,7 +176,7 @@
return true;
}
-bool Prefs::FileStorage::GetSubKeys(const string& ns,
+bool Prefs::FileStorage::GetSubKeys(std::string_view ns,
vector<string>* keys) const {
base::FilePath filename;
TEST_AND_RETURN_FALSE(GetFileNameForKey(ns, &filename));
@@ -192,7 +194,7 @@
return true;
}
-bool Prefs::FileStorage::SetKey(const string& key, std::string_view value) {
+bool Prefs::FileStorage::SetKey(std::string_view key, std::string_view value) {
base::FilePath filename;
TEST_AND_RETURN_FALSE(GetFileNameForKey(key, &filename));
if (!base::DirectoryExists(filename.DirName())) {
@@ -205,13 +207,13 @@
return true;
}
-bool Prefs::FileStorage::KeyExists(const string& key) const {
+bool Prefs::FileStorage::KeyExists(std::string_view key) const {
base::FilePath filename;
TEST_AND_RETURN_FALSE(GetFileNameForKey(key, &filename));
return base::PathExists(filename);
}
-bool Prefs::FileStorage::DeleteKey(const string& key) {
+bool Prefs::FileStorage::DeleteKey(std::string_view key) {
base::FilePath filename;
TEST_AND_RETURN_FALSE(GetFileNameForKey(key, &filename));
#if BASE_VER < 800000
@@ -222,20 +224,21 @@
return true;
}
-bool Prefs::FileStorage::GetFileNameForKey(const string& key,
+bool Prefs::FileStorage::GetFileNameForKey(std::string_view key,
base::FilePath* filename) const {
// Allows only non-empty keys containing [A-Za-z0-9_-/].
TEST_AND_RETURN_FALSE(!key.empty());
for (char c : key)
TEST_AND_RETURN_FALSE(base::IsAsciiAlpha(c) || base::IsAsciiDigit(c) ||
c == '_' || c == '-' || c == kKeySeparator);
- *filename = prefs_dir_.Append(key);
+ *filename = prefs_dir_.Append(
+ base::FilePath::StringPieceType(key.data(), key.size()));
return true;
}
// MemoryPrefs
-bool MemoryPrefs::MemoryStorage::GetKey(const string& key,
+bool MemoryPrefs::MemoryStorage::GetKey(std::string_view key,
string* value) const {
auto it = values_.find(key);
if (it == values_.end())
@@ -244,15 +247,13 @@
return true;
}
-bool MemoryPrefs::MemoryStorage::GetSubKeys(const string& ns,
+bool MemoryPrefs::MemoryStorage::GetSubKeys(std::string_view ns,
vector<string>* keys) const {
- using value_type = decltype(values_)::value_type;
- using key_type = decltype(values_)::key_type;
- auto lower_comp = [](const value_type& pr, const key_type& ns) {
- return pr.first.substr(0, ns.length()) < ns;
+ auto lower_comp = [](const auto& pr, const auto& ns) {
+ return std::string_view{pr.first.data(), ns.length()} < ns;
};
- auto upper_comp = [](const key_type& ns, const value_type& pr) {
- return ns < pr.first.substr(0, ns.length());
+ auto upper_comp = [](const auto& ns, const auto& pr) {
+ return ns < std::string_view{pr.first.data(), ns.length()};
};
auto lower_it =
std::lower_bound(begin(values_), end(values_), ns, lower_comp);
@@ -262,17 +263,17 @@
return true;
}
-bool MemoryPrefs::MemoryStorage::SetKey(const string& key,
+bool MemoryPrefs::MemoryStorage::SetKey(std::string_view key,
std::string_view value) {
- values_[key] = value;
+ values_[std::string{key}] = value;
return true;
}
-bool MemoryPrefs::MemoryStorage::KeyExists(const string& key) const {
+bool MemoryPrefs::MemoryStorage::KeyExists(std::string_view key) const {
return values_.find(key) != values_.end();
}
-bool MemoryPrefs::MemoryStorage::DeleteKey(const string& key) {
+bool MemoryPrefs::MemoryStorage::DeleteKey(std::string_view key) {
auto it = values_.find(key);
if (it != values_.end())
values_.erase(it);
diff --git a/common/prefs.h b/common/prefs.h
index 93477dd..c3105c6 100644
--- a/common/prefs.h
+++ b/common/prefs.h
@@ -17,6 +17,7 @@
#ifndef UPDATE_ENGINE_COMMON_PREFS_H_
#define UPDATE_ENGINE_COMMON_PREFS_H_
+#include <functional>
#include <map>
#include <string>
#include <string_view>
@@ -41,23 +42,23 @@
// Get the key named |key| and store its value in the referenced |value|.
// Returns whether the operation succeeded.
- virtual bool GetKey(const std::string& key, std::string* value) const = 0;
+ virtual bool GetKey(std::string_view key, std::string* value) const = 0;
// Get the keys stored within the namespace. If there are no keys in the
// namespace, |keys| will be empty. Returns whether the operation succeeded.
- virtual bool GetSubKeys(const std::string& ns,
+ virtual bool GetSubKeys(std::string_view ns,
std::vector<std::string>* keys) const = 0;
// Set the value of the key named |key| to |value| regardless of the
// previous value. Returns whether the operation succeeded.
- virtual bool SetKey(const std::string& key, std::string_view value) = 0;
+ virtual bool SetKey(std::string_view key, std::string_view value) = 0;
// Returns whether the key named |key| exists.
- virtual bool KeyExists(const std::string& key) const = 0;
+ virtual bool KeyExists(std::string_view key) const = 0;
// Deletes the value associated with the key name |key|. Returns whether the
// key was deleted.
- virtual bool DeleteKey(const std::string& key) = 0;
+ virtual bool DeleteKey(std::string_view key) = 0;
private:
DISALLOW_COPY_AND_ASSIGN(StorageInterface);
@@ -66,29 +67,29 @@
explicit PrefsBase(StorageInterface* storage) : storage_(storage) {}
// PrefsInterface methods.
- bool GetString(const std::string& key, std::string* value) const override;
- bool SetString(const std::string& key, std::string_view value) override;
- bool GetInt64(const std::string& key, int64_t* value) const override;
- bool SetInt64(const std::string& key, const int64_t value) override;
- bool GetBoolean(const std::string& key, bool* value) const override;
- bool SetBoolean(const std::string& key, const bool value) override;
+ bool GetString(std::string_view key, std::string* value) const override;
+ bool SetString(std::string_view key, std::string_view value) override;
+ bool GetInt64(std::string_view key, int64_t* value) const override;
+ bool SetInt64(std::string_view key, const int64_t value) override;
+ bool GetBoolean(std::string_view key, bool* value) const override;
+ bool SetBoolean(std::string_view key, const bool value) override;
- bool Exists(const std::string& key) const override;
- bool Delete(const std::string& key) override;
- bool Delete(const std::string& pref_key,
+ bool Exists(std::string_view key) const override;
+ bool Delete(std::string_view key) override;
+ bool Delete(std::string_view pref_key,
const std::vector<std::string>& nss) override;
- bool GetSubKeys(const std::string& ns,
+ bool GetSubKeys(std::string_view ns,
std::vector<std::string>* keys) const override;
- void AddObserver(const std::string& key,
- ObserverInterface* observer) override;
- void RemoveObserver(const std::string& key,
+ void AddObserver(std::string_view key, ObserverInterface* observer) override;
+ void RemoveObserver(std::string_view key,
ObserverInterface* observer) override;
private:
// The registered observers watching for changes.
- std::map<std::string, std::vector<ObserverInterface*>> observers_;
+ std::map<std::string, std::vector<ObserverInterface*>, std::less<>>
+ observers_;
// The concrete implementation of the storage used for the keys.
StorageInterface* storage_;
@@ -121,12 +122,12 @@
bool Init(const base::FilePath& prefs_dir);
// PrefsBase::StorageInterface overrides.
- bool GetKey(const std::string& key, std::string* value) const override;
- bool GetSubKeys(const std::string& ns,
+ bool GetKey(std::string_view key, std::string* value) const override;
+ bool GetSubKeys(std::string_view ns,
std::vector<std::string>* keys) const override;
- bool SetKey(const std::string& key, std::string_view value) override;
- bool KeyExists(const std::string& key) const override;
- bool DeleteKey(const std::string& key) override;
+ bool SetKey(std::string_view key, std::string_view value) override;
+ bool KeyExists(std::string_view key) const override;
+ bool DeleteKey(std::string_view key) override;
private:
FRIEND_TEST(PrefsTest, GetFileNameForKey);
@@ -135,7 +136,7 @@
// Sets |filename| to the full path to the file containing the data
// associated with |key|. Returns true on success, false otherwise.
- bool GetFileNameForKey(const std::string& key,
+ bool GetFileNameForKey(std::string_view key,
base::FilePath* filename) const;
// Preference store directory.
@@ -161,16 +162,16 @@
MemoryStorage() = default;
// PrefsBase::StorageInterface overrides.
- bool GetKey(const std::string& key, std::string* value) const override;
- bool GetSubKeys(const std::string& ns,
+ bool GetKey(std::string_view, std::string* value) const override;
+ bool GetSubKeys(std::string_view ns,
std::vector<std::string>* keys) const override;
- bool SetKey(const std::string& key, std::string_view value) override;
- bool KeyExists(const std::string& key) const override;
- bool DeleteKey(const std::string& key) override;
+ bool SetKey(std::string_view key, std::string_view value) override;
+ bool KeyExists(std::string_view key) const override;
+ bool DeleteKey(std::string_view key) override;
private:
// The std::map holding the values in memory.
- std::map<std::string, std::string> values_;
+ std::map<std::string, std::string, std::less<>> values_;
};
// The concrete memory storage implementation.
diff --git a/common/prefs_interface.h b/common/prefs_interface.h
index e773a35..69ccf68 100644
--- a/common/prefs_interface.h
+++ b/common/prefs_interface.h
@@ -37,10 +37,10 @@
virtual ~ObserverInterface() = default;
// Called when the value is set for the observed |key|.
- virtual void OnPrefSet(const std::string& key) = 0;
+ virtual void OnPrefSet(std::string_view key) = 0;
// Called when the observed |key| is deleted.
- virtual void OnPrefDeleted(const std::string& key) = 0;
+ virtual void OnPrefDeleted(std::string_view key) = 0;
};
virtual ~PrefsInterface() = default;
@@ -48,61 +48,61 @@
// Gets a string |value| associated with |key|. Returns true on
// success, false on failure (including when the |key| is not
// present in the store).
- virtual bool GetString(const std::string& key, std::string* value) const = 0;
+ virtual bool GetString(std::string_view key, std::string* value) const = 0;
// Associates |key| with a string |value|. Returns true on success,
// false otherwise.
- virtual bool SetString(const std::string& key, std::string_view value) = 0;
+ virtual bool SetString(std::string_view key, std::string_view value) = 0;
// Gets an int64_t |value| associated with |key|. Returns true on
// success, false on failure (including when the |key| is not
// present in the store).
- virtual bool GetInt64(const std::string& key, int64_t* value) const = 0;
+ virtual bool GetInt64(std::string_view key, int64_t* value) const = 0;
// Associates |key| with an int64_t |value|. Returns true on success,
// false otherwise.
- virtual bool SetInt64(const std::string& key, const int64_t value) = 0;
+ virtual bool SetInt64(std::string_view key, const int64_t value) = 0;
// Gets a boolean |value| associated with |key|. Returns true on
// success, false on failure (including when the |key| is not
// present in the store).
- virtual bool GetBoolean(const std::string& key, bool* value) const = 0;
+ virtual bool GetBoolean(std::string_view key, bool* value) const = 0;
// Associates |key| with a boolean |value|. Returns true on success,
// false otherwise.
- virtual bool SetBoolean(const std::string& key, const bool value) = 0;
+ virtual bool SetBoolean(std::string_view key, const bool value) = 0;
// Returns true if the setting exists (i.e. a file with the given key
// exists in the prefs directory)
- virtual bool Exists(const std::string& key) const = 0;
+ virtual bool Exists(std::string_view key) const = 0;
// Returns true if successfully deleted the file corresponding to
// this key. Calling with non-existent keys does nothing.
- virtual bool Delete(const std::string& key) = 0;
+ virtual bool Delete(std::string_view key) = 0;
// Deletes the pref key from platform and given namespace subdirectories.
// Keys are matched against end of pref keys in each namespace.
// Returns true if all deletes were successful.
- virtual bool Delete(const std::string& pref_key,
+ virtual bool Delete(std::string_view pref_key,
const std::vector<std::string>& nss) = 0;
// Creates a key which is part of a sub preference.
static std::string CreateSubKey(const std::vector<std::string>& ns_with_key);
// Returns a list of keys within the namespace.
- virtual bool GetSubKeys(const std::string& ns,
+ virtual bool GetSubKeys(std::string_view ns,
std::vector<std::string>* keys) const = 0;
// Add an observer to watch whenever the given |key| is modified. The
// OnPrefSet() and OnPrefDelete() methods will be called whenever any of the
// Set*() methods or the Delete() method are called on the given key,
// respectively.
- virtual void AddObserver(const std::string& key,
+ virtual void AddObserver(std::string_view key,
ObserverInterface* observer) = 0;
// Remove an observer added with AddObserver(). The observer won't be called
// anymore for future Set*() and Delete() method calls.
- virtual void RemoveObserver(const std::string& key,
+ virtual void RemoveObserver(std::string_view key,
ObserverInterface* observer) = 0;
protected:
diff --git a/common/prefs_unittest.cc b/common/prefs_unittest.cc
index a5f46e5..cef6d44 100644
--- a/common/prefs_unittest.cc
+++ b/common/prefs_unittest.cc
@@ -507,8 +507,8 @@
class MockPrefsObserver : public PrefsInterface::ObserverInterface {
public:
- MOCK_METHOD1(OnPrefSet, void(const string&));
- MOCK_METHOD1(OnPrefDeleted, void(const string& key));
+ MOCK_METHOD1(OnPrefSet, void(std::string_view));
+ MOCK_METHOD1(OnPrefDeleted, void(std::string_view));
};
TEST_F(PrefsTest, ObserversCalled) {
diff --git a/metrics_utils.cc b/metrics_utils.cc
index 34da5a1..ade024a 100644
--- a/metrics_utils.cc
+++ b/metrics_utils.cc
@@ -294,7 +294,7 @@
return metrics::ConnectionType::kUnknown;
}
-int64_t GetPersistedValue(const std::string& key, PrefsInterface* prefs) {
+int64_t GetPersistedValue(std::string_view key, PrefsInterface* prefs) {
CHECK(prefs);
if (!prefs->Exists(key))
return 0;
diff --git a/metrics_utils.h b/metrics_utils.h
index 3aac4e5..16e9eec 100644
--- a/metrics_utils.h
+++ b/metrics_utils.h
@@ -50,7 +50,7 @@
// Returns the persisted value from prefs for the given key. It also
// validates that the value returned is non-negative.
-int64_t GetPersistedValue(const std::string& key, PrefsInterface* prefs);
+int64_t GetPersistedValue(std::string_view key, PrefsInterface* prefs);
// Persists the reboot count of the update attempt to |kPrefsNumReboots|.
void SetNumReboots(int64_t num_reboots, PrefsInterface* prefs);