Update sync when networks are added/updated/deleted locally.

This change refactors LocalNetworkCollector::GetSyncableNetwork to
accept a network's GUID instead of NetworkIdentifier in order to keep
logic related to local network lookups encapsulated in
LocalNetworkCollector.

Bug: 966270
Change-Id: I36946e0fbb1cfc85e972cf5169f1bf4555080f5c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2107062
Commit-Queue: Jon Mann <jonmann@chromium.org>
Reviewed-by: James Vecore <vecore@google.com>
Cr-Commit-Position: refs/heads/master@{#753818}

chromeos/components/sync_wifi/BUILD.gn

@@ -95,6 +95,7 @@ source_set("unit_tests") {
     "//chromeos/services/network_config",
     "//chromeos/services/network_config:in_process_instance",
     "//chromeos/services/network_config/public/cpp:test_support",
+    "//components/prefs:test_support",
     "//components/sync:test_support",
     "//components/sync_preferences:test_support",
     "//components/user_manager:test_support",

chromeos/components/sync_wifi/fake_local_network_collector.cc

@@ -19,10 +19,10 @@ void FakeLocalNetworkCollector::GetAllSyncableNetworks(
   std::move(callback).Run(networks_);
 }
 
-void FakeLocalNetworkCollector::GetSyncableNetwork(const NetworkIdentifier& id,
+void FakeLocalNetworkCollector::GetSyncableNetwork(const std::string& guid,
                                                    ProtoCallback callback) {
   for (sync_pb::WifiConfigurationSpecifics proto : networks_) {
-    if (NetworkIdentifier::FromProto(proto) == id) {
+    if (NetworkIdentifier::FromProto(proto).SerializeToString() == guid) {
       std::move(callback).Run(proto);
       return;
     }
@@ -31,6 +31,18 @@ void FakeLocalNetworkCollector::GetSyncableNetwork(const NetworkIdentifier& id,
   std::move(callback).Run(base::nullopt);
 }
 
+base::Optional<NetworkIdentifier>
+FakeLocalNetworkCollector::GetNetworkIdentifierFromGuid(
+    const std::string& guid) {
+  for (sync_pb::WifiConfigurationSpecifics proto : networks_) {
+    auto id = NetworkIdentifier::FromProto(proto);
+    if (id.SerializeToString() == guid) {
+      return id;
+    }
+  }
+  return base::nullopt;
+}
+
 void FakeLocalNetworkCollector::AddNetwork(
     sync_pb::WifiConfigurationSpecifics proto) {
   networks_.push_back(proto);

chromeos/components/sync_wifi/fake_local_network_collector.h

@@ -24,8 +24,12 @@ class FakeLocalNetworkCollector : public LocalNetworkCollector {
 
   // sync_wifi::LocalNetworkCollector::
   void GetAllSyncableNetworks(ProtoListCallback callback) override;
-  void GetSyncableNetwork(const NetworkIdentifier& id,
+  // For test purposes, |guid| == serialized NetworkIdentifier.
+  void GetSyncableNetwork(const std::string& guid,
                           ProtoCallback callback) override;
+  // For test purposes, |guid| == serialized NetworkIdentifier.
+  base::Optional<NetworkIdentifier> GetNetworkIdentifierFromGuid(
+      const std::string& guid) override;
 
   void AddNetwork(sync_pb::WifiConfigurationSpecifics proto);
   void ClearNetworks();

chromeos/components/sync_wifi/local_network_collector.h

@@ -10,6 +10,7 @@
 
 #include "base/memory/weak_ptr.h"
 #include "base/optional.h"
+#include "chromeos/components/sync_wifi/network_identifier.h"
 #include "chromeos/services/network_config/public/mojom/cros_network_config.mojom.h"
 #include "mojo/public/cpp/bindings/receiver.h"
 
@@ -23,8 +24,6 @@ class NetworkMetadataStore;
 
 namespace sync_wifi {
 
-class NetworkIdentifier;
-
 // Handles the retrieval, filtering, and conversion of local network
 // configurations to syncable protos.
 class LocalNetworkCollector {
@@ -48,9 +47,14 @@ class LocalNetworkCollector {
   // Creates a WifiConfigurationSpecifics proto with the relevant network
   // details for the network with the given |id|.  If that network doesn't
   // exist or isn't syncable it will provide base::nullopt to the callback.
-  virtual void GetSyncableNetwork(const NetworkIdentifier& id,
+  virtual void GetSyncableNetwork(const std::string& guid,
                                   ProtoCallback callback) = 0;
 
+  // Retrieves the NetworkIdentifier for a given local network's |guid|
+  // if the network no longer exists it returns nullopt.
+  virtual base::Optional<NetworkIdentifier> GetNetworkIdentifierFromGuid(
+      const std::string& guid) = 0;
+
   // Provides the metadata store which gets constructed later.
   virtual void SetNetworkMetadataStore(
       base::WeakPtr<NetworkMetadataStore> network_metadata_store) = 0;

chromeos/components/sync_wifi/local_network_collector_impl.cc

@@ -70,12 +70,12 @@ void LocalNetworkCollectorImpl::GetAllSyncableNetworks(
   }
 }
 
-void LocalNetworkCollectorImpl::GetSyncableNetwork(const NetworkIdentifier& id,
+void LocalNetworkCollectorImpl::GetSyncableNetwork(const std::string& guid,
                                                    ProtoCallback callback) {
   const network_config::mojom::NetworkStateProperties* network = nullptr;
   for (const network_config::mojom::NetworkStatePropertiesPtr& n :
        mojo_networks_) {
-    if (NetworkIdentifier::FromMojoNetwork(n) == id) {
+    if (n->guid == guid) {
       if (IsEligible(n)) {
         network = n.get();
       }
@@ -95,6 +95,18 @@ void LocalNetworkCollectorImpl::GetSyncableNetwork(const NetworkIdentifier& id,
   StartGetNetworkDetails(network, request_guid);
 }
 
+base::Optional<NetworkIdentifier>
+LocalNetworkCollectorImpl::GetNetworkIdentifierFromGuid(
+    const std::string& guid) {
+  for (const network_config::mojom::NetworkStatePropertiesPtr& network :
+       mojo_networks_) {
+    if (network->guid == guid) {
+      return NetworkIdentifier::FromMojoNetwork(network);
+    }
+  }
+  return base::nullopt;
+}
+
 void LocalNetworkCollectorImpl::SetNetworkMetadataStore(
     base::WeakPtr<NetworkMetadataStore> network_metadata_store) {
   network_metadata_store_ = network_metadata_store;

chromeos/components/sync_wifi/local_network_collector_impl.h

@@ -46,9 +46,12 @@ class LocalNetworkCollectorImpl
   void GetAllSyncableNetworks(ProtoListCallback callback) override;
 
   // Can be called on multiple networks simultaneously.
-  void GetSyncableNetwork(const NetworkIdentifier& id,
+  void GetSyncableNetwork(const std::string& guid,
                           ProtoCallback callback) override;
 
+  base::Optional<NetworkIdentifier> GetNetworkIdentifierFromGuid(
+      const std::string& guid) override;
+
   void SetNetworkMetadataStore(
       base::WeakPtr<NetworkMetadataStore> network_metadata_store) override;
 

chromeos/components/sync_wifi/local_network_collector_impl_unittest.cc

@@ -146,30 +146,29 @@ TEST_F(LocalNetworkCollectorImplTest,
 }
 
 TEST_F(LocalNetworkCollectorImplTest, TestGetSyncableNetwork) {
-  helper()->ConfigureWiFiNetwork(kFredSsid, /*is_secured=*/true,
-                                 /*in_profile=*/true, /*has_connected=*/true);
-
-  NetworkIdentifier id = GeneratePskNetworkId(kFredSsid);
+  std::string guid = helper()->ConfigureWiFiNetwork(
+      kFredSsid, /*is_secured=*/true,
+      /*in_profile=*/true, /*has_connected=*/true);
   local_network_collector()->GetSyncableNetwork(
-      id, base::BindOnce(&LocalNetworkCollectorImplTest::OnGetSyncableNetwork,
-                         base::Unretained(this), kFredSsid));
+      guid, base::BindOnce(&LocalNetworkCollectorImplTest::OnGetSyncableNetwork,
+                           base::Unretained(this), kFredSsid));
 }
 
 TEST_F(LocalNetworkCollectorImplTest, TestGetSyncableNetwork_DoesntExist) {
-  NetworkIdentifier id = GeneratePskNetworkId(kFredSsid);
   local_network_collector()->GetSyncableNetwork(
-      id, base::BindOnce(&LocalNetworkCollectorImplTest::OnGetSyncableNetwork,
-                         base::Unretained(this), std::string()));
+      "test_guid",
+      base::BindOnce(&LocalNetworkCollectorImplTest::OnGetSyncableNetwork,
+                     base::Unretained(this), std::string()));
 }
 
 TEST_F(LocalNetworkCollectorImplTest, TestGetSyncableNetwork_NeverConnected) {
-  helper()->ConfigureWiFiNetwork(kFredSsid, /*is_secured=*/true,
-                                 /*in_profile=*/true, /*has_connected=*/false);
+  std::string guid = helper()->ConfigureWiFiNetwork(
+      kFredSsid, /*is_secured=*/true,
+      /*in_profile=*/true, /*has_connected=*/false);
 
-  NetworkIdentifier id = GeneratePskNetworkId(kFredSsid);
   local_network_collector()->GetSyncableNetwork(
-      id, base::BindOnce(&LocalNetworkCollectorImplTest::OnGetSyncableNetwork,
-                         base::Unretained(this), std::string()));
+      guid, base::BindOnce(&LocalNetworkCollectorImplTest::OnGetSyncableNetwork,
+                           base::Unretained(this), std::string()));
 }
 
 }  // namespace sync_wifi

chromeos/components/sync_wifi/network_identifier.cc

@@ -8,6 +8,7 @@
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "chromeos/components/sync_wifi/network_type_conversions.h"
+#include "chromeos/network/network_state.h"
 #include "chromeos/services/network_config/public/mojom/cros_network_config.mojom.h"
 #include "components/sync/protocol/wifi_configuration_specifics.pb.h"
 #include "third_party/cros_system_api/dbus/shill/dbus-constants.h"
@@ -48,6 +49,12 @@ NetworkIdentifier NetworkIdentifier::DeserializeFromString(
   return NetworkIdentifier(pieces[0], pieces[1]);
 }
 
+// static
+NetworkIdentifier NetworkIdentifier::FromNetworkState(
+    const NetworkState* network) {
+  return NetworkIdentifier(network->GetHexSsid(), network->security_class());
+}
+
 NetworkIdentifier::NetworkIdentifier(const std::string& hex_ssid,
                                      const std::string& security_type)
     : security_type_(security_type) {

chromeos/components/sync_wifi/network_identifier.h

@@ -15,6 +15,8 @@ class WifiConfigurationSpecifics;
 
 namespace chromeos {
 
+class NetworkState;
+
 namespace sync_wifi {
 
 // A unique identifier for synced networks which contains the properties
@@ -25,6 +27,7 @@ class NetworkIdentifier {
       const sync_pb::WifiConfigurationSpecifics& specifics);
   static NetworkIdentifier FromMojoNetwork(
       const network_config::mojom::NetworkStatePropertiesPtr& network);
+  static NetworkIdentifier FromNetworkState(const NetworkState* network);
   // |serialized_string| is in the format of hex_ssid and security_type
   // concatenated with an underscore.  security_type is the shill constant
   // returned from NetworkState::security_class(). For example, it would be

chromeos/components/sync_wifi/network_test_helper.cc

@@ -77,10 +77,10 @@ void NetworkTestHelper::SetUp() {
   base::RunLoop().RunUntilIdle();
 }
 
-void NetworkTestHelper::ConfigureWiFiNetwork(const std::string& ssid,
-                                             bool is_secured,
-                                             bool in_profile,
-                                             bool has_connected) {
+std::string NetworkTestHelper::ConfigureWiFiNetwork(const std::string& ssid,
+                                                    bool is_secured,
+                                                    bool in_profile,
+                                                    bool has_connected) {
   std::string security_entry =
       is_secured ? R"("SecurityClass": "psk", "Passphrase": "secretsauce", )"
                  : R"("SecurityClass": "none", )";
@@ -88,12 +88,13 @@ void NetworkTestHelper::ConfigureWiFiNetwork(const std::string& ssid,
       in_profile ? base::StringPrintf(R"("Profile": "%s", )",
                                       network_state_helper_->UserHash())
                  : std::string();
+  std::string guid = base::StringPrintf("%s_guid", ssid.c_str());
   std::string service_path =
       network_state_helper_->ConfigureService(base::StringPrintf(
-          R"({"GUID": "%s_guid", "Type": "wifi", "SSID": "%s",
+          R"({"GUID": "%s", "Type": "wifi", "SSID": "%s",
             %s "State": "ready", "Strength": 100,
             %s "AutoConnect": true, "Connectable": true})",
-          ssid.c_str(), ssid.c_str(), security_entry.c_str(),
+          guid.c_str(), ssid.c_str(), security_entry.c_str(),
           profile_entry.c_str()));
 
   base::RunLoop().RunUntilIdle();
@@ -102,6 +103,8 @@ void NetworkTestHelper::ConfigureWiFiNetwork(const std::string& ssid,
     NetworkHandler::Get()->network_metadata_store()->ConnectSucceeded(
         service_path);
   }
+
+  return guid;
 }
 
 NetworkStateTestHelper* NetworkTestHelper::network_state_test_helper() {

chromeos/components/sync_wifi/network_test_helper.h

@@ -29,10 +29,12 @@ class NetworkTestHelper : public network_config::CrosNetworkConfigTestHelper {
   virtual ~NetworkTestHelper();
 
   void SetUp();
-  void ConfigureWiFiNetwork(const std::string& ssid,
-                            bool is_secured,
-                            bool in_profile,
-                            bool has_connected);
+
+  // Returns the |guid| of the newly configured network.
+  std::string ConfigureWiFiNetwork(const std::string& ssid,
+                                   bool is_secured,
+                                   bool in_profile,
+                                   bool has_connected);
 
   NetworkStateTestHelper* network_state_test_helper();
 

chromeos/components/sync_wifi/wifi_configuration_bridge.cc

@@ -11,10 +11,12 @@
 #include "base/memory/ptr_util.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/optional.h"
+#include "base/strings/stringprintf.h"
 #include "base/time/clock.h"
 #include "base/time/time.h"
 #include "chromeos/components/sync_wifi/network_identifier.h"
 #include "chromeos/components/sync_wifi/synced_network_updater.h"
+#include "chromeos/network/network_configuration_handler.h"
 #include "chromeos/network/network_metadata_store.h"
 #include "components/device_event_log/device_event_log.h"
 #include "components/sync/model/entity_change.h"
@@ -23,6 +25,7 @@
 #include "components/sync/model/model_type_change_processor.h"
 #include "components/sync/model/mutable_data_batch.h"
 #include "components/sync/protocol/model_type_state.pb.h"
+#include "third_party/cros_system_api/dbus/shill/dbus-constants.h"
 
 namespace chromeos {
 
@@ -42,22 +45,30 @@ std::unique_ptr<syncer::EntityData> GenerateWifiEntityData(
 WifiConfigurationBridge::WifiConfigurationBridge(
     SyncedNetworkUpdater* synced_network_updater,
     LocalNetworkCollector* local_network_collector,
+    NetworkConfigurationHandler* network_configuration_handler,
     std::unique_ptr<syncer::ModelTypeChangeProcessor> change_processor,
     syncer::OnceModelTypeStoreFactory create_store_callback)
     : ModelTypeSyncBridge(std::move(change_processor)),
       synced_network_updater_(synced_network_updater),
       local_network_collector_(local_network_collector),
+      network_configuration_handler_(network_configuration_handler),
       network_metadata_store_(nullptr) {
   std::move(create_store_callback)
       .Run(syncer::WIFI_CONFIGURATIONS,
            base::BindOnce(&WifiConfigurationBridge::OnStoreCreated,
                           weak_ptr_factory_.GetWeakPtr()));
+  if (network_configuration_handler_) {
+    network_configuration_handler_->AddObserver(this);
+  }
 }
 
 WifiConfigurationBridge::~WifiConfigurationBridge() {
   if (network_metadata_store_) {
     network_metadata_store_->RemoveObserver(this);
   }
+  if (network_configuration_handler_) {
+    network_configuration_handler_->RemoveObserver(this);
+  }
 }
 
 std::unique_ptr<syncer::MetadataChangeList>
@@ -302,7 +313,87 @@ std::vector<NetworkIdentifier> WifiConfigurationBridge::GetAllIdsForTesting() {
 
 void WifiConfigurationBridge::OnFirstConnectionToNetwork(
     const std::string& guid) {
-  // TODO(jonmann): Add network to sync.
+  if (network_metadata_store_->GetIsConfiguredBySync(guid)) {
+    // Don't have to upload a configuration that came from sync.
+    return;
+  }
+
+  local_network_collector_->GetSyncableNetwork(
+      guid, base::BindOnce(&WifiConfigurationBridge::SaveNetworkToSync,
+                           weak_ptr_factory_.GetWeakPtr()));
+}
+
+void WifiConfigurationBridge::SaveNetworkToSync(
+    base::Optional<sync_pb::WifiConfigurationSpecifics> proto) {
+  if (!proto) {
+    return;
+  }
+
+  std::unique_ptr<syncer::EntityData> entity_data =
+      GenerateWifiEntityData(*proto);
+  std::string storage_key = GetStorageKey(*entity_data);
+
+  std::unique_ptr<syncer::ModelTypeStore::WriteBatch> batch =
+      store_->CreateWriteBatch();
+  batch->WriteData(storage_key, proto->SerializeAsString());
+  change_processor()->Put(storage_key, std::move(entity_data),
+                          batch->GetMetadataChangeList());
+  entries_[storage_key] = *proto;
+  Commit(std::move(batch));
+}
+
+void WifiConfigurationBridge::OnBeforeConfigurationRemoved(
+    const std::string& service_path,
+    const std::string& guid) {
+  base::Optional<NetworkIdentifier> id =
+      local_network_collector_->GetNetworkIdentifierFromGuid(guid);
+  if (!id) {
+    return;
+  }
+
+  std::string storage_key = id->SerializeToString();
+  if (entries_.contains(storage_key))
+    pending_deletes_[guid] = storage_key;
+}
+
+void WifiConfigurationBridge::OnConfigurationRemoved(
+    const std::string& service_path,
+    const std::string& network_guid) {
+  LOG(ERROR) << "WifiConfigurationBridge::RemoveNetworkFromSync" << this;
+  if (!pending_deletes_.contains(network_guid))
+    return;
+
+  std::string storage_key = pending_deletes_[network_guid];
+
+  std::unique_ptr<syncer::ModelTypeStore::WriteBatch> batch =
+      store_->CreateWriteBatch();
+  batch->DeleteData(storage_key);
+  change_processor()->Delete(storage_key, batch->GetMetadataChangeList());
+  entries_.erase(storage_key);
+  Commit(std::move(batch));
+}
+
+void WifiConfigurationBridge::OnConfigurationModified(
+    const std::string& service_path,
+    const std::string& guid,
+    base::DictionaryValue* set_properties) {
+  if (!set_properties)
+    return;
+
+  if (network_metadata_store_->GetIsConfiguredBySync(guid)) {
+    // Don't have to upload a configuration that came from sync.
+    return;
+  }
+  if (set_properties->HasKey(shill::kAutoConnectProperty) ||
+      set_properties->HasKey(shill::kPriorityProperty) ||
+      set_properties->HasKey(shill::kProxyConfigProperty) ||
+      set_properties->FindPath(
+          base::StringPrintf("%s.%s", shill::kStaticIPConfigProperty,
+                             shill::kNameServersProperty))) {
+    local_network_collector_->GetSyncableNetwork(
+        guid, base::BindOnce(&WifiConfigurationBridge::SaveNetworkToSync,
+                             weak_ptr_factory_.GetWeakPtr()));
+  }
 }
 
 void WifiConfigurationBridge::SetNetworkMetadataStore(

chromeos/components/sync_wifi/wifi_configuration_bridge.h

@@ -16,6 +16,7 @@
 #include "base/time/time.h"
 #include "chromeos/components/sync_wifi/local_network_collector.h"
 #include "chromeos/components/sync_wifi/synced_network_updater.h"
+#include "chromeos/network/network_configuration_observer.h"
 #include "chromeos/network/network_metadata_observer.h"
 #include "components/sync/base/model_type.h"
 #include "components/sync/model/model_type_store.h"
@@ -27,6 +28,7 @@ class ModelTypeChangeProcessor;
 
 namespace chromeos {
 
+class NetworkConfigurationHandler;
 class NetworkMetadataStore;
 
 namespace sync_wifi {
@@ -34,11 +36,13 @@ namespace sync_wifi {
 // Receives updates to network configurations from the Chrome sync back end and
 // from the system network stack and keeps both lists in sync.
 class WifiConfigurationBridge : public syncer::ModelTypeSyncBridge,
+                                public NetworkConfigurationObserver,
                                 public NetworkMetadataObserver {
  public:
   WifiConfigurationBridge(
       SyncedNetworkUpdater* synced_network_updater,
       LocalNetworkCollector* local_network_collector,
+      NetworkConfigurationHandler* network_configuration_handler,
       std::unique_ptr<syncer::ModelTypeChangeProcessor> change_processor,
       syncer::OnceModelTypeStoreFactory create_store_callback);
   ~WifiConfigurationBridge() override;
@@ -60,6 +64,15 @@ class WifiConfigurationBridge : public syncer::ModelTypeSyncBridge,
   // NetworkMetadataObserver:
   void OnFirstConnectionToNetwork(const std::string& guid) override;
 
+  // NetworkConfigurationObserver::
+  void OnConfigurationModified(const std::string& service_path,
+                               const std::string& guid,
+                               base::DictionaryValue* set_properties) override;
+  void OnBeforeConfigurationRemoved(const std::string& service_path,
+                                    const std::string& guid) override;
+  void OnConfigurationRemoved(const std::string& service_path,
+                              const std::string& guid) override;
+
   // Comes from |entries_| the in-memory map.
   std::vector<NetworkIdentifier> GetAllIdsForTesting();
 
@@ -84,12 +97,22 @@ class WifiConfigurationBridge : public syncer::ModelTypeSyncBridge,
       syncer::EntityChangeList change_list,
       std::vector<sync_pb::WifiConfigurationSpecifics> local_network_list);
 
+  void SaveNetworkToSync(
+      base::Optional<sync_pb::WifiConfigurationSpecifics> proto);
+  void RemoveNetworkFromSync(
+      base::Optional<sync_pb::WifiConfigurationSpecifics> proto);
+
   // An in-memory list of the proto's that mirrors what is on the sync server.
   // This gets updated when changes are received from the server and after local
   // changes have been committed.  On initialization of this class, it is
   // populated with the contents of |store_|.
   base::flat_map<std::string, sync_pb::WifiConfigurationSpecifics> entries_;
 
+  // Map of network |guid| to |storage_key|.  After a network is deleted, we
+  // no longer have access to its metadata so this stores the necessary
+  // information to delete it from sync.
+  base::flat_map<std::string, std::string> pending_deletes_;
+
   // The on disk store of WifiConfigurationSpecifics protos that mirrors what
   // is on the sync server.  This gets updated when changes are received from
   // the server and after local changes have been committed to the server.
@@ -97,6 +120,7 @@ class WifiConfigurationBridge : public syncer::ModelTypeSyncBridge,
 
   SyncedNetworkUpdater* synced_network_updater_;
   LocalNetworkCollector* local_network_collector_;
+  NetworkConfigurationHandler* network_configuration_handler_;
   base::WeakPtr<NetworkMetadataStore> network_metadata_store_;
 
   base::WeakPtrFactory<WifiConfigurationBridge> weak_ptr_factory_{this};

chromeos/components/sync_wifi/wifi_configuration_bridge_unittest.cc

@@ -16,6 +16,9 @@
 #include "chromeos/components/sync_wifi/network_identifier.h"
 #include "chromeos/components/sync_wifi/synced_network_updater.h"
 #include "chromeos/components/sync_wifi/test_data_generator.h"
+#include "chromeos/network/network_metadata_store.h"
+#include "components/prefs/pref_registry_simple.h"
+#include "components/prefs/testing_pref_service.h"
 #include "components/sync/model/data_batch.h"
 #include "components/sync/model/entity_change.h"
 #include "components/sync/model/metadata_batch.h"
@@ -24,8 +27,10 @@
 #include "components/sync/model_impl/in_memory_metadata_change_list.h"
 #include "components/sync/protocol/model_type_state.pb.h"
 #include "components/sync/test/test_matchers.h"
+#include "components/sync_preferences/testing_pref_service_syncable.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/cros_system_api/dbus/shill/dbus-constants.h"
 
 namespace chromeos {
 
@@ -46,12 +51,13 @@ using testing::UnorderedElementsAre;
 const char kSsidMeow[] = "meow";
 const char kSsidWoof[] = "woof";
 const char kSsidHonk[] = "honk";
+const char kSyncPsk[] = "sync_psk";
+const char kLocalPsk[] = "local_psk";
 
 syncer::EntityData GenerateWifiEntityData(
     const sync_pb::WifiConfigurationSpecifics& data) {
   syncer::EntityData entity_data;
-  entity_data.specifics.mutable_wifi_configuration()
-      ->CopyFrom(data);
+  entity_data.specifics.mutable_wifi_configuration()->CopyFrom(data);
   entity_data.name = data.hex_ssid();
   return entity_data;
 }
@@ -117,10 +123,24 @@ class WifiConfigurationBridgeTest : public testing::Test {
     ON_CALL(mock_processor_, IsTrackingMetadata()).WillByDefault(Return(true));
     synced_network_updater_ = std::make_unique<TestSyncedNetworkUpdater>();
     local_network_collector_ = std::make_unique<FakeLocalNetworkCollector>();
+
+    user_prefs_ =
+        std::make_unique<sync_preferences::TestingPrefServiceSyncable>();
+    device_prefs_ = std::make_unique<TestingPrefServiceSimple>();
+    NetworkMetadataStore::RegisterPrefs(user_prefs_->registry());
+    NetworkMetadataStore::RegisterPrefs(device_prefs_->registry());
+    network_metadata_store_ = std::make_unique<NetworkMetadataStore>(
+        /*network_configuration_handler=*/nullptr,
+        /*network_connection_handler=*/nullptr,
+        /*network_state_handler=*/nullptr, user_prefs_.get(),
+        device_prefs_.get());
+
     bridge_ = std::make_unique<WifiConfigurationBridge>(
         synced_network_updater(), local_network_collector(),
+        /*network_configuration_handler=*/nullptr,
         mock_processor_.CreateForwardingProcessor(),
         syncer::ModelTypeStoreTestUtil::MoveStoreToFactory(std::move(store_)));
+    bridge_->SetNetworkMetadataStore(network_metadata_store_->GetWeakPtr());
   }
 
   void DisableBridge() {
@@ -166,28 +186,22 @@ class WifiConfigurationBridgeTest : public testing::Test {
   }
 
   const NetworkIdentifier& woof_network_id() const { return woof_network_id_; }
-
   const NetworkIdentifier& meow_network_id() const { return meow_network_id_; }
-
   const NetworkIdentifier& honk_network_id() const { return honk_network_id_; }
 
  private:
   base::test::TaskEnvironment task_environment_;
-
   std::unique_ptr<syncer::ModelTypeStore> store_;
-
   testing::NiceMock<syncer::MockModelTypeChangeProcessor> mock_processor_;
-
   std::unique_ptr<WifiConfigurationBridge> bridge_;
-
   std::unique_ptr<TestSyncedNetworkUpdater> synced_network_updater_;
-
   std::unique_ptr<FakeLocalNetworkCollector> local_network_collector_;
+  std::unique_ptr<TestingPrefServiceSimple> device_prefs_;
+  std::unique_ptr<sync_preferences::TestingPrefServiceSyncable> user_prefs_;
+  std::unique_ptr<NetworkMetadataStore> network_metadata_store_;
 
   const NetworkIdentifier woof_network_id_ = GeneratePskNetworkId(kSsidWoof);
-
   const NetworkIdentifier meow_network_id_ = GeneratePskNetworkId(kSsidMeow);
-
   const NetworkIdentifier honk_network_id_ = GeneratePskNetworkId(kSsidHonk);
 
   DISALLOW_COPY_AND_ASSIGN(WifiConfigurationBridgeTest);
@@ -307,8 +321,6 @@ TEST_F(WifiConfigurationBridgeTest, MergeSyncData) {
   auto metadata_change_list =
       std::make_unique<syncer::InMemoryMetadataChangeList>();
   syncer::EntityChangeList entity_data;
-  const char kSyncPsk[] = "sync_psk";
-  const char kLocalPsk[] = "local_psk";
 
   WifiConfigurationSpecifics meow_sync =
       GenerateTestWifiSpecifics(meow_network_id(), kSyncPsk, /*timestamp=*/100);
@@ -359,6 +371,67 @@ TEST_F(WifiConfigurationBridgeTest, MergeSyncData) {
   EXPECT_TRUE(VectorContainsProto(sync_networks, honk_sync));
 }
 
+TEST_F(WifiConfigurationBridgeTest, LocalFirstConnect) {
+  WifiConfigurationSpecifics meow_local =
+      GenerateTestWifiSpecifics(meow_network_id(), kSyncPsk, /*timestamp=*/100);
+  local_network_collector()->AddNetwork(meow_local);
+
+  std::string storage_key;
+  EXPECT_CALL(*processor(), Put(_, _, _))
+      .WillOnce(testing::SaveArg<0>(&storage_key));
+  bridge()->OnFirstConnectionToNetwork(meow_network_id().SerializeToString());
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(storage_key, meow_network_id().SerializeToString());
+}
+
+TEST_F(WifiConfigurationBridgeTest, LocalUpdate) {
+  WifiConfigurationSpecifics meow_local =
+      GenerateTestWifiSpecifics(meow_network_id(), kSyncPsk, /*timestamp=*/100);
+  local_network_collector()->AddNetwork(meow_local);
+
+  std::string storage_key;
+  EXPECT_CALL(*processor(), Put(_, _, _))
+      .WillOnce(testing::SaveArg<0>(&storage_key));
+  std::string guid = meow_network_id().SerializeToString();
+  base::DictionaryValue set_properties;
+  set_properties.SetBoolean(shill::kAutoConnectProperty, true);
+  bridge()->OnConfigurationModified("service_path", guid, &set_properties);
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(storage_key, meow_network_id().SerializeToString());
+}
+
+TEST_F(WifiConfigurationBridgeTest, LocalUpdate_UntrackedField) {
+  WifiConfigurationSpecifics meow_local =
+      GenerateTestWifiSpecifics(meow_network_id(), kSyncPsk, /*timestamp=*/100);
+  local_network_collector()->AddNetwork(meow_local);
+
+  EXPECT_CALL(*processor(), Put(_, _, _)).Times(testing::Exactly(0));
+  std::string guid = meow_network_id().SerializeToString();
+  base::DictionaryValue set_properties;
+  set_properties.SetString(shill::kUIDataProperty, "random_change");
+  bridge()->OnConfigurationModified("service_path", guid, &set_properties);
+  base::RunLoop().RunUntilIdle();
+}
+
+TEST_F(WifiConfigurationBridgeTest, LocalRemove) {
+  WifiConfigurationSpecifics meow_local =
+      GenerateTestWifiSpecifics(meow_network_id(), kSyncPsk, /*timestamp=*/100);
+  local_network_collector()->AddNetwork(meow_local);
+  std::string guid = meow_network_id().SerializeToString();
+
+  bridge()->OnFirstConnectionToNetwork(guid);
+  base::RunLoop().RunUntilIdle();
+
+  bridge()->OnBeforeConfigurationRemoved("service_path", guid);
+
+  std::string storage_key;
+  EXPECT_CALL(*processor(), Delete(_, _))
+      .WillOnce(testing::SaveArg<0>(&storage_key));
+  bridge()->OnConfigurationRemoved("service_path", guid);
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(storage_key, meow_network_id().SerializeToString());
+}
+
 }  // namespace
 
 }  // namespace sync_wifi

chromeos/components/sync_wifi/wifi_configuration_sync_service.cc

@@ -36,8 +36,10 @@ WifiConfigurationSyncService::WifiConfigurationSyncService(
       remote_cros_network_config_.get(), std::make_unique<TimerFactory>());
   collector_ = std::make_unique<LocalNetworkCollectorImpl>(
       remote_cros_network_config_.get());
+  NetworkHandler* network_handler = NetworkHandler::Get();
   bridge_ = std::make_unique<sync_wifi::WifiConfigurationBridge>(
       updater_.get(), collector_.get(),
+      network_handler->network_configuration_handler(),
       std::make_unique<syncer::ClientTagBasedModelTypeProcessor>(
           syncer::WIFI_CONFIGURATIONS,
           base::BindRepeating(&syncer::ReportUnrecoverableError, channel)),