[Wifi Sync] Implement the SyncedNetworkUpdater class.

This is responsible for updating the local network stack to match
incoming WifiConfigurationSpecifics protos from the sync backend.

Bug: 966270
Change-Id: Ib4bdf1c22604b2528bc2abad5bc2ac1672e58290
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1768877
Commit-Queue: Jon Mann <jonmann@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#709627}

chrome/browser/sync/wifi_configuration_sync_service_factory.cc

@@ -39,10 +39,10 @@ WifiConfigurationSyncServiceFactory::~WifiConfigurationSyncServiceFactory() =
 
 KeyedService* WifiConfigurationSyncServiceFactory::BuildServiceInstanceFor(
     content::BrowserContext* context) const {
+  Profile* profile = Profile::FromBrowserContext(context);
   return new chromeos::sync_wifi::WifiConfigurationSyncService(
-      chrome::GetChannel(), ModelTypeStoreServiceFactory::GetForProfile(
-                                Profile::FromBrowserContext(context))
-                                ->GetStoreFactory());
+      chrome::GetChannel(), profile->GetPrefs(),
+      ModelTypeStoreServiceFactory::GetForProfile(profile)->GetStoreFactory());
 }
 
 void WifiConfigurationSyncServiceFactory::RegisterProfilePrefs(

chromeos/components/sync_wifi/BUILD.gn

@@ -6,22 +6,30 @@ static_library("sync_wifi") {
   sources = [
     "network_identifier.cc",
     "network_identifier.h",
+    "network_type_conversions.cc",
+    "network_type_conversions.h",
     "pending_network_configuration_tracker.h",
     "pending_network_configuration_tracker_impl.cc",
     "pending_network_configuration_tracker_impl.h",
     "pending_network_configuration_update.cc",
     "pending_network_configuration_update.h",
     "synced_network_updater.h",
+    "synced_network_updater_impl.cc",
+    "synced_network_updater_impl.h",
     "wifi_configuration_bridge.cc",
     "wifi_configuration_bridge.h",
     "wifi_configuration_sync_service.cc",
     "wifi_configuration_sync_service.h",
   ]
   deps = [
+    "//ash/public/cpp",
     "//base",
-    "//chromeos/network:network",
+    "//chromeos/network",
+    "//chromeos/services/network_config/public/mojom",
+    "//components/device_event_log",
     "//components/keyed_service/core",
-    "//components/prefs:prefs",
+    "//components/prefs",
+    "//components/proxy_config",
     "//components/sync",
     "//components/version_info",
   ]
@@ -30,6 +38,8 @@ static_library("sync_wifi") {
 source_set("test_support") {
   testonly = true
   sources = [
+    "fake_pending_network_configuration_tracker.cc",
+    "fake_pending_network_configuration_tracker.h",
     "test_data_generator.cc",
     "test_data_generator.h",
   ]
@@ -45,16 +55,24 @@ source_set("unit_tests") {
   sources = [
     "network_identifier_unittest.cc",
     "pending_network_configuration_tracker_impl_unittest.cc",
+    "synced_network_updater_impl_unittest.cc",
     "wifi_configuration_bridge_unittest.cc",
   ]
   deps = [
     ":sync_wifi",
     ":test_support",
+    "//ash/public/cpp",
     "//base/test:test_support",
     "//chromeos/dbus/shill:test_support",
+    "//chromeos/login/login_state",
+    "//chromeos/login/login_state:test_support",
     "//chromeos/network:test_support",
+    "//chromeos/services/network_config",
+    "//chromeos/services/network_config:in_process_instance",
+    "//chromeos/services/network_config/public/cpp:test_support",
     "//components/sync:test_support",
     "//components/sync_preferences:test_support",
+    "//components/user_manager:test_support",
     "//testing/gtest",
   ]
 }

chromeos/components/sync_wifi/DEPS

 include_rules = [
+  "+ash/public/cpp",
   "+chromeos/network",
   "+components/keyed_service/core",
   "+components/prefs",
+  "+components/proxy_config",
+  "+components/user_manager",
   "+components/version_info",
   "+components/sync",
-  "+components/sync_preferences"
+  "+components/sync_preferences",
 ]

chromeos/components/sync_wifi/fake_pending_network_configuration_tracker.cc

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chromeos/components/sync_wifi/fake_pending_network_configuration_tracker.h"
+
+#include "base/guid.h"
+#include "base/stl_util.h"
+#include "chromeos/components/sync_wifi/pending_network_configuration_update.h"
+
+namespace chromeos {
+
+namespace sync_wifi {
+
+FakePendingNetworkConfigurationTracker::
+    FakePendingNetworkConfigurationTracker() = default;
+
+FakePendingNetworkConfigurationTracker::
+    ~FakePendingNetworkConfigurationTracker() = default;
+
+std::string FakePendingNetworkConfigurationTracker::TrackPendingUpdate(
+    const NetworkIdentifier& id,
+    const base::Optional<sync_pb::WifiConfigurationSpecificsData>& specifics) {
+  std::string change_id = base::GenerateGUID();
+  id_to_pending_update_map_.emplace(
+      id, PendingNetworkConfigurationUpdate(id, change_id, specifics,
+                                            /*completed_attempts=*/0));
+  id_to_completed_attempts_map_[id] = 0;
+
+  return change_id;
+}
+
+void FakePendingNetworkConfigurationTracker::MarkComplete(
+    const std::string& change_guid,
+    const NetworkIdentifier& id) {
+  if (base::Contains(id_to_pending_update_map_, id) &&
+      id_to_pending_update_map_.at(id).change_guid() == change_guid) {
+    id_to_pending_update_map_.erase(id);
+  }
+}
+
+void FakePendingNetworkConfigurationTracker::IncrementCompletedAttempts(
+    const std::string& change_id,
+    const NetworkIdentifier& id) {
+  base::Optional<PendingNetworkConfigurationUpdate> existing_update =
+      GetPendingUpdate(change_id, id);
+  id_to_pending_update_map_.emplace(
+      std::piecewise_construct, std::forward_as_tuple(id),
+      std::forward_as_tuple(id, change_id, existing_update->specifics(),
+                            existing_update->completed_attempts() + 1));
+  id_to_completed_attempts_map_[id]++;
+}
+
+std::vector<PendingNetworkConfigurationUpdate>
+FakePendingNetworkConfigurationTracker::GetPendingUpdates() {
+  std::vector<PendingNetworkConfigurationUpdate> list;
+  for (const auto& it : id_to_pending_update_map_)
+    list.emplace_back(it.second);
+  return list;
+}
+
+base::Optional<PendingNetworkConfigurationUpdate>
+FakePendingNetworkConfigurationTracker::GetPendingUpdate(
+    const std::string& change_guid,
+    const NetworkIdentifier& id) {
+  if (!base::Contains(id_to_pending_update_map_, id) ||
+      id_to_pending_update_map_.at(id).change_guid() != change_guid) {
+    return base::nullopt;
+  }
+
+  return id_to_pending_update_map_.at(id);
+}
+
+PendingNetworkConfigurationUpdate*
+FakePendingNetworkConfigurationTracker::GetPendingUpdateById(
+    const NetworkIdentifier& id) {
+  if (!base::Contains(id_to_pending_update_map_, id))
+    return nullptr;
+
+  return &id_to_pending_update_map_.at(id);
+}
+
+int FakePendingNetworkConfigurationTracker::GetCompletedAttempts(
+    const NetworkIdentifier& id) {
+  DCHECK(id_to_completed_attempts_map_.count(id));
+  return id_to_completed_attempts_map_[id];
+}
+
+}  // namespace sync_wifi
+
+}  // namespace chromeos

chromeos/components/sync_wifi/fake_pending_network_configuration_tracker.h

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROMEOS_COMPONENTS_SYNC_WIFI_FAKE_PENDING_NETWORK_CONFIGURATION_TRACKER_H_
+#define CHROMEOS_COMPONENTS_SYNC_WIFI_FAKE_PENDING_NETWORK_CONFIGURATION_TRACKER_H_
+
+#include <map>
+
+#include "base/containers/flat_map.h"
+#include "base/optional.h"
+#include "chromeos/components/sync_wifi/network_identifier.h"
+#include "chromeos/components/sync_wifi/pending_network_configuration_tracker.h"
+
+namespace chromeos {
+
+namespace sync_wifi {
+
+class FakePendingNetworkConfigurationTracker
+    : public PendingNetworkConfigurationTracker {
+ public:
+  FakePendingNetworkConfigurationTracker();
+  ~FakePendingNetworkConfigurationTracker() override;
+
+  // sync_wifi::PendingNetworkConfigurationtracker::
+  std::string TrackPendingUpdate(
+      const NetworkIdentifier& id,
+      const base::Optional<sync_pb::WifiConfigurationSpecificsData>& specifics)
+      override;
+  void MarkComplete(const std::string& change_guid,
+                    const NetworkIdentifier& id) override;
+  void IncrementCompletedAttempts(const std::string& change_id,
+                                  const NetworkIdentifier& id) override;
+  std::vector<PendingNetworkConfigurationUpdate> GetPendingUpdates() override;
+  base::Optional<PendingNetworkConfigurationUpdate> GetPendingUpdate(
+      const std::string& change_guid,
+      const NetworkIdentifier& id) override;
+
+  // Get the matching PendingNetworkConfigurationUpdate by for a given |id|.
+  // This is needed because some tests don't have insight into what
+  // change_guids are used.
+  PendingNetworkConfigurationUpdate* GetPendingUpdateById(
+      const NetworkIdentifier& id);
+
+  // This includes ssids which have already been removed from the tracker.
+  int GetCompletedAttempts(const NetworkIdentifier& id);
+
+ private:
+  std::map<NetworkIdentifier, PendingNetworkConfigurationUpdate>
+      id_to_pending_update_map_;
+
+  // This map is not cleared when MarkComplete is called to allow tests to
+  // verify that the expected number of retries were performed before removal.
+  std::map<NetworkIdentifier, int> id_to_completed_attempts_map_;
+
+  DISALLOW_COPY_AND_ASSIGN(FakePendingNetworkConfigurationTracker);
+};
+
+}  // namespace sync_wifi
+
+}  // namespace chromeos
+
+#endif  // CHROMEOS_COMPONENTS_SYNC_WIFI_FAKE_PENDING_NETWORK_CONFIGURATION_TRACKER_H_

chromeos/components/sync_wifi/network_identifier.cc

@@ -7,8 +7,9 @@
 #include "base/strings/string_split.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
-#include "chromeos/network/network_state.h"
-#include "components/sync/protocol/model_type_state.pb.h"
+#include "chromeos/components/sync_wifi/network_type_conversions.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"
 
 namespace chromeos {
@@ -19,32 +20,22 @@ namespace {
 
 const char kDelimeter[] = "_";
 
-std::string GetSecurityType(
-    sync_pb::WifiConfigurationSpecificsData_SecurityType security_type) {
-  switch (security_type) {
-    case sync_pb::WifiConfigurationSpecificsData::SECURITY_TYPE_PSK:
-      return shill::kSecurityPsk;
-    case sync_pb::WifiConfigurationSpecificsData::SECURITY_TYPE_WEP:
-      return shill::kSecurityWep;
-    default:
-      NOTREACHED();
-      return "";
-  }
-}
-
 }  // namespace
 
 // static
 NetworkIdentifier NetworkIdentifier::FromProto(
     const sync_pb::WifiConfigurationSpecificsData& specifics) {
-  return NetworkIdentifier(specifics.hex_ssid(),
-                           GetSecurityType(specifics.security_type()));
+  return NetworkIdentifier(
+      specifics.hex_ssid(),
+      SecurityTypeStringFromProto(specifics.security_type()));
 }
 
 // static
-NetworkIdentifier NetworkIdentifier::FromNetwork(
-    const chromeos::NetworkState& network) {
-  return NetworkIdentifier(network.GetHexSsid(), network.security_class());
+NetworkIdentifier NetworkIdentifier::FromMojoNetwork(
+    const network_config::mojom::NetworkStatePropertiesPtr& network) {
+  return NetworkIdentifier(
+      network->type_state->get_wifi()->hex_ssid,
+      SecurityTypeStringFromMojo(network->type_state->get_wifi()->security));
 }
 
 // static
@@ -77,6 +68,18 @@ bool NetworkIdentifier::operator==(const NetworkIdentifier& o) const {
   return hex_ssid_ == o.hex_ssid_ && security_type_ == o.security_type_;
 }
 
+bool NetworkIdentifier::operator!=(const NetworkIdentifier& o) const {
+  return hex_ssid_ != o.hex_ssid_ || security_type_ != o.security_type_;
+}
+
+bool NetworkIdentifier::operator>(const NetworkIdentifier& o) const {
+  return *this != o && !(*this < o);
+}
+
+bool NetworkIdentifier::operator<(const NetworkIdentifier& o) const {
+  return SerializeToString() < o.SerializeToString();
+}
+
 }  // namespace sync_wifi
 
 }  // namespace chromeos

chromeos/components/sync_wifi/network_identifier.h

@@ -7,14 +7,14 @@
 
 #include <string>
 
+#include "chromeos/services/network_config/public/mojom/cros_network_config.mojom-forward.h"
+
 namespace sync_pb {
 class WifiConfigurationSpecificsData;
 }
 
 namespace chromeos {
 
-class NetworkState;
-
 namespace sync_wifi {
 
 // A unique identifier for synced networks which contains the properties
@@ -23,7 +23,8 @@ class NetworkIdentifier {
  public:
   static NetworkIdentifier FromProto(
       const sync_pb::WifiConfigurationSpecificsData& specifics);
-  static NetworkIdentifier FromNetwork(const chromeos::NetworkState& network);
+  static NetworkIdentifier FromMojoNetwork(
+      const network_config::mojom::NetworkStatePropertiesPtr& 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
@@ -40,6 +41,9 @@ class NetworkIdentifier {
   virtual ~NetworkIdentifier();
 
   bool operator==(const NetworkIdentifier& o) const;
+  bool operator!=(const NetworkIdentifier& o) const;
+  bool operator<(const NetworkIdentifier& o) const;
+  bool operator>(const NetworkIdentifier& o) const;
 
   std::string SerializeToString() const;
 

chromeos/components/sync_wifi/network_identifier_unittest.cc

@@ -38,16 +38,6 @@ TEST_F(NetworkIdentifierTest, FromProto) {
   EXPECT_EQ(expected_id, id);
 }
 
-TEST_F(NetworkIdentifierTest, FromNetwork) {
-  chromeos::NetworkState network("dummy_path");
-  network.PropertyChanged(shill::kWifiHexSsid, base::Value(kHexSsid));
-  network.PropertyChanged(shill::kSecurityClassProperty,
-                          base::Value(shill::kSecurityPsk));
-  NetworkIdentifier id = NetworkIdentifier::FromNetwork(network);
-  EXPECT_EQ(kHexSsid, id.hex_ssid());
-  EXPECT_EQ(shill::kSecurityPsk, id.security_type());
-}
-
 TEST_F(NetworkIdentifierTest, FromString) {
   std::string string_id("0123456789ABCDEF_psk");
   NetworkIdentifier id = NetworkIdentifier::DeserializeFromString(string_id);

chromeos/components/sync_wifi/network_type_conversions.cc

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chromeos/components/sync_wifi/network_type_conversions.h"
+
+#include "base/strings/string_number_conversions.h"
+#include "chromeos/components/sync_wifi/network_identifier.h"
+#include "chromeos/network/network_handler.h"
+#include "chromeos/network/network_state.h"
+#include "chromeos/network/network_state_handler.h"
+#include "chromeos/services/network_config/public/mojom/cros_network_config.mojom.h"
+#include "third_party/cros_system_api/dbus/shill/dbus-constants.h"
+
+namespace chromeos {
+
+namespace sync_wifi {
+
+namespace {
+
+std::string DecodeHexString(const std::string& base_16) {
+  std::string decoded;
+  DCHECK_EQ(base_16.size() % 2, 0u) << "Must be a multiple of 2";
+  decoded.reserve(base_16.size() / 2);
+
+  std::vector<uint8_t> v;
+  if (!base::HexStringToBytes(base_16, &v)) {
+    NOTREACHED();
+  }
+  decoded.assign(reinterpret_cast<const char*>(&v[0]), v.size());
+  return decoded;
+}
+
+}  // namespace
+
+std::string SecurityTypeStringFromMojo(
+    const network_config::mojom::SecurityType& security_type) {
+  switch (security_type) {
+    case network_config::mojom::SecurityType::kWpaPsk:
+      return shill::kSecurityPsk;
+    case network_config::mojom::SecurityType::kWepPsk:
+      return shill::kSecurityWep;
+    default:
+      // Only PSK and WEP secured networks are supported by sync.
+      NOTREACHED();
+      return "";
+  }
+}
+
+std::string SecurityTypeStringFromProto(
+    const sync_pb::WifiConfigurationSpecificsData_SecurityType& security_type) {
+  switch (security_type) {
+    case sync_pb::WifiConfigurationSpecificsData::SECURITY_TYPE_PSK:
+      return shill::kSecurityPsk;
+    case sync_pb::WifiConfigurationSpecificsData::SECURITY_TYPE_WEP:
+      return shill::kSecurityWep;
+    default:
+      // Only PSK and WEP secured networks are supported by sync.
+      NOTREACHED();
+      return "";
+  }
+}
+
+network_config::mojom::SecurityType MojoSecurityTypeFromProto(
+    const sync_pb::WifiConfigurationSpecificsData_SecurityType& security_type) {
+  switch (security_type) {
+    case sync_pb::WifiConfigurationSpecificsData::SECURITY_TYPE_PSK:
+      return network_config::mojom::SecurityType::kWpaPsk;
+    case sync_pb::WifiConfigurationSpecificsData::SECURITY_TYPE_WEP:
+      return network_config::mojom::SecurityType::kWepPsk;
+    default:
+      // Only PSK and WEP secured networks are supported by sync.
+      NOTREACHED();
+      return network_config::mojom::SecurityType::kNone;
+  }
+}
+
+network_config::mojom::ConfigPropertiesPtr MojoNetworkConfigFromProto(
+    const sync_pb::WifiConfigurationSpecificsData& specifics) {
+  auto config = network_config::mojom::ConfigProperties::New();
+  auto wifi = network_config::mojom::WiFiConfigProperties::New();
+
+  wifi->ssid = DecodeHexString(specifics.hex_ssid());
+  wifi->security = MojoSecurityTypeFromProto(specifics.security_type());
+  wifi->passphrase = specifics.passphrase();
+
+  config->type_config =
+      network_config::mojom::NetworkTypeConfigProperties::NewWifi(
+          std::move(wifi));
+
+  config->auto_connect = network_config::mojom::AutoConnectConfig::New(
+      specifics.automatically_connect() ==
+      sync_pb::WifiConfigurationSpecificsData::AUTOMATICALLY_CONNECT_ENABLED);
+
+  config->priority = network_config::mojom::PriorityConfig::New(
+      specifics.is_preferred() ==
+              sync_pb::WifiConfigurationSpecificsData::IS_PREFERRED_ENABLED
+          ? 1
+          : 0);
+
+  return config;
+}
+
+}  // namespace sync_wifi
+
+}  // namespace chromeos

chromeos/components/sync_wifi/network_type_conversions.h

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROMEOS_COMPONENTS_SYNC_WIFI_NETWORK_TYPE_CONVERSIONS_H_
+#define CHROMEOS_COMPONENTS_SYNC_WIFI_NETWORK_TYPE_CONVERSIONS_H_
+
+#include "chromeos/services/network_config/public/mojom/cros_network_config.mojom.h"
+#include "components/sync/protocol/wifi_configuration_specifics.pb.h"
+
+namespace chromeos {
+
+namespace sync_wifi {
+
+std::string SecurityTypeStringFromMojo(
+    const network_config::mojom::SecurityType& security_type);
+
+std::string SecurityTypeStringFromProto(
+    const sync_pb::WifiConfigurationSpecificsData_SecurityType& security_type);
+
+network_config::mojom::SecurityType MojoSecurityTypeFromString(
+    const std::string& security_type);
+
+network_config::mojom::SecurityType MojoSecurityTypeFromProto(
+    const sync_pb::WifiConfigurationSpecificsData_SecurityType& security_type);
+
+network_config::mojom::ConfigPropertiesPtr MojoNetworkConfigFromProto(
+    const sync_pb::WifiConfigurationSpecificsData& specifics);
+
+}  // namespace sync_wifi
+
+}  // namespace chromeos
+
+#endif  // CHROMEOS_COMPONENTS_SYNC_WIFI_NETWORK_TYPE_CONVERSIONS_H_

chromeos/components/sync_wifi/pending_network_configuration_tracker.h

@@ -28,9 +28,8 @@ class PendingNetworkConfigurationTracker {
   // Adds an update to the list of in flight changes.  |change_uuid| is a
   // unique identifier for each update, |id| is the identifier for the network
   // which is getting updated, and |specifics| should be nullopt if the network
-  // is being deleted.
-  virtual void TrackPendingUpdate(
-      const std::string& change_guid,
+  // is being deleted.  Returns the change_guid.
+  virtual std::string TrackPendingUpdate(
       const NetworkIdentifier& id,
       const base::Optional<sync_pb::WifiConfigurationSpecificsData>&
           specifics) = 0;

chromeos/components/sync_wifi/pending_network_configuration_tracker_impl.cc

@@ -4,6 +4,7 @@
 
 #include "chromeos/components/sync_wifi/pending_network_configuration_tracker_impl.h"
 
+#include "base/guid.h"
 #include "base/optional.h"
 #include "base/strings/stringprintf.h"
 #include "chromeos/components/sync_wifi/network_identifier.h"
@@ -66,8 +67,7 @@ PendingNetworkConfigurationTrackerImpl::PendingNetworkConfigurationTrackerImpl(
 PendingNetworkConfigurationTrackerImpl::
     ~PendingNetworkConfigurationTrackerImpl() = default;
 
-void PendingNetworkConfigurationTrackerImpl::TrackPendingUpdate(
-    const std::string& change_guid,
+std::string PendingNetworkConfigurationTrackerImpl::TrackPendingUpdate(
     const NetworkIdentifier& id,
     const base::Optional<sync_pb::WifiConfigurationSpecificsData>& specifics) {
   std::string serialized_specifics;
@@ -76,11 +76,15 @@ void PendingNetworkConfigurationTrackerImpl::TrackPendingUpdate(
   else
     CHECK(specifics->SerializeToString(&serialized_specifics));
 
+  std::string change_guid = base::GenerateGUID();
+
   dict_.SetPath(GeneratePath(id, kChangeGuidKey), base::Value(change_guid));
   dict_.SetPath(GeneratePath(id, kSpecificsKey),
                 base::Value(serialized_specifics));
   dict_.SetPath(GeneratePath(id, kCompletedAttemptsKey), base::Value(0));
   pref_service_->Set(kPendingNetworkConfigurationsPref, dict_);
+
+  return change_guid;
 }
 
 void PendingNetworkConfigurationTrackerImpl::MarkComplete(

chromeos/components/sync_wifi/pending_network_configuration_tracker_impl.h

@@ -28,8 +28,7 @@ class PendingNetworkConfigurationTrackerImpl
   static void RegisterProfilePrefs(PrefRegistrySimple* registry);
 
   // sync_wifi::PendingNetworkConfigurationTracker::
-  void TrackPendingUpdate(
-      const std::string& change_guid,
+  std::string TrackPendingUpdate(
       const NetworkIdentifier& id,
       const base::Optional<sync_pb::WifiConfigurationSpecificsData>& specifics)
       override;

chromeos/components/sync_wifi/pending_network_configuration_tracker_impl_unittest.cc

@@ -22,8 +22,6 @@ namespace {
 
 const char kFredSsid[] = "Fred";
 const char kMangoSsid[] = "Mango";
-const char kChangeGuid1[] = "change-1";
-const char kChangeGuid2[] = "change-2";
 
 const char kPendingNetworkConfigurationsPref[] =
     "sync_wifi.pending_network_configuration_updates";
@@ -100,102 +98,109 @@ class PendingNetworkConfigurationTrackerImplTest : public testing::Test {
 };
 
 TEST_F(PendingNetworkConfigurationTrackerImplTest, TestMarkComplete) {
-  tracker()->TrackPendingUpdate(kChangeGuid1, fred_network_id(),
-                                /*specifics=*/base::nullopt);
-  AssertTrackerHasMatchingUpdate(kChangeGuid1, fred_network_id());
+  std::string change_guid = tracker()->TrackPendingUpdate(
+      fred_network_id(), /*specifics=*/base::nullopt);
+  AssertTrackerHasMatchingUpdate(change_guid, fred_network_id());
   EXPECT_EQ(1u, GetPref()->DictSize());
-  EXPECT_TRUE(DoesPrefContainPendingUpdate(fred_network_id(), kChangeGuid1));
-  tracker()->MarkComplete(kChangeGuid1, fred_network_id());
-  EXPECT_FALSE(tracker()->GetPendingUpdate(kChangeGuid1, fred_network_id()));
+  EXPECT_TRUE(DoesPrefContainPendingUpdate(fred_network_id(), change_guid));
+  tracker()->MarkComplete(change_guid, fred_network_id());
+  EXPECT_FALSE(tracker()->GetPendingUpdate(change_guid, fred_network_id()));
   EXPECT_EQ(0u, GetPref()->DictSize());
 }
 
 TEST_F(PendingNetworkConfigurationTrackerImplTest, TestTwoChangesSameNetwork) {
-  tracker()->TrackPendingUpdate(kChangeGuid1, fred_network_id(),
+  std::string change_guid =
+      tracker()->TrackPendingUpdate(fred_network_id(),
                                     /*specifics=*/base::nullopt);
-  tracker()->IncrementCompletedAttempts(kChangeGuid1, fred_network_id());
-  AssertTrackerHasMatchingUpdate(kChangeGuid1, fred_network_id(),
+  tracker()->IncrementCompletedAttempts(change_guid, fred_network_id());
+  AssertTrackerHasMatchingUpdate(change_guid, fred_network_id(),
                                  /*completed_attempts=*/1);
   EXPECT_EQ(1u, GetPref()->DictSize());
   EXPECT_EQ(1, tracker()
-                   ->GetPendingUpdate(kChangeGuid1, fred_network_id())
+                   ->GetPendingUpdate(change_guid, fred_network_id())
                    ->completed_attempts());
 
-  tracker()->TrackPendingUpdate(kChangeGuid2, fred_network_id(),
+  std::string second_change_guid =
+      tracker()->TrackPendingUpdate(fred_network_id(),
                                     /*specifics=*/base::nullopt);
-  EXPECT_FALSE(tracker()->GetPendingUpdate(kChangeGuid1, fred_network_id()));
-  AssertTrackerHasMatchingUpdate(kChangeGuid2, fred_network_id());
+  EXPECT_FALSE(tracker()->GetPendingUpdate(change_guid, fred_network_id()));
+  AssertTrackerHasMatchingUpdate(second_change_guid, fred_network_id());
   EXPECT_EQ(0, tracker()
-                   ->GetPendingUpdate(kChangeGuid2, fred_network_id())
+                   ->GetPendingUpdate(second_change_guid, fred_network_id())
                    ->completed_attempts());
   EXPECT_EQ(1u, GetPref()->DictSize());
 }
 
 TEST_F(PendingNetworkConfigurationTrackerImplTest,
        TestTwoChangesDifferentNetworks) {
-  tracker()->TrackPendingUpdate(kChangeGuid1, fred_network_id(),
+  std::string change_guid =
+      tracker()->TrackPendingUpdate(fred_network_id(),
                                     /*specifics=*/base::nullopt);
-  AssertTrackerHasMatchingUpdate(kChangeGuid1, fred_network_id());
-  EXPECT_TRUE(DoesPrefContainPendingUpdate(fred_network_id(), kChangeGuid1));
+  AssertTrackerHasMatchingUpdate(change_guid, fred_network_id());
+  EXPECT_TRUE(DoesPrefContainPendingUpdate(fred_network_id(), change_guid));
   EXPECT_EQ(1u, GetPref()->DictSize());
-  tracker()->TrackPendingUpdate(kChangeGuid2, mango_network_id(),
+  std::string second_change_guid =
+      tracker()->TrackPendingUpdate(mango_network_id(),
                                     /*specifics=*/base::nullopt);
-  AssertTrackerHasMatchingUpdate(kChangeGuid1, fred_network_id());
-  AssertTrackerHasMatchingUpdate(kChangeGuid2, mango_network_id());
-  EXPECT_TRUE(DoesPrefContainPendingUpdate(fred_network_id(), kChangeGuid1));
-  EXPECT_TRUE(DoesPrefContainPendingUpdate(mango_network_id(), kChangeGuid2));
+  AssertTrackerHasMatchingUpdate(change_guid, fred_network_id());
+  AssertTrackerHasMatchingUpdate(second_change_guid, mango_network_id());
+  EXPECT_TRUE(DoesPrefContainPendingUpdate(fred_network_id(), change_guid));
+  EXPECT_TRUE(
+      DoesPrefContainPendingUpdate(mango_network_id(), second_change_guid));
   EXPECT_EQ(2u, GetPref()->DictSize());
 }
 
 TEST_F(PendingNetworkConfigurationTrackerImplTest, TestGetPendingUpdates) {
-  tracker()->TrackPendingUpdate(kChangeGuid1, fred_network_id(),
+  std::string change_guid =
+      tracker()->TrackPendingUpdate(fred_network_id(),
                                     /*specifics=*/base::nullopt);
-  tracker()->TrackPendingUpdate(kChangeGuid2, mango_network_id(),
+  std::string second_change_guid =
+      tracker()->TrackPendingUpdate(mango_network_id(),
                                     /*specifics=*/base::nullopt);
   std::vector<PendingNetworkConfigurationUpdate> list =
       tracker()->GetPendingUpdates();
   EXPECT_EQ(2u, list.size());
-  EXPECT_EQ(kChangeGuid1, list[0].change_guid());
+  EXPECT_EQ(change_guid, list[0].change_guid());
   EXPECT_EQ(fred_network_id(), list[0].id());
-  EXPECT_EQ(kChangeGuid2, list[1].change_guid());
+  EXPECT_EQ(second_change_guid, list[1].change_guid());
   EXPECT_EQ(mango_network_id(), list[1].id());
 
-  tracker()->MarkComplete(kChangeGuid1, fred_network_id());
+  tracker()->MarkComplete(change_guid, fred_network_id());
   list = tracker()->GetPendingUpdates();
   EXPECT_EQ(1u, list.size());
-  EXPECT_EQ(kChangeGuid2, list[0].change_guid());
+  EXPECT_EQ(second_change_guid, list[0].change_guid());
   EXPECT_EQ(mango_network_id(), list[0].id());
 }
 
 TEST_F(PendingNetworkConfigurationTrackerImplTest, TestGetPendingUpdate) {
   sync_pb::WifiConfigurationSpecificsData specifics =
       GenerateTestWifiSpecifics(fred_network_id());
-  tracker()->TrackPendingUpdate(kChangeGuid1, fred_network_id(), specifics);
+  std::string change_guid =
+      tracker()->TrackPendingUpdate(fred_network_id(), specifics);
 
-  AssertTrackerHasMatchingUpdate(kChangeGuid1, fred_network_id(),
+  AssertTrackerHasMatchingUpdate(change_guid, fred_network_id(),
                                  /*completed_attempts=*/0, specifics);
-
-  EXPECT_FALSE(tracker()->GetPendingUpdate(kChangeGuid2, mango_network_id()));
 }
 
 TEST_F(PendingNetworkConfigurationTrackerImplTest, TestRetryCounting) {
-  tracker()->TrackPendingUpdate(kChangeGuid1, fred_network_id(),
+  std::string change_guid =
+      tracker()->TrackPendingUpdate(fred_network_id(),
                                     /*specifics=*/base::nullopt);
-  AssertTrackerHasMatchingUpdate(kChangeGuid1, fred_network_id());
+  AssertTrackerHasMatchingUpdate(change_guid, fred_network_id());
   EXPECT_EQ(1u, GetPref()->DictSize());
   EXPECT_EQ(0, tracker()
-                   ->GetPendingUpdate(kChangeGuid1, fred_network_id())
+                   ->GetPendingUpdate(change_guid, fred_network_id())
                    ->completed_attempts());
-  tracker()->IncrementCompletedAttempts(kChangeGuid1, fred_network_id());
-  tracker()->IncrementCompletedAttempts(kChangeGuid1, fred_network_id());
-  tracker()->IncrementCompletedAttempts(kChangeGuid1, fred_network_id());
+  tracker()->IncrementCompletedAttempts(change_guid, fred_network_id());
+  tracker()->IncrementCompletedAttempts(change_guid, fred_network_id());
+  tracker()->IncrementCompletedAttempts(change_guid, fred_network_id());
   EXPECT_EQ(3, tracker()
-                   ->GetPendingUpdate(kChangeGuid1, fred_network_id())
+                   ->GetPendingUpdate(change_guid, fred_network_id())
                    ->completed_attempts());
-  tracker()->IncrementCompletedAttempts(kChangeGuid1, fred_network_id());
-  tracker()->IncrementCompletedAttempts(kChangeGuid1, fred_network_id());
+  tracker()->IncrementCompletedAttempts(change_guid, fred_network_id());
+  tracker()->IncrementCompletedAttempts(change_guid, fred_network_id());
   EXPECT_EQ(5, tracker()
-                   ->GetPendingUpdate(kChangeGuid1, fred_network_id())
+                   ->GetPendingUpdate(change_guid, fred_network_id())
                    ->completed_attempts());
 }
 

chromeos/components/sync_wifi/pending_network_configuration_update.cc

@@ -24,6 +24,9 @@ PendingNetworkConfigurationUpdate::PendingNetworkConfigurationUpdate(
 PendingNetworkConfigurationUpdate::~PendingNetworkConfigurationUpdate() =
     default;
 
+PendingNetworkConfigurationUpdate& PendingNetworkConfigurationUpdate::operator=(
+    PendingNetworkConfigurationUpdate& update) = default;
+
 bool PendingNetworkConfigurationUpdate::IsDeleteOperation() const {
   return !specifics_.has_value();
 }

chromeos/components/sync_wifi/pending_network_configuration_update.h

@@ -28,6 +28,8 @@ class PendingNetworkConfigurationUpdate {
       int completed_attempts);
   PendingNetworkConfigurationUpdate(
       const PendingNetworkConfigurationUpdate& update);
+  PendingNetworkConfigurationUpdate& operator=(
+      PendingNetworkConfigurationUpdate& update);
   virtual ~PendingNetworkConfigurationUpdate();
 
   // The identifier for the network.
@@ -49,9 +51,9 @@ class PendingNetworkConfigurationUpdate {
   bool IsDeleteOperation() const;
 
  private:
-  const NetworkIdentifier id_;
-  const std::string change_guid_;
-  const base::Optional<sync_pb::WifiConfigurationSpecificsData> specifics_;
+  NetworkIdentifier id_;
+  std::string change_guid_;
+  base::Optional<sync_pb::WifiConfigurationSpecificsData> specifics_;
   int completed_attempts_;
 };
 

chromeos/components/sync_wifi/synced_network_updater_impl.cc

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chromeos/components/sync_wifi/synced_network_updater_impl.h"
+
+#include "base/bind.h"
+#include "base/guid.h"
+#include "base/values.h"
+#include "chromeos/components/sync_wifi/network_type_conversions.h"
+#include "chromeos/network/network_configuration_handler.h"
+#include "chromeos/network/network_profile_handler.h"
+#include "chromeos/network/network_state.h"
+#include "components/device_event_log/device_event_log.h"
+#include "components/proxy_config/proxy_config_dictionary.h"
+#include "third_party/cros_system_api/dbus/shill/dbus-constants.h"
+
+namespace chromeos {
+
+namespace sync_wifi {
+
+SyncedNetworkUpdaterImpl::SyncedNetworkUpdaterImpl(
+    std::unique_ptr<PendingNetworkConfigurationTracker> tracker,
+    network_config::mojom::CrosNetworkConfig* cros_network_config)
+    : tracker_(std::move(tracker)), cros_network_config_(cros_network_config) {
+  cros_network_config_->AddObserver(
+      cros_network_config_observer_receiver_.BindNewPipeAndPassRemote());
+  // Load the current list of networks.
+  OnNetworkStateListChanged();
+}
+
+SyncedNetworkUpdaterImpl::~SyncedNetworkUpdaterImpl() = default;
+
+void SyncedNetworkUpdaterImpl::AddOrUpdateNetwork(
+    const sync_pb::WifiConfigurationSpecificsData& specifics) {
+  auto id = NetworkIdentifier::FromProto(specifics);
+  network_config::mojom::NetworkStatePropertiesPtr existing_network =
+      FindLocalNetwork(id);
+  std::string change_guid = tracker_->TrackPendingUpdate(id, specifics);
+  network_config::mojom::ConfigPropertiesPtr config =
+      MojoNetworkConfigFromProto(specifics);
+
+  if (existing_network) {
+    cros_network_config_->SetProperties(
+        existing_network->guid, std::move(config),
+        base::BindOnce(&SyncedNetworkUpdaterImpl::OnSetPropertiesResult,
+                       weak_ptr_factory_.GetWeakPtr(), change_guid, id));
+    return;
+  }
+  cros_network_config_->ConfigureNetwork(
+      std::move(config), /* shared= */ false,
+      base::BindOnce(&SyncedNetworkUpdaterImpl::OnConfigureNetworkResult,
+                     weak_ptr_factory_.GetWeakPtr(), change_guid, id));
+}
+
+void SyncedNetworkUpdaterImpl::RemoveNetwork(const NetworkIdentifier& id) {
+  network_config::mojom::NetworkStatePropertiesPtr network =
+      FindLocalNetwork(id);
+  if (!network)
+    return;
+
+  std::string change_guid =
+      tracker_->TrackPendingUpdate(id, /*specifics=*/base::nullopt);
+
+  cros_network_config_->ForgetNetwork(
+      network->guid,
+      base::BindOnce(&SyncedNetworkUpdaterImpl::OnForgetNetworkResult,
+                     weak_ptr_factory_.GetWeakPtr(), change_guid, id));
+}
+
+network_config::mojom::NetworkStatePropertiesPtr
+SyncedNetworkUpdaterImpl::FindLocalNetwork(const NetworkIdentifier& id) {
+  for (const network_config::mojom::NetworkStatePropertiesPtr& network :
+       networks_) {
+    if (id == NetworkIdentifier::FromMojoNetwork(network))
+      return network.Clone();
+  }
+  return nullptr;
+}
+
+void SyncedNetworkUpdaterImpl::OnNetworkStateListChanged() {
+  cros_network_config_->GetNetworkStateList(
+      network_config::mojom::NetworkFilter::New(
+          network_config::mojom::FilterType::kConfigured,
+          network_config::mojom::NetworkType::kWiFi,
+          /* limit= */ 0),
+      base::BindOnce(&SyncedNetworkUpdaterImpl::OnGetNetworkList,
+                     base::Unretained(this)));
+}
+
+void SyncedNetworkUpdaterImpl::OnGetNetworkList(
+    std::vector<network_config::mojom::NetworkStatePropertiesPtr> networks) {
+  networks_ = std::move(networks);
+}
+
+void SyncedNetworkUpdaterImpl::OnError(const std::string& change_guid,
+                                       const NetworkIdentifier& id,
+                                       const std::string& error_name) {
+  NET_LOG(ERROR) << "Failed to update id:" << id.SerializeToString()
+                 << " error:" << error_name;
+}
+
+void SyncedNetworkUpdaterImpl::OnConfigureNetworkResult(
+    const std::string& change_guid,
+    const NetworkIdentifier& id,
+    const base::Optional<std::string>& guid,
+    const std::string& error_message) {
+  if (!guid) {
+    OnError(change_guid, id, "Failed to configure network.");
+    return;
+  }
+  VLOG(1) << "Successfully updated network with id " << id.SerializeToString();
+  CleanupUpdate(change_guid, id);
+}
+
+void SyncedNetworkUpdaterImpl::OnSetPropertiesResult(
+    const std::string& change_guid,
+    const NetworkIdentifier& id,
+    bool success,
+    const std::string& error_message) {
+  if (!success) {
+    OnError(change_guid, id, "Failed to update properties on network.");
+    return;
+  }
+  VLOG(1) << "Successfully updated network with id " << id.SerializeToString();
+  CleanupUpdate(change_guid, id);
+}
+
+void SyncedNetworkUpdaterImpl::OnForgetNetworkResult(
+    const std::string& change_guid,
+    const NetworkIdentifier& id,
+    bool success) {
+  if (!success) {
+    OnError(change_guid, id, "Failed to remove network.");
+    return;
+  }
+
+  VLOG(1) << "Successfully deleted network with id " << id.SerializeToString();
+  CleanupUpdate(change_guid, id);
+}
+
+void SyncedNetworkUpdaterImpl::CleanupUpdate(const std::string& change_guid,
+                                             const NetworkIdentifier& id) {
+  tracker_->MarkComplete(change_guid, id);
+}
+
+}  // namespace sync_wifi
+
+}  // namespace chromeos

chromeos/components/sync_wifi/synced_network_updater_impl.h

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROMEOS_COMPONENTS_SYNC_WIFI_SYNCED_NETWORK_UPDATER_IMPL_H_
+#define CHROMEOS_COMPONENTS_SYNC_WIFI_SYNCED_NETWORK_UPDATER_IMPL_H_
+
+#include "base/memory/weak_ptr.h"
+#include "base/values.h"
+#include "chromeos/components/sync_wifi/network_identifier.h"
+#include "chromeos/components/sync_wifi/pending_network_configuration_tracker.h"
+#include "chromeos/components/sync_wifi/synced_network_updater.h"
+#include "chromeos/services/network_config/public/mojom/cros_network_config.mojom.h"
+#include "components/sync/protocol/model_type_state.pb.h"
+#include "mojo/public/cpp/bindings/receiver.h"
+
+namespace chromeos {
+
+namespace sync_wifi {
+
+// Implementation of SyncedNetworkUpdater. This class takes add/update/delete
+// requests from the sync backend and applies them to the local network stack
+// using chromeos::NetworkConfigurationHandler.
+class SyncedNetworkUpdaterImpl
+    : public SyncedNetworkUpdater,
+      public chromeos::network_config::mojom::CrosNetworkConfigObserver {
+ public:
+  // |cros_network_config| must outlive this class.
+  SyncedNetworkUpdaterImpl(
+      std::unique_ptr<PendingNetworkConfigurationTracker> tracker,
+      network_config::mojom::CrosNetworkConfig* cros_network_config);
+  ~SyncedNetworkUpdaterImpl() override;
+
+  void AddOrUpdateNetwork(
+      const sync_pb::WifiConfigurationSpecificsData& specifics) override;
+
+  void RemoveNetwork(const NetworkIdentifier& id) override;
+
+  // CrosNetworkConfigObserver:
+  void OnNetworkStateListChanged() override;
+  void OnActiveNetworksChanged(
+      std::vector<
+          network_config::mojom::NetworkStatePropertiesPtr> /* networks */)
+      override {}
+  void OnNetworkStateChanged(
+      chromeos::network_config::mojom::NetworkStatePropertiesPtr /* network */)
+      override {}
+  void OnDeviceStateListChanged() override {}
+  void OnVpnProvidersChanged() override {}
+  void OnNetworkCertificatesChanged() override {}
+
+ private:
+  void CleanupUpdate(const std::string& change_guid,
+                     const NetworkIdentifier& id);
+  network_config::mojom::NetworkStatePropertiesPtr FindLocalNetwork(
+      const NetworkIdentifier& id);
+
+  base::Optional<base::DictionaryValue> ConvertToDictionary(
+      const sync_pb::WifiConfigurationSpecificsData& specifics,
+      const std::string& guid);
+  void OnGetNetworkList(
+      std::vector<network_config::mojom::NetworkStatePropertiesPtr> networks);
+  void OnError(const std::string& change_guid,
+               const NetworkIdentifier& id,
+               const std::string& error_name);
+  void OnSetPropertiesResult(const std::string& change_guid,
+                             const NetworkIdentifier& id,
+                             bool success,
+                             const std::string& error_message);
+  void OnConfigureNetworkResult(const std::string& change_guid,
+                                const NetworkIdentifier& id,
+                                const base::Optional<std::string>& guid,
+                                const std::string& error_message);
+  void OnForgetNetworkResult(const std::string& change_guid,
+                             const NetworkIdentifier& id,
+                             bool success);
+
+  std::unique_ptr<PendingNetworkConfigurationTracker> tracker_;
+  network_config::mojom::CrosNetworkConfig* cros_network_config_;
+  mojo::Receiver<chromeos::network_config::mojom::CrosNetworkConfigObserver>
+      cros_network_config_observer_receiver_{this};
+  std::vector<network_config::mojom::NetworkStatePropertiesPtr> networks_;
+
+  base::WeakPtrFactory<SyncedNetworkUpdaterImpl> weak_ptr_factory_{this};
+};
+
+}  // namespace sync_wifi
+
+}  // namespace chromeos
+
+#endif  // CHROMEOS_COMPONENTS_SYNC_WIFI_SYNCED_NETWORK_UPDATER_IMPL_H_

chromeos/components/sync_wifi/synced_network_updater_impl_unittest.cc

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include <memory>
+
+#include "ash/public/cpp/network_config_service.h"
+#include "base/bind.h"
+#include "base/logging.h"
+#include "base/macros.h"
+#include "base/optional.h"
+#include "base/run_loop.h"
+#include "base/test/task_environment.h"
+#include "chromeos/components/sync_wifi/fake_pending_network_configuration_tracker.h"
+#include "chromeos/components/sync_wifi/network_identifier.h"
+#include "chromeos/components/sync_wifi/pending_network_configuration_tracker_impl.h"
+#include "chromeos/components/sync_wifi/synced_network_updater_impl.h"
+#include "chromeos/components/sync_wifi/test_data_generator.h"
+#include "chromeos/dbus/shill/fake_shill_simulated_result.h"
+#include "chromeos/dbus/shill/shill_clients.h"
+#include "chromeos/dbus/shill/shill_manager_client.h"
+#include "chromeos/dbus/shill/shill_profile_client.h"
+#include "chromeos/dbus/shill/shill_service_client.h"
+#include "chromeos/login/login_state/login_state.h"
+#include "chromeos/network/managed_network_configuration_handler.h"
+#include "chromeos/network/network_cert_loader.h"
+#include "chromeos/network/network_certificate_handler.h"
+#include "chromeos/network/network_configuration_handler.h"
+#include "chromeos/network/network_connection_handler.h"
+#include "chromeos/network/network_device_handler.h"
+#include "chromeos/network/network_handler.h"
+#include "chromeos/network/network_profile_handler.h"
+#include "chromeos/network/network_state_handler.h"
+#include "chromeos/services/network_config/cros_network_config.h"
+#include "chromeos/services/network_config/in_process_instance.h"
+#include "chromeos/services/network_config/public/cpp/cros_network_config_test_helper.h"
+#include "chromeos/services/network_config/public/mojom/cros_network_config.mojom.h"
+#include "components/user_manager/fake_user_manager.h"
+#include "components/user_manager/scoped_user_manager.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/cros_system_api/dbus/shill/dbus-constants.h"
+
+namespace chromeos {
+
+class NetworkDeviceHandler;
+class NetworkProfileHandler;
+class NetworkConfigurationHandler;
+class NetworkConnectionHandler;
+class ManagedNetworkConfigurationHandler;
+
+namespace sync_wifi {
+
+namespace {
+
+const char kFredSsid[] = "Fred";
+const char kMangoSsid[] = "Mango";
+
+const chromeos::NetworkState* FindLocalNetworkById(
+    const NetworkIdentifier& id) {
+  chromeos::NetworkStateHandler::NetworkStateList network_list;
+  chromeos::NetworkHandler::Get()
+      ->network_state_handler()
+      ->GetNetworkListByType(
+          chromeos::NetworkTypePattern::WiFi(), /* configured_only= */ true,
+          /* visible_only= */ false, /* limit= */ 0, &network_list);
+  for (const chromeos::NetworkState* network : network_list) {
+    if (network->GetHexSsid() == id.hex_ssid() &&
+        network->security_class() == id.security_type()) {
+      return network;
+    }
+  }
+  return nullptr;
+}
+
+}  // namespace
+
+class SyncedNetworkUpdaterImplTest : public testing::Test {
+ public:
+  SyncedNetworkUpdaterImplTest()
+      : task_environment_(base::test::TaskEnvironment::MainThreadType::DEFAULT,
+                          base::test::TaskEnvironment::TimeSource::MOCK_TIME) {
+    LoginState::Initialize();
+    network_state_helper_ = std::make_unique<NetworkStateTestHelper>(
+        /*use_default_devices_and_services=*/false);
+    network_device_handler_ =
+        chromeos::NetworkDeviceHandler::InitializeForTesting(
+            network_state_helper_->network_state_handler());
+    network_profile_handler_ = NetworkProfileHandler::InitializeForTesting();
+    network_device_handler_ = NetworkDeviceHandler::InitializeForTesting(
+        network_state_helper_->network_state_handler());
+    network_configuration_handler_ =
+        base::WrapUnique<NetworkConfigurationHandler>(
+            NetworkConfigurationHandler::InitializeForTest(
+                network_state_helper_->network_state_handler(),
+                network_device_handler_.get()));
+    managed_network_configuration_handler_ =
+        ManagedNetworkConfigurationHandler::InitializeForTesting(
+            network_state_helper_->network_state_handler(),
+            network_profile_handler_.get(), network_device_handler_.get(),
+            network_configuration_handler_.get());
+    managed_network_configuration_handler_->SetPolicy(
+        ::onc::ONC_SOURCE_DEVICE_POLICY,
+        /*userhash=*/std::string(),
+        /*network_configs_onc=*/base::ListValue(),
+        /*global_network_config=*/base::DictionaryValue());
+    cros_network_config_impl_ =
+        std::make_unique<chromeos::network_config::CrosNetworkConfig>(
+            network_state_helper_->network_state_handler(),
+            network_device_handler_.get(),
+            managed_network_configuration_handler_.get(),
+            network_connection_handler_.get(),
+            /*network_certificate_handler=*/nullptr);
+    OverrideInProcessInstanceForTesting(cros_network_config_impl_.get());
+
+    ash::GetNetworkConfigService(
+        remote_cros_network_config_.BindNewPipeAndPassReceiver());
+
+    auto fake_user_manager = std::make_unique<user_manager::FakeUserManager>();
+    scoped_user_manager_ = std::make_unique<user_manager::ScopedUserManager>(
+        std::move(fake_user_manager));
+  }
+
+  ~SyncedNetworkUpdaterImplTest() override {
+    LoginState::Shutdown();
+    network_config::OverrideInProcessInstanceForTesting(nullptr);
+  }
+
+  void SetUp() override {
+    testing::Test::SetUp();
+    NetworkHandler::Initialize();
+    network_state_helper()->ResetDevicesAndServices();
+    base::RunLoop().RunUntilIdle();
+
+    auto tracker_unique_ptr =
+        std::make_unique<FakePendingNetworkConfigurationTracker>();
+    tracker_ = tracker_unique_ptr.get();
+    updater_ = std::make_unique<SyncedNetworkUpdaterImpl>(
+        std::move(tracker_unique_ptr), remote_cros_network_config_.get());
+  }
+
+  void TearDown() override {
+    chromeos::NetworkHandler::Shutdown();
+    testing::Test::TearDown();
+  }
+
+  FakePendingNetworkConfigurationTracker* tracker() { return tracker_; }
+  SyncedNetworkUpdaterImpl* updater() { return updater_.get(); }
+  chromeos::NetworkStateTestHelper* network_state_helper() {
+    return network_state_helper_.get();
+  }
+  NetworkIdentifier fred_network_id() { return fred_network_id_; }
+  NetworkIdentifier mango_network_id() { return mango_network_id_; }
+
+ private:
+  base::test::TaskEnvironment task_environment_;
+  FakePendingNetworkConfigurationTracker* tracker_;
+  std::unique_ptr<NetworkStateTestHelper> network_state_helper_;
+  std::unique_ptr<SyncedNetworkUpdaterImpl> updater_;
+  std::unique_ptr<network_config::CrosNetworkConfig> cros_network_config_impl_;
+  std::unique_ptr<NetworkProfileHandler> network_profile_handler_;
+  std::unique_ptr<NetworkDeviceHandler> network_device_handler_;
+  std::unique_ptr<NetworkConfigurationHandler> network_configuration_handler_;
+  std::unique_ptr<ManagedNetworkConfigurationHandler>
+      managed_network_configuration_handler_;
+  std::unique_ptr<NetworkConnectionHandler> network_connection_handler_;
+  mojo::Remote<chromeos::network_config::mojom::CrosNetworkConfig>
+      remote_cros_network_config_;
+  std::unique_ptr<user_manager::ScopedUserManager> scoped_user_manager_;
+
+  NetworkIdentifier fred_network_id_ = GeneratePskNetworkId(kFredSsid);
+  NetworkIdentifier mango_network_id_ = GeneratePskNetworkId(kMangoSsid);
+
+  DISALLOW_COPY_AND_ASSIGN(SyncedNetworkUpdaterImplTest);
+};
+
+TEST_F(SyncedNetworkUpdaterImplTest, TestAdd_OneNetwork) {
+  sync_pb::WifiConfigurationSpecificsData specifics =
+      GenerateTestWifiSpecifics(fred_network_id());
+  NetworkIdentifier id = NetworkIdentifier::FromProto(specifics);
+  updater()->AddOrUpdateNetwork(specifics);
+  EXPECT_TRUE(tracker()->GetPendingUpdateById(id));
+  base::RunLoop().RunUntilIdle();
+  EXPECT_TRUE(FindLocalNetworkById(id));
+  EXPECT_FALSE(tracker()->GetPendingUpdateById(id));
+}
+
+TEST_F(SyncedNetworkUpdaterImplTest, TestAdd_ThenRemove) {
+  EXPECT_FALSE(FindLocalNetworkById(fred_network_id()));
+  updater()->AddOrUpdateNetwork(GenerateTestWifiSpecifics(fred_network_id()));
+  EXPECT_TRUE(tracker()->GetPendingUpdateById(fred_network_id()));
+  base::RunLoop().RunUntilIdle();
+  EXPECT_TRUE(FindLocalNetworkById(fred_network_id()));
+  updater()->RemoveNetwork(fred_network_id());
+  PendingNetworkConfigurationUpdate* update =
+      tracker()->GetPendingUpdateById(fred_network_id());
+  EXPECT_TRUE(update);
+  EXPECT_FALSE(update->specifics());
+  base::RunLoop().RunUntilIdle();
+  EXPECT_FALSE(tracker()->GetPendingUpdateById(fred_network_id()));
+  EXPECT_FALSE(FindLocalNetworkById(fred_network_id()));
+}
+
+TEST_F(SyncedNetworkUpdaterImplTest, TestAdd_TwoNetworks) {
+  updater()->AddOrUpdateNetwork(GenerateTestWifiSpecifics(fred_network_id()));
+  updater()->AddOrUpdateNetwork(GenerateTestWifiSpecifics(mango_network_id()));
+  EXPECT_TRUE(tracker()->GetPendingUpdateById(fred_network_id()));
+  EXPECT_TRUE(tracker()->GetPendingUpdateById(mango_network_id()));
+  base::RunLoop().RunUntilIdle();
+  EXPECT_TRUE(FindLocalNetworkById(fred_network_id()));
+  EXPECT_TRUE(FindLocalNetworkById(mango_network_id()));
+  EXPECT_FALSE(tracker()->GetPendingUpdateById(fred_network_id()));
+  EXPECT_FALSE(tracker()->GetPendingUpdateById(mango_network_id()));
+}
+
+TEST_F(SyncedNetworkUpdaterImplTest, TestFailToAdd_Error) {
+  network_state_helper()->manager_test()->SetSimulateConfigurationResult(
+      chromeos::FakeShillSimulatedResult::kFailure);
+
+  updater()->AddOrUpdateNetwork(GenerateTestWifiSpecifics(fred_network_id()));
+  EXPECT_TRUE(tracker()->GetPendingUpdateById(fred_network_id()));
+
+  base::RunLoop().RunUntilIdle();
+
+  EXPECT_FALSE(FindLocalNetworkById(fred_network_id()));
+  EXPECT_TRUE(tracker()->GetPendingUpdateById(fred_network_id()));
+}
+
+TEST_F(SyncedNetworkUpdaterImplTest, TestFailToRemove) {
+  network_state_helper()->profile_test()->SetSimulateDeleteResult(
+      chromeos::FakeShillSimulatedResult::kFailure);
+  updater()->AddOrUpdateNetwork(GenerateTestWifiSpecifics(fred_network_id()));
+  base::RunLoop().RunUntilIdle();
+  EXPECT_TRUE(FindLocalNetworkById(fred_network_id()));
+  EXPECT_FALSE(tracker()->GetPendingUpdateById(fred_network_id()));
+
+  updater()->RemoveNetwork(fred_network_id());
+  EXPECT_TRUE(tracker()->GetPendingUpdateById(fred_network_id()));
+
+  base::RunLoop().RunUntilIdle();
+
+  EXPECT_TRUE(FindLocalNetworkById(fred_network_id()));
+  EXPECT_TRUE(tracker()->GetPendingUpdateById(fred_network_id()));
+}
+
+}  // namespace sync_wifi
+
+}  // namespace chromeos

chromeos/components/sync_wifi/wifi_configuration_sync_service.cc

@@ -6,9 +6,13 @@
 
 #include <utility>
 
+#include "ash/public/cpp/network_config_service.h"
 #include "base/bind_helpers.h"
 #include "base/time/default_clock.h"
+#include "chromeos/components/sync_wifi/pending_network_configuration_tracker_impl.h"
+#include "chromeos/components/sync_wifi/synced_network_updater_impl.h"
 #include "chromeos/components/sync_wifi/wifi_configuration_bridge.h"
+#include "chromeos/services/network_config/public/mojom/cros_network_config.mojom.h"
 #include "components/sync/base/report_unrecoverable_error.h"
 #include "components/sync/model/model_type_store.h"
 #include "components/sync/model_impl/client_tag_based_model_type_processor.h"
@@ -19,9 +23,15 @@ namespace sync_wifi {
 
 WifiConfigurationSyncService::WifiConfigurationSyncService(
     version_info::Channel channel,
+    PrefService* pref_service,
     syncer::OnceModelTypeStoreFactory create_store_callback) {
+  ash::GetNetworkConfigService(
+      remote_cros_network_config_.BindNewPipeAndPassReceiver());
+  updater_ = std::make_unique<SyncedNetworkUpdaterImpl>(
+      std::make_unique<PendingNetworkConfigurationTrackerImpl>(pref_service),
+      remote_cros_network_config_.get());
   bridge_ = std::make_unique<sync_wifi::WifiConfigurationBridge>(
-      /* synced_network_updater= */ nullptr,
+      updater_.get(),
       std::make_unique<syncer::ClientTagBasedModelTypeProcessor>(
           syncer::WIFI_CONFIGURATIONS,
           base::BindRepeating(&syncer::ReportUnrecoverableError, channel)),

chromeos/components/sync_wifi/wifi_configuration_sync_service.h

@@ -9,10 +9,13 @@
 #include <string>
 
 #include "base/memory/weak_ptr.h"
+#include "chromeos/services/network_config/public/mojom/cros_network_config.mojom.h"
 #include "components/keyed_service/core/keyed_service.h"
+#include "components/prefs/pref_service.h"
 #include "components/sync/model/model_type_store.h"
 #include "components/sync/model/model_type_store_service.h"
 #include "components/version_info/channel.h"
+#include "mojo/public/cpp/bindings/remote.h"
 
 namespace syncer {
 class ModelTypeControllerDelegate;
@@ -22,6 +25,7 @@ namespace chromeos {
 
 namespace sync_wifi {
 
+class SyncedNetworkUpdaterImpl;
 class WifiConfigurationBridge;
 
 // A profile keyed service which instantiates and provides access to an instance
@@ -30,6 +34,7 @@ class WifiConfigurationSyncService : public KeyedService {
  public:
   WifiConfigurationSyncService(
       version_info::Channel channel,
+      PrefService* pref_service,
       syncer::OnceModelTypeStoreFactory create_store_callback);
   ~WifiConfigurationSyncService() override;
 
@@ -37,6 +42,9 @@ class WifiConfigurationSyncService : public KeyedService {
 
  private:
   std::unique_ptr<WifiConfigurationBridge> bridge_;
+  std::unique_ptr<SyncedNetworkUpdaterImpl> updater_;
+  mojo::Remote<chromeos::network_config::mojom::CrosNetworkConfig>
+      remote_cros_network_config_;
 
   DISALLOW_COPY_AND_ASSIGN(WifiConfigurationSyncService);
 };

chromeos/dbus/shill/fake_shill_manager_client.cc

@@ -369,8 +369,20 @@ void FakeShillManagerClient::ConfigureService(
     return;
   }
 
-  if (type == shill::kTypeWifi)
+  if (type == shill::kTypeWifi) {
     properties.GetString(shill::kSSIDProperty, &name);
+
+    if (name.empty()) {
+      std::string hex_name;
+      properties.GetString(shill::kWifiHexSsid, &hex_name);
+      if (!hex_name.empty()) {
+        std::vector<uint8_t> bytes;
+        if (base::HexStringToBytes(hex_name, &bytes)) {
+          name.assign(reinterpret_cast<const char*>(&bytes[0]), bytes.size());
+        }
+      }
+    }
+  }
   if (name.empty())
     properties.GetString(shill::kNameProperty, &name);
   if (name.empty())