[Nearby] Use local device data manager in settings mojo implementation

Instead of accessing the device name pref directly in the settings mojo
implementation, use the local device data manager to get and set the
device name in addition to observing for device name changes.

Fixed: b/168138411
Change-Id: I610d653b640a36a5128ff66bb9e3c99b5aa7e538
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2404715
Commit-Queue: Josh Nohle <nohle@chromium.org>
Reviewed-by: James Vecore <vecore@google.com>
Cr-Commit-Position: refs/heads/master@{#806314}

chrome/browser/nearby_sharing/nearby_share_settings.cc

@@ -10,8 +10,11 @@
 #include "components/prefs/pref_service.h"
 #include "components/prefs/scoped_user_pref_update.h"
 
-NearbyShareSettings::NearbyShareSettings(PrefService* pref_service)
-    : pref_service_(pref_service) {
+NearbyShareSettings::NearbyShareSettings(
+    PrefService* pref_service,
+    NearbyShareLocalDeviceDataManager* local_device_data_manager)
+    : pref_service_(pref_service),
+      local_device_data_manager_(local_device_data_manager) {
   pref_change_registrar_.Init(pref_service_);
   pref_change_registrar_.Add(
       prefs::kNearbySharingEnabledPrefName,
@@ -25,32 +28,36 @@ NearbyShareSettings::NearbyShareSettings(PrefService* pref_service)
       prefs::kNearbySharingDataUsageName,
       base::BindRepeating(&NearbyShareSettings::OnDataUsagePrefChanged,
                           base::Unretained(this)));
-  pref_change_registrar_.Add(
-      prefs::kNearbySharingDeviceNamePrefName,
-      base::BindRepeating(&NearbyShareSettings::OnDeviceNamePrefChanged,
-                          base::Unretained(this)));
   pref_change_registrar_.Add(
       prefs::kNearbySharingAllowedContactsPrefName,
       base::BindRepeating(&NearbyShareSettings::OnAllowedContactsPrefChanged,
                           base::Unretained(this)));
+
+  local_device_data_manager_->AddObserver(this);
 }
 
-NearbyShareSettings::~NearbyShareSettings() = default;
+NearbyShareSettings::~NearbyShareSettings() {
+  local_device_data_manager_->RemoveObserver(this);
+}
 
 bool NearbyShareSettings::GetEnabled() const {
   return pref_service_->GetBoolean(prefs::kNearbySharingEnabledPrefName);
 }
+
 std::string NearbyShareSettings::GetDeviceName() const {
-  return pref_service_->GetString(prefs::kNearbySharingDeviceNamePrefName);
+  return local_device_data_manager_->GetDeviceName().value_or(std::string());
 }
+
 DataUsage NearbyShareSettings::GetDataUsage() const {
   return static_cast<DataUsage>(
       pref_service_->GetInteger(prefs::kNearbySharingDataUsageName));
 }
+
 Visibility NearbyShareSettings::GetVisibility() const {
   return static_cast<Visibility>(
       pref_service_->GetInteger(prefs::kNearbySharingBackgroundVisibilityName));
 }
+
 const std::vector<std::string> NearbyShareSettings::GetAllowedContacts() const {
   std::vector<std::string> allowed_contacts;
   const base::ListValue* list =
@@ -84,8 +91,7 @@ void NearbyShareSettings::GetDeviceName(
 }
 
 void NearbyShareSettings::SetDeviceName(const std::string& device_name) {
-  pref_service_->SetString(prefs::kNearbySharingDeviceNamePrefName,
-                           device_name);
+  local_device_data_manager_->SetDeviceName(device_name);
 }
 
 void NearbyShareSettings::GetDataUsage(
@@ -129,17 +135,22 @@ void NearbyShareSettings::Bind(
   receiver_set_.Add(this, std::move(receiver));
 }
 
-void NearbyShareSettings::OnEnabledPrefChanged() {
-  bool enabled = GetEnabled();
+void NearbyShareSettings::OnLocalDeviceDataChanged(bool did_device_name_change,
+                                                   bool did_full_name_change,
+                                                   bool did_icon_url_change) {
+  if (!did_device_name_change)
+    return;
+
+  std::string device_name = GetDeviceName();
   for (auto& remote : observers_set_) {
-    remote->OnEnabledChanged(enabled);
+    remote->OnDeviceNameChanged(device_name);
   }
 }
 
-void NearbyShareSettings::OnDeviceNamePrefChanged() {
-  std::string device_name = GetDeviceName();
+void NearbyShareSettings::OnEnabledPrefChanged() {
+  bool enabled = GetEnabled();
   for (auto& remote : observers_set_) {
-    remote->OnDeviceNameChanged(device_name);
+    remote->OnEnabledChanged(enabled);
   }
 }
 

chrome/browser/nearby_sharing/nearby_share_settings.h

@@ -7,6 +7,7 @@
 
 #include <vector>
 
+#include "chrome/browser/nearby_sharing/local_device_data/nearby_share_local_device_data_manager.h"
 #include "chrome/browser/ui/webui/nearby_share/public/mojom/nearby_share_settings.mojom.h"
 #include "components/prefs/pref_change_registrar.h"
 #include "mojo/public/cpp/bindings/pending_receiver.h"
@@ -34,9 +35,15 @@ class PrefService;
 // will not synchronously trigger the observer event. Generally this is not a
 // problem because these settings should only be changed by user interaction,
 // but this is necessary to know when writing unit-tests.
-class NearbyShareSettings : public nearby_share::mojom::NearbyShareSettings {
+//
+// TODO(nohle): Use the NearbyShareContactManager to implement
+// Get/SetAllowedContacts().
+class NearbyShareSettings : public nearby_share::mojom::NearbyShareSettings,
+                            public NearbyShareLocalDeviceDataManager::Observer {
  public:
-  explicit NearbyShareSettings(PrefService* pref_service_);
+  NearbyShareSettings(
+      PrefService* pref_service_,
+      NearbyShareLocalDeviceDataManager* local_device_data_manager);
   ~NearbyShareSettings() override;
 
   // Synchronous getters for C++ clients, mojo setters can be used as is
@@ -69,9 +76,13 @@ class NearbyShareSettings : public nearby_share::mojom::NearbyShareSettings {
   void Bind(
       mojo::PendingReceiver<nearby_share::mojom::NearbyShareSettings> receiver);
 
+  // NearbyShareLocalDeviceDataManager::Observer:
+  void OnLocalDeviceDataChanged(bool did_device_name_change,
+                                bool did_full_name_change,
+                                bool did_icon_url_change) override;
+
  private:
   void OnEnabledPrefChanged();
-  void OnDeviceNamePrefChanged();
   void OnDataUsagePrefChanged();
   void OnVisibilityPrefChanged();
   void OnAllowedContactsPrefChanged();
@@ -79,7 +90,8 @@ class NearbyShareSettings : public nearby_share::mojom::NearbyShareSettings {
   mojo::RemoteSet<nearby_share::mojom::NearbyShareSettingsObserver>
       observers_set_;
   mojo::ReceiverSet<nearby_share::mojom::NearbyShareSettings> receiver_set_;
-  PrefService* pref_service_;
+  PrefService* pref_service_ = nullptr;
+  NearbyShareLocalDeviceDataManager* local_device_data_manager_ = nullptr;
   PrefChangeRegistrar pref_change_registrar_;
 };
 

chrome/browser/nearby_sharing/nearby_share_settings_unittest.cc

@@ -11,6 +11,7 @@
 #include "chrome/browser/browser_features.h"
 #include "chrome/browser/nearby_sharing/common/nearby_share_enums.h"
 #include "chrome/browser/nearby_sharing/common/nearby_share_prefs.h"
+#include "chrome/browser/nearby_sharing/local_device_data/fake_nearby_share_local_device_data_manager.h"
 #include "chrome/browser/ui/webui/nearby_share/public/mojom/nearby_share_settings.mojom-test-utils.h"
 #include "chrome/browser/ui/webui/nearby_share/public/mojom/nearby_share_settings.mojom.h"
 #include "chrome/test/base/testing_browser_process.h"
@@ -75,8 +76,10 @@ class NearbyShareSettingsTest : public ::testing::Test {
   content::BrowserTaskEnvironment task_environment_;
   base::test::ScopedFeatureList scoped_feature_list_;
   TestingPrefServiceSimple pref_service_;
+  FakeNearbyShareLocalDeviceDataManager local_device_data_manager_;
   FakeNearbyShareSettingsObserver observer_;
-  NearbyShareSettings nearby_share_settings_{&pref_service_};
+  NearbyShareSettings nearby_share_settings_{&pref_service_,
+                                             &local_device_data_manager_};
   NearbyShareSettingsAsyncWaiter nearby_share_settings_waiter_{
       &nearby_share_settings_};
 };
@@ -118,6 +121,12 @@ TEST_F(NearbyShareSettingsTest, GetAndSetDeviceName) {
   EXPECT_EQ("uncalled", observer_.device_name);
   nearby_share_settings_.SetDeviceName("d");
   EXPECT_EQ("d", nearby_share_settings_.GetDeviceName());
+
+  EXPECT_EQ("uncalled", observer_.device_name);
+  local_device_data_manager_.NotifyLocalDeviceDataChanged(
+      /*did_device_name_change=*/true,
+      /*did_full_name_change=*/false,
+      /*did_icon_url_change=*/false);
   FlushMojoMessages();
   EXPECT_EQ("d", observer_.device_name);
 

chrome/browser/nearby_sharing/nearby_sharing_service_impl.cc

@@ -203,7 +203,6 @@ NearbySharingServiceImpl::NearbySharingServiceImpl(
     std::unique_ptr<NearbyConnectionsManager> nearby_connections_manager,
     NearbyProcessManager* process_manager)
     : profile_(profile),
-      settings_(prefs),
       nearby_connections_manager_(std::move(nearby_connections_manager)),
       process_manager_(process_manager),
       http_client_factory_(std::make_unique<NearbyShareClientFactoryImpl>(
@@ -225,7 +224,8 @@ NearbySharingServiceImpl::NearbySharingServiceImpl(
           content::BrowserContext::GetDefaultStoragePartition(profile)
               ->GetProtoDatabaseProvider(),
           profile->GetPath(),
-          http_client_factory_.get())) {
+          http_client_factory_.get())),
+      settings_(prefs, local_device_data_manager_.get()) {
   DCHECK(profile_);
   DCHECK(nearby_connections_manager_);
 

chrome/browser/nearby_sharing/nearby_sharing_service_impl.h

@@ -298,7 +298,6 @@ class NearbySharingServiceImpl
   void SetInHighVisibility(bool in_high_visibility);
 
   Profile* profile_;
-  NearbyShareSettings settings_;
   std::unique_ptr<NearbyConnectionsManager> nearby_connections_manager_;
   NearbyProcessManager* process_manager_;
   ScopedObserver<NearbyProcessManager, NearbyProcessManager::Observer>
@@ -311,6 +310,7 @@ class NearbySharingServiceImpl
   std::unique_ptr<NearbyShareLocalDeviceDataManager> local_device_data_manager_;
   std::unique_ptr<NearbyShareContactManager> contact_manager_;
   std::unique_ptr<NearbyShareCertificateManager> certificate_manager_;
+  NearbyShareSettings settings_;
   NearbyFileHandler file_handler_;
 
   // A list of service observers.

chrome/browser/nearby_sharing/nearby_sharing_service_impl_unittest.cc

@@ -413,7 +413,7 @@ class NearbySharingServiceImplTest : public testing::Test {
   }
 
   void SetVisibility(nearby_share::mojom::Visibility visibility) {
-    NearbyShareSettings settings(&prefs_);
+    NearbyShareSettings settings(&prefs_, local_device_data_manager());
     settings.SetVisibility(visibility);
   }