NOT FOR REVIEW

components/wifi_sync/wifi_config_observer_chromeos_unittest.cc

Index: components/wifi_sync/wifi_config_observer_chromeos_unittest.cc
diff --git a/components/wifi_sync/wifi_config_observer_chromeos_unittest.cc b/components/wifi_sync/wifi_config_observer_chromeos_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..52c11272cf7874f040e456f5b32f7ec529108d11
--- /dev/null
+++ b/components/wifi_sync/wifi_config_observer_chromeos_unittest.cc
@@ -0,0 +1,985 @@
+// Copyright 2015 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 "components/wifi_sync/wifi_config_observer_chromeos.h"
+
+#include <set>
+
+#include "base/logging.h"
+#include "base/macros.h"
+#include "base/message_loop/message_loop.h"
+#include "base/strings/string_number_conversions.h"
+#include "base/strings/stringprintf.h"
+#include "base/values.h"
+#include "chromeos/dbus/dbus_thread_manager.h"
+#include "chromeos/network/managed_network_configuration_handler.h"
+#include "chromeos/network/network_configuration_handler.h"
+#include "chromeos/network/network_handler_callbacks.h"
+#include "chromeos/network/network_state.h"
+#include "chromeos/network/network_state_handler.h"
+#include "components/wifi_sync/wifi_credential.h"
+#include "components/wifi_sync/wifi_credential_syncable_service.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/cros_system_api/dbus/service_constants.h"
+
+using chromeos::network_handler::DictionaryResultCallback;
+using chromeos::network_handler::ErrorCallback;
+using chromeos::network_handler::StringResultCallback;
+
+namespace wifi_sync {
+
+namespace {
+
+const char kPassphrasePsk[] = "psk-passphrase";
+const char kPassphraseWep[] = "abcde";
+const char kServiceGuid[] = "32938100-1456-4138-88e6-6edee41c88c8";
+const char kService2Guid[] = "4e715282-1299-40c8-b873-8d992b5999e2";
+const char kServicePath[] = "/service/fake";
+const char kService2Path[] = "/service/fake2";
+const char kUserHash[] = "fake-user-hash";
+const char kProfilePath[] = "/profile/fake";
+
+class FakeWifiCredentialSyncableService : public WifiCredentialSyncableService {
+ public:
+  FakeWifiCredentialSyncableService()
+      : add_to_synced_networks_call_count_(0) {}
+  ~FakeWifiCredentialSyncableService() override {}
+
+  // syncer::SyncableService implementation.
+  syncer::SyncMergeResult MergeDataAndStartSyncing(
+      syncer::ModelType type,
+      const syncer::SyncDataList& initial_sync_data,
+      scoped_ptr<syncer::SyncChangeProcessor> sync_processor,
+      scoped_ptr<syncer::SyncErrorFactory> error_handler) override {
+    NOTIMPLEMENTED();
+    return syncer::SyncMergeResult(type);
+  }
+  void StopSyncing(syncer::ModelType type) override {
+    NOTIMPLEMENTED();
+  }
+  syncer::SyncDataList GetAllSyncData(syncer::ModelType type) const override {
+    NOTIMPLEMENTED();
+    return syncer::SyncDataList();
+  }
+  syncer::SyncError ProcessSyncChanges(
+      const tracked_objects::Location& caller_location,
+      const syncer::SyncChangeList& change_list) override {
+    NOTIMPLEMENTED();
+    return syncer::SyncError();
+  }
+
+  // WifiCredentialSyncableService implementation.
+  bool AddToSyncedNetworks(const WifiCredential& credential) override {
+    LOG(ERROR) << "*** QUICHE: " << __func__ << " on " << this;
+    ++add_to_synced_networks_call_count_;
+    last_passphrase_ = credential.passphrase();
+    return true;
+  }
+
+  // XXX doc
+  size_t add_to_synced_networks_call_count() const {
+    return add_to_synced_networks_call_count_;
+  }
+
+  // XXX doc
+  std::string last_passphrase() const {
+    return last_passphrase_;
+  }
+
+ private:
+  // The number of times AddToSyncedNetworks has been called on this fake.
+  size_t add_to_synced_networks_call_count_;
+
+  // The passphrase received in the most recent call to AddToSyncedNetworks.
+  std::string last_passphrase_;
+};
+
+class FakeManagedNetworkConfigurationHandler
+    : public chromeos::ManagedNetworkConfigurationHandler {
+ public:
+  ~FakeManagedNetworkConfigurationHandler() override {}
+
+  // ManagedNetworkConfigurationHandler implementation.
+  void AddObserver(chromeos::NetworkPolicyObserver* observer) override {
+    NOTIMPLEMENTED();
+  }
+  void RemoveObserver(chromeos::NetworkPolicyObserver* observer) override {
+    NOTIMPLEMENTED();
+  }
+  void GetProperties(
+      const std::string& service_path,
+      const DictionaryResultCallback& callback,
+      const ErrorCallback& error_callback) override {
+    NOTIMPLEMENTED();
+  }
+  void GetManagedProperties(
+      const std::string& userhash,
+      const std::string& service_path,
+      const DictionaryResultCallback& callback,
+      const ErrorCallback& error_callback) override {
+    NOTIMPLEMENTED();
+  }
+  void SetProperties(
+      const std::string& service_path,
+      const base::DictionaryValue& user_settings,
+      const base::Closure& callback,
+      const ErrorCallback& error_callback) const override {
+    NOTIMPLEMENTED();
+  }
+  void CreateConfiguration(
+      const std::string& userhash,
+      const base::DictionaryValue& properties,
+      const StringResultCallback& callback,
+      const ErrorCallback& error_callback) const override {
+    NOTIMPLEMENTED();
+  }
+  void RemoveConfiguration(
+      const std::string& service_path,
+      const base::Closure& callback,
+      const ErrorCallback& error_callback) const override {
+    NOTIMPLEMENTED();
+  }
+  void SetPolicy(
+      ::onc::ONCSource onc_source,
+      const std::string& userhash,
+      const base::ListValue& network_configs_onc,
+      const base::DictionaryValue& global_network_config) override {
+    NOTIMPLEMENTED();
+  }
+  bool IsAnyPolicyApplicationRunning() const override {
+    NOTIMPLEMENTED();
+    return false;
+  }
+  const base::DictionaryValue* FindPolicyByGUID(
+      const std::string userhash,
+      const std::string& guid,
+      ::onc::ONCSource* onc_source) const override {
+    if (userhash == "" &&
+        device_policy_network_guids_.find(guid) !=
+        device_policy_network_guids_.end())
+      return &dummy_dictionary_;
+
+    if (userhash != "" &&
+        user_policy_network_guids_.find(guid) !=
+        user_policy_network_guids_.end())
+      return &dummy_dictionary_;
+
+    return nullptr;
+  }
+  const GuidToPolicyMap* GetNetworkConfigsFromPolicy(
+      const std::string& userhash) const override {
+    NOTIMPLEMENTED();
+    return nullptr;
+  }
+  const base::DictionaryValue* GetGlobalConfigFromPolicy(
+      const std::string& userhash) const override {
+    NOTIMPLEMENTED();
+    return nullptr;
+  }
+  const base::DictionaryValue* FindPolicyByGuidAndProfile(
+      const std::string& guid,
+      const std::string& profile_path) const override {
+    NOTIMPLEMENTED();
+    return nullptr;
+  }
+
+  // XXX doc
+  void AddToDevicePolicyNetworks(const std::string& guid) {
+    device_policy_network_guids_.insert(guid);
+  }
+
+  // XXX doc
+  void AddToUserPolicyNetworks(const std::string& userhash,
+                               const std::string& guid) {
+    CHECK(userhash_.empty() || userhash_ == userhash)
+        << "The fake only supports one user";
+    CHECK(!userhash.empty());
+    user_policy_network_guids_.insert(guid);
+    userhash_ = userhash;
+  }
+
+ private:
+  // XXX doc
+  const base::DictionaryValue dummy_dictionary_;
+  std::set<std::string> device_policy_network_guids_;
+  std::set<std::string> user_policy_network_guids_;
+  std::string userhash_;
+};
+
+class TestNetworkStateHandler : public chromeos::NetworkStateHandler {
+ public:
+  // XXX doc
+  void AddNetwork(
+      const std::string& service_path,
+      const base::DictionaryValue& service_properties) {
+    AddToKnownNetworks(service_path);
+    UpdateManagedStateProperties(
+        chromeos::ManagedState::MANAGED_TYPE_NETWORK,
+        service_path, service_properties);
+  }
+
+  // XXX doc
+  void UpdateConnectionState(
+      const std::string& service_path,
+      const std::string& new_connection_state) {
+    base::DictionaryValue changed_properties;
+    changed_properties.SetStringWithoutPathExpansion(
+        shill::kStateProperty, new_connection_state);
+    UpdateManagedStateProperties(
+        chromeos::ManagedState::MANAGED_TYPE_NETWORK,
+        service_path, changed_properties);
+  }
+
+ private:
+  // XXX doc
+  void AddToKnownNetworks(const std::string& network_path) {
+    known_network_paths_.push_back(network_path);
+    base::ListValue known_networks_list_value;
+    known_networks_list_value.AppendStrings(known_network_paths_);
+    UpdateManagedList(chromeos::ManagedState::MANAGED_TYPE_NETWORK,
+                      known_networks_list_value);
+  }
+
+  std::vector<std::string> known_network_paths_;  // XXX doc
+};
+
+}  // namespace
+
+class WifiConfigObserverChromeOsTest : public testing::Test {
+ protected:
+  WifiConfigObserverChromeOsTest() {
+    network_state_handler_.reset(
+        new TestNetworkStateHandler());
+    network_configuration_handler_.reset(
+        chromeos::NetworkConfigurationHandler::InitializeForTest(
+            network_state_handler_.get(),
+            nullptr /* network_device_handler */));
+    config_observer_.reset(
+        new WifiConfigObserverChromeOs(
+            kUserHash,
+            &managed_network_configuration_handler_,
+            network_configuration_handler_.get(),
+            network_state_handler_.get()));
+  }
+
+  // XXX consider nuking. (we no longer initialize DBusThreadManager)
+  ~WifiConfigObserverChromeOsTest() override {
+    // We can't leave resetting these scoped_ptrs to the default dtor,
+    // because a) we need to shut down DBusThreadManager, and b) the
+    // objects managed by the scoped_ptrs depend on DBusThreadManager
+    // (either directly, or transitively).
+    config_observer_.reset();
+    network_configuration_handler_.reset();
+    network_state_handler_.reset();
+  }
+
+  // Wrappers for WifiConfigObserver methods.
+  void StartSyncing() {
+    // XXX hdr?
+    config_observer_->StartSyncing(base::AsWeakPtr(&syncable_service_));
+  }
+  void StopSyncing() {
+    config_observer_->StopSyncing();
+  }
+  void OnConfigurationCreated(
+      const std::string& service_path,
+      const base::DictionaryValue& properties,
+      chromeos::NetworkConfigurationObserver::Source source) {
+    config_observer_->OnConfigurationCreated(
+        service_path, kProfilePath, properties, source);
+  }
+  void OnConfigurationProfileChanged(
+      const std::string& service_path,
+      const std::string& profile_path,
+      chromeos::NetworkConfigurationObserver::Source source) {
+    config_observer_->OnConfigurationProfileChanged(
+        service_path, profile_path, source);
+  }
+  void OnPropertiesSet(
+      const std::string& service_path,
+      const std::string& guid,
+      const base::DictionaryValue& properties,
+      chromeos::NetworkConfigurationObserver::Source source) {
+    config_observer_->OnPropertiesSet(
+        service_path, guid, properties, source);
+    // XXX strictly speaking, this should update NetworkStateHandler's
+    // state too, no? but we can get away with this cheat, because the
+    // only thing WCOCO reads out of NetworkState is the GUID.
+  }
+  // XXX merge in MakeNetworkState?
+  void NetworkConnectionStateChanged(
+      chromeos::NetworkState* network, const std::string& new_state) {
+    network->PropertyChanged(
+        shill::kStateProperty, base::StringValue(new_state));
+    config_observer_->NetworkConnectionStateChanged(network);
+  }
+
+  scoped_ptr<base::DictionaryValue> MakeCommonServiceProperties(
+      const std::string& guid) {
+    scoped_ptr<base::DictionaryValue> properties(new base::DictionaryValue());
+    CHECK(properties);
+    const std::string ssid(kSsid);  // XXX factor out?
+    properties->SetStringWithoutPathExpansion(
+        shill::kTypeProperty, shill::kTypeWifi);
+    properties->SetStringWithoutPathExpansion(
+        shill::kWifiHexSsid, base::HexEncode(ssid.data(), ssid.size()));
+    properties->SetStringWithoutPathExpansion(shill::kGuidProperty, guid);
+    return properties;
+  }
+
+  scoped_ptr<base::DictionaryValue> Make8021XServiceProperties(
+      const std::string& guid) {
+    scoped_ptr<base::DictionaryValue> properties(
+        MakeCommonServiceProperties(guid));
+    properties->SetStringWithoutPathExpansion(
+        shill::kSecurityClassProperty, shill::kSecurity8021x);
+    return properties;
+  }
+
+  // XXX doc. properties that would be passed by NCH to OnConfigurationCreated.
+  scoped_ptr<base::DictionaryValue> MakeOpenServiceProperties(
+      const std::string& guid) {
+    scoped_ptr<base::DictionaryValue> properties(
+        MakeCommonServiceProperties(guid));
+    properties->SetStringWithoutPathExpansion(
+        shill::kSecurityClassProperty, shill::kSecurityNone);
+    return properties;
+  }
+
+  // XXX doc. properties that would be passed by NCH to OnConfigurationCreated.
+  scoped_ptr<base::DictionaryValue> MakePskServiceProperties(
+      const std::string& guid) {
+    scoped_ptr<base::DictionaryValue> properties(
+        MakeCommonServiceProperties(guid));
+    properties->SetStringWithoutPathExpansion(
+        shill::kSecurityClassProperty, shill::kSecurityPsk);
+    properties->SetStringWithoutPathExpansion(
+        shill::kPassphraseProperty, kPassphrasePsk);  // XXX factor out?
+    return properties;
+  }
+
+  // XXX doc. properties that would be passed by NCH to OnConfigurationCreated.
+  scoped_ptr<base::DictionaryValue> MakeWepServiceProperties(
+      const std::string& guid) {
+    scoped_ptr<base::DictionaryValue> properties(
+        MakeCommonServiceProperties(guid));
+    properties->SetStringWithoutPathExpansion(
+        shill::kSecurityClassProperty, shill::kSecurityWep);
+    properties->SetStringWithoutPathExpansion(
+        shill::kPassphraseProperty, kPassphraseWep);  // XXX factor out?
+    return properties;
+  }
+
+  // XXX document.
+  scoped_ptr<chromeos::NetworkState> MakeNetworkState(
+      const std::string& service_path,
+      const base::DictionaryValue& service_properties) {
+    // XXX don't return NetworkState
+    scoped_ptr<chromeos::NetworkState> network_state(
+        new chromeos::NetworkState(service_path));
+    CHECK(network_state);
+    network_state->PropertyChanged(shill::kVisibleProperty,
+                                  base::FundamentalValue(true));
+    for (base::DictionaryValue::Iterator it(service_properties);
+         !it.IsAtEnd(); it.Advance()) {
+      if (it.key() == shill::kPassphraseProperty)
+        continue;  // Shill doesn't echo back the passphrase.
+      network_state->PropertyChanged(it.key(), it.value());
+    }
+    network_state->InitialPropertiesReceived(service_properties);
+    scoped_ptr<base::DictionaryValue> filtered_properties(
+        service_properties.DeepCopyWithoutEmptyChildren());
+    // Remove passphrase, since Shill doesn't echo it back.
+    filtered_properties->Remove(shill::kPassphraseProperty, nullptr);
+    network_state_handler_->AddNetwork(service_path, *filtered_properties);
+    return network_state;
+  }
+
+  // XXX document.
+  void RegisterNetwork(
+      const std::string& service_path,
+      const base::DictionaryValue& service_properties) {
+    scoped_ptr<base::DictionaryValue> filtered_properties(
+        service_properties.DeepCopyWithoutEmptyChildren());
+    // Remove passphrase, since Shill doesn't echo it back.
+    filtered_properties->Remove(shill::kPassphraseProperty, nullptr);
+    // Add the visible property, as Shill would.
+    filtered_properties->SetBooleanWithoutPathExpansion(
+        shill::kVisibleProperty, true);
+    network_state_handler_->AddNetwork(service_path, *filtered_properties);
+  }
+
+  // XXX doc
+  void SetNetworkPassphrase(
+      const std::string& service_path,
+      const std::string& service_guid,
+      const std::string& passphrase) {
+    base::DictionaryValue properties;
+    properties.SetStringWithoutPathExpansion(
+        shill::kPassphraseProperty, passphrase);
+    OnPropertiesSet(
+        service_path, service_guid, properties,
+        chromeos::NetworkConfigurationObserver::SOURCE_USER_ACTION);
+  }
+
+  // XXX doc
+  void UpdateConnectionState(
+      const std::string& service_path,
+      const std::string& new_connection_state) {
+    network_state_handler_->UpdateConnectionState(
+        service_path, new_connection_state);
+  }
+
+  // XXX doc
+  void AddToDevicePolicyNetworks(const std::string& guid) {
+    managed_network_configuration_handler_.AddToDevicePolicyNetworks(guid);
+  }
+
+  void AddToUserPolicyNetworks(const std::string& guid) {
+    managed_network_configuration_handler_.AddToUserPolicyNetworks(
+        kUserHash, guid);
+  }
+
+  // XXX doc
+  size_t add_to_synced_networks_call_count() const {
+    return syncable_service_.add_to_synced_networks_call_count();
+  }
+
+  // XXX doc
+  std::string syncable_service_last_passphrase() const {
+    return syncable_service_.last_passphrase();
+  }
+
+ private:
+  static const char kSsid[];
+
+  base::MessageLoopForUI message_loop_;  // For NetworkStateHandler.
+  FakeManagedNetworkConfigurationHandler managed_network_configuration_handler_;
+  scoped_ptr<TestNetworkStateHandler> network_state_handler_;
+  scoped_ptr<chromeos::NetworkConfigurationHandler>
+    network_configuration_handler_;
+  scoped_ptr<WifiConfigObserverChromeOs> config_observer_;
+  FakeWifiCredentialSyncableService syncable_service_;
+
+  DISALLOW_COPY_AND_ASSIGN(WifiConfigObserverChromeOsTest);
+};
+
+const char WifiConfigObserverChromeOsTest::kSsid[] = "fake-ssid";
+
+// Success tests for networks found in scan results. (These are the
+// most common cases.)
+
+TEST_F(WifiConfigObserverChromeOsTest, FromScan_SyncNewOpenNetwork) {
+  StartSyncing();
+  scoped_ptr<base::DictionaryValue> service_properties =
+      MakeOpenServiceProperties(kServiceGuid);
+  RegisterNetwork(kServicePath, *service_properties);
+  UpdateConnectionState(kServicePath, shill::kStateOnline);
+  OnConfigurationProfileChanged(
+      kServicePath, kProfilePath,
+      chromeos::NetworkConfigurationObserver::SOURCE_USER_ACTION);
+  EXPECT_EQ(1U, add_to_synced_networks_call_count());
+}
+
+TEST_F(WifiConfigObserverChromeOsTest, FromScan_SyncNewWepNetwork) {
+  StartSyncing();
+  scoped_ptr<base::DictionaryValue> service_properties =
+      MakeWepServiceProperties(kServiceGuid);
+  RegisterNetwork(kServicePath, *service_properties);
+  SetNetworkPassphrase(kServicePath, kServiceGuid, kPassphraseWep);
+  UpdateConnectionState(kServicePath, shill::kStateOnline);
+  OnConfigurationProfileChanged(
+      kServicePath, kProfilePath,
+      chromeos::NetworkConfigurationObserver::SOURCE_USER_ACTION);
+  EXPECT_EQ(1U, add_to_synced_networks_call_count());
+}
+
+TEST_F(WifiConfigObserverChromeOsTest, FromScan_SyncNewPskNetwork) {
+  StartSyncing();
+  scoped_ptr<base::DictionaryValue> service_properties =
+      MakePskServiceProperties(kServiceGuid);
+  RegisterNetwork(kServicePath, *service_properties);
+  SetNetworkPassphrase(kServicePath, kServiceGuid, kPassphrasePsk);
+  UpdateConnectionState(kServicePath, shill::kStateOnline);
+  OnConfigurationProfileChanged(
+      kServicePath, kProfilePath,
+      chromeos::NetworkConfigurationObserver::SOURCE_USER_ACTION);
+  EXPECT_EQ(1U, add_to_synced_networks_call_count());
+}
+
+TEST_F(WifiConfigObserverChromeOsTest,
+       FromScan_SyncNewPskNetworkWithOtherSettingChanges) {
+  StartSyncing();
+  scoped_ptr<base::DictionaryValue> service_properties =
+      MakePskServiceProperties(kServiceGuid);
+  RegisterNetwork(kServicePath, *service_properties);
+  SetNetworkPassphrase(kServicePath, kServiceGuid, kPassphrasePsk);
+
+  base::DictionaryValue new_properties;
+  new_properties.SetBooleanWithoutPathExpansion(shill::kWifiHiddenSsid, true);
+  OnPropertiesSet(kServicePath, kServiceGuid, new_properties,
+                  chromeos::NetworkConfigurationObserver::SOURCE_USER_ACTION);
+  UpdateConnectionState(kServicePath, shill::kStateOnline);
+  OnConfigurationProfileChanged(
+      kServicePath, kProfilePath,
+      chromeos::NetworkConfigurationObserver::SOURCE_USER_ACTION);
+
+  // Changing a non-passphrase property shouldn't wipe out the
+  // passphrase. So this network should still sync.
+  EXPECT_EQ(1U, add_to_synced_networks_call_count());
+}
+
+TEST_F(WifiConfigObserverChromeOsTest, FromScan_SyncChangedPskPassphrase) {
+  StartSyncing();
+  scoped_ptr<base::DictionaryValue> service_properties =
+      MakePskServiceProperties(kServiceGuid);
+  RegisterNetwork(kServicePath, *service_properties);
+
+  std::string new_passphrase = base::StringPrintf("not-%s", kPassphrasePsk);
+  SetNetworkPassphrase(kServicePath, kServiceGuid, new_passphrase);
+  UpdateConnectionState(kServicePath, shill::kStateOnline);
+  // OnConfigurationProfileChanged() omitted when updating an existing network.
+  EXPECT_EQ(1U, add_to_synced_networks_call_count());
+  EXPECT_EQ(new_passphrase, syncable_service_last_passphrase());
+}
+
+// Test of manually configured networks. These are created via the
+// "Join other..." dialog. Users will generally only do this for
+// hidden networks.
+
+// XXX should these continue using MakeNetworkState and
+// NetworkConnectionStateChanged?
+
+TEST_F(WifiConfigObserverChromeOsTest, ManualConfig_SyncOpenNetwork) {
+  StartSyncing();
+  scoped_ptr<base::DictionaryValue> service_properties =
+      MakeOpenServiceProperties(kServiceGuid);
+  OnConfigurationCreated(
+      kServicePath, *service_properties,
+      chromeos::NetworkConfigurationObserver::SOURCE_USER_ACTION);
+  RegisterNetwork(kServicePath, *service_properties);
+  UpdateConnectionState(kServicePath, shill::kStateOnline);
+  EXPECT_EQ(1U, add_to_synced_networks_call_count());
+}
+
+TEST_F(WifiConfigObserverChromeOsTest, ManualConfig_SyncWepNetwork) {
+  StartSyncing();
+  scoped_ptr<base::DictionaryValue> service_properties =
+      MakeWepServiceProperties(kServiceGuid);
+  OnConfigurationCreated(
+      kServicePath, *service_properties,
+      chromeos::NetworkConfigurationObserver::SOURCE_USER_ACTION);
+  RegisterNetwork(kServicePath, *service_properties);
+  UpdateConnectionState(kServicePath, shill::kStateOnline);
+  EXPECT_EQ(1U, add_to_synced_networks_call_count());
+}
+
+TEST_F(WifiConfigObserverChromeOsTest, ManualConfig_SyncPskNetwork) {
+  StartSyncing();
+  scoped_ptr<base::DictionaryValue> service_properties =
+      MakePskServiceProperties(kServiceGuid);
+  OnConfigurationCreated(
+      kServicePath, *service_properties,
+      chromeos::NetworkConfigurationObserver::SOURCE_USER_ACTION);
+  RegisterNetwork(kServicePath, *service_properties);
+  UpdateConnectionState(kServicePath, shill::kStateOnline);
+  EXPECT_EQ(1U, add_to_synced_networks_call_count());
+}
+
+TEST_F(WifiConfigObserverChromeOsTest,
+       ManualConfig_SyncLatestPassphraseWhenChangedAfterCreation) {
+  StartSyncing();
+
+  scoped_ptr<base::DictionaryValue> service_properties =
+      MakePskServiceProperties(kServiceGuid);
+  OnConfigurationCreated(
+      kServicePath, *service_properties,
+      chromeos::NetworkConfigurationObserver::SOURCE_USER_ACTION);
+  RegisterNetwork(kServicePath, *service_properties);
+
+  const std::string new_passphrase =
+      base::StringPrintf("not-%s", kPassphrasePsk);
+  SetNetworkPassphrase(kServicePath, kServiceGuid, new_passphrase);
+  UpdateConnectionState(kServicePath, shill::kStateOnline);
+  EXPECT_EQ(1U, add_to_synced_networks_call_count());
+  EXPECT_EQ(new_passphrase, syncable_service_last_passphrase());
+}
+
+// Tests of network connection state handling. "Normal" tests, such as
+// SyncNewPskNetworkFromScan, exercise shill::kStateOnline. The tests
+// in the group below check behavior for other network connection states.
+
+TEST_F(WifiConfigObserverChromeOsTest, ConnectionState_DoNotSyncAtAssociation) {
+  StartSyncing();
+  scoped_ptr<base::DictionaryValue> service_properties =
+      MakePskServiceProperties(kServiceGuid);
+  RegisterNetwork(kServicePath, *service_properties);
+  SetNetworkPassphrase(kServicePath, kServiceGuid, kPassphrasePsk);
+  UpdateConnectionState(kServicePath, shill::kStateAssociation);
+  OnConfigurationProfileChanged(
+      kServicePath, kProfilePath,
+      chromeos::NetworkConfigurationObserver::SOURCE_USER_ACTION);
+  EXPECT_EQ(0U, add_to_synced_networks_call_count());
+}
+
+TEST_F(WifiConfigObserverChromeOsTest, ConnectionState_SyncAtPortal) {
+  StartSyncing();
+  scoped_ptr<base::DictionaryValue> service_properties =
+      MakePskServiceProperties(kServiceGuid);
+  RegisterNetwork(kServicePath, *service_properties);
+  SetNetworkPassphrase(kServicePath, kServiceGuid, kPassphrasePsk);
+  UpdateConnectionState(kServicePath, shill::kStatePortal);
+  OnConfigurationProfileChanged(
+      kServicePath, kProfilePath,
+      chromeos::NetworkConfigurationObserver::SOURCE_USER_ACTION);
+  // kStatePortal means that the network layer is up (we have an IP
+  // address), but we don't have Internet connectivity. We should still
+  // sync the network, because, if the network layer is up, then so is
+  // the link layer.
+  EXPECT_EQ(1U, add_to_synced_networks_call_count());
+}
+
+TEST_F(WifiConfigObserverChromeOsTest, ConnectionState_SyncAtReady) {
+  StartSyncing();
+  scoped_ptr<base::DictionaryValue> service_properties =
+      MakePskServiceProperties(kServiceGuid);
+  RegisterNetwork(kServicePath, *service_properties);
+  SetNetworkPassphrase(kServicePath, kServiceGuid, kPassphrasePsk);
+  UpdateConnectionState(kServicePath, shill::kStateReady);
+  OnConfigurationProfileChanged(
+      kServicePath, kProfilePath,
+      chromeos::NetworkConfigurationObserver::SOURCE_USER_ACTION);
+  // kStateReady means that the network layer is up (we have an IP
+  // address), but we haven't tested Internet connectivity. If the
+  // network layer is up, the link layer must be up. So go ahead and
+  // sync. (We could wait until reaching kStatePortal, or
+  // kStateOnline. But triggering on kStateReady lets us get started
+  // earlier, in some cases.)
+  EXPECT_EQ(1U, add_to_synced_networks_call_count());
+}
+
+// Tests for configurations which should not be synced. Note that
+// "negative" tests related to NetworkConfigurationObserver::Source
+// are grouped separately.
+TEST_F(WifiConfigObserverChromeOsTest,
+       BadConfig_OpenNetworkWithoutUserActionIsNotSent) {
+  StartSyncing();
+
+  scoped_ptr<base::DictionaryValue> service_properties =
+      MakeOpenServiceProperties(kServiceGuid);
+  RegisterNetwork(kServicePath, *service_properties);
+  UpdateConnectionState(kServicePath, shill::kStateOnline);
+  // OnConfigurationProfileChanged() not called for autoconnect.
+
+  // While we have sufficient information to sync this network, we
+  // shouldn't do so. The reason is that the user hasn't explicitly
+  // indicated that they trust this network. (The user might, e.g., be
+  // using a friend's device. That friend may trust SketchyOpenWifi,
+  // but we don't want to assume the user does as well.)
+  EXPECT_EQ(0U, add_to_synced_networks_call_count());
+}
+
+TEST_F(WifiConfigObserverChromeOsTest,
+       BadConfig_PskNetworkWithoutPassphraseIsNotSent) {
+  StartSyncing();
+  scoped_ptr<base::DictionaryValue> service_properties =
+      MakePskServiceProperties(kServiceGuid);
+  service_properties->RemoveWithoutPathExpansion(
+      shill::kPassphraseProperty, nullptr);
+  RegisterNetwork(kServicePath, *service_properties);
+  // Assume the upper layers forgot to call OnPropertiesSet().
+  UpdateConnectionState(kServicePath, shill::kStateOnline);
+  OnConfigurationProfileChanged(
+      kServicePath, kProfilePath,
+      chromeos::NetworkConfigurationObserver::SOURCE_USER_ACTION);
+  EXPECT_EQ(0U, add_to_synced_networks_call_count());
+}
+
+TEST_F(WifiConfigObserverChromeOsTest, BadConfig_8021XNetworkIsNotSent) {
+  StartSyncing();
+  scoped_ptr<base::DictionaryValue> service_properties =
+      MakeCommonServiceProperties(kServiceGuid);
+  service_properties->SetStringWithoutPathExpansion(
+      shill::kSecurityClassProperty, shill::kSecurity8021x);
+  RegisterNetwork(kServicePath, *service_properties);
+  UpdateConnectionState(kServicePath, shill::kStateOnline);
+  OnConfigurationProfileChanged(
+      kServicePath, kProfilePath,
+      chromeos::NetworkConfigurationObserver::SOURCE_USER_ACTION);
+  EXPECT_EQ(0U, add_to_synced_networks_call_count());
+}
+
+TEST_F(WifiConfigObserverChromeOsTest, BadConfig_NonWifiNetworkIsNotSent) {
+  StartSyncing();
+  base::DictionaryValue service_properties;
+  service_properties.SetStringWithoutPathExpansion(
+      shill::kTypeProperty, shill::kTypeEthernet);
+  service_properties.SetStringWithoutPathExpansion(
+      shill::kGuidProperty, kServiceGuid);
+  RegisterNetwork(kServicePath, service_properties);
+  UpdateConnectionState(kServicePath, shill::kStateOnline);
+  OnConfigurationProfileChanged(
+      kServicePath, kProfilePath,
+      chromeos::NetworkConfigurationObserver::SOURCE_USER_ACTION);
+  EXPECT_EQ(0U, add_to_synced_networks_call_count());
+}
+
+// XXX group name
+// XXX revisit this group
+
+TEST_F(WifiConfigObserverChromeOsTest, PolicyCreatedNetworkNotSent) {
+  StartSyncing();
+
+  scoped_ptr<base::DictionaryValue> service_properties =
+      MakePskServiceProperties(kServiceGuid);
+  AddToDevicePolicyNetworks(kServiceGuid);
+  OnConfigurationCreated(
+      kServicePath, *service_properties,
+      chromeos::NetworkConfigurationObserver::SOURCE_POLICY);
+  NetworkConnectionStateChanged(
+      MakeNetworkState(kServicePath, *service_properties).get(),
+      shill::kStateOnline);
+  EXPECT_EQ(0U, add_to_synced_networks_call_count());
+}
+
+TEST_F(WifiConfigObserverChromeOsTest,
+       PolicyCreatedUserUpdatedNetworkNotSent) {
+  StartSyncing();
+
+  scoped_ptr<base::DictionaryValue> service_properties =
+      MakePskServiceProperties(kServiceGuid);
+  OnConfigurationCreated(
+      kServicePath, *service_properties,
+      chromeos::NetworkConfigurationObserver::SOURCE_POLICY);
+  AddToDevicePolicyNetworks(kServiceGuid);
+
+  base::DictionaryValue new_properties;
+  new_properties.SetStringWithoutPathExpansion(
+      shill::kPassphraseProperty, "new-psk-passphrase-via-policy");
+  OnPropertiesSet(kServicePath, kServiceGuid, new_properties,
+                  chromeos::NetworkConfigurationObserver::SOURCE_USER_ACTION);
+  new_properties.MergeDictionary(service_properties.get());
+  NetworkConnectionStateChanged(
+      MakeNetworkState(kServicePath, new_properties).get(),
+      shill::kStateOnline);
+  EXPECT_EQ(0U, add_to_synced_networks_call_count());
+}
+
+TEST_F(WifiConfigObserverChromeOsTest, UserCreatedPolicyUpdatedNetworkNotSent) {
+  StartSyncing();
+
+  scoped_ptr<base::DictionaryValue> service_properties =
+      MakePskServiceProperties(kServiceGuid);
+  OnConfigurationCreated(
+      kServicePath, *service_properties,
+      chromeos::NetworkConfigurationObserver::SOURCE_USER_ACTION);
+
+  base::DictionaryValue new_properties;
+  new_properties.SetStringWithoutPathExpansion(
+      shill::kPassphraseProperty, "new-psk-passphrase-via-policy");
+  OnPropertiesSet(kServicePath, kServiceGuid, new_properties,
+                  chromeos::NetworkConfigurationObserver::SOURCE_POLICY);
+  new_properties.MergeDictionary(service_properties.get());
+  NetworkConnectionStateChanged(
+      MakeNetworkState(kServicePath, new_properties).get(),
+      shill::kStateOnline);
+  // We can't take the new passphrase, because it didn't come from the
+  // user. And we can't take the old passphrase, because we don't know
+  // if it works. (The successful connection may have been due to the
+  // new passphrase.) Hence, we should not sync this network at all.
+  EXPECT_EQ(0U, add_to_synced_networks_call_count());
+}
+
+// XXX test for bad source on ConfigurationProfileChange
+// XXX test with extension source
+
+TEST_F(WifiConfigObserverChromeOsTest,
+       DevicePolicyOwnedUserUpdatedNetworkNotSent) {
+  StartSyncing();
+  // No call to OnConfigurationCreated, because this network was
+  // loaded from persistent settings.
+  AddToDevicePolicyNetworks(kServiceGuid);
+
+  base::DictionaryValue new_properties;
+  new_properties.SetStringWithoutPathExpansion(
+      shill::kPassphraseProperty, "new-psk-passphrase-by-user");
+  OnPropertiesSet(kServicePath, kServiceGuid, new_properties,
+                  chromeos::NetworkConfigurationObserver::SOURCE_USER_ACTION);
+  new_properties.MergeDictionary(MakePskServiceProperties(kServiceGuid).get());
+  NetworkConnectionStateChanged(
+      MakeNetworkState(kServicePath, new_properties).get(),
+      shill::kStateOnline);
+
+  EXPECT_EQ(0U, add_to_synced_networks_call_count());
+}
+
+TEST_F(WifiConfigObserverChromeOsTest,
+       UserPolicyOwnedUserUpdatedNetworkNotSent) {
+  StartSyncing();
+  // No call to OnConfigurationCreated, because this network was
+  // loaded from persistent settings.
+  AddToUserPolicyNetworks(kServiceGuid);
+
+  base::DictionaryValue new_properties;
+  new_properties.SetStringWithoutPathExpansion(
+      shill::kPassphraseProperty, "new-psk-passphrase-by-user");
+  OnPropertiesSet(kServicePath, kServiceGuid, new_properties,
+                  chromeos::NetworkConfigurationObserver::SOURCE_USER_ACTION);
+  new_properties.MergeDictionary(MakePskServiceProperties(kServiceGuid).get());
+  NetworkConnectionStateChanged(
+      MakeNetworkState(kServicePath, new_properties).get(),
+      shill::kStateOnline);
+
+  EXPECT_EQ(0U, add_to_synced_networks_call_count());
+}
+
+// XXX create a group name?
+// XXX maybe merge with next, using OnConfigurationCreated for policy,
+// and OnPropertiesSet/OnConfigurationProfileChanged for user.
+TEST_F(WifiConfigObserverChromeOsTest,
+       SyncOnNonConflictingUserAndPolicyConfigs) {
+  StartSyncing();
+
+  scoped_ptr<base::DictionaryValue> user_service_properties =
+      MakeOpenServiceProperties(kServiceGuid);
+  OnConfigurationCreated(
+      kServicePath, *user_service_properties,
+      chromeos::NetworkConfigurationObserver::SOURCE_USER_ACTION);
+
+  scoped_ptr<base::DictionaryValue> policy_service_properties =
+      MakeOpenServiceProperties(kService2Guid);
+  AddToDevicePolicyNetworks(kService2Guid);
+  OnConfigurationCreated(
+      kService2Path, *policy_service_properties,
+      chromeos::NetworkConfigurationObserver::SOURCE_POLICY);
+
+  NetworkConnectionStateChanged(
+      MakeNetworkState(kServicePath, *user_service_properties).get(),
+      shill::kStateOnline);
+  EXPECT_EQ(1U, add_to_synced_networks_call_count());
+}
+
+TEST_F(WifiConfigObserverChromeOsTest,
+       SyncOnNonConflictingUserAndPolicyChanges) {
+  StartSyncing();
+
+  base::DictionaryValue new_user_service_properties;
+  new_user_service_properties.SetStringWithoutPathExpansion(
+      shill::kPassphraseProperty, kPassphrasePsk);
+  OnPropertiesSet(kServicePath, kServiceGuid, new_user_service_properties,
+                  chromeos::NetworkConfigurationObserver::SOURCE_USER_ACTION);
+
+  base::DictionaryValue new_policy_service_properties;
+  new_policy_service_properties.SetStringWithoutPathExpansion(
+      shill::kPassphraseProperty, kPassphraseWep);
+  AddToDevicePolicyNetworks(kService2Guid);
+  OnPropertiesSet(kService2Path, kService2Guid, new_policy_service_properties,
+                  chromeos::NetworkConfigurationObserver::SOURCE_POLICY);
+
+  NetworkConnectionStateChanged(
+      MakeNetworkState(kServicePath,
+                       *MakePskServiceProperties(kServiceGuid)).get(),
+      shill::kStateOnline);
+  EXPECT_EQ(1U, add_to_synced_networks_call_count());
+}
+
+// Tests of our Start/Stop behavior.
+
+TEST_F(WifiConfigObserverChromeOsTest, StartStop_StartSendsQueuedCredentials) {
+  scoped_ptr<base::DictionaryValue> service_properties =
+      MakePskServiceProperties(kServiceGuid);
+  RegisterNetwork(kServicePath, *service_properties);
+  SetNetworkPassphrase(kServicePath, kServiceGuid, kPassphrasePsk);
+  UpdateConnectionState(kServicePath, shill::kStateOnline);
+  OnConfigurationProfileChanged(
+      kServicePath, kProfilePath,
+      chromeos::NetworkConfigurationObserver::SOURCE_USER_ACTION);
+  EXPECT_EQ(0U, add_to_synced_networks_call_count());
+
+  StartSyncing();
+  EXPECT_EQ(1U, add_to_synced_networks_call_count());
+}
+
+TEST_F(WifiConfigObserverChromeOsTest, StartStop_StopPreventsSend) {
+  StartSyncing();
+  scoped_ptr<base::DictionaryValue> service_properties =
+      MakePskServiceProperties(kServiceGuid);
+  RegisterNetwork(kServicePath, *service_properties);
+  SetNetworkPassphrase(kServicePath, kServiceGuid, kPassphrasePsk);
+  StopSyncing();
+
+  UpdateConnectionState(kServicePath, shill::kStateOnline);
+  OnConfigurationProfileChanged(
+      kServicePath, kProfilePath,
+      chromeos::NetworkConfigurationObserver::SOURCE_USER_ACTION);
+  EXPECT_EQ(0U, add_to_synced_networks_call_count());
+}
+
+TEST_F(WifiConfigObserverChromeOsTest, StartStop_RestartDoesNotResend) {
+  scoped_ptr<base::DictionaryValue> service_properties =
+      MakeOpenServiceProperties(kServiceGuid);
+  RegisterNetwork(kServicePath, *service_properties);
+  UpdateConnectionState(kServicePath, shill::kStateOnline);
+  OnConfigurationProfileChanged(
+      kServicePath, kProfilePath,
+      chromeos::NetworkConfigurationObserver::SOURCE_USER_ACTION);
+  EXPECT_EQ(0U, add_to_synced_networks_call_count());
+
+  StartSyncing();
+  EXPECT_EQ(1U, add_to_synced_networks_call_count());
+
+  StopSyncing();
+  StartSyncing();
+  EXPECT_EQ(1U, add_to_synced_networks_call_count());
+}
+
+// Tests related to the connection manager ("Shill") restarting.
+
+TEST_F(WifiConfigObserverChromeOsTest,
+       ShillRestart_ServicePathReuseDoesNotCauseSpuriousSync) {
+  StartSyncing();
+
+  scoped_ptr<base::DictionaryValue> service_a_properties =
+      MakePskServiceProperties(kServiceGuid);
+  RegisterNetwork(kServicePath, *service_a_properties);
+  SetNetworkPassphrase(kServicePath, kServiceGuid, kPassphrasePsk);
+
+  // Due to Shill restarting, a later network gets assigned the same
+  // service path.
+  scoped_ptr<base::DictionaryValue> service_b_properties =
+      MakeWepServiceProperties(kService2Guid);
+  RegisterNetwork(kServicePath, *service_b_properties);
+  // Don't SetNetworkPassphrase(), as that might cause service B to be
+  // synced.
+
+  // Since service A has not connected, and service B was already
+  // configured, there is nothing for us to sync.
+  UpdateConnectionState(kServicePath, shill::kStateOnline);
+  EXPECT_EQ(0U, add_to_synced_networks_call_count());
+}
+
+TEST_F(WifiConfigObserverChromeOsTest,
+       ShillRestart_SyncDespiteServicePathChange) {
+  StartSyncing();
+
+  scoped_ptr<base::DictionaryValue> service_properties =
+      MakePskServiceProperties(kServiceGuid);
+  RegisterNetwork(kServicePath, *service_properties);
+  SetNetworkPassphrase(kServicePath, kServiceGuid, kPassphrasePsk);
+
+  // Shill has restarted, and assigned our service a different service
+  // path.  We shouldn't let that get in the way of syncing the
+  // network.
+  RegisterNetwork(kService2Path, *service_properties);
+  UpdateConnectionState(kService2Path, shill::kStateOnline);
+  EXPECT_EQ(1U, add_to_synced_networks_call_count());
+}
+
+// XXX not tested: user changes passphrase while connection is in
+// progress, and old connection attempt succeeds.
+
+// XXX No test for case where passphrase is supplied after
+// connect. Behavior is undefined in that case.
+
+}  // namespace wifi_sync