[WiFi-Sync] Merge local and remote networks when sync is enabled.

This implements a simple merge of the local networks with the
remote list of synced networks.

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

chromeos/components/sync_wifi/BUILD.gn

@@ -45,6 +45,8 @@ static_library("sync_wifi") {
 source_set("test_support") {
   testonly = true
   sources = [
+    "fake_local_network_collector.cc",
+    "fake_local_network_collector.h",
     "fake_one_shot_timer.cc",
     "fake_one_shot_timer.h",
     "fake_pending_network_configuration_tracker.cc",

chromeos/components/sync_wifi/fake_local_network_collector.cc

+// Copyright 2020 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_local_network_collector.h"
+
+#include "components/sync/protocol/wifi_configuration_specifics.pb.h"
+
+namespace chromeos {
+
+namespace sync_wifi {
+
+FakeLocalNetworkCollector::FakeLocalNetworkCollector() = default;
+
+FakeLocalNetworkCollector::~FakeLocalNetworkCollector() = default;
+
+void FakeLocalNetworkCollector::GetAllSyncableNetworks(
+    ProtoListCallback callback) {
+  std::move(callback).Run(networks_);
+}
+
+void FakeLocalNetworkCollector::GetSyncableNetwork(const NetworkIdentifier& id,
+                                                   ProtoCallback callback) {
+  for (sync_pb::WifiConfigurationSpecifics proto : networks_) {
+    if (NetworkIdentifier::FromProto(proto) == id) {
+      std::move(callback).Run(proto);
+      return;
+    }
+  }
+
+  std::move(callback).Run(base::nullopt);
+}
+
+void FakeLocalNetworkCollector::AddNetwork(
+    sync_pb::WifiConfigurationSpecifics proto) {
+  networks_.push_back(proto);
+}
+
+void FakeLocalNetworkCollector::ClearNetworks() {
+  networks_.clear();
+}
+
+}  // namespace sync_wifi
+
+}  // namespace chromeos
\ No newline at end of file

chromeos/components/sync_wifi/fake_local_network_collector.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_LOCAL_NETWORK_COLLECTOR_H_
+#define CHROMEOS_COMPONENTS_SYNC_WIFI_FAKE_LOCAL_NETWORK_COLLECTOR_H_
+
+#include <map>
+
+#include "base/containers/flat_map.h"
+#include "base/optional.h"
+#include "chromeos/components/sync_wifi/local_network_collector.h"
+#include "chromeos/components/sync_wifi/network_identifier.h"
+
+namespace chromeos {
+
+namespace sync_wifi {
+
+// Test implementation of LocalNetworkCollector.
+class FakeLocalNetworkCollector : public LocalNetworkCollector {
+ public:
+  FakeLocalNetworkCollector();
+  ~FakeLocalNetworkCollector() override;
+
+  // sync_wifi::LocalNetworkCollector::
+  void GetAllSyncableNetworks(ProtoListCallback callback) override;
+  void GetSyncableNetwork(const NetworkIdentifier& id,
+                          ProtoCallback callback) override;
+
+  void AddNetwork(sync_pb::WifiConfigurationSpecifics proto);
+  void ClearNetworks();
+
+ private:
+  std::vector<sync_pb::WifiConfigurationSpecifics> networks_;
+
+  DISALLOW_COPY_AND_ASSIGN(FakeLocalNetworkCollector);
+};
+
+}  // namespace sync_wifi
+
+}  // namespace chromeos
+
+#endif  // CHROMEOS_COMPONENTS_SYNC_WIFI_FAKE_LOCAL_NETWORK_COLLECTOR_H_

chromeos/components/sync_wifi/wifi_configuration_bridge.cc

@@ -15,6 +15,7 @@
 #include "base/time/time.h"
 #include "chromeos/components/sync_wifi/network_identifier.h"
 #include "chromeos/components/sync_wifi/synced_network_updater.h"
+#include "components/device_event_log/device_event_log.h"
 #include "components/sync/model/entity_change.h"
 #include "components/sync/model/metadata_batch.h"
 #include "components/sync/model/metadata_change_list.h"
@@ -39,10 +40,12 @@ std::unique_ptr<syncer::EntityData> GenerateWifiEntityData(
 
 WifiConfigurationBridge::WifiConfigurationBridge(
     SyncedNetworkUpdater* synced_network_updater,
+    LocalNetworkCollector* local_network_collector,
     std::unique_ptr<syncer::ModelTypeChangeProcessor> change_processor,
     syncer::OnceModelTypeStoreFactory create_store_callback)
     : ModelTypeSyncBridge(std::move(change_processor)),
-      synced_network_updater_(synced_network_updater) {
+      synced_network_updater_(synced_network_updater),
+      local_network_collector_(local_network_collector) {
   std::move(create_store_callback)
       .Run(syncer::WIFI_CONFIGURATIONS,
            base::BindOnce(&WifiConfigurationBridge::OnStoreCreated,
@@ -58,10 +61,35 @@ WifiConfigurationBridge::CreateMetadataChangeList() {
 
 base::Optional<syncer::ModelError> WifiConfigurationBridge::MergeSyncData(
     std::unique_ptr<syncer::MetadataChangeList> metadata_change_list,
-    syncer::EntityChangeList entity_data) {
+    syncer::EntityChangeList change_list) {
   DCHECK(entries_.empty());
-  return ApplySyncChanges(std::move(metadata_change_list),
-                          std::move(entity_data));
+  DCHECK(local_network_collector_);
+
+  local_network_collector_->GetAllSyncableNetworks(
+      base::BindOnce(&WifiConfigurationBridge::OnGetAllSyncableNetworksResult,
+                     weak_ptr_factory_.GetWeakPtr(),
+                     std::move(metadata_change_list), std::move(change_list)));
+
+  return base::nullopt;
+}
+
+void WifiConfigurationBridge::OnGetAllSyncableNetworksResult(
+    std::unique_ptr<syncer::MetadataChangeList> metadata_change_list,
+    syncer::EntityChangeList change_list,
+    std::vector<sync_pb::WifiConfigurationSpecifics> list) {
+  NET_LOG(DEBUG) << list.size() << " local networks eligible for sync.";
+
+  for (sync_pb::WifiConfigurationSpecifics& proto : list) {
+    // TODO(jonmann) Don't override local configurations that are newer.
+    std::unique_ptr<syncer::EntityData> entity_data =
+        GenerateWifiEntityData(proto);
+    std::string storage_key = GetStorageKey(*entity_data);
+    change_processor()->Put(storage_key, std::move(entity_data),
+                            metadata_change_list.get());
+    entries_[storage_key] = proto;
+  }
+
+  ApplySyncChanges(std::move(metadata_change_list), std::move(change_list));
 }
 
 base::Optional<syncer::ModelError> WifiConfigurationBridge::ApplySyncChanges(
@@ -70,6 +98,11 @@ base::Optional<syncer::ModelError> WifiConfigurationBridge::ApplySyncChanges(
   std::unique_ptr<syncer::ModelTypeStore::WriteBatch> batch =
       store_->CreateWriteBatch();
 
+  NET_LOG(DEBUG) << "Applying  " << entity_changes.size()
+                 << " pending changes.";
+
+  // TODO(jonmann) Don't override synced network configurations that are newer
+  // than the local configurations.
   for (std::unique_ptr<syncer::EntityChange>& change : entity_changes) {
     if (change->type() == syncer::EntityChange::ACTION_DELETE) {
       auto it = entries_.find(change->storage_key());

chromeos/components/sync_wifi/wifi_configuration_bridge.h

@@ -14,6 +14,7 @@
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "base/time/time.h"
+#include "chromeos/components/sync_wifi/local_network_collector.h"
 #include "chromeos/components/sync_wifi/synced_network_updater.h"
 #include "components/sync/base/model_type.h"
 #include "components/sync/model/model_type_store.h"
@@ -33,6 +34,7 @@ class WifiConfigurationBridge : public syncer::ModelTypeSyncBridge {
  public:
   WifiConfigurationBridge(
       SyncedNetworkUpdater* synced_network_updater,
+      LocalNetworkCollector* local_network_collector,
       std::unique_ptr<syncer::ModelTypeChangeProcessor> change_processor,
       syncer::OnceModelTypeStoreFactory create_store_callback);
   ~WifiConfigurationBridge() override;
@@ -67,6 +69,11 @@ class WifiConfigurationBridge : public syncer::ModelTypeSyncBridge {
                          std::unique_ptr<syncer::MetadataBatch> metadata_batch);
   void OnCommit(const base::Optional<syncer::ModelError>& error);
 
+  void OnGetAllSyncableNetworksResult(
+      std::unique_ptr<syncer::MetadataChangeList> metadata_change_list,
+      syncer::EntityChangeList change_list,
+      std::vector<sync_pb::WifiConfigurationSpecifics> list);
+
   // 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
@@ -80,6 +87,8 @@ class WifiConfigurationBridge : public syncer::ModelTypeSyncBridge {
 
   SyncedNetworkUpdater* synced_network_updater_;
 
+  LocalNetworkCollector* local_network_collector_;
+
   base::WeakPtrFactory<WifiConfigurationBridge> weak_ptr_factory_{this};
 
   DISALLOW_COPY_AND_ASSIGN(WifiConfigurationBridge);

chromeos/components/sync_wifi/wifi_configuration_bridge_unittest.cc

@@ -10,10 +10,13 @@
 
 #include "base/bind.h"
 #include "base/run_loop.h"
+#include "base/test/bind_test_util.h"
 #include "base/test/task_environment.h"
+#include "chromeos/components/sync_wifi/fake_local_network_collector.h"
 #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 "components/sync/model/data_batch.h"
 #include "components/sync/model/entity_change.h"
 #include "components/sync/model/metadata_batch.h"
 #include "components/sync/model/mock_model_type_change_processor.h"
@@ -62,6 +65,15 @@ bool ProtoVectorContainsId(
              }) != protos.end();
 }
 
+void ExtractProtosFromDataBatch(
+    std::unique_ptr<syncer::DataBatch> batch,
+    std::vector<sync_pb::WifiConfigurationSpecifics>* output) {
+  while (batch->HasNext()) {
+    const syncer::KeyAndData& data_pair = batch->Next();
+    output->push_back(data_pair.second->specifics.wifi_configuration());
+  }
+}
+
 // Implementation of SyncedNetworkUpdater. This class takes add/update/delete
 // network requests and stores them in its internal data structures without
 // actually updating anything external.
@@ -99,8 +111,10 @@ class WifiConfigurationBridgeTest : public testing::Test {
   void SetUp() override {
     ON_CALL(mock_processor_, IsTrackingMetadata()).WillByDefault(Return(true));
     synced_network_updater_ = std::make_unique<TestSyncedNetworkUpdater>();
+    local_network_collector_ = std::make_unique<FakeLocalNetworkCollector>();
     bridge_ = std::make_unique<WifiConfigurationBridge>(
-        synced_network_updater(), mock_processor_.CreateForwardingProcessor(),
+        synced_network_updater(), local_network_collector(),
+        mock_processor_.CreateForwardingProcessor(),
         syncer::ModelTypeStoreTestUtil::MoveStoreToFactory(std::move(store_)));
   }
 
@@ -122,6 +136,18 @@ class WifiConfigurationBridgeTest : public testing::Test {
     return changes;
   }
 
+  std::vector<sync_pb::WifiConfigurationSpecifics> GetAllSyncedData() {
+    std::vector<WifiConfigurationSpecifics> data;
+    base::RunLoop loop;
+    bridge()->GetAllDataForDebugging(base::BindLambdaForTesting(
+        [&loop, &data](std::unique_ptr<syncer::DataBatch> batch) {
+          ExtractProtosFromDataBatch(std::move(batch), &data);
+          loop.Quit();
+        }));
+    loop.Run();
+    return data;
+  }
+
   syncer::MockModelTypeChangeProcessor* processor() { return &mock_processor_; }
 
   WifiConfigurationBridge* bridge() { return bridge_.get(); }
@@ -130,6 +156,10 @@ class WifiConfigurationBridgeTest : public testing::Test {
     return synced_network_updater_.get();
   }
 
+  FakeLocalNetworkCollector* local_network_collector() {
+    return local_network_collector_.get();
+  }
+
   const NetworkIdentifier& woof_network_id() const { return woof_network_id_; }
 
   const NetworkIdentifier& meow_network_id() const { return meow_network_id_; }
@@ -145,6 +175,8 @@ class WifiConfigurationBridgeTest : public testing::Test {
 
   std::unique_ptr<TestSyncedNetworkUpdater> synced_network_updater_;
 
+  std::unique_ptr<FakeLocalNetworkCollector> local_network_collector_;
+
   const NetworkIdentifier woof_network_id_ = GeneratePskNetworkId(kSsidWoof);
 
   const NetworkIdentifier meow_network_id_ = GeneratePskNetworkId(kSsidMeow);
@@ -260,6 +292,43 @@ TEST_F(WifiConfigurationBridgeTest, ApplySyncChangesOneDeletion) {
   EXPECT_EQ(removed_networks[0], id);
 }
 
+TEST_F(WifiConfigurationBridgeTest, MergeSyncData) {
+  auto metadata_change_list =
+      std::make_unique<syncer::InMemoryMetadataChangeList>();
+  syncer::EntityChangeList entity_data;
+
+  WifiConfigurationSpecifics meow_network =
+      GenerateTestWifiSpecifics(meow_network_id());
+  entity_data.push_back(
+      syncer::EntityChange::CreateAdd(meow_network_id().SerializeToString(),
+                                      GenerateWifiEntityData(meow_network)));
+
+  WifiConfigurationSpecifics woof_network =
+      GenerateTestWifiSpecifics(woof_network_id());
+  local_network_collector()->AddNetwork(woof_network);
+
+  std::string storage_key;
+  EXPECT_CALL(*processor(), Put(_, _, _))
+      .WillOnce(testing::SaveArg<0>(&storage_key));
+
+  bridge()->MergeSyncData(std::move(metadata_change_list),
+                          std::move(entity_data));
+  base::RunLoop().RunUntilIdle();
+
+  // Verify local network was added to sync.
+  EXPECT_EQ(storage_key, woof_network_id().SerializeToString());
+
+  // Verify sync network was added to local stack.
+  const std::vector<sync_pb::WifiConfigurationSpecifics>&
+      updated_local_networks = synced_network_updater()->add_or_update_calls();
+  EXPECT_EQ(1u, updated_local_networks.size());
+  EXPECT_TRUE(ProtoVectorContainsId(updated_local_networks, meow_network_id()));
+
+  std::vector<sync_pb::WifiConfigurationSpecifics> sync_networks =
+      GetAllSyncedData();
+  EXPECT_EQ(2u, sync_networks.size());
+}
+
 }  // namespace
 
 }  // namespace sync_wifi

chromeos/components/sync_wifi/wifi_configuration_sync_service.cc

@@ -9,6 +9,7 @@
 #include "ash/public/cpp/network_config_service.h"
 #include "base/bind_helpers.h"
 #include "base/time/default_clock.h"
+#include "chromeos/components/sync_wifi/local_network_collector_impl.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/timer_factory.h"
@@ -31,8 +32,10 @@ WifiConfigurationSyncService::WifiConfigurationSyncService(
   updater_ = std::make_unique<SyncedNetworkUpdaterImpl>(
       std::make_unique<PendingNetworkConfigurationTrackerImpl>(pref_service),
       remote_cros_network_config_.get(), std::make_unique<TimerFactory>());
+  collector_ = std::make_unique<LocalNetworkCollectorImpl>(
+      remote_cros_network_config_.get());
   bridge_ = std::make_unique<sync_wifi::WifiConfigurationBridge>(
-      updater_.get(),
+      updater_.get(), collector_.get(),
       std::make_unique<syncer::ClientTagBasedModelTypeProcessor>(
           syncer::WIFI_CONFIGURATIONS,
           base::BindRepeating(&syncer::ReportUnrecoverableError, channel)),

chromeos/components/sync_wifi/wifi_configuration_sync_service.h

@@ -25,6 +25,7 @@ namespace chromeos {
 
 namespace sync_wifi {
 
+class LocalNetworkCollectorImpl;
 class SyncedNetworkUpdaterImpl;
 class WifiConfigurationBridge;
 
@@ -43,6 +44,7 @@ class WifiConfigurationSyncService : public KeyedService {
  private:
   std::unique_ptr<WifiConfigurationBridge> bridge_;
   std::unique_ptr<SyncedNetworkUpdaterImpl> updater_;
+  std::unique_ptr<LocalNetworkCollectorImpl> collector_;
   mojo::Remote<chromeos::network_config::mojom::CrosNetworkConfig>
       remote_cros_network_config_;