0

Use std::string_view in PrefChangeRegistrar and OverlayUserPrefStore.

Bug: 349741884
Change-Id: I19a45d2f45f8e4d8447c6b86665ac2a2084302bd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5742334
Auto-Submit: Jan Keitel <jkeitel@google.com>
Commit-Queue: Jan Keitel <jkeitel@google.com>
Reviewed-by: Dominic Battré <battre@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1333436}
This commit is contained in:
Jan Keitel
2024-07-26 09:00:14 +00:00
committed by Chromium LUCI CQ
parent 32ee423e67
commit 6288ad6b75
7 changed files with 34 additions and 44 deletions

@ -55,7 +55,7 @@ OverlayUserPrefStore::OverlayUserPrefStore(PersistentPrefStore* ephemeral,
persistent_pref_store_observer_.get()); persistent_pref_store_observer_.get());
} }
bool OverlayUserPrefStore::IsSetInOverlay(const std::string& key) const { bool OverlayUserPrefStore::IsSetInOverlay(std::string_view key) const {
return ephemeral_user_pref_store_->GetValue(key, nullptr); return ephemeral_user_pref_store_->GetValue(key, nullptr);
} }
@ -204,11 +204,11 @@ void OverlayUserPrefStore::ReportValueChanged(std::string_view key,
observer.OnPrefValueChanged(key); observer.OnPrefValueChanged(key);
} }
void OverlayUserPrefStore::RegisterPersistentPref(const std::string& key) { void OverlayUserPrefStore::RegisterPersistentPref(std::string_view key) {
DCHECK(!key.empty()) << "Key is empty"; DCHECK(!key.empty()) << "Key is empty";
DCHECK(persistent_names_set_.find(key) == persistent_names_set_.end()) DCHECK(persistent_names_set_.find(key) == persistent_names_set_.end())
<< "Key already registered: " << key; << "Key already registered: " << key;
persistent_names_set_.insert(key); persistent_names_set_.insert(std::string(key));
} }
void OverlayUserPrefStore::OnStoreDeletionFromDisk() { void OverlayUserPrefStore::OnStoreDeletionFromDisk() {

@ -8,7 +8,6 @@
#include <stdint.h> #include <stdint.h>
#include <map> #include <map>
#include <string>
#include <string_view> #include <string_view>
#include "base/memory/ref_counted.h" #include "base/memory/ref_counted.h"
@ -38,7 +37,7 @@ class COMPONENTS_PREFS_EXPORT OverlayUserPrefStore
// Returns true if a value has been set for the |key| in this // Returns true if a value has been set for the |key| in this
// OverlayUserPrefStore, i.e. if it potentially overrides a value // OverlayUserPrefStore, i.e. if it potentially overrides a value
// from the |persistent_user_pref_store_|. // from the |persistent_user_pref_store_|.
virtual bool IsSetInOverlay(const std::string& key) const; virtual bool IsSetInOverlay(std::string_view key) const;
// Methods of PrefStore. // Methods of PrefStore.
void AddObserver(PrefStore::Observer* observer) override; void AddObserver(PrefStore::Observer* observer) override;
@ -70,7 +69,7 @@ class COMPONENTS_PREFS_EXPORT OverlayUserPrefStore
// Registers preferences that should be stored in the persistent preferences // Registers preferences that should be stored in the persistent preferences
// (|persistent_user_pref_store_|). // (|persistent_user_pref_store_|).
void RegisterPersistentPref(const std::string& key); void RegisterPersistentPref(std::string_view key);
void OnStoreDeletionFromDisk() override; void OnStoreDeletionFromDisk() override;
bool HasReadErrorDelegate() const override; bool HasReadErrorDelegate() const override;

@ -35,13 +35,13 @@ void PrefChangeRegistrar::Reset() {
service_ = nullptr; service_ = nullptr;
} }
void PrefChangeRegistrar::Add(const std::string& path, void PrefChangeRegistrar::Add(std::string_view path,
const base::RepeatingClosure& obs) { const base::RepeatingClosure& obs) {
Add(path, Add(path,
base::BindRepeating(&PrefChangeRegistrar::InvokeUnnamedCallback, obs)); base::BindRepeating(&PrefChangeRegistrar::InvokeUnnamedCallback, obs));
} }
void PrefChangeRegistrar::Add(const std::string& path, void PrefChangeRegistrar::Add(std::string_view path,
const NamedChangeCallback& obs) { const NamedChangeCallback& obs) {
if (!service_) { if (!service_) {
NOTREACHED_IN_MIGRATION(); NOTREACHED_IN_MIGRATION();
@ -51,13 +51,15 @@ void PrefChangeRegistrar::Add(const std::string& path,
<< "\", registered."; << "\", registered.";
service_->AddPrefObserver(path, this); service_->AddPrefObserver(path, this);
observers_[path] = obs; observers_.insert_or_assign(std::string(path), obs);
} }
void PrefChangeRegistrar::Remove(const std::string& path) { void PrefChangeRegistrar::Remove(std::string_view path) {
DCHECK(IsObserved(path)); DCHECK(IsObserved(path));
observers_.erase(path); // Use std::map::erase directly once C++23 is supported.
auto it = observers_.find(path);
observers_.erase(it);
service_->RemovePrefObserver(path, this); service_->RemovePrefObserver(path, this);
} }

@ -49,11 +49,11 @@ class COMPONENTS_PREFS_EXPORT PrefChangeRegistrar final : public PrefObserver {
// the preference that is changing as its parameter. // the preference that is changing as its parameter.
// //
// Only one observer may be registered per path. // Only one observer may be registered per path.
void Add(const std::string& path, const base::RepeatingClosure& obs); void Add(std::string_view path, const base::RepeatingClosure& obs);
void Add(const std::string& path, const NamedChangeCallback& obs); void Add(std::string_view path, const NamedChangeCallback& obs);
// Removes the pref observer registered for |path|. // Removes the pref observer registered for |path|.
void Remove(const std::string& path); void Remove(std::string_view path);
// Removes all observers that have been previously added with a call to Add. // Removes all observers that have been previously added with a call to Add.
void RemoveAll(); void RemoveAll();

@ -5,6 +5,7 @@
#include "components/prefs/pref_change_registrar.h" #include "components/prefs/pref_change_registrar.h"
#include <memory> #include <memory>
#include <string_view>
#include "base/functional/bind.h" #include "base/functional/bind.h"
#include "base/functional/callback_helpers.h" #include "base/functional/callback_helpers.h"
@ -14,12 +15,11 @@
#include "testing/gmock/include/gmock/gmock.h" #include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
using testing::Mock;
using testing::Eq;
namespace base { namespace base {
namespace { namespace {
using testing::Mock;
const char kHomePage[] = "homepage"; const char kHomePage[] = "homepage";
const char kHomePageIsNewTabPage[] = "homepage_is_newtabpage"; const char kHomePageIsNewTabPage[] = "homepage_is_newtabpage";
const char kApplicationLocale[] = "intl.app_locale"; const char kApplicationLocale[] = "intl.app_locale";
@ -32,11 +32,11 @@ class MockPrefService : public TestingPrefServiceSimple {
MOCK_METHOD(void, MOCK_METHOD(void,
AddPrefObserver, AddPrefObserver,
(const std::string&, PrefObserver*), (std::string_view, PrefObserver*),
(override)); (override));
MOCK_METHOD(void, MOCK_METHOD(void,
RemovePrefObserver, RemovePrefObserver,
(const std::string&, PrefObserver*), (std::string_view, PrefObserver*),
(override)); (override));
}; };
@ -71,20 +71,16 @@ TEST_F(PrefChangeRegistrarTest, AddAndRemove) {
registrar.Init(service()); registrar.Init(service());
// Test adding. // Test adding.
EXPECT_CALL(*service(), EXPECT_CALL(*service(), AddPrefObserver("test.pref.1", &registrar));
AddPrefObserver(Eq(std::string("test.pref.1")), &registrar)); EXPECT_CALL(*service(), AddPrefObserver("test.pref.2", &registrar));
EXPECT_CALL(*service(),
AddPrefObserver(Eq(std::string("test.pref.2")), &registrar));
registrar.Add("test.pref.1", DoNothingClosure()); registrar.Add("test.pref.1", DoNothingClosure());
registrar.Add("test.pref.2", DoNothingClosure()); registrar.Add("test.pref.2", DoNothingClosure());
EXPECT_FALSE(registrar.IsEmpty()); EXPECT_FALSE(registrar.IsEmpty());
// Test removing. // Test removing.
Mock::VerifyAndClearExpectations(service()); Mock::VerifyAndClearExpectations(service());
EXPECT_CALL(*service(), EXPECT_CALL(*service(), RemovePrefObserver("test.pref.1", &registrar));
RemovePrefObserver(Eq(std::string("test.pref.1")), &registrar)); EXPECT_CALL(*service(), RemovePrefObserver("test.pref.2", &registrar));
EXPECT_CALL(*service(),
RemovePrefObserver(Eq(std::string("test.pref.2")), &registrar));
registrar.Remove("test.pref.1"); registrar.Remove("test.pref.1");
registrar.Remove("test.pref.2"); registrar.Remove("test.pref.2");
EXPECT_TRUE(registrar.IsEmpty()); EXPECT_TRUE(registrar.IsEmpty());
@ -99,33 +95,27 @@ TEST_F(PrefChangeRegistrarTest, AutoRemove) {
registrar.Init(service()); registrar.Init(service());
// Setup of auto-remove. // Setup of auto-remove.
EXPECT_CALL(*service(), EXPECT_CALL(*service(), AddPrefObserver("test.pref.1", &registrar));
AddPrefObserver(Eq(std::string("test.pref.1")), &registrar));
registrar.Add("test.pref.1", DoNothingClosure()); registrar.Add("test.pref.1", DoNothingClosure());
Mock::VerifyAndClearExpectations(service()); Mock::VerifyAndClearExpectations(service());
EXPECT_FALSE(registrar.IsEmpty()); EXPECT_FALSE(registrar.IsEmpty());
// Test auto-removing. // Test auto-removing.
EXPECT_CALL(*service(), EXPECT_CALL(*service(), RemovePrefObserver("test.pref.1", &registrar));
RemovePrefObserver(Eq(std::string("test.pref.1")), &registrar));
} }
TEST_F(PrefChangeRegistrarTest, RemoveAll) { TEST_F(PrefChangeRegistrarTest, RemoveAll) {
PrefChangeRegistrar registrar; PrefChangeRegistrar registrar;
registrar.Init(service()); registrar.Init(service());
EXPECT_CALL(*service(), EXPECT_CALL(*service(), AddPrefObserver("test.pref.1", &registrar));
AddPrefObserver(Eq(std::string("test.pref.1")), &registrar)); EXPECT_CALL(*service(), AddPrefObserver("test.pref.2", &registrar));
EXPECT_CALL(*service(),
AddPrefObserver(Eq(std::string("test.pref.2")), &registrar));
registrar.Add("test.pref.1", DoNothingClosure()); registrar.Add("test.pref.1", DoNothingClosure());
registrar.Add("test.pref.2", DoNothingClosure()); registrar.Add("test.pref.2", DoNothingClosure());
Mock::VerifyAndClearExpectations(service()); Mock::VerifyAndClearExpectations(service());
EXPECT_CALL(*service(), EXPECT_CALL(*service(), RemovePrefObserver("test.pref.1", &registrar));
RemovePrefObserver(Eq(std::string("test.pref.1")), &registrar)); EXPECT_CALL(*service(), RemovePrefObserver("test.pref.2", &registrar));
EXPECT_CALL(*service(),
RemovePrefObserver(Eq(std::string("test.pref.2")), &registrar));
registrar.RemoveAll(); registrar.RemoveAll();
EXPECT_TRUE(registrar.IsEmpty()); EXPECT_TRUE(registrar.IsEmpty());

@ -399,12 +399,11 @@ const base::Value* PrefService::GetDefaultPrefValue(
return value; return value;
} }
void PrefService::AddPrefObserver(const std::string& path, PrefObserver* obs) { void PrefService::AddPrefObserver(std::string_view path, PrefObserver* obs) {
pref_notifier_->AddPrefObserver(path, obs); pref_notifier_->AddPrefObserver(path, obs);
} }
void PrefService::RemovePrefObserver(const std::string& path, void PrefService::RemovePrefObserver(std::string_view path, PrefObserver* obs) {
PrefObserver* obs) {
pref_notifier_->RemovePrefObserver(path, obs); pref_notifier_->RemovePrefObserver(path, obs);
} }

@ -477,8 +477,8 @@ class COMPONENTS_PREFS_EXPORT PrefService {
// make sure the observer gets cleaned up properly. // make sure the observer gets cleaned up properly.
// //
// Virtual for testing. // Virtual for testing.
virtual void AddPrefObserver(const std::string& path, PrefObserver* obs); virtual void AddPrefObserver(std::string_view path, PrefObserver* obs);
virtual void RemovePrefObserver(const std::string& path, PrefObserver* obs); virtual void RemovePrefObserver(std::string_view path, PrefObserver* obs);
// A PrefStore::Observer which reports loading errors from // A PrefStore::Observer which reports loading errors from
// PersistentPrefStores after they are loaded. Usually this is only user_prefs // PersistentPrefStores after they are loaded. Usually this is only user_prefs