Use std::string_view in PrefStore::Observer.

Bug: 349741884
Change-Id: Ifa66cb4a47c17864247eee8006fcaba33a7d90f8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5666575
Reviewed-by: Dominic Battré <battre@chromium.org>
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Code-Coverage: findit-for-me@appspot.gserviceaccount.com <findit-for-me@appspot.gserviceaccount.com>
Auto-Submit: Jan Keitel <jkeitel@google.com>
Commit-Queue: Sylvain Defresne <sdefresne@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1320933}

chrome/browser/prefs/profile_pref_store_manager_unittest.cc

@@ -58,7 +58,7 @@ class RegistryVerifier : public PrefStore::Observer {
       : pref_registry_(pref_registry) {}
 
   // PrefStore::Observer implementation
-  void OnPrefValueChanged(const std::string& key) override {
+  void OnPrefValueChanged(std::string_view key) override {
     EXPECT_TRUE(base::Contains(*pref_registry_, key,
                                &PrefValueMap::Map::value_type::first))
         << "Unregistered key " << key << " was changed.";
@@ -91,8 +91,6 @@ class PrefStoreReadObserver : public PrefStore::Observer {
   }
 
   // PrefStore::Observer implementation
-  void OnPrefValueChanged(const std::string& key) override {}
-
   void OnInitializationCompleted(bool succeeded) override {
     if (stop_waiting_) {
       std::move(stop_waiting_).Run();

components/prefs/default_pref_store_unittest.cc

@@ -24,8 +24,7 @@ class MockPrefStoreObserver : public PrefStore::Observer {
   }
 
   // PrefStore::Observer implementation:
-  void OnPrefValueChanged(const std::string& key) override;
-  void OnInitializationCompleted(bool succeeded) override {}
+  void OnPrefValueChanged(std::string_view key) override;
 
  private:
   raw_ptr<DefaultPrefStore> pref_store_;
@@ -42,7 +41,7 @@ MockPrefStoreObserver::~MockPrefStoreObserver() {
   pref_store_->RemoveObserver(this);
 }
 
-void MockPrefStoreObserver::OnPrefValueChanged(const std::string& key) {
+void MockPrefStoreObserver::OnPrefValueChanged(std::string_view key) {
   change_count_++;
 }
 

components/prefs/json_pref_store_unittest.cc

@@ -118,13 +118,12 @@ void InterceptingPrefFilter::ReleasePrefs() {
 
 class MockPrefStoreObserver : public PrefStore::Observer {
  public:
-  MOCK_METHOD1(OnPrefValueChanged, void (const std::string&));
-  MOCK_METHOD1(OnInitializationCompleted, void (bool));
+  MOCK_METHOD(void, OnInitializationCompleted, (bool), (override));
 };
 
 class MockReadErrorDelegate : public PersistentPrefStore::ReadErrorDelegate {
  public:
-  MOCK_METHOD1(OnError, void(PersistentPrefStore::PrefReadError));
+  MOCK_METHOD(void, OnError, (PersistentPrefStore::PrefReadError), (override));
 };
 
 enum class CommitPendingWriteMode {

components/prefs/overlay_user_pref_store.cc

@@ -25,8 +25,10 @@ class OverlayUserPrefStore::ObserverAdapter : public PrefStore::Observer {
       : ephemeral_user_pref_store_(ephemeral), parent_(parent) {}
 
   // Methods of PrefStore::Observer.
-  void OnPrefValueChanged(const std::string& key) override {
-    parent_->OnPrefValueChanged(ephemeral_user_pref_store_, key);
+  void OnPrefValueChanged(std::string_view key) override {
+    // TODO: crbug.com/349741884 - Pass `std::string_view` once the interface is
+    // changed.
+    parent_->OnPrefValueChanged(ephemeral_user_pref_store_, std::string(key));
   }
   void OnInitializationCompleted(bool succeeded) override {
     parent_->OnInitializationCompleted(ephemeral_user_pref_store_, succeeded);

components/prefs/pref_service.h

@@ -492,7 +492,6 @@ class COMPONENTS_PREFS_EXPORT PrefService {
     explicit PersistentPrefStoreLoadingObserver(PrefService* pref_service_);
 
     // PrefStore::Observer implementation
-    void OnPrefValueChanged(const std::string& key) override {}
     void OnInitializationCompleted(bool succeeded) override;
 
    private:

components/prefs/pref_store.h

@@ -6,7 +6,6 @@
 #define COMPONENTS_PREFS_PREF_STORE_H_
 
 #include <memory>
-#include <string>
 #include <string_view>
 
 #include "base/memory/ref_counted.h"
@@ -25,8 +24,8 @@ class COMPONENTS_PREFS_EXPORT PrefStore : public base::RefCounted<PrefStore> {
   // Observer interface for monitoring PrefStore.
   class COMPONENTS_PREFS_EXPORT Observer {
    public:
-    // Called when the value for the given |key| in the store changes.
-    virtual void OnPrefValueChanged(const std::string& key) {}
+    // Called when the value for the given `key` in the store changes.
+    virtual void OnPrefValueChanged(std::string_view key) {}
     // Notification about the PrefStore being fully initialized.
     virtual void OnInitializationCompleted(bool succeeded) {}
 
@@ -47,9 +46,9 @@ class COMPONENTS_PREFS_EXPORT PrefStore : public base::RefCounted<PrefStore> {
   // Whether the store has completed all asynchronous initialization.
   virtual bool IsInitializationComplete() const;
 
-  // Get the value for a given preference |key| and stores it in |*result|.
-  // |*result| is only modified if the return value is true and if |result|
-  // is not NULL. Ownership of the |*result| value remains with the PrefStore.
+  // Get the value for a given preference `key` and stores it in `*result`.
+  // `*result` is only modified if the return value is true and if `result`
+  // is not NULL. Ownership of the `*result` value remains with the PrefStore.
   virtual bool GetValue(std::string_view key,
                         const base::Value** result) const = 0;
 

components/prefs/pref_store_observer_mock.cc

@@ -4,6 +4,8 @@
 
 #include "components/prefs/pref_store_observer_mock.h"
 
+#include <string_view>
+
 #include "testing/gtest/include/gtest/gtest.h"
 
 PrefStoreObserverMock::PrefStoreObserverMock()
@@ -19,8 +21,8 @@ void PrefStoreObserverMock::VerifyAndResetChangedKey(
   changed_keys.clear();
 }
 
-void PrefStoreObserverMock::OnPrefValueChanged(const std::string& key) {
-  changed_keys.push_back(key);
+void PrefStoreObserverMock::OnPrefValueChanged(std::string_view key) {
+  changed_keys.emplace_back(key);
 }
 
 void PrefStoreObserverMock::OnInitializationCompleted(bool success) {

components/prefs/pref_store_observer_mock.h

@@ -5,7 +5,7 @@
 #ifndef COMPONENTS_PREFS_PREF_STORE_OBSERVER_MOCK_H_
 #define COMPONENTS_PREFS_PREF_STORE_OBSERVER_MOCK_H_
 
-#include <string>
+#include <string_view>
 #include <vector>
 
 #include "base/compiler_specific.h"
@@ -24,7 +24,7 @@ class PrefStoreObserverMock : public PrefStore::Observer {
   void VerifyAndResetChangedKey(const std::string& expected);
 
   // PrefStore::Observer implementation
-  void OnPrefValueChanged(const std::string& key) override;
+  void OnPrefValueChanged(std::string_view key) override;
   void OnInitializationCompleted(bool success) override;
 
   std::vector<std::string> changed_keys;

components/prefs/pref_value_store.cc

@@ -39,9 +39,10 @@ void PrefValueStore::PrefStoreKeeper::Initialize(
     pref_store_->AddObserver(this);
 }
 
-void PrefValueStore::PrefStoreKeeper::OnPrefValueChanged(
-    const std::string& key) {
-  pref_value_store_->OnPrefValueChanged(type_, key);
+void PrefValueStore::PrefStoreKeeper::OnPrefValueChanged(std::string_view key) {
+  // TODO: crbug.com/349741884 - Pass `std::string_view` once the interface is
+  // changed.
+  pref_value_store_->OnPrefValueChanged(type_, std::string(key));
 }
 
 void PrefValueStore::PrefStoreKeeper::OnInitializationCompleted(

components/prefs/pref_value_store.h

@@ -192,7 +192,7 @@ class COMPONENTS_PREFS_EXPORT PrefValueStore {
 
    private:
     // PrefStore::Observer implementation.
-    void OnPrefValueChanged(const std::string& key) override;
+    void OnPrefValueChanged(std::string_view key) override;
     void OnInitializationCompleted(bool succeeded) override;
 
     // PrefValueStore this keeper is part of.

components/prefs/pref_value_store_unittest.cc

@@ -23,8 +23,8 @@ namespace {
 // Allows to capture pref notifications through gmock.
 class MockPrefNotifier : public PrefNotifier {
  public:
-  MOCK_METHOD1(OnPreferenceChanged, void(const std::string&));
-  MOCK_METHOD1(OnInitializationCompleted, void(bool));
+  MOCK_METHOD(void, OnPreferenceChanged, (const std::string&), (override));
+  MOCK_METHOD(void, OnInitializationCompleted, (bool), (override));
 };
 
 }  // namespace

components/prefs/segregated_pref_store.cc

@@ -23,7 +23,7 @@ SegregatedPrefStore::UnderlyingPrefStoreObserver::UnderlyingPrefStoreObserver(
 }
 
 void SegregatedPrefStore::UnderlyingPrefStoreObserver::OnPrefValueChanged(
-    const std::string& key) {
+    std::string_view key) {
   // Notify Observers only after all underlying PrefStores of the outer
   // SegregatedPrefStore are initialized.
   if (!outer_->IsInitializationComplete())

components/prefs/segregated_pref_store.h

@@ -100,7 +100,7 @@ class COMPONENTS_PREFS_EXPORT SegregatedPrefStore : public PersistentPrefStore {
         delete;
 
     // PrefStore::Observer implementation
-    void OnPrefValueChanged(const std::string& key) override;
+    void OnPrefValueChanged(std::string_view key) override;
     void OnInitializationCompleted(bool succeeded) override;
 
     bool initialization_succeeded() const { return initialization_succeeded_; }

components/prefs/testing_pref_store.cc

@@ -51,7 +51,7 @@ class ChangedValueWaiter : public PrefStore::Observer {
     QuitRunLoopIfNewValueIsPresent();
   }
 
-  void OnPrefValueChanged(const std::string& key) override {
+  void OnPrefValueChanged(std::string_view key) override {
     if (key == key_) {
       QuitRunLoopIfNewValueIsPresent();
     }

components/prefs/wrap_with_prefix_pref_store.cc

@@ -132,11 +132,11 @@ void WrapWithPrefixPrefStore::ReportValueChanged(const std::string& key,
   return target_pref_store_->ReportValueChanged(AddDottedPrefix(key), flags);
 }
 
-void WrapWithPrefixPrefStore::OnPrefValueChanged(const std::string& key) {
+void WrapWithPrefixPrefStore::OnPrefValueChanged(std::string_view key) {
   if (!HasDottedPrefix(key)) {
     return;
   }
-  std::string original_key(RemoveDottedPrefix(key));
+  std::string_view original_key(RemoveDottedPrefix(key));
   for (PrefStore::Observer& observer : observers_) {
     observer.OnPrefValueChanged(original_key);
   }

components/prefs/wrap_with_prefix_pref_store.h

@@ -72,7 +72,7 @@ class COMPONENTS_PREFS_EXPORT WrapWithPrefixPrefStore
   bool HasReadErrorDelegate() const override;
 
   // PrefStore::Observer implementation.
-  void OnPrefValueChanged(const std::string& key) override;
+  void OnPrefValueChanged(std::string_view key) override;
   void OnInitializationCompleted(bool succeeded) override;
 
  protected:

components/prefs/wrap_with_prefix_pref_store_unittest.cc

@@ -58,7 +58,7 @@ class MockPrefStoreObserver : public PrefStore::Observer {
  public:
   ~MockPrefStoreObserver() override = default;
 
-  MOCK_METHOD(void, OnPrefValueChanged, (const std::string& key), (override));
+  MOCK_METHOD(void, OnPrefValueChanged, (std::string_view key), (override));
   MOCK_METHOD(void, OnInitializationCompleted, (bool succeeded), (override));
 };
 

components/supervised_user/core/browser/supervised_user_pref_store_unittest.cc

@@ -35,7 +35,7 @@ class SupervisedUserPrefStoreFixture : public PrefStore::Observer {
   bool initialization_completed() const { return initialization_completed_; }
 
   // PrefStore::Observer implementation:
-  void OnPrefValueChanged(const std::string& key) override;
+  void OnPrefValueChanged(std::string_view key) override;
   void OnInitializationCompleted(bool succeeded) override;
 
  private:
@@ -55,8 +55,7 @@ SupervisedUserPrefStoreFixture::~SupervisedUserPrefStoreFixture() {
   pref_store_->RemoveObserver(this);
 }
 
-void SupervisedUserPrefStoreFixture::OnPrefValueChanged(
-    const std::string& key) {
+void SupervisedUserPrefStoreFixture::OnPrefValueChanged(std::string_view key) {
   const base::Value* value = nullptr;
   ASSERT_TRUE(pref_store_->GetValue(key, &value));
   changed_prefs_.SetByDottedPath(key, value->Clone());

components/supervised_user/core/browser/supervised_user_settings_service.cc

@@ -459,9 +459,6 @@ SupervisedUserSettingsService::AsWeakPtr() {
   return weak_ptr_factory_.GetWeakPtr();
 }
 
-void SupervisedUserSettingsService::OnPrefValueChanged(const std::string& key) {
-}
-
 void SupervisedUserSettingsService::OnInitializationCompleted(bool success) {
   if (!success) {
     // If this happens, it means the profile directory was not found. There is

components/supervised_user/core/browser/supervised_user_settings_service.h

@@ -174,7 +174,6 @@ class SupervisedUserSettingsService : public KeyedService,
   base::WeakPtr<SyncableService> AsWeakPtr() override;
 
   // PrefStore::Observer implementation:
-  void OnPrefValueChanged(const std::string& key) override;
   void OnInitializationCompleted(bool success) override;
 
   bool IsCustomPassphraseAllowed() const;

components/sync_preferences/dual_layer_user_pref_store.cc

@@ -32,7 +32,7 @@ DualLayerUserPrefStore::UnderlyingPrefStoreObserver::
 }
 
 void DualLayerUserPrefStore::UnderlyingPrefStoreObserver::OnPrefValueChanged(
-    const std::string& key) {
+    std::string_view key) {
   // Ignore this notification if it originated from the outer store - in that
   // case, `DualLayerUserPrefStore` itself will send notifications as
   // appropriate. This avoids dual notifications even though there are dual
@@ -550,7 +550,7 @@ void DualLayerUserPrefStore::DisableTypeAndClearAccountStore(
   }
 }
 
-bool DualLayerUserPrefStore::IsPrefKeyMergeable(const std::string& key) const {
+bool DualLayerUserPrefStore::IsPrefKeyMergeable(std::string_view key) const {
   if (!pref_model_associator_client_) {
     return false;
   }

components/sync_preferences/dual_layer_user_pref_store.h

@@ -133,7 +133,7 @@ class DualLayerUserPrefStore : public PersistentPrefStore,
         delete;
 
     // PrefStore::Observer implementation.
-    void OnPrefValueChanged(const std::string& key) override;
+    void OnPrefValueChanged(std::string_view key) override;
     void OnInitializationCompleted(bool succeeded) override;
 
     bool initialization_succeeded() const;
@@ -158,7 +158,7 @@ class DualLayerUserPrefStore : public PersistentPrefStore,
   bool ShouldGetValueFromAccountStore(const std::string& key) const;
 
   // Returns whether the pref with the given `key` is mergeable.
-  bool IsPrefKeyMergeable(const std::string& key) const;
+  bool IsPrefKeyMergeable(std::string_view key) const;
 
   // Produces a "merged" view of `account_value` and `local_value`. In case
   // `pref_name` is a mergeable pref, a new merged pref is returned, which is

components/sync_preferences/dual_layer_user_pref_store_unittest.cc

@@ -6,6 +6,7 @@
 
 #include <map>
 #include <set>
+#include <string_view>
 
 #include "base/memory/scoped_refptr.h"
 #include "base/test/mock_callback.h"
@@ -123,7 +124,7 @@ class MockPrefStoreObserver : public PrefStore::Observer {
  public:
   ~MockPrefStoreObserver() override = default;
 
-  MOCK_METHOD(void, OnPrefValueChanged, (const std::string& key), (override));
+  MOCK_METHOD(void, OnPrefValueChanged, (std::string_view), (override));
   MOCK_METHOD(void, OnInitializationCompleted, (bool succeeded), (override));
 };
 

components/sync_preferences/pref_model_associator.cc

@@ -413,7 +413,7 @@ bool PrefModelAssociator::IsPrefRegistered(std::string_view name) const {
   return registered_preferences_.contains(name);
 }
 
-void PrefModelAssociator::OnPrefValueChanged(const std::string& name) {
+void PrefModelAssociator::OnPrefValueChanged(std::string_view name) {
   if (processing_syncer_changes_) {
     return;  // These are changes originating from us, ignore.
   }
@@ -433,15 +433,18 @@ void PrefModelAssociator::OnPrefValueChanged(const std::string& name) {
 
   base::AutoReset<bool> processing_changes(&processing_syncer_changes_, true);
 
-  NotifySyncedPrefObservers(name, /*from_sync=*/false);
+  // TODO: crbug.com/349741884: Eliminate this copy once the underlying methods
+  // accept std::string_view.
+  std::string name_str = std::string(name);
+  NotifySyncedPrefObservers(name_str, /*from_sync=*/false);
 
   syncer::SyncChangeList changes;
 
-  if (synced_preferences_.count(name) == 0) {
+  if (!synced_preferences_.contains(name)) {
     // Not in synced_preferences_ means no synced data. InitPrefAndAssociate(..)
     // will determine if we care about its data (e.g. if it has a default value
     // and hasn't been changed yet we don't) and take care syncing any new data.
-    InitPrefAndAssociate(syncer::SyncData(), name, &changes);
+    InitPrefAndAssociate(syncer::SyncData(), name_str, &changes);
   } else {
     // We are already syncing this preference, just update or delete its sync
     // node.
@@ -450,7 +453,7 @@ void PrefModelAssociator::OnPrefValueChanged(const std::string& name) {
     if (user_pref_value) {
       // If the pref was updated, update it.
       syncer::SyncData sync_data;
-      if (!CreatePrefSyncData(name, *user_pref_value, &sync_data)) {
+      if (!CreatePrefSyncData(name_str, *user_pref_value, &sync_data)) {
         LOG(ERROR) << "Failed to update preference.";
         return;
       }
@@ -458,8 +461,9 @@ void PrefModelAssociator::OnPrefValueChanged(const std::string& name) {
                            sync_data);
     } else {
       // Otherwise, the pref must have been cleared and hence delete it.
-      changes.emplace_back(FROM_HERE, syncer::SyncChange::ACTION_DELETE,
-                           syncer::SyncData::CreateLocalDelete(name, type_));
+      changes.emplace_back(
+          FROM_HERE, syncer::SyncChange::ACTION_DELETE,
+          syncer::SyncData::CreateLocalDelete(name_str, type_));
     }
   }
 

components/sync_preferences/pref_model_associator.h

@@ -97,7 +97,7 @@ class PrefModelAssociator final : public syncer::SyncableService,
   base::WeakPtr<SyncableService> AsWeakPtr() override;
 
   // PrefStore::Observer implementation.
-  void OnPrefValueChanged(const std::string& key) override;
+  void OnPrefValueChanged(std::string_view name) override;
   void OnInitializationCompleted(bool succeeded) override;
 
   // Register a preference with the specified name for syncing. We do not care
@@ -195,7 +195,7 @@ class PrefModelAssociator final : public syncer::SyncableService,
   // to eventually match `registered_preferences_` as more preferences are
   // synced. It determines whether a preference change should update an existing
   // sync node or create a new sync node.
-  std::set<std::string> synced_preferences_;
+  std::set<std::string, std::less<>> synced_preferences_;
 
   // Sync's handler for outgoing changes. Non-null between
   // MergeDataAndStartSyncing() and StopSyncing().

ios/chrome/browser/net/model/http_server_properties_factory.cc

@@ -63,7 +63,6 @@ class PrefServiceAdapter : public net::HttpServerProperties::PrefDelegate,
   }
 
   // PrefStore::Observer implementation.
-  void OnPrefValueChanged(const std::string& key) override {}
   void OnInitializationCompleted(bool succeeded) override {
     if (on_pref_load_callback_) {
       pref_store_->RemoveObserver(this);

third_party/libaddressinput/chromium/chrome_storage_impl.cc

@@ -32,8 +32,6 @@ void ChromeStorageImpl::Get(const std::string& key,
   const_cast<ChromeStorageImpl*>(this)->DoGet(key, data_ready);
 }
 
-void ChromeStorageImpl::OnPrefValueChanged(const std::string& key) {}
-
 void ChromeStorageImpl::OnInitializationCompleted(bool succeeded) {
   for (const auto& request : outstanding_requests_)
     DoGet(request->key, request->callback);

third_party/libaddressinput/chromium/chrome_storage_impl.h

@@ -31,13 +31,11 @@ class ChromeStorageImpl : public ::i18n::addressinput::Storage,
   virtual ~ChromeStorageImpl();
 
   // ::i18n::addressinput::Storage implementation.
-  virtual void Put(const std::string& key, std::string* data) override;
-  virtual void Get(const std::string& key, const Callback& data_ready)
-      const override;
+  void Put(const std::string& key, std::string* data) override;
+  void Get(const std::string& key, const Callback& data_ready) const override;
 
   // PrefStore::Observer implementation.
-  virtual void OnPrefValueChanged(const std::string& key) override;
-  virtual void OnInitializationCompleted(bool succeeded) override;
+  void OnInitializationCompleted(bool succeeded) override;
 
  private:
   struct Request {