Wi-Fi Sync: Fix check that prevents syncing networks added by sync.
In the recent change to sync networks after they are configured, the
check to see if a network was configured by sync was executed before
the metadata has actually been set.
Bug: 966270
Change-Id: If8bce1c693f8ee6f55f35bc49a64d4ceaf734bc2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2259378
Commit-Queue: Jon Mann <jonmann@chromium.org>
Reviewed-by: Azeem Arshad <azeemarshad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#781595}
diff --git a/chromeos/components/sync_wifi/wifi_configuration_bridge.cc b/chromeos/components/sync_wifi/wifi_configuration_bridge.cc
index 5529609..d44eedd 100644
--- a/chromeos/components/sync_wifi/wifi_configuration_bridge.cc
+++ b/chromeos/components/sync_wifi/wifi_configuration_bridge.cc
@@ -418,13 +418,6 @@
}
void WifiConfigurationBridge::OnNetworkCreated(const std::string& guid) {
- if (network_metadata_store_->GetIsConfiguredBySync(guid)) {
- // Don't have to upload a configuration that came from sync.
- NET_LOG(EVENT) << "Not uploading newly configured network "
- << NetworkGuidId(guid) << ", it was added by sync.";
- return;
- }
-
network_guid_to_timer_map_[guid] = timer_factory_->CreateOneShotTimer();
network_guid_to_timer_map_[guid]->Start(
FROM_HERE, kSyncAfterCreatedTimeout,
@@ -434,10 +427,19 @@
void WifiConfigurationBridge::OnNetworkConfiguredDelayComplete(
const std::string& network_guid) {
- NET_LOG(EVENT) << "Attempting to sync new network after delay.";
if (network_guid_to_timer_map_.contains(network_guid)) {
network_guid_to_timer_map_.erase(network_guid);
}
+
+ // This check to prevent uploading networks that were added by sync happens
+ // after the delay because the metadata isn't available in OnNetworkCreated.
+ if (network_metadata_store_->GetIsConfiguredBySync(network_guid)) {
+ NET_LOG(EVENT) << "Not uploading newly configured network "
+ << NetworkGuidId(network_guid) << ", it was added by sync.";
+ return;
+ }
+
+ NET_LOG(EVENT) << "Attempting to sync new network after delay.";
local_network_collector_->GetSyncableNetwork(
network_guid, base::BindOnce(&WifiConfigurationBridge::SaveNetworkToSync,
weak_ptr_factory_.GetWeakPtr()));
diff --git a/chromeos/components/sync_wifi/wifi_configuration_bridge_unittest.cc b/chromeos/components/sync_wifi/wifi_configuration_bridge_unittest.cc
index 4b2b175..1a733e6d 100644
--- a/chromeos/components/sync_wifi/wifi_configuration_bridge_unittest.cc
+++ b/chromeos/components/sync_wifi/wifi_configuration_bridge_unittest.cc
@@ -18,6 +18,7 @@
#include "chromeos/components/sync_wifi/fake_local_network_collector.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/synced_network_metrics_logger.h"
#include "chromeos/components/sync_wifi/synced_network_updater.h"
#include "chromeos/components/sync_wifi/test_data_generator.h"
@@ -137,11 +138,12 @@
class WifiConfigurationBridgeTest : public testing::Test {
protected:
WifiConfigurationBridgeTest()
- : store_(syncer::ModelTypeStoreTestUtil::CreateInMemoryStoreForTest()) {}
+ : store_(syncer::ModelTypeStoreTestUtil::CreateInMemoryStoreForTest()) {
+ network_test_helper_ = std::make_unique<NetworkTestHelper>();
+ }
void SetUp() override {
- shill_clients::InitializeFakes();
- NetworkHandler::Initialize();
+ network_test_helper_->SetUp();
ON_CALL(mock_processor_, IsTrackingMetadata()).WillByDefault(Return(true));
timer_factory_ = std::make_unique<FakeTimerFactory>();
@@ -158,8 +160,9 @@
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(), /*is_enterprise_enrolled=*/false);
+ network_test_helper_->network_state_helper().network_state_handler(),
+ user_prefs_.get(), device_prefs_.get(),
+ /*is_enterprise_enrolled=*/false);
bridge_ = std::make_unique<WifiConfigurationBridge>(
synced_network_updater(), local_network_collector(),
@@ -170,11 +173,6 @@
base::RunLoop().RunUntilIdle();
}
- void TearDown() override {
- NetworkHandler::Shutdown();
- shill_clients::Shutdown();
- }
-
void DisableBridge() {
ON_CALL(mock_processor_, IsTrackingMetadata()).WillByDefault(Return(false));
}
@@ -218,6 +216,12 @@
}
FakeTimerFactory* timer_factory() { return timer_factory_.get(); }
+ NetworkMetadataStore* network_metadata_store() {
+ return network_metadata_store_.get();
+ }
+ NetworkTestHelper* network_test_helper() {
+ return network_test_helper_.get();
+ }
const NetworkIdentifier& woof_network_id() const { return woof_network_id_; }
const NetworkIdentifier& meow_network_id() const { return meow_network_id_; }
@@ -235,6 +239,7 @@
std::unique_ptr<sync_preferences::TestingPrefServiceSyncable> user_prefs_;
std::unique_ptr<NetworkMetadataStore> network_metadata_store_;
std::unique_ptr<SyncedNetworkMetricsLogger> metrics_logger_;
+ std::unique_ptr<NetworkTestHelper> network_test_helper_;
const NetworkIdentifier woof_network_id_ = GeneratePskNetworkId(kSsidWoof);
const NetworkIdentifier meow_network_id_ = GeneratePskNetworkId(kSsidMeow);
@@ -442,6 +447,22 @@
base::RunLoop().RunUntilIdle();
}
+TEST_F(WifiConfigurationBridgeTest, LocalConfigured_FromSync) {
+ WifiConfigurationSpecifics meow_local =
+ GenerateTestWifiSpecifics(meow_network_id(), kSyncPsk, /*timestamp=*/0);
+ local_network_collector()->AddNetwork(meow_local);
+
+ EXPECT_CALL(*processor(), Put(_, _, _)).Times(testing::Exactly(0));
+
+ std::string guid = meow_network_id().SerializeToString();
+ bridge()->OnNetworkCreated(guid);
+ network_metadata_store()->SetIsConfiguredBySync(guid);
+ base::RunLoop().RunUntilIdle();
+
+ timer_factory()->FireAll();
+ base::RunLoop().RunUntilIdle();
+}
+
TEST_F(WifiConfigurationBridgeTest, LocalFirstConnect) {
base::HistogramTester histogram_tester;
WifiConfigurationSpecifics meow_local =
