Record that a network was added by sync to NetworkMetadataStore.

After a network is successfully added or updated by Chrome Sync
ensure that the IsAddedBySync bit is appropriately set in the
metadata store.

Bug: 966270
Change-Id: I3eec02cf6390cf42f160136714c74b385f5640ce
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2088175Reviewed-by: Azeem Arshad <azeemarshad@chromium.org>
Commit-Queue: Jon Mann <jonmann@chromium.org>
Cr-Commit-Position: refs/heads/master@{#747047}

chromeos/components/sync_wifi/network_test_helper.cc

@@ -9,6 +9,7 @@
 #include "chromeos/components/sync_wifi/network_type_conversions.h"
 #include "chromeos/login/login_state/login_state.h"
 #include "chromeos/network/network_handler.h"
+#include "chromeos/network/network_metadata_store.h"
 #include "chromeos/services/network_config/in_process_instance.h"
 #include "components/onc/onc_pref_names.h"
 #include "components/proxy_config/pref_proxy_config_tracker_impl.h"
@@ -26,6 +27,8 @@ NetworkTestHelper::NetworkTestHelper()
   PrefProxyConfigTrackerImpl::RegisterPrefs(local_state_.registry());
   ::onc::RegisterProfilePrefs(user_prefs_.registry());
   ::onc::RegisterPrefs(local_state_.registry());
+  NetworkMetadataStore::RegisterPrefs(user_prefs_.registry());
+  NetworkMetadataStore::RegisterPrefs(local_state_.registry());
 
   network_profile_handler_ = NetworkProfileHandler::InitializeForTesting();
   network_configuration_handler_ =
@@ -65,6 +68,7 @@ NetworkTestHelper::~NetworkTestHelper() {
 
 void NetworkTestHelper::SetUp() {
   NetworkHandler::Initialize();
+  NetworkHandler::Get()->InitializePrefServices(&user_prefs_, &local_state_);
   network_state_helper_->ResetDevicesAndServices();
 
   base::RunLoop().RunUntilIdle();
@@ -89,6 +93,10 @@ void NetworkTestHelper::ConfigureWiFiNetwork(const std::string& ssid,
   base::RunLoop().RunUntilIdle();
 }
 
+NetworkStateTestHelper* NetworkTestHelper::network_state_test_helper() {
+  return network_state_helper_.get();
+}
+
 }  // namespace sync_wifi
 
 }  // namespace chromeos
\ No newline at end of file

chromeos/components/sync_wifi/network_test_helper.h

@@ -33,6 +33,8 @@ class NetworkTestHelper : public network_config::CrosNetworkConfigTestHelper {
                             bool is_secured,
                             bool in_profile);
 
+  NetworkStateTestHelper* network_state_test_helper();
+
  private:
   std::unique_ptr<NetworkProfileHandler> network_profile_handler_;
   std::unique_ptr<NetworkConfigurationHandler> network_configuration_handler_;

chromeos/components/sync_wifi/synced_network_updater_impl.cc

@@ -10,6 +10,8 @@
 #include "chromeos/components/sync_wifi/network_type_conversions.h"
 #include "chromeos/components/sync_wifi/timer_factory.h"
 #include "chromeos/network/network_configuration_handler.h"
+#include "chromeos/network/network_handler.h"
+#include "chromeos/network/network_metadata_store.h"
 #include "chromeos/network/network_profile_handler.h"
 #include "chromeos/network/network_state.h"
 #include "components/device_event_log/device_event_log.h"
@@ -69,7 +71,8 @@ void SyncedNetworkUpdaterImpl::StartAddOrUpdateOperation(
     cros_network_config_->SetProperties(
         existing_network->guid, std::move(config),
         base::BindOnce(&SyncedNetworkUpdaterImpl::OnSetPropertiesResult,
-                       weak_ptr_factory_.GetWeakPtr(), change_guid, id));
+                       weak_ptr_factory_.GetWeakPtr(), change_guid,
+                       existing_network->guid, id));
     return;
   }
 
@@ -136,26 +139,31 @@ void SyncedNetworkUpdaterImpl::OnError(const std::string& change_guid,
 void SyncedNetworkUpdaterImpl::OnConfigureNetworkResult(
     const std::string& change_guid,
     const NetworkIdentifier& id,
-    const base::Optional<std::string>& guid,
+    const base::Optional<std::string>& network_guid,
     const std::string& error_message) {
-  if (guid) {
+  if (network_guid) {
     VLOG(1) << "Successfully configured network with id "
             << id.SerializeToString();
+    NetworkHandler::Get()->network_metadata_store()->SetIsConfiguredBySync(
+        *network_guid);
   } else {
     NET_LOG(ERROR) << "Failed to configure network with id "
                    << id.SerializeToString() << ". " << error_message;
   }
-  HandleShillResult(change_guid, id, guid.has_value());
+  HandleShillResult(change_guid, id, network_guid.has_value());
 }
 
 void SyncedNetworkUpdaterImpl::OnSetPropertiesResult(
     const std::string& change_guid,
+    const std::string& network_guid,
     const NetworkIdentifier& id,
     bool is_success,
     const std::string& error_message) {
   if (is_success) {
     VLOG(1) << "Successfully updated network with id "
             << id.SerializeToString();
+    NetworkHandler::Get()->network_metadata_store()->SetIsConfiguredBySync(
+        network_guid);
   } else {
     NET_LOG(ERROR) << "Failed to update network with id "
                    << id.SerializeToString();

chromeos/components/sync_wifi/synced_network_updater_impl.h

@@ -80,12 +80,13 @@ class SyncedNetworkUpdaterImpl
                const NetworkIdentifier& id,
                const std::string& error_name);
   void OnSetPropertiesResult(const std::string& change_guid,
+                             const std::string& network_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 base::Optional<std::string>& network_guid,
                                 const std::string& error_message);
   void OnForgetNetworkResult(const std::string& change_guid,
                              const NetworkIdentifier& id,

chromeos/components/sync_wifi/synced_network_updater_impl_unittest.cc

@@ -14,30 +14,17 @@
 #include "chromeos/components/sync_wifi/fake_pending_network_configuration_tracker.h"
 #include "chromeos/components/sync_wifi/fake_timer_factory.h"
 #include "chromeos/components/sync_wifi/network_identifier.h"
+#include "chromeos/components/sync_wifi/network_test_helper.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/network/network_metadata_store.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"
 
@@ -80,56 +67,16 @@ class SyncedNetworkUpdaterImplTest : public testing::Test {
   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(),
-            nullptr /* ui_proxy_config_service */);
-    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());
-
+    local_test_helper_ = std::make_unique<NetworkTestHelper>();
     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);
-  }
+  ~SyncedNetworkUpdaterImplTest() override { local_test_helper_.reset(); }
 
   void SetUp() override {
     testing::Test::SetUp();
-    NetworkHandler::Initialize();
+    local_test_helper_->SetUp();
     network_state_helper()->ResetDevicesAndServices();
     base::RunLoop().RunUntilIdle();
 
@@ -152,27 +99,19 @@ class SyncedNetworkUpdaterImplTest : public testing::Test {
   FakeTimerFactory* timer_factory() { return timer_factory_; }
   SyncedNetworkUpdaterImpl* updater() { return updater_.get(); }
   chromeos::NetworkStateTestHelper* network_state_helper() {
-    return network_state_helper_.get();
+    return local_test_helper_->network_state_test_helper();
   }
   NetworkIdentifier fred_network_id() { return fred_network_id_; }
   NetworkIdentifier mango_network_id() { return mango_network_id_; }
 
  private:
   base::test::TaskEnvironment task_environment_;
+  std::unique_ptr<NetworkTestHelper> local_test_helper_;
   FakeTimerFactory* timer_factory_;
   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);
@@ -187,8 +126,12 @@ TEST_F(SyncedNetworkUpdaterImplTest, TestAdd_OneNetwork) {
   updater()->AddOrUpdateNetwork(specifics);
   EXPECT_TRUE(tracker()->GetPendingUpdateById(id));
   base::RunLoop().RunUntilIdle();
-  EXPECT_TRUE(FindLocalNetworkById(id));
+  const chromeos::NetworkState* network = FindLocalNetworkById(id);
+  EXPECT_TRUE(network);
   EXPECT_FALSE(tracker()->GetPendingUpdateById(id));
+  EXPECT_TRUE(
+      NetworkHandler::Get()->network_metadata_store()->GetIsConfiguredBySync(
+          network->guid()));
 }
 
 TEST_F(SyncedNetworkUpdaterImplTest, TestAdd_ThenRemove) {
@@ -213,8 +156,20 @@ TEST_F(SyncedNetworkUpdaterImplTest, TestAdd_TwoNetworks) {
   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()));
+
+  const chromeos::NetworkState* fred_network =
+      FindLocalNetworkById(fred_network_id());
+  const chromeos::NetworkState* mango_network =
+      FindLocalNetworkById(mango_network_id());
+  EXPECT_TRUE(fred_network);
+  EXPECT_TRUE(mango_network);
+  EXPECT_TRUE(
+      NetworkHandler::Get()->network_metadata_store()->GetIsConfiguredBySync(
+          fred_network->guid()));
+  EXPECT_TRUE(
+      NetworkHandler::Get()->network_metadata_store()->GetIsConfiguredBySync(
+          mango_network->guid()));
+
   EXPECT_FALSE(tracker()->GetPendingUpdateById(fred_network_id()));
   EXPECT_FALSE(tracker()->GetPendingUpdateById(mango_network_id()));
 }