Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(145)

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: chromeos/network/network_configuration_handler_unittest.cc

Issue 2754903002: Prevent networkingPrivate.forgetNetwork from removing shared configs (Closed)
Patch Set: . Created 3 years, 9 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« chromeos/dbus/fake_shill_profile_client.cc ('K') | « chromeos/network/network_configuration_handler.cc ('k') | components/sync_wifi/wifi_config_delegate_chromeos_unittest.cc » ('j') | extensions/browser/api/networking_private/networking_private_chromeos.cc » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: chromeos/network/network_configuration_handler_unittest.cc
diff --git a/chromeos/network/network_configuration_handler_unittest.cc b/chromeos/network/network_configuration_handler_unittest.cc
index 2ae84f39ce3f6cceee98f6ab6023f076ca0f3dd3..554d9303b1c2f4436a7db08109e529f32f932a94 100644
--- a/chromeos/network/network_configuration_handler_unittest.cc
+++ b/chromeos/network/network_configuration_handler_unittest.cc
@@ -65,13 +65,17 @@ void DictionaryValueCallback(const std::string& expected_id,
EXPECT_EQ(expected_id, service_path);
}
-void ErrorCallback(bool error_expected,
- const std::string& expected_id,
- const std::string& error_name,
+void ErrorCallback(const std::string& error_name,
std::unique_ptr<base::DictionaryValue> error_data) {
- EXPECT_TRUE(error_expected) << "Unexpected error: " << error_name
- << " with associated data: \n"
- << PrettyJson(*error_data);
+ ADD_FAILURE() << "Unexpected error: " << error_name
+ << " with associated data: \n"
+ << PrettyJson(*error_data);
+}
+
+void RecordError(std::string* error_name_ptr,
+ const std::string& error_name,
+ std::unique_ptr<base::DictionaryValue> error_data) {
+ *error_name_ptr = error_name;
}
void ServiceResultCallback(const std::string& expected_result,
@@ -194,11 +198,19 @@ class NetworkConfigurationHandlerTest : public testing::Test {
std::unique_ptr<ShillServiceClient>(mock_service_client_));
EXPECT_CALL(*mock_service_client_, GetProperties(_, _)).Times(AnyNumber());
+ EXPECT_CALL(*mock_service_client_, AddPropertyChangedObserver(_, _))
+ .Times(AnyNumber());
+ EXPECT_CALL(*mock_service_client_, RemovePropertyChangedObserver(_, _))
+ .Times(AnyNumber());
EXPECT_CALL(*mock_manager_client_, GetProperties(_)).Times(AnyNumber());
EXPECT_CALL(*mock_manager_client_, AddPropertyChangedObserver(_))
- .Times(AnyNumber());
+ .WillRepeatedly(Invoke(
+ this,
+ &NetworkConfigurationHandlerTest::AddPropertyChangedObserver));
EXPECT_CALL(*mock_manager_client_, RemovePropertyChangedObserver(_))
- .Times(AnyNumber());
+ .WillRepeatedly(Invoke(
+ this,
+ &NetworkConfigurationHandlerTest::RemovePropertyChangedObserver));
network_state_handler_ = NetworkStateHandler::InitializeForTest();
network_configuration_handler_.reset(new NetworkConfigurationHandler());
@@ -214,6 +226,14 @@ class NetworkConfigurationHandlerTest : public testing::Test {
DBusThreadManager::Shutdown();
}
+ void AddPropertyChangedObserver(ShillPropertyChangedObserver* observer) {
+ property_changed_observers_.AddObserver(observer);
+ }
+
+ void RemovePropertyChangedObserver(ShillPropertyChangedObserver* observer) {
+ property_changed_observers_.RemoveObserver(observer);
+ }
+
// Handles responses for GetProperties method calls.
void OnGetProperties(
const dbus::ObjectPath& path,
@@ -258,6 +278,15 @@ class NetworkConfigurationHandlerTest : public testing::Test {
const ObjectPathCallback& callback,
const ShillClientHelper::ErrorCallback& error_callback) {
callback.Run(dbus::ObjectPath("/service/2"));
+
+ // Notify property changed observer that service list was changed - the
+ // goal is to have network state handler update it's network state list.
+ base::ListValue value;
+ value.AppendString("/service/1");
+ value.AppendString("/service/2");
+ for (auto& observer : property_changed_observers_) {
+ observer.OnPropertyChanged("ServiceCompleteList", value);
+ }
}
void OnGetLoadableProfileEntries(
@@ -287,7 +316,7 @@ class NetworkConfigurationHandlerTest : public testing::Test {
network_configuration_handler_->CreateShillConfiguration(
properties, NetworkConfigurationObserver::SOURCE_USER_ACTION,
base::Bind(&ServiceResultCallback, service_path),
- base::Bind(&ErrorCallback, false, std::string()));
+ base::Bind(&ErrorCallback));
}
protected:
@@ -298,6 +327,9 @@ class NetworkConfigurationHandlerTest : public testing::Test {
std::unique_ptr<NetworkConfigurationHandler> network_configuration_handler_;
base::MessageLoopForUI message_loop_;
base::DictionaryValue* dictionary_value_result_;
+
+ base::ObserverList<ShillPropertyChangedObserver, true>
+ property_changed_observers_;
};
TEST_F(NetworkConfigurationHandlerTest, GetProperties) {
@@ -325,7 +357,7 @@ TEST_F(NetworkConfigurationHandlerTest, GetProperties) {
network_configuration_handler_->GetShillProperties(
service_path,
base::Bind(&DictionaryValueCallback, service_path, expected_json),
- base::Bind(&ErrorCallback, false, service_path));
+ base::Bind(&ErrorCallback));
base::RunLoop().RunUntilIdle();
}
@@ -343,8 +375,7 @@ TEST_F(NetworkConfigurationHandlerTest, SetProperties) {
Invoke(this, &NetworkConfigurationHandlerTest::OnSetProperties));
network_configuration_handler_->SetShillProperties(
service_path, value, NetworkConfigurationObserver::SOURCE_USER_ACTION,
- base::Bind(&base::DoNothing),
- base::Bind(&ErrorCallback, false, service_path));
+ base::Bind(&base::DoNothing), base::Bind(&ErrorCallback));
base::RunLoop().RunUntilIdle();
}
@@ -363,8 +394,7 @@ TEST_F(NetworkConfigurationHandlerTest, ClearProperties) {
Invoke(this, &NetworkConfigurationHandlerTest::OnSetProperties));
network_configuration_handler_->SetShillProperties(
service_path, value, NetworkConfigurationObserver::SOURCE_USER_ACTION,
- base::Bind(&base::DoNothing),
- base::Bind(&ErrorCallback, false, service_path));
+ base::Bind(&base::DoNothing), base::Bind(&ErrorCallback));
base::RunLoop().RunUntilIdle();
// Now clear it.
@@ -375,7 +405,7 @@ TEST_F(NetworkConfigurationHandlerTest, ClearProperties) {
Invoke(this, &NetworkConfigurationHandlerTest::OnClearProperties));
network_configuration_handler_->ClearShillProperties(
service_path, values_to_clear, base::Bind(&base::DoNothing),
- base::Bind(&ErrorCallback, false, service_path));
+ base::Bind(&ErrorCallback));
base::RunLoop().RunUntilIdle();
}
@@ -394,8 +424,7 @@ TEST_F(NetworkConfigurationHandlerTest, ClearPropertiesError) {
Invoke(this, &NetworkConfigurationHandlerTest::OnSetProperties));
network_configuration_handler_->SetShillProperties(
service_path, value, NetworkConfigurationObserver::SOURCE_USER_ACTION,
- base::Bind(&base::DoNothing),
- base::Bind(&ErrorCallback, false, service_path));
+ base::Bind(&base::DoNothing), base::Bind(&ErrorCallback));
base::RunLoop().RunUntilIdle();
// Now clear it.
@@ -404,9 +433,11 @@ TEST_F(NetworkConfigurationHandlerTest, ClearPropertiesError) {
EXPECT_CALL(*mock_service_client_, ClearProperties(_, _, _, _))
.WillOnce(Invoke(
this, &NetworkConfigurationHandlerTest::OnClearPropertiesError));
+
+ std::string error;
network_configuration_handler_->ClearShillProperties(
service_path, values_to_clear, base::Bind(&base::DoNothing),
- base::Bind(&ErrorCallback, true, service_path));
+ base::Bind(&ErrorCallback));
base::RunLoop().RunUntilIdle();
}
@@ -430,25 +461,137 @@ TEST_F(NetworkConfigurationHandlerTest, CreateConfiguration) {
}
TEST_F(NetworkConfigurationHandlerTest, RemoveConfiguration) {
- std::string service_path = "/service/1";
+ std::string service_path = "/service/2";
- TestCallback test_callback;
- EXPECT_CALL(*mock_service_client_, GetLoadableProfileEntries(_, _))
+ // Set up network configuration so the associated network service has the
+ // profile path set to |profile|.
+ std::string key = "SSID";
+ std::string type = "wifi";
+ base::DictionaryValue value;
+ shill_property_util::SetSSID("Service", &value);
+ value.SetWithoutPathExpansion(shill::kTypeProperty, new base::Value(type));
+ value.SetWithoutPathExpansion(shill::kProfileProperty,
+ new base::Value("profile2"));
+ EXPECT_CALL(*mock_manager_client_,
+ ConfigureServiceForProfile(dbus::ObjectPath("profile2"), _, _, _))
+ .WillOnce(
+ Invoke(this, &NetworkConfigurationHandlerTest::OnConfigureService));
+
+ dictionary_value_result_ = &value;
+ EXPECT_CALL(*mock_service_client_, GetProperties(_, _))
+ .WillRepeatedly(
+ Invoke(this, &NetworkConfigurationHandlerTest::OnGetProperties));
+
+ CreateConfiguration(service_path, value);
+ base::RunLoop().RunUntilIdle();
+
+ // Set Up mock flor GetLoadableProfileEntries - it returns
+ // [(profile1, entry1), (profile2, entry2)] profile-entry pairs.
+ EXPECT_CALL(*mock_service_client_,
+ GetLoadableProfileEntries(dbus::ObjectPath(service_path), _))
.WillOnce(Invoke(
this, &NetworkConfigurationHandlerTest::OnGetLoadableProfileEntries));
- EXPECT_CALL(*mock_profile_client_, DeleteEntry(_, _, _, _))
- .WillRepeatedly(
- Invoke(this, &NetworkConfigurationHandlerTest::OnDeleteEntry));
+ // Expectations for entries deleted during the test.
+ EXPECT_CALL(*mock_profile_client_,
+ DeleteEntry(dbus::ObjectPath("profile1"), "entry1", _, _))
+ .WillOnce(Invoke(this, &NetworkConfigurationHandlerTest::OnDeleteEntry));
+ EXPECT_CALL(*mock_profile_client_,
+ DeleteEntry(dbus::ObjectPath("profile2"), "entry2", _, _))
+ .WillOnce(Invoke(this, &NetworkConfigurationHandlerTest::OnDeleteEntry));
+
+ TestCallback test_callback;
network_configuration_handler_->RemoveConfiguration(
service_path, NetworkConfigurationObserver::SOURCE_USER_ACTION,
base::Bind(&TestCallback::Run, base::Unretained(&test_callback)),
- base::Bind(&ErrorCallback, false, service_path));
+ base::Bind(&ErrorCallback));
+
+ base::RunLoop().RunUntilIdle();
+ EXPECT_EQ(1, test_callback.run_count());
+ EXPECT_FALSE(PendingProfileEntryDeleterForTest(service_path));
+}
+
+TEST_F(NetworkConfigurationHandlerTest, RemoveConfigurationFromCurrentProfile) {
+ std::string service_path = "/service/2";
+
+ // Set up network configuration so the associated network service has the
+ // profile path set to |profile|.
+ std::string key = "SSID";
+ std::string type = "wifi";
+ base::DictionaryValue value;
+ shill_property_util::SetSSID("Service", &value);
+ value.SetWithoutPathExpansion(shill::kTypeProperty, new base::Value(type));
+ value.SetWithoutPathExpansion(shill::kProfileProperty,
+ new base::Value("profile2"));
+ EXPECT_CALL(*mock_manager_client_,
+ ConfigureServiceForProfile(dbus::ObjectPath("profile2"), _, _, _))
+ .WillOnce(
+ Invoke(this, &NetworkConfigurationHandlerTest::OnConfigureService));
+
+ dictionary_value_result_ = &value;
+ EXPECT_CALL(*mock_service_client_, GetProperties(_, _))
+ .WillRepeatedly(
+ Invoke(this, &NetworkConfigurationHandlerTest::OnGetProperties));
+
+ CreateConfiguration(service_path, value);
+ base::RunLoop().RunUntilIdle();
+
+ // Set Up mock flor GetLoadableProfileEntries - it returns
+ // [(profile1, entry1), (profile2, entry2)] profile-entry pairs.
+ EXPECT_CALL(*mock_service_client_,
+ GetLoadableProfileEntries(dbus::ObjectPath(service_path), _))
+ .WillOnce(Invoke(
+ this, &NetworkConfigurationHandlerTest::OnGetLoadableProfileEntries));
+
+ // Expectations for entries deleted during the test.
+ EXPECT_CALL(*mock_profile_client_,
+ DeleteEntry(dbus::ObjectPath("profile2"), "entry2", _, _))
+ .WillOnce(Invoke(this, &NetworkConfigurationHandlerTest::OnDeleteEntry));
+ EXPECT_CALL(*mock_profile_client_,
+ DeleteEntry(dbus::ObjectPath("profile1"), "entry1", _, _))
+ .Times(0);
+
+ TestCallback test_callback;
+ network_configuration_handler_->RemoveConfigurationFromCurrentProfile(
+ service_path, NetworkConfigurationObserver::SOURCE_USER_ACTION,
+ base::Bind(&TestCallback::Run, base::Unretained(&test_callback)),
+ base::Bind(&ErrorCallback));
+
base::RunLoop().RunUntilIdle();
EXPECT_EQ(1, test_callback.run_count());
EXPECT_FALSE(PendingProfileEntryDeleterForTest(service_path));
}
+TEST_F(NetworkConfigurationHandlerTest,
+ RemoveNonExistentConfigurationFromCurrentProfile) {
+ // Set Up mock flor GetLoadableProfileEntries - it returns
+ // [(profile1, entry1), (profile2, entry2)] profile-entry pairs.
+ EXPECT_CALL(*mock_service_client_,
+ GetLoadableProfileEntries(dbus::ObjectPath("/service/3"), _))
+ .WillRepeatedly(Invoke(
+ this, &NetworkConfigurationHandlerTest::OnGetLoadableProfileEntries));
+
+ // Expectations for entries deleted during the test.
+ EXPECT_CALL(*mock_profile_client_,
+ DeleteEntry(dbus::ObjectPath("profile2"), "entry2", _, _))
+ .Times(0);
+ EXPECT_CALL(*mock_profile_client_,
+ DeleteEntry(dbus::ObjectPath("profile1"), "entry1", _, _))
+ .Times(0);
+
+ TestCallback test_callback;
+ std::string error;
+ network_configuration_handler_->RemoveConfigurationFromCurrentProfile(
+ "/service/3", NetworkConfigurationObserver::SOURCE_USER_ACTION,
+ base::Bind(&TestCallback::Run, base::Unretained(&test_callback)),
+ base::Bind(&RecordError, base::Unretained(&error)));
+ EXPECT_EQ("NetworkNotConfigured", error);
+
+ base::RunLoop().RunUntilIdle();
+ EXPECT_EQ(0, test_callback.run_count());
+ EXPECT_FALSE(PendingProfileEntryDeleterForTest("/service/3"));
+}
+
////////////////////////////////////////////////////////////////////////////////
// Stub based tests
@@ -545,7 +688,7 @@ class NetworkConfigurationHandlerStubTest : public testing::Test {
base::Bind(
&NetworkConfigurationHandlerStubTest::CreateConfigurationCallback,
base::Unretained(this)),
- base::Bind(&ErrorCallback, false, service_path));
+ base::Bind(&ErrorCallback));
base::RunLoop().RunUntilIdle();
}
@@ -600,7 +743,7 @@ TEST_F(NetworkConfigurationHandlerStubTest, StubSetAndClearProperties) {
NetworkConfigurationObserver::SOURCE_USER_ACTION,
base::Bind(&NetworkConfigurationHandlerStubTest::SuccessCallback,
base::Unretained(this), "SetProperties"),
- base::Bind(&ErrorCallback, false, service_path));
+ base::Bind(&ErrorCallback));
base::RunLoop().RunUntilIdle();
EXPECT_EQ("SetProperties", success_callback_name_);
@@ -621,7 +764,7 @@ TEST_F(NetworkConfigurationHandlerStubTest, StubSetAndClearProperties) {
service_path, properties_to_clear,
base::Bind(&NetworkConfigurationHandlerStubTest::SuccessCallback,
base::Unretained(this), "ClearProperties"),
- base::Bind(&ErrorCallback, false, service_path));
+ base::Bind(&ErrorCallback));
base::RunLoop().RunUntilIdle();
EXPECT_EQ("ClearProperties", success_callback_name_);
@@ -645,8 +788,7 @@ TEST_F(NetworkConfigurationHandlerStubTest, StubGetNameFromWifiHex) {
network_configuration_handler_->SetShillProperties(
service_path, properties_to_set,
NetworkConfigurationObserver::SOURCE_USER_ACTION,
- base::Bind(&base::DoNothing),
- base::Bind(&ErrorCallback, false, service_path));
+ base::Bind(&base::DoNothing), base::Bind(&ErrorCallback));
base::RunLoop().RunUntilIdle();
std::string wifi_hex_result;
EXPECT_TRUE(GetServiceStringProperty(service_path, shill::kWifiHexSsid,
@@ -658,7 +800,7 @@ TEST_F(NetworkConfigurationHandlerStubTest, StubGetNameFromWifiHex) {
service_path,
base::Bind(&NetworkConfigurationHandlerStubTest::GetPropertiesCallback,
base::Unretained(this)),
- base::Bind(&ErrorCallback, false, service_path));
+ base::Bind(&ErrorCallback));
base::RunLoop().RunUntilIdle();
EXPECT_EQ(service_path, get_properties_path_);
@@ -706,8 +848,7 @@ TEST_F(NetworkConfigurationHandlerStubTest, NetworkConfigurationObserver) {
network_configuration_handler_->SetShillProperties(
service_path, properties_to_set,
NetworkConfigurationObserver::SOURCE_USER_ACTION,
- base::Bind(&base::DoNothing),
- base::Bind(&ErrorCallback, false, service_path));
+ base::Bind(&base::DoNothing), base::Bind(&ErrorCallback));
base::RunLoop().RunUntilIdle();
EXPECT_EQ(test_passphrase, test_observer->GetStringProperty(
service_path, shill::kPassphraseProperty));
@@ -722,8 +863,7 @@ TEST_F(NetworkConfigurationHandlerStubTest, NetworkConfigurationObserver) {
network_configuration_handler_->SetNetworkProfile(
service_path, user_profile,
NetworkConfigurationObserver::SOURCE_USER_ACTION,
- base::Bind(&base::DoNothing),
- base::Bind(&ErrorCallback, false, service_path));
+ base::Bind(&base::DoNothing), base::Bind(&ErrorCallback));
base::RunLoop().RunUntilIdle();
EXPECT_TRUE(test_observer->HasConfiguration(service_path));
EXPECT_FALSE(test_observer->HasConfigurationInProfile(
@@ -733,8 +873,7 @@ TEST_F(NetworkConfigurationHandlerStubTest, NetworkConfigurationObserver) {
network_configuration_handler_->RemoveConfiguration(
service_path, NetworkConfigurationObserver::SOURCE_USER_ACTION,
- base::Bind(&base::DoNothing),
- base::Bind(&ErrorCallback, false, service_path));
+ base::Bind(&base::DoNothing), base::Bind(&ErrorCallback));
base::RunLoop().RunUntilIdle();
EXPECT_FALSE(test_observer->HasConfiguration(service_path));
« chromeos/dbus/fake_shill_profile_client.cc ('K') | « chromeos/network/network_configuration_handler.cc ('k') | components/sync_wifi/wifi_config_delegate_chromeos_unittest.cc » ('j') | extensions/browser/api/networking_private/networking_private_chromeos.cc » ('J')

Powered by Google App Engine
This is Rietveld 408576698