shill: Emit more property change notifications for Services.
Add property change notifications for all the Service properties that
Chrome currently pays attention to. Also add unit tests for these
property change notifications.
While there:
- add some more tips to TESTING
- update service-api documentation for AutoConnect
(it is settable for non-favorite services, and has been for
some time)
- update some ServiceTests to no longer set_favorite before
changing AutoConnect (set_favorite is no longer necessary,
as noted above)
- clarify service-api documentation for Device property
- make the security_ field of WiFiService const (it is only
set in the ctor, and designating it const makes it obvious
why there's no property change for it)
- fix bug where VPNService would change the Favorite property,
but not signal the change
BUG=chromium:230329
TEST=new unit tests
Change-Id: Ia387b1ab90fa80fd218cd69e0fd8126ff4c5a2ae
Reviewed-on: https://gerrit.chromium.org/gerrit/48459
Reviewed-by: Paul Stewart <pstew@chromium.org>
Commit-Queue: mukesh agrawal <quiche@chromium.org>
Tested-by: mukesh agrawal <quiche@chromium.org>
diff --git a/Makefile b/Makefile
index 0274460..ce401c7 100644
--- a/Makefile
+++ b/Makefile
@@ -463,6 +463,7 @@
rtnl_listener_unittest.o \
rtnl_message_unittest.o \
scope_logger_unittest.o \
+ service_property_change_test.o \
service_under_test.o \
service_unittest.o \
supplicant_eap_state_handler_unittest.o \
diff --git a/TESTING b/TESTING
index 2d4d226..f93d4a8 100644
--- a/TESTING
+++ b/TESTING
@@ -38,10 +38,22 @@
(chroot)$ GTEST_ARGS="--v=1000 --gtest_filter=WiFiPropertyTest.*" \
CXXFLAGS=-g cros_workon_make --board x86-generic --reconf --test shill
+- if you want to set a breakpoint in gdb, make sure to include the shill
+ namespace. e.g., run
+ (cros-gdb) b shill::EthernetService::GetStorageIdentifier
+ Breakpoint 2 at 0x5555563cc270: file ethernet_service.cc, line 63.
+ rather than
+ (cros-gdb) b EthernetService::GetStorageIdentifier
+ Function "EthernetService::GetStorageIdentifier" not defined.
+ Make breakpoint pending on future shared library load? (y or [n]) n
+
- alternate build arguments:
- to see the actual compiler commands that are run:
- (chroot)$ CFLAGS="-print-cmdline" cros_workon_make --reconf \
- --board=link shill
+ - to see the actual compiler commands that are run:
+ (chroot)$ CFLAGS="-print-cmdline" cros_workon_make --reconf \
+ --board=link shill
+ - to abort compilation on the first error
+ (chroot)$ MAKEFLAGS="--stop" cros_workon_make --test --board=link \
+ --reconf shill
Running integration tests
-------------------------
diff --git a/cellular_service.cc b/cellular_service.cc
index 1d110a7..e546b9d 100644
--- a/cellular_service.cc
+++ b/cellular_service.cc
@@ -106,7 +106,7 @@
num_connect_attempts_(0),
out_of_credits_detection_in_progress_(false),
out_of_credits_(false) {
- set_connectable(true);
+ SetConnectable(true);
PropertyStore *store = this->mutable_store();
store->RegisterConstBool(kActivateOverNonCellularNetworkProperty,
&activate_over_non_cellular_network_);
@@ -461,7 +461,7 @@
}
activation_state_ = state;
adaptor()->EmitStringChanged(flimflam::kActivationStateProperty, state);
- SetConnectable(state != flimflam::kActivationStateNotActivated);
+ SetConnectableFull(state != flimflam::kActivationStateNotActivated);
}
void CellularService::SetOLP(const OLP &olp) {
diff --git a/cellular_service_unittest.cc b/cellular_service_unittest.cc
index 951b330..ba46a09 100644
--- a/cellular_service_unittest.cc
+++ b/cellular_service_unittest.cc
@@ -19,6 +19,7 @@
#include "shill/mock_store.h"
#include "shill/nice_mock_control.h"
#include "shill/proxy_factory.h"
+#include "shill/service_property_change_test.h"
using std::string;
using testing::_;
@@ -363,7 +364,7 @@
// The following test cases are copied from ServiceTest.IsAutoConnectable
- service_->set_connectable(true);
+ service_->SetConnectable(true);
EXPECT_TRUE(service_->IsAutoConnectable(&reason));
// We should not auto-connect to a Service that a user has
@@ -533,4 +534,9 @@
dispatcher_.DispatchPendingEvents();
}
+TEST_F(CellularServiceTest, PropertyChanges) {
+ TestCommonPropertyChanges(service_, adaptor_);
+ TestAutoConnectPropertyChange(service_, adaptor_);
+}
+
} // namespace shill
diff --git a/dbus_adaptor.cc b/dbus_adaptor.cc
index 6dd566d..5341d6a 100644
--- a/dbus_adaptor.cc
+++ b/dbus_adaptor.cc
@@ -25,15 +25,13 @@
namespace shill {
-// static
+// public static
+const char DBusAdaptor::kNullPath[] = "/";
+// private statics
const char DBusAdaptor::kByteArraysSig[] = "aay";
-// static
const char DBusAdaptor::kPathsSig[] = "ao";
-// static
const char DBusAdaptor::kStringmapSig[] = "a{ss}";
-// static
const char DBusAdaptor::kStringmapsSig[] = "aa{ss}";
-// static
const char DBusAdaptor::kStringsSig[] = "as";
DBusAdaptor::DBusAdaptor(DBus::Connection* conn, const string &object_path)
diff --git a/dbus_adaptor.h b/dbus_adaptor.h
index e51a52d..dcb9bd1 100644
--- a/dbus_adaptor.h
+++ b/dbus_adaptor.h
@@ -32,6 +32,8 @@
public DBus::IntrospectableAdaptor,
public base::SupportsWeakPtr<DBusAdaptor> {
public:
+ static const char kNullPath[];
+
DBusAdaptor(DBus::Connection* conn, const std::string &object_path);
virtual ~DBusAdaptor();
diff --git a/doc/service-api.txt b/doc/service-api.txt
index adc2c1d..de8ef46 100644
--- a/doc/service-api.txt
+++ b/doc/service-api.txt
@@ -172,11 +172,6 @@
services are marked for auto-connect then the highest
priority available service will be selected.
- For favorite services it is possible to change
- this value to prevent or permit automatic
- connection attempts. For non-favorite services, setting
- AutoConnect to TRUE causes favorite to be set to TRUE.
-
boolean Cellular.ActivateOverNonCellularNetwork [readonly]
(Cellular only) If set to true, this service must be
@@ -350,6 +345,9 @@
This value may be used to retrieve and manipulate
Layer 3 configuration state.
+ A value of "/" indicates that the service is
+ not bound to any device.
+
array{string} Diagnostics.Disconnects [readonly]
History (wall-clock timestamps) of connection drops.
diff --git a/ethernet_service.cc b/ethernet_service.cc
index af19822..e9dcbb0 100644
--- a/ethernet_service.cc
+++ b/ethernet_service.cc
@@ -38,8 +38,8 @@
: Service(control_interface, dispatcher, metrics, manager,
Technology::kEthernet),
ethernet_(device) {
- set_connectable(true);
- set_auto_connect(true);
+ SetConnectable(true);
+ SetAutoConnect(true);
set_friendly_name("Ethernet");
SetStrength(kStrengthMax);
}
@@ -64,14 +64,15 @@
kServiceType, ethernet_->address().c_str());
}
-void EthernetService::SetAutoConnect(const bool &connect, Error *error) {
+void EthernetService::SetAutoConnectFull(const bool &connect,
+ Error *error) {
if (!connect) {
Error::PopulateAndLog(
error, Error::kInvalidArguments,
"Auto-connect on Ethernet services must not be disabled.");
return;
}
- Service::SetAutoConnect(connect, error);
+ Service::SetAutoConnectFull(connect, error);
}
void EthernetService::Remove(Error *error) {
diff --git a/ethernet_service.h b/ethernet_service.h
index 38b01aa..af32816 100644
--- a/ethernet_service.h
+++ b/ethernet_service.h
@@ -34,7 +34,7 @@
// ethernet_<MAC>
virtual std::string GetStorageIdentifier() const;
virtual bool IsAutoConnectByDefault() const { return true; }
- virtual void SetAutoConnect(const bool &connect, Error *error);
+ virtual void SetAutoConnectFull(const bool &connect, Error *error);
virtual void Remove(Error *error);
diff --git a/ethernet_service_unittest.cc b/ethernet_service_unittest.cc
index 69f9844..427231b 100644
--- a/ethernet_service_unittest.cc
+++ b/ethernet_service_unittest.cc
@@ -7,9 +7,12 @@
#include <gmock/gmock.h>
#include <gtest/gtest.h>
+#include "shill/mock_adaptors.h"
#include "shill/mock_ethernet.h"
+#include "shill/mock_manager.h"
#include "shill/property_store_unittest.h"
#include "shill/refptr_types.h"
+#include "shill/service_property_change_test.h"
using ::testing::NiceMock;
@@ -18,11 +21,12 @@
class EthernetServiceTest : public PropertyStoreTest {
public:
EthernetServiceTest()
- : ethernet_(
+ : mock_manager_(control_interface(), dispatcher(), metrics(), glib()),
+ ethernet_(
new NiceMock<MockEthernet>(control_interface(),
dispatcher(),
metrics(),
- manager(),
+ &mock_manager_,
"ethernet",
fake_mac,
0)),
@@ -30,7 +34,7 @@
new EthernetService(control_interface(),
dispatcher(),
metrics(),
- manager(),
+ &mock_manager_,
ethernet_)) {}
virtual ~EthernetServiceTest() {}
@@ -42,9 +46,14 @@
}
void SetAutoConnect(const bool connect, Error *error) {
- return service_->SetAutoConnect(connect, error);
+ return service_->SetAutoConnectFull(connect, error);
}
+ ServiceMockAdaptor *GetAdaptor() {
+ return dynamic_cast<ServiceMockAdaptor *>(service_->adaptor());
+ }
+
+ MockManager mock_manager_;
scoped_refptr<MockEthernet> ethernet_;
EthernetServiceRefPtr service_;
};
@@ -77,4 +86,8 @@
service_->Disconnect(&error);
}
+TEST_F(EthernetServiceTest, PropertyChanges) {
+ TestCommonPropertyChanges(service_, GetAdaptor());
+}
+
} // namespace shill
diff --git a/manager_unittest.cc b/manager_unittest.cc
index 13998bc..b6d1164 100644
--- a/manager_unittest.cc
+++ b/manager_unittest.cc
@@ -378,7 +378,7 @@
metrics(),
manager());
service->MakeFavorite();
- service->set_connectable(true);
+ service->SetConnectable(true);
return service;
}
@@ -1917,7 +1917,7 @@
metrics(),
manager()));
const string kGUID = "a guid";
- mock_service->set_guid(kGUID);
+ mock_service->SetGuid(kGUID, NULL);
manager()->RegisterService(mock_service);
ServiceRefPtr mock_service_generic(mock_service.get());
@@ -2254,7 +2254,7 @@
EXPECT_TRUE(ServiceOrderIs(mock_service0, mock_service1));
// Priority.
- mock_service0->set_priority(1);
+ mock_service0->SetPriority(1, NULL);
manager()->UpdateService(mock_service0);
EXPECT_TRUE(ServiceOrderIs(mock_service0, mock_service1));
@@ -2264,9 +2264,9 @@
EXPECT_TRUE(ServiceOrderIs(mock_service1, mock_service0));
// Auto-connect.
- mock_service0->set_auto_connect(true);
+ mock_service0->SetAutoConnect(true);
manager()->UpdateService(mock_service0);
- mock_service1->set_auto_connect(false);
+ mock_service1->SetAutoConnect(false);
manager()->UpdateService(mock_service1);
EXPECT_TRUE(ServiceOrderIs(mock_service0, mock_service1));
@@ -2283,9 +2283,9 @@
EXPECT_TRUE(ServiceOrderIs(mock_service0, mock_service1));
// Connectable.
- mock_service1->set_connectable(true);
+ mock_service1->SetConnectable(true);
manager()->UpdateService(mock_service1);
- mock_service0->set_connectable(false);
+ mock_service0->SetConnectable(false);
manager()->UpdateService(mock_service0);
EXPECT_TRUE(ServiceOrderIs(mock_service1, mock_service0));
@@ -2360,11 +2360,11 @@
EXPECT_CALL(mock_metrics, NotifyDefaultServiceChanged(NULL));
manager()->SortServicesTask();
- mock_service1->set_priority(1);
+ mock_service1->SetPriority(1, NULL);
EXPECT_CALL(mock_metrics, NotifyDefaultServiceChanged(NULL));
manager()->SortServicesTask();
- mock_service1->set_priority(0);
+ mock_service1->SetPriority(0, NULL);
EXPECT_CALL(mock_metrics, NotifyDefaultServiceChanged(NULL));
manager()->SortServicesTask();
@@ -2382,7 +2382,7 @@
service_watcher.AsWeakPtr()));
EXPECT_EQ(1, tag);
- mock_service1->set_priority(1);
+ mock_service1->SetPriority(1, NULL);
EXPECT_CALL(*mock_connection0.get(), SetIsDefault(false));
EXPECT_CALL(*mock_connection1.get(), SetIsDefault(true));
EXPECT_CALL(service_watcher, OnDefaultServiceChanged(_));
@@ -2746,9 +2746,9 @@
TEST_F(ManagerTest, AutoConnectOnUpdate) {
MockServiceRefPtr service1 = MakeAutoConnectableService();
- service1->set_priority(1);
+ service1->SetPriority(1, NULL);
MockServiceRefPtr service2 = MakeAutoConnectableService();
- service2->set_priority(2);
+ service2->SetPriority(2, NULL);
manager()->RegisterService(service1);
manager()->RegisterService(service2);
dispatcher()->DispatchPendingEvents();
@@ -2766,9 +2766,9 @@
TEST_F(ManagerTest, AutoConnectOnDeregister) {
MockServiceRefPtr service1 = MakeAutoConnectableService();
- service1->set_priority(1);
+ service1->SetPriority(1, NULL);
MockServiceRefPtr service2 = MakeAutoConnectableService();
- service2->set_priority(2);
+ service2->SetPriority(2, NULL);
manager()->RegisterService(service1);
manager()->RegisterService(service2);
dispatcher()->DispatchPendingEvents();
@@ -3034,8 +3034,8 @@
EXPECT_FALSE(service);
}
- mock_service0->set_guid(kGUID0);
- mock_service1->set_guid(kGUID1);
+ mock_service0->SetGuid(kGUID0, NULL);
+ mock_service1->SetGuid(kGUID1, NULL);
{
Error error;
@@ -3355,8 +3355,8 @@
.WillRepeatedly(Return(Service::kStateIdle));
EXPECT_CALL(*wifi_service0.get(), IsConnected())
.WillRepeatedly(Return(false));
- wifi_service0->set_connectable(true);
- wifi_service0->set_auto_connect(true);
+ wifi_service0->SetConnectable(true);
+ wifi_service0->SetAutoConnect(true);
wifi_service0->SetSecurity(Service::kCryptoAes, true, true);
EXPECT_CALL(*wifi_service0.get(), technology())
.WillRepeatedly(Return(Technology::kWifi));
@@ -3370,8 +3370,8 @@
.WillRepeatedly(Return(Service::kStateIdle));
EXPECT_CALL(*wifi_service1.get(), IsConnected())
.WillRepeatedly(Return(false));
- wifi_service1->set_auto_connect(true);
- wifi_service1->set_connectable(true);
+ wifi_service1->SetAutoConnect(true);
+ wifi_service1->SetConnectable(true);
wifi_service1->SetSecurity(Service::kCryptoRc4, true, true);
EXPECT_CALL(*wifi_service1.get(), technology())
.WillRepeatedly(Return(Technology::kWifi));
@@ -3385,8 +3385,8 @@
.WillRepeatedly(Return(Service::kStateConnected));
EXPECT_CALL(*wifi_service2.get(), IsConnected())
.WillRepeatedly(Return(true));
- wifi_service2->set_auto_connect(true);
- wifi_service2->set_connectable(true);
+ wifi_service2->SetAutoConnect(true);
+ wifi_service2->SetConnectable(true);
wifi_service2->SetSecurity(Service::kCryptoNone, false, false);
EXPECT_CALL(*wifi_service2.get(), technology())
.WillRepeatedly(Return(Technology::kWifi));
@@ -3408,8 +3408,8 @@
.WillRepeatedly(Return(Service::kStateConnected));
EXPECT_CALL(*cell_service.get(), IsConnected())
.WillRepeatedly(Return(true));
- wifi_service2->set_auto_connect(true);
- cell_service->set_connectable(true);
+ wifi_service2->SetAutoConnect(true);
+ cell_service->SetConnectable(true);
EXPECT_CALL(*cell_service.get(), technology())
.WillRepeatedly(Return(Technology::kCellular));
manager()->RegisterService(cell_service);
@@ -3424,8 +3424,8 @@
.WillRepeatedly(Return(Service::kStateIdle));
EXPECT_CALL(*vpn_service.get(), IsConnected())
.WillRepeatedly(Return(false));
- wifi_service2->set_auto_connect(false);
- vpn_service->set_connectable(true);
+ wifi_service2->SetAutoConnect(false);
+ vpn_service->SetConnectable(true);
EXPECT_CALL(*vpn_service.get(), technology())
.WillRepeatedly(Return(Technology::kVPN));
manager()->RegisterService(vpn_service);
diff --git a/metrics_unittest.cc b/metrics_unittest.cc
index e37ab32..370948d 100644
--- a/metrics_unittest.cc
+++ b/metrics_unittest.cc
@@ -42,22 +42,40 @@
&dispatcher_,
&metrics_,
&manager_)),
- wifi_service_(new MockWiFiService(&control_interface_,
- &dispatcher_,
- &metrics_,
- &manager_,
- manager_.wifi_provider(),
- ssid_,
- flimflam::kModeManaged,
- flimflam::kSecurityNone,
- false)),
+ open_wifi_service_(new MockWiFiService(&control_interface_,
+ &dispatcher_,
+ &metrics_,
+ &manager_,
+ manager_.wifi_provider(),
+ ssid_,
+ flimflam::kModeManaged,
+ flimflam::kSecurityNone,
+ false)),
+ wep_wifi_service_(new MockWiFiService(&control_interface_,
+ &dispatcher_,
+ &metrics_,
+ &manager_,
+ manager_.wifi_provider(),
+ ssid_,
+ flimflam::kModeManaged,
+ flimflam::kSecurityWep,
+ false)),
+ eap_wifi_service_(new MockWiFiService(&control_interface_,
+ &dispatcher_,
+ &metrics_,
+ &manager_,
+ manager_.wifi_provider(),
+ ssid_,
+ flimflam::kModeManaged,
+ flimflam::kSecurity8021x,
+ false)),
eap_(new MockEapCredentials()) {}
virtual ~MetricsTest() {}
virtual void SetUp() {
metrics_.set_library(&library_);
- wifi_service_->eap_.reset(eap_); // Passes ownership.
+ eap_wifi_service_->eap_.reset(eap_); // Passes ownership.
metrics_.collect_bootstats_ = false;
}
@@ -87,12 +105,14 @@
MockEventDispatcher dispatcher_;
MockGLib glib_;
MockManager manager_;
- Metrics metrics_; // This must be destroyed after service_ and wifi_service_
+ Metrics metrics_; // This must be destroyed after all |service_|s.
MetricsLibraryMock library_;
scoped_refptr<MockService> service_;
const std::vector<uint8_t> ssid_;
- scoped_refptr<MockWiFiService> wifi_service_;
- MockEapCredentials *eap_; // Owned by |wifi_service_|.
+ scoped_refptr<MockWiFiService> open_wifi_service_;
+ scoped_refptr<MockWiFiService> wep_wifi_service_;
+ scoped_refptr<MockWiFiService> eap_wifi_service_;
+ MockEapCredentials *eap_; // Owned by |eap_wifi_service_|.
};
TEST_F(MetricsTest, TimeToConfig) {
@@ -144,9 +164,11 @@
Metrics::kTimerHistogramMillisecondsMin,
Metrics::kTimerHistogramMillisecondsMax,
Metrics::kTimerHistogramNumBuckets));
- metrics_.RegisterService(wifi_service_);
- metrics_.NotifyServiceStateChanged(wifi_service_, Service::kStateAssociating);
- metrics_.NotifyServiceStateChanged(wifi_service_, Service::kStateConfiguring);
+ metrics_.RegisterService(open_wifi_service_);
+ metrics_.NotifyServiceStateChanged(open_wifi_service_,
+ Service::kStateAssociating);
+ metrics_.NotifyServiceStateChanged(open_wifi_service_,
+ Service::kStateConfiguring);
}
TEST_F(MetricsTest, WiFiServicePostReady) {
@@ -166,12 +188,12 @@
_, _)).Times(0);
EXPECT_CALL(library_, SendEnumToUMA("Network.Shill.Wifi.EapInnerProtocol",
_, _)).Times(0);
- wifi_service_->frequency_ = 2412;
- wifi_service_->physical_mode_ = Metrics::kWiFiNetworkPhyMode11a;
- wifi_service_->security_ = flimflam::kSecurityWep;
- wifi_service_->raw_signal_strength_ = kStrength;
- metrics_.RegisterService(wifi_service_);
- metrics_.NotifyServiceStateChanged(wifi_service_, Service::kStateConnected);
+ wep_wifi_service_->frequency_ = 2412;
+ wep_wifi_service_->physical_mode_ = Metrics::kWiFiNetworkPhyMode11a;
+ wep_wifi_service_->raw_signal_strength_ = kStrength;
+ metrics_.RegisterService(wep_wifi_service_);
+ metrics_.NotifyServiceStateChanged(wep_wifi_service_,
+ Service::kStateConnected);
Mock::VerifyAndClearExpectations(&library_);
// Simulate a system suspend, resume and an AP reconnect.
@@ -188,7 +210,8 @@
WillOnce(DoAll(SetArgumentPointee<0>(non_zero_time_delta), Return(true)));
metrics_.NotifyPowerStateChange(PowerManagerProxyDelegate::kMem);
metrics_.NotifyPowerStateChange(PowerManagerProxyDelegate::kOn);
- metrics_.NotifyServiceStateChanged(wifi_service_, Service::kStateConnected);
+ metrics_.NotifyServiceStateChanged(wep_wifi_service_,
+ Service::kStateConnected);
Mock::VerifyAndClearExpectations(&library_);
Mock::VerifyAndClearExpectations(mock_time_resume_to_ready_timer);
@@ -199,7 +222,8 @@
-kStrength);
EXPECT_CALL(library_, SendToUMA("Network.Shill.Wifi.TimeResumeToReady",
_, _, _, _)).Times(0);
- metrics_.NotifyServiceStateChanged(wifi_service_, Service::kStateConnected);
+ metrics_.NotifyServiceStateChanged(wep_wifi_service_,
+ Service::kStateConnected);
}
TEST_F(MetricsTest, WiFiServicePostReadyEAP) {
@@ -208,13 +232,13 @@
Metrics::kWiFiNetworkPhyMode11a,
Metrics::kWiFiSecurity8021x,
-kStrength);
- wifi_service_->frequency_ = 2412;
- wifi_service_->physical_mode_ = Metrics::kWiFiNetworkPhyMode11a;
- wifi_service_->security_ = flimflam::kSecurity8021x;
- wifi_service_->raw_signal_strength_ = kStrength;
+ eap_wifi_service_->frequency_ = 2412;
+ eap_wifi_service_->physical_mode_ = Metrics::kWiFiNetworkPhyMode11a;
+ eap_wifi_service_->raw_signal_strength_ = kStrength;
EXPECT_CALL(*eap_, OutputConnectionMetrics(&metrics_, Technology::kWifi));
- metrics_.RegisterService(wifi_service_);
- metrics_.NotifyServiceStateChanged(wifi_service_, Service::kStateConnected);
+ metrics_.RegisterService(eap_wifi_service_);
+ metrics_.NotifyServiceStateChanged(eap_wifi_service_,
+ Service::kStateConnected);
}
TEST_F(MetricsTest, FrequencyToChannel) {
@@ -269,7 +293,7 @@
EXPECT_CALL(*mock_time_online_timer, Start()).Times(2);
EXPECT_CALL(*mock_time_to_drop_timer, Start());
metrics_.NotifyDefaultServiceChanged(service_);
- metrics_.NotifyDefaultServiceChanged(wifi_service_);
+ metrics_.NotifyDefaultServiceChanged(open_wifi_service_);
EXPECT_CALL(*mock_time_online_timer, Start());
EXPECT_CALL(*mock_time_to_drop_timer, Start()).Times(0);
diff --git a/profile_unittest.cc b/profile_unittest.cc
index 559042d..d7d2af2 100644
--- a/profile_unittest.cc
+++ b/profile_unittest.cc
@@ -259,7 +259,8 @@
dispatcher(),
metrics(),
manager()));
- service1->set_priority(service1->priority() + 1); // Change from default.
+ // Change prioirty from default.
+ service1->SetPriority(service1->priority() + 1, NULL);
ASSERT_TRUE(profile_->AdoptService(service1));
ASSERT_TRUE(profile_->ContainsService(service1));
diff --git a/service.cc b/service.cc
index 9d12995..49e2ea4 100644
--- a/service.cc
+++ b/service.cc
@@ -137,7 +137,7 @@
diagnostics_reporter_(DiagnosticsReporter::GetInstance()) {
HelpRegisterDerivedBool(flimflam::kAutoConnectProperty,
&Service::GetAutoConnect,
- &Service::SetAutoConnect);
+ &Service::SetAutoConnectFull);
// flimflam::kActivationStateProperty: Registered in CellularService
// flimflam::kCellularApnProperty: Registered in CellularService
@@ -158,7 +158,9 @@
NULL);
store_.RegisterConstStrings(kEapRemoteCertificationProperty,
&remote_certification_);
- store_.RegisterString(flimflam::kGuidProperty, &guid_);
+ HelpRegisterDerivedString(flimflam::kGuidProperty,
+ &Service::GetGuid,
+ &Service::SetGuid);
// TODO(ers): in flimflam clearing Error has the side-effect of
// setting the service state to IDLE. Is this important? I could
@@ -182,7 +184,9 @@
&Service::SetNameProperty);
// flimflam::kPassphraseProperty: Registered in WiFiService
// flimflam::kPassphraseRequiredProperty: Registered in WiFiService
- store_.RegisterInt32(flimflam::kPriorityProperty, &priority_);
+ HelpRegisterDerivedInt32(flimflam::kPriorityProperty,
+ &Service::GetPriority,
+ &Service::SetPriority);
HelpRegisterDerivedString(flimflam::kProfileProperty,
&Service::GetProfileRpcId,
&Service::SetProfileRpcId);
@@ -588,8 +592,8 @@
return;
}
- auto_connect_ = true;
- favorite_ = true;
+ MarkAsFavorite();
+ SetAutoConnect(true);
}
void Service::SetConnection(const ConnectionRefPtr &connection) {
@@ -641,6 +645,14 @@
remote_certification_.clear();
}
+void Service::SetAutoConnect(bool connect) {
+ if (auto_connect() == connect) {
+ return;
+ }
+ auto_connect_ = connect;
+ adaptor_->EmitBoolChanged(flimflam::kAutoConnectProperty, auto_connect());
+}
+
void Service::SetEapCredentials(EapCredentials *eap) {
// This operation must be done at most once for the lifetime of the service.
CHECK(eap && !eap_);
@@ -944,7 +956,7 @@
string Service::GetIPConfigRpcIdentifier(Error *error) {
if (!connection_) {
error->Populate(Error::kNotFound);
- return "/";
+ return DBusAdaptor::kNullPath;
}
string id = connection_->ipconfig_rpc_identifier();
@@ -952,23 +964,24 @@
if (id.empty()) {
// Do not return an empty IPConfig.
error->Populate(Error::kNotFound);
- return "/";
+ return DBusAdaptor::kNullPath;
}
return id;
}
-void Service::set_connectable(bool connectable) {
+void Service::SetConnectable(bool connectable) {
+ if (connectable_ == connectable)
+ return;
connectable_ = connectable;
adaptor_->EmitBoolChanged(flimflam::kConnectableProperty, connectable_);
}
-void Service::SetConnectable(bool connectable) {
+void Service::SetConnectableFull(bool connectable) {
if (connectable_ == connectable) {
return;
}
- connectable_ = connectable;
- adaptor_->EmitBoolChanged(flimflam::kConnectableProperty, connectable_);
+ SetConnectable(connectable);
if (manager_->HasService(this)) {
manager_->UpdateService(this);
}
@@ -1054,6 +1067,15 @@
BoolAccessor(new CustomAccessor<Service, bool>(this, get, set)));
}
+void Service::HelpRegisterDerivedInt32(
+ const string &name,
+ int32(Service::*get)(Error *),
+ void(Service::*set)(const int32&, Error *)) {
+ store_.RegisterDerivedInt32(
+ name,
+ Int32Accessor(new CustomAccessor<Service, int32>(this, get, set)));
+}
+
void Service::HelpRegisterDerivedString(
const string &name,
string(Service::*get)(Error *),
@@ -1125,13 +1147,13 @@
return auto_connect();
}
-void Service::SetAutoConnect(const bool &connect, Error */*error*/) {
+void Service::SetAutoConnectFull(const bool &connect, Error */*error*/) {
LOG(INFO) << "Service " << unique_name() << ": AutoConnect="
<< auto_connect() << "->" << connect;
if (auto_connect() == connect) {
return;
}
- set_auto_connect(connect);
+ SetAutoConnect(connect);
manager_->UpdateService(this);
}
@@ -1155,6 +1177,23 @@
check_portal_ = check_portal;
}
+string Service::GetGuid(Error *error) {
+ return guid_;
+}
+
+void Service::SetGuid(const string &guid, Error */*error*/) {
+ if (guid == guid_) {
+ return;
+ }
+ guid_ = guid;
+ adaptor_->EmitStringChanged(flimflam::kGuidProperty, guid_);
+}
+
+void Service::MarkAsFavorite() {
+ favorite_ = true;
+ adaptor_->EmitBoolChanged(flimflam::kFavoriteProperty, favorite_);
+}
+
void Service::SetSecurity(CryptoAlgorithm crypto_algorithm, bool key_rotation,
bool endpoint_auth) {
crypto_algorithm_ = crypto_algorithm;
@@ -1175,6 +1214,18 @@
}
}
+int32 Service::GetPriority(Error *error) {
+ return priority_;
+}
+
+void Service::SetPriority(const int32 &priority, Error *error) {
+ if (priority == priority_) {
+ return;
+ }
+ priority_ = priority;
+ adaptor_->EmitIntChanged(flimflam::kPriorityProperty, priority_);
+}
+
string Service::GetProfileRpcId(Error *error) {
if (!profile_) {
// This happens in some unit tests where profile_ is not set.
@@ -1186,6 +1237,8 @@
void Service::SetProfileRpcId(const string &profile, Error *error) {
manager_->SetProfileForService(this, profile, error);
+ // No need to Emit here, since SetProfileForService will call into
+ // SetProfile (if the profile actually changes).
}
uint16 Service::GetHTTPProxyPort(Error */*error*/) {
diff --git a/service.h b/service.h
index 7a92a28..aabe624 100644
--- a/service.h
+++ b/service.h
@@ -45,6 +45,7 @@
class Manager;
class Metrics;
class ServiceAdaptorInterface;
+class ServiceMockAdaptor;
class Sockets;
class StoreInterface;
@@ -295,14 +296,16 @@
int64 /*time_resume_to_ready_milliseconds*/) const {}
bool auto_connect() const { return auto_connect_; }
- void set_auto_connect(bool connect) { auto_connect_ = connect; }
+ void SetAutoConnect(bool connect);
bool connectable() const { return connectable_; }
- // TODO(petkov): Remove this method in favor of SetConnectable.
- void set_connectable(bool connectable);
- // Sets the connectable property of the service. Broadcasts the new value and
- // alerts the manager if necessary.
+ // Sets the connectable property of the service, and broadcast the
+ // new value. Does not update the manager.
+ // TODO(petkov): Remove this method in favor of SetConnectableFull.
void SetConnectable(bool connectable);
+ // Sets the connectable property of the service, broadcasts the new
+ // value, and alerts the manager if necessary.
+ void SetConnectableFull(bool connectable);
virtual bool explicitly_disconnected() const {
return explicitly_disconnected_;
@@ -311,18 +314,18 @@
bool favorite() const { return favorite_; }
// Setter is deliberately omitted; use MakeFavorite.
- // Sets the flimflam::kNameProperty
- void SetFriendlyName(const std::string &friendly_name);
void set_friendly_name(const std::string &n) { friendly_name_ = n; }
const std::string &friendly_name() const { return friendly_name_; }
+ // Sets the flimflam::kNameProperty and broadcasts the change.
+ void SetFriendlyName(const std::string &friendly_name);
const std::string &guid() const { return guid_; }
- void set_guid(const std::string &guid) { guid_ = guid; }
+ void SetGuid(const std::string &guid, Error *error);
bool has_ever_connected() const { return has_ever_connected_; }
int32 priority() const { return priority_; }
- void set_priority(int32 priority) { priority_ = priority; }
+ void SetPriority(const int32 &priority, Error *error);
size_t crypto_algorithm() const { return crypto_algorithm_; }
bool key_rotation() const { return key_rotation_; }
@@ -453,6 +456,10 @@
const std::string &name,
bool(Service::*get)(Error *error),
void(Service::*set)(const bool &value, Error *error));
+ void HelpRegisterDerivedInt32(
+ const std::string &name,
+ int32(Service::*get)(Error *error),
+ void(Service::*set)(const int32 &value, Error *error));
void HelpRegisterDerivedString(
const std::string &name,
std::string(Service::*get)(Error *error),
@@ -493,8 +500,9 @@
// failure_ enum.
void UpdateErrorProperty();
- // RPC setter for the the "AutoConnect" property.
- virtual void SetAutoConnect(const bool &connect, Error *error);
+ // RPC setter for the the "AutoConnect" property. Updates the |manager_|.
+ // (cf. SetAutoConnect, which does not update the manager.)
+ virtual void SetAutoConnectFull(const bool &connect, Error *error);
// Property accessors reserved for subclasses
EventDispatcher *dispatcher() const { return dispatcher_; }
@@ -503,7 +511,9 @@
Manager *manager() const { return manager_; }
Metrics *metrics() const { return metrics_; }
- void set_favorite(bool favorite) { favorite_ = favorite; }
+ // Mark the serivce as a favorite, without affecting its auto_connect
+ // property. (cf. MakeFavorite)
+ void MarkAsFavorite();
// Inform base class of the security properties for the service.
//
// NB: When adding a call to this function from a subclass, please check
@@ -521,6 +531,8 @@
friend class WiFiServiceTest;
friend class WiMaxProviderTest;
friend class WiMaxServiceTest;
+ friend void TestCommonPropertyChanges(ServiceRefPtr, ServiceMockAdaptor *);
+ friend void TestNamePropertyChange(ServiceRefPtr, ServiceMockAdaptor *);
FRIEND_TEST(AllMockServiceTest, AutoConnectWithFailures);
FRIEND_TEST(CellularCapabilityGSMTest, SetStorageIdentifier);
FRIEND_TEST(CellularCapabilityUniversalMainTest, UpdateStorageIdentifier);
@@ -550,7 +562,7 @@
FRIEND_TEST(ServiceTest, SaveStringEmpty);
FRIEND_TEST(ServiceTest, SecurityLevel);
FRIEND_TEST(ServiceTest, SetCheckPortal);
- FRIEND_TEST(ServiceTest, SetConnectable);
+ FRIEND_TEST(ServiceTest, SetConnectableFull);
FRIEND_TEST(ServiceTest, SetFriendlyName);
FRIEND_TEST(ServiceTest, SetProperty);
FRIEND_TEST(ServiceTest, State);
@@ -596,14 +608,18 @@
std::string GetCheckPortal(Error *error);
void SetCheckPortal(const std::string &check_portal, Error *error);
+ std::string GetGuid(Error *error);
+
virtual std::string GetDeviceRpcId(Error *error) = 0;
std::string GetIPConfigRpcIdentifier(Error *error);
+ std::string GetNameProperty(Error *error);
// The base implementation asserts that |name| matches the current Name
// property value.
virtual void SetNameProperty(const std::string &name, Error *error);
- std::string GetNameProperty(Error *error);
+
+ int32 GetPriority(Error *error);
std::string GetProfileRpcId(Error *error);
void SetProfileRpcId(const std::string &profile, Error *error);
diff --git a/service_property_change_test.cc b/service_property_change_test.cc
new file mode 100644
index 0000000..9e08e6f
--- /dev/null
+++ b/service_property_change_test.cc
@@ -0,0 +1,119 @@
+// Copyright (c) 2013 The Chromium OS 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 "shill/service_property_change_test.h"
+
+#include <string>
+
+#include <chromeos/dbus/service_constants.h>
+#include <gtest/gtest.h>
+#include <gmock/gmock.h>
+
+#include "shill/error.h"
+#include "shill/mock_adaptors.h"
+#include "shill/service.h"
+
+using std::string;
+using testing::_;
+using testing::AnyNumber;
+using testing::Mock;
+
+namespace shill {
+
+// Some of these tests are duplicative, as we also have broader tests
+// for specific setters. However, it's convenient to have all the property
+// change notifications documented (and tested) in one place.
+
+void TestCommonPropertyChanges(ServiceRefPtr service,
+ ServiceMockAdaptor *adaptor) {
+ Error error;
+
+ EXPECT_EQ(Service::kStateIdle, service->state());
+ EXPECT_CALL(*adaptor, EmitStringChanged(flimflam::kStateProperty, _));
+ service->SetState(Service::kStateConnected);
+ Mock::VerifyAndClearExpectations(adaptor);
+
+ // TODO(quiche): Once crosbug.com/34528 is resolved, add a test
+ // that service->SetConnection emits kIPConfigProperty changed.
+
+ bool connectable = service->connectable();
+ EXPECT_CALL(*adaptor, EmitBoolChanged(flimflam::kConnectableProperty, _));
+ service->SetConnectable(!connectable);
+ Mock::VerifyAndClearExpectations(adaptor);
+
+ EXPECT_EQ(string(), service->guid());
+ EXPECT_CALL(*adaptor, EmitStringChanged(flimflam::kGuidProperty, _));
+ service->SetGuid("some garbage", &error);
+ Mock::VerifyAndClearExpectations(adaptor);
+
+ EXPECT_FALSE(service->favorite());
+ EXPECT_CALL(*adaptor, EmitBoolChanged(flimflam::kAutoConnectProperty, _))
+ .Times(AnyNumber());
+ EXPECT_CALL(*adaptor, EmitBoolChanged(flimflam::kFavoriteProperty, _));
+ service->MakeFavorite();
+ Mock::VerifyAndClearExpectations(adaptor);
+
+ EXPECT_EQ(0, service->priority());
+ EXPECT_CALL(*adaptor, EmitIntChanged(flimflam::kPriorityProperty, _));
+ service->SetPriority(1, &error);
+ Mock::VerifyAndClearExpectations(adaptor);
+
+ EXPECT_EQ(string(), service->GetProxyConfig(&error));
+ EXPECT_CALL(*adaptor, EmitStringChanged(flimflam::kProxyConfigProperty, _));
+ service->SetProxyConfig("some garbage", &error);
+ Mock::VerifyAndClearExpectations(adaptor);
+
+ uint8 strength = service->strength();
+ EXPECT_CALL(*adaptor,
+ EmitUint8Changed(flimflam::kSignalStrengthProperty, _));
+ service->SetStrength(strength+1);
+ Mock::VerifyAndClearExpectations(adaptor);
+
+ EXPECT_EQ(string(), service->error_details());
+ EXPECT_CALL(*adaptor, EmitStringChanged(kErrorDetailsProperty, _));
+ service->SetErrorDetails("some garbage");
+ Mock::VerifyAndClearExpectations(adaptor);
+
+ EXPECT_EQ(Service::kFailureUnknown, service->failure());
+ EXPECT_EQ(Service::ConnectFailureToString(Service::kFailureUnknown),
+ service->error());
+ EXPECT_CALL(*adaptor, EmitStringChanged(flimflam::kStateProperty, _));
+ EXPECT_CALL(*adaptor, EmitStringChanged(flimflam::kErrorProperty, _));
+ service->SetFailure(Service::kFailureAAA);
+ Mock::VerifyAndClearExpectations(adaptor);
+
+ EXPECT_NE(Service::ConnectFailureToString(Service::kFailureUnknown),
+ service->error());
+ EXPECT_CALL(*adaptor, EmitStringChanged(flimflam::kStateProperty, _));
+ EXPECT_CALL(*adaptor, EmitStringChanged(kErrorDetailsProperty, _));
+ EXPECT_CALL(*adaptor, EmitStringChanged(flimflam::kErrorProperty, _));
+ service->SetState(Service::kStateConnected);
+ Mock::VerifyAndClearExpectations(adaptor);
+
+ EXPECT_EQ(Service::ConnectFailureToString(Service::kFailureUnknown),
+ service->error());
+ EXPECT_CALL(*adaptor, EmitStringChanged(flimflam::kStateProperty, _));
+ EXPECT_CALL(*adaptor, EmitStringChanged(flimflam::kErrorProperty, _));
+ service->SetFailureSilent(Service::kFailureAAA);
+ Mock::VerifyAndClearExpectations(adaptor);
+}
+
+void TestAutoConnectPropertyChange(ServiceRefPtr service,
+ ServiceMockAdaptor *adaptor) {
+ bool auto_connect = service->auto_connect();
+ EXPECT_CALL(*adaptor, EmitBoolChanged(flimflam::kAutoConnectProperty, _));
+ service->SetAutoConnect(!auto_connect);
+ Mock::VerifyAndClearExpectations(adaptor);
+}
+
+void TestNamePropertyChange(ServiceRefPtr service,
+ ServiceMockAdaptor *adaptor) {
+ Error error;
+ string name = service->GetNameProperty(&error);
+ EXPECT_CALL(*adaptor, EmitStringChanged(flimflam::kNameProperty, _));
+ service->SetNameProperty(name + " and some new stuff", &error);
+ Mock::VerifyAndClearExpectations(adaptor);
+}
+
+} // namespace shill
diff --git a/service_property_change_test.h b/service_property_change_test.h
new file mode 100644
index 0000000..ecf8433
--- /dev/null
+++ b/service_property_change_test.h
@@ -0,0 +1,29 @@
+// Copyright (c) 2013 The Chromium OS 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 SHILL_TEST_COMMON_H_
+#define SHILL_TEST_COMMON_H_
+
+#include "shill/refptr_types.h"
+
+namespace shill {
+
+class ServiceMockAdaptor;
+
+// Test property change notifications that are implemented by all
+// Services.
+void TestCommonPropertyChanges(ServiceRefPtr service,
+ ServiceMockAdaptor *adaptor);
+// Test AutoConnect property change notification. Implemented by
+// all Services except EthernetService.
+void TestAutoConnectPropertyChange(ServiceRefPtr service,
+ ServiceMockAdaptor *adaptor);
+// Test Name property change notification. Only VPNService allows
+// changing the name property.
+void TestNamePropertyChange(ServiceRefPtr service,
+ ServiceMockAdaptor *adaptor);
+
+} // namespace shill
+
+#endif // SHILL_TEST_COMMON_H_
diff --git a/service_unittest.cc b/service_unittest.cc
index eab8380..4c672f4 100644
--- a/service_unittest.cc
+++ b/service_unittest.cc
@@ -33,6 +33,7 @@
#include "shill/mock_store.h"
#include "shill/mock_time.h"
#include "shill/property_store_unittest.h"
+#include "shill/service_property_change_test.h"
#include "shill/service_under_test.h"
using base::Bind;
@@ -173,8 +174,8 @@
return service_->GetAutoConnect(error);
}
- void SetAutoConnect(bool connect, Error *error) {
- service_->SetAutoConnect(connect, error);
+ void SetAutoConnectFull(bool connect, Error *error) {
+ service_->SetAutoConnectFull(connect, error);
}
MockManager mock_manager_;
@@ -246,7 +247,6 @@
{
::DBus::Error dbus_error;
bool expected = true;
- service_->set_favorite(true);
service_->mutable_store()->SetBoolProperty(flimflam::kAutoConnectProperty,
expected,
&error);
@@ -330,7 +330,6 @@
}
{
::DBus::Error error;
- service_->set_favorite(true);
EXPECT_TRUE(DBusAdaptor::SetProperty(service_->mutable_store(),
flimflam::kAutoConnectProperty,
PropertyStoreTest::kBoolV,
@@ -338,7 +337,6 @@
}
{
::DBus::Error error;
- service_->set_favorite(false);
EXPECT_TRUE(DBusAdaptor::SetProperty(service_->mutable_store(),
flimflam::kAutoConnectProperty,
PropertyStoreTest::kBoolV,
@@ -582,7 +580,7 @@
EXPECT_TRUE(service_->favorite());
EXPECT_TRUE(service_->auto_connect());
- service_->set_auto_connect(false);
+ service_->SetAutoConnect(false);
service_->MakeFavorite();
EXPECT_TRUE(service_->favorite());
EXPECT_FALSE(service_->auto_connect());
@@ -590,7 +588,7 @@
TEST_F(ServiceTest, IsAutoConnectable) {
const char *reason = NULL;
- service_->set_connectable(true);
+ service_->SetConnectable(true);
// Services with non-primary connectivity technologies should not auto-connect
// when the system is offline.
@@ -645,7 +643,7 @@
TEST_F(ServiceTest, AutoConnectLogging) {
ScopedMockLog log;
EXPECT_CALL(log, Log(_, _, _));
- service_->set_connectable(true);
+ service_->SetConnectable(true);
ScopeLogger::GetInstance()->EnableScopesByName("+service");
ScopeLogger::GetInstance()->set_verbose_level(1);
@@ -657,14 +655,14 @@
ScopeLogger::GetInstance()->set_verbose_level(0);
EXPECT_CALL(log, Log(logging::LOG_INFO, _,
HasSubstr(Service::kAutoConnNotConnectable)));
- service_->set_connectable(false);
+ service_->SetConnectable(false);
service_->AutoConnect();
}
TEST_F(AllMockServiceTest, AutoConnectWithFailures) {
const char *reason;
- service_->set_connectable(true);
+ service_->SetConnectable(true);
service_->technology_ = Technology::kEthernet;
EXPECT_TRUE(service_->IsAutoConnectable(&reason));
@@ -755,7 +753,7 @@
TEST_F(ServiceTest, ConfigureBoolProperty) {
service_->MakeFavorite();
- service_->set_auto_connect(false);
+ service_->SetAutoConnect(false);
ASSERT_FALSE(service_->auto_connect());
KeyValueStore args;
args.SetBool(flimflam::kAutoConnectProperty, true);
@@ -768,7 +766,7 @@
TEST_F(ServiceTest, ConfigureStringProperty) {
const string kGuid0 = "guid_zero";
const string kGuid1 = "guid_one";
- service_->set_guid(kGuid0);
+ service_->SetGuid(kGuid0, NULL);
ASSERT_EQ(kGuid0, service_->guid());
KeyValueStore args;
args.SetString(flimflam::kGuidProperty, kGuid1);
@@ -800,7 +798,7 @@
TEST_F(ServiceTest, ConfigureIntProperty) {
const int kPriority0 = 100;
const int kPriority1 = 200;
- service_->set_priority(kPriority0);
+ service_->SetPriority(kPriority0, NULL);
ASSERT_EQ(kPriority0, service_->priority());
KeyValueStore args;
args.SetInt(flimflam::kPriorityProperty, kPriority1);
@@ -812,7 +810,7 @@
TEST_F(ServiceTest, ConfigureIgnoredProperty) {
service_->MakeFavorite();
- service_->set_auto_connect(false);
+ service_->SetAutoConnect(false);
ASSERT_FALSE(service_->auto_connect());
KeyValueStore args;
args.SetBool(flimflam::kAutoConnectProperty, true);
@@ -824,13 +822,13 @@
}
TEST_F(ServiceTest, DoPropertiesMatch) {
- service_->set_auto_connect(false);
+ service_->SetAutoConnect(false);
const string kGUID0 = "guid_zero";
const string kGUID1 = "guid_one";
- service_->set_guid(kGUID0);
+ service_->SetGuid(kGUID0, NULL);
const uint32 kPriority0 = 100;
const uint32 kPriority1 = 200;
- service_->set_priority(kPriority0);
+ service_->SetPriority(kPriority0, NULL);
{
KeyValueStore args;
@@ -1013,32 +1011,32 @@
EXPECT_EQ("Test Name 2", service_->friendly_name_);
}
-TEST_F(ServiceTest, SetConnectable) {
+TEST_F(ServiceTest, SetConnectableFull) {
EXPECT_FALSE(service_->connectable());
ServiceMockAdaptor *adaptor = GetAdaptor();
EXPECT_CALL(*adaptor, EmitBoolChanged(_, _)).Times(0);
EXPECT_CALL(mock_manager_, HasService(_)).Times(0);
- service_->SetConnectable(false);
+ service_->SetConnectableFull(false);
EXPECT_FALSE(service_->connectable());
EXPECT_CALL(*adaptor, EmitBoolChanged(flimflam::kConnectableProperty, true));
EXPECT_CALL(mock_manager_, HasService(_)).WillOnce(Return(false));
EXPECT_CALL(mock_manager_, UpdateService(_)).Times(0);
- service_->SetConnectable(true);
+ service_->SetConnectableFull(true);
EXPECT_TRUE(service_->connectable());
EXPECT_CALL(*adaptor, EmitBoolChanged(flimflam::kConnectableProperty, false));
EXPECT_CALL(mock_manager_, HasService(_)).WillOnce(Return(true));
EXPECT_CALL(mock_manager_, UpdateService(_));
- service_->SetConnectable(false);
+ service_->SetConnectableFull(false);
EXPECT_FALSE(service_->connectable());
EXPECT_CALL(*adaptor, EmitBoolChanged(flimflam::kConnectableProperty, true));
EXPECT_CALL(mock_manager_, HasService(_)).WillOnce(Return(true));
EXPECT_CALL(mock_manager_, UpdateService(_));
- service_->SetConnectable(true);
+ service_->SetConnectableFull(true);
EXPECT_TRUE(service_->connectable());
}
@@ -1472,7 +1470,7 @@
service_->SetErrorDetails(kDetails);
}
-TEST_F(ServiceTest, SetAutoConnect) {
+TEST_F(ServiceTest, SetAutoConnectFull) {
EXPECT_FALSE(service_->auto_connect());
Error error;
EXPECT_FALSE(GetAutoConnect(&error));
@@ -1480,7 +1478,7 @@
// false -> false
EXPECT_CALL(mock_manager_, UpdateService(_)).Times(0);
- SetAutoConnect(false, &error);
+ SetAutoConnectFull(false, &error);
EXPECT_TRUE(error.IsSuccess());
EXPECT_FALSE(service_->auto_connect());
EXPECT_FALSE(GetAutoConnect(NULL));
@@ -1488,7 +1486,7 @@
// false -> true
EXPECT_CALL(mock_manager_, UpdateService(_)).Times(1);
- SetAutoConnect(true, &error);
+ SetAutoConnectFull(true, &error);
EXPECT_TRUE(error.IsSuccess());
EXPECT_TRUE(service_->auto_connect());
EXPECT_TRUE(GetAutoConnect(NULL));
@@ -1496,7 +1494,7 @@
// true -> true
EXPECT_CALL(mock_manager_, UpdateService(_)).Times(0);
- SetAutoConnect(true, &error);
+ SetAutoConnectFull(true, &error);
EXPECT_TRUE(error.IsSuccess());
EXPECT_TRUE(service_->auto_connect());
EXPECT_TRUE(GetAutoConnect(NULL));
@@ -1504,11 +1502,16 @@
// true -> false
EXPECT_CALL(mock_manager_, UpdateService(_)).Times(1);
- SetAutoConnect(false, &error);
+ SetAutoConnectFull(false, &error);
EXPECT_TRUE(error.IsSuccess());
EXPECT_FALSE(service_->auto_connect());
EXPECT_FALSE(GetAutoConnect(NULL));
Mock::VerifyAndClearExpectations(&mock_manager_);
}
+TEST_F(ServiceTest, PropertyChanges) {
+ TestCommonPropertyChanges(service_, GetAdaptor());
+ TestAutoConnectPropertyChange(service_, GetAdaptor());
+}
+
} // namespace shill
diff --git a/vpn_service.cc b/vpn_service.cc
index f5f3e16..f893152 100644
--- a/vpn_service.cc
+++ b/vpn_service.cc
@@ -35,7 +35,7 @@
VPNDriver *driver)
: Service(control, dispatcher, metrics, manager, Technology::kVPN),
driver_(driver) {
- set_connectable(true);
+ SetConnectable(true);
set_save_credentials(false);
mutable_store()->RegisterString(flimflam::kVPNDomainProperty, &vpn_domain_);
}
@@ -116,7 +116,7 @@
void VPNService::MakeFavorite() {
// The base MakeFavorite method also sets auto_connect_ to true
// which is not desirable for VPN services.
- set_favorite(true);
+ MarkAsFavorite();
}
void VPNService::SetConnection(const ConnectionRefPtr &connection) {
diff --git a/vpn_service_unittest.cc b/vpn_service_unittest.cc
index 5936f12..28086bd 100644
--- a/vpn_service_unittest.cc
+++ b/vpn_service_unittest.cc
@@ -20,6 +20,7 @@
#include "shill/mock_store.h"
#include "shill/mock_vpn_driver.h"
#include "shill/mock_vpn_provider.h"
+#include "shill/service_property_change_test.h"
using std::string;
using testing::_;
@@ -91,6 +92,10 @@
manager_.vpn_provider_.reset(provider);
}
+ ServiceMockAdaptor *GetAdaptor() {
+ return dynamic_cast<ServiceMockAdaptor *>(service_->adaptor());
+ }
+
std::string interface_name_;
std::string ipconfig_rpc_identifier_;
MockVPNDriver *driver_; // Owned by |service_|.
@@ -198,7 +203,7 @@
}
TEST_F(VPNServiceTest, Unload) {
- service_->set_auto_connect(true);
+ service_->SetAutoConnect(true);
service_->set_save_credentials(true);
EXPECT_CALL(*driver_, Disconnect());
EXPECT_CALL(*driver_, UnloadCredentials());
@@ -318,4 +323,16 @@
EXPECT_EQ(kName, service_->friendly_name());
}
+TEST_F(VPNServiceTest, PropertyChanges) {
+ TestCommonPropertyChanges(service_, GetAdaptor());
+ TestAutoConnectPropertyChange(service_, GetAdaptor());
+
+ const string kHost = "1.2.3.4";
+ scoped_refptr<MockProfile> profile(
+ new NiceMock<MockProfile>(&control_, &metrics_, &manager_));
+ service_->set_profile(profile);
+ driver_->args()->SetString(flimflam::kProviderHostProperty, kHost);
+ TestNamePropertyChange(service_, GetAdaptor());
+}
+
} // namespace shill
diff --git a/wifi_service.cc b/wifi_service.cc
index c70215d..70c8310 100644
--- a/wifi_service.cc
+++ b/wifi_service.cc
@@ -18,6 +18,7 @@
#include "shill/adaptor_interfaces.h"
#include "shill/certificate_file.h"
#include "shill/control_interface.h"
+#include "shill/dbus_adaptor.h"
#include "shill/device.h"
#include "shill/eap_credentials.h"
#include "shill/error.h"
@@ -557,7 +558,7 @@
string WiFiService::GetDeviceRpcId(Error *error) {
if (!wifi_) {
error->Populate(Error::kNotFound, "Not associated with a device");
- return "/";
+ return DBusAdaptor::kNullPath;
}
return wifi_->GetRpcIdentifier();
}
@@ -577,7 +578,7 @@
need_passphrase_ = passphrase_.empty();
is_connectable = !need_passphrase_;
}
- set_connectable(is_connectable);
+ SetConnectable(is_connectable);
}
void WiFiService::UpdateFromEndpoints() {
@@ -1022,15 +1023,22 @@
SetWiFi(NULL);
}
-void WiFiService::SetWiFi(const WiFiRefPtr &wifi) {
- if (wifi_ == wifi) {
+void WiFiService::SetWiFi(const WiFiRefPtr &new_wifi) {
+ if (wifi_ == new_wifi) {
return;
}
ClearCachedCredentials();
if (wifi_) {
wifi_->DisassociateFromService(this);
}
- wifi_ = wifi;
+ if (new_wifi) {
+ adaptor()->EmitRpcIdentifierChanged(flimflam::kDeviceProperty,
+ new_wifi->GetRpcIdentifier());
+ } else {
+ adaptor()->EmitRpcIdentifierChanged(flimflam::kDeviceProperty,
+ DBusAdaptor::kNullPath);
+ }
+ wifi_ = new_wifi;
}
} // namespace shill
diff --git a/wifi_service.h b/wifi_service.h
index dc21c0a..caeae84 100644
--- a/wifi_service.h
+++ b/wifi_service.h
@@ -218,12 +218,12 @@
// endpoints).
WiFiRefPtr ChooseDevice();
- void SetWiFi(const WiFiRefPtr &wifi);
+ void SetWiFi(const WiFiRefPtr &new_wifi);
// Properties
std::string passphrase_;
bool need_passphrase_;
- std::string security_;
+ const std::string security_;
// TODO(cmasone): see if the below can be pulled from the endpoint associated
// with this service instead.
const std::string mode_;
diff --git a/wifi_service_unittest.cc b/wifi_service_unittest.cc
index 3078eee..4c3e88a 100644
--- a/wifi_service_unittest.cc
+++ b/wifi_service_unittest.cc
@@ -24,6 +24,7 @@
#include "shill/mock_control.h"
#include "shill/mock_eap_credentials.h"
#include "shill/mock_log.h"
+#include "shill/mock_manager.h"
#include "shill/mock_nss.h"
#include "shill/mock_profile.h"
#include "shill/mock_service.h"
@@ -32,6 +33,7 @@
#include "shill/mock_wifi_provider.h"
#include "shill/property_store_unittest.h"
#include "shill/refptr_types.h"
+#include "shill/service_property_change_test.h"
#include "shill/wifi_endpoint.h"
#include "shill/wpa_supplicant.h"
@@ -59,16 +61,17 @@
class WiFiServiceTest : public PropertyStoreTest {
public:
WiFiServiceTest()
- : wifi_(new NiceMock<MockWiFi>(
- control_interface(),
- dispatcher(),
- metrics(),
- manager(),
- "wifi",
- fake_mac,
- 0)),
- simple_ssid_(1, 'a'),
- simple_ssid_string_("a") {}
+ : mock_manager_(control_interface(), dispatcher(), metrics(), glib()),
+ wifi_(
+ new NiceMock<MockWiFi>(control_interface(),
+ dispatcher(),
+ metrics(),
+ manager(),
+ "wifi",
+ fake_mac,
+ 0)),
+ simple_ssid_(1, 'a'),
+ simple_ssid_string_("a") {}
virtual ~WiFiServiceTest() {}
protected:
@@ -124,6 +127,9 @@
WiFiServiceRefPtr MakeGenericService() {
return MakeSimpleService(flimflam::kSecurityWep);
}
+ void SetWiFi(WiFiServiceRefPtr service, WiFiRefPtr wifi) {
+ service->SetWiFi(wifi); // Has side-effects.
+ }
void SetWiFiForService(WiFiServiceRefPtr service, WiFiRefPtr wifi) {
service->wifi_ = wifi;
}
@@ -132,6 +138,17 @@
SetWiFiForService(service, wifi_);
return service;
}
+ WiFiServiceRefPtr MakeServiceWithMockManager() {
+ return new WiFiService(control_interface(),
+ dispatcher(),
+ metrics(),
+ &mock_manager_,
+ &provider_,
+ simple_ssid_,
+ flimflam::kModeManaged,
+ flimflam::kSecurityWep,
+ false);
+ }
ServiceMockAdaptor *GetAdaptor(WiFiService *service) {
return dynamic_cast<ServiceMockAdaptor *>(service->adaptor());
}
@@ -153,6 +170,7 @@
const string &simple_ssid_string() { return simple_ssid_string_; }
private:
+ MockManager mock_manager_;
scoped_refptr<MockWiFi> wifi_;
MockWiFiProvider provider_;
const vector<uint8_t> simple_ssid_;
@@ -882,7 +900,7 @@
WiFiServiceRefPtr service = MakeSimpleService(flimflam::kSecurity8021x);
// Hack the GUID in so that we don't have to mess about with WiFi to regsiter
// our service. This way, Manager will handle the lookup itself.
- service->set_guid(guid);
+ service->SetGuid(guid, NULL);
manager()->RegisterService(service);
EXPECT_FALSE(service->connectable());
EXPECT_EQ(service.get(), manager()->GetService(args, &error).get());
@@ -1638,5 +1656,21 @@
service->Unload();
}
+TEST_F(WiFiServiceTest, PropertyChanges) {
+ WiFiServiceRefPtr service = MakeServiceWithMockManager();
+ ServiceMockAdaptor *adaptor = GetAdaptor(service);
+ TestCommonPropertyChanges(service, adaptor);
+ TestAutoConnectPropertyChange(service, adaptor);
+
+ EXPECT_CALL(*adaptor,
+ EmitRpcIdentifierChanged(flimflam::kDeviceProperty, _));
+ SetWiFi(service, wifi());
+ Mock::VerifyAndClearExpectations(adaptor);
+
+ EXPECT_CALL(*adaptor,
+ EmitRpcIdentifierChanged(flimflam::kDeviceProperty, _));
+ service->ResetWiFi();
+ Mock::VerifyAndClearExpectations(adaptor);
+}
} // namespace shill
diff --git a/wimax_service.cc b/wimax_service.cc
index 4b75461..28c7c84 100644
--- a/wimax_service.cc
+++ b/wimax_service.cc
@@ -11,6 +11,7 @@
#include <base/stringprintf.h>
#include <chromeos/dbus/service_constants.h>
+#include "shill/dbus_adaptor.h"
#include "shill/eap_credentials.h"
#include "shill/key_value_store.h"
#include "shill/logging.h"
@@ -27,7 +28,6 @@
const char WiMaxService::kStorageNetworkId[] = "NetworkId";
const char WiMaxService::kNetworkIdProperty[] = "NetworkId";
-
WiMaxService::WiMaxService(ControlInterface *control,
EventDispatcher *dispatcher,
Metrics *metrics,
@@ -72,7 +72,7 @@
SetStrength(0);
if (device_) {
device_->OnServiceStopped(this);
- device_ = NULL;
+ SetDevice(NULL);
}
UpdateConnectable();
}
@@ -146,7 +146,7 @@
if (error->IsSuccess()) {
// Associate with the carrier device if the connection process has been
// initiated successfully.
- device_ = carrier;
+ SetDevice(carrier);
}
}
@@ -158,7 +158,7 @@
}
Service::Disconnect(error);
device_->DisconnectFrom(this, error);
- device_ = NULL;
+ SetDevice(NULL);
// Set |need_passphrase_| to true so that after users explicitly disconnect
// from the network, the UI will prompt for credentials when they try to
// re-connect to the same network. This works around the fact that there is
@@ -176,7 +176,7 @@
string WiMaxService::GetDeviceRpcId(Error *error) {
if (!device_) {
error->Populate(Error::kNotFound, "Not associated with a device");
- return "/";
+ return DBusAdaptor::kNullPath;
}
return device_->GetRpcIdentifier();
}
@@ -204,7 +204,7 @@
}
void WiMaxService::UpdateConnectable() {
- SetConnectable(IsStarted() && !need_passphrase_);
+ SetConnectableFull(IsStarted() && !need_passphrase_);
}
void WiMaxService::OnSignalStrengthChanged(int strength) {
@@ -212,6 +212,19 @@
SetStrength(strength);
}
+void WiMaxService::SetDevice(WiMaxRefPtr new_device) {
+ if (device_ == new_device)
+ return;
+ if (new_device) {
+ adaptor()->EmitRpcIdentifierChanged(flimflam::kDeviceProperty,
+ new_device->GetRpcIdentifier());
+ } else {
+ adaptor()->EmitRpcIdentifierChanged(flimflam::kDeviceProperty,
+ DBusAdaptor::kNullPath);
+ }
+ device_ = new_device;
+}
+
bool WiMaxService::Save(StoreInterface *storage) {
SLOG(WiMax, 2) << __func__;
if (!Service::Save(storage)) {
@@ -236,7 +249,7 @@
Service::SetState(state);
if (!IsConnecting() && !IsConnected()) {
// Disassociate from any carrier device if it's not connected anymore.
- device_ = NULL;
+ SetDevice(NULL);
}
}
diff --git a/wimax_service.h b/wimax_service.h
index a63314d..c98633f 100644
--- a/wimax_service.h
+++ b/wimax_service.h
@@ -92,7 +92,10 @@
void UpdateConnectable();
- WiMaxRefPtr device_;
+ // Update |device_|, and inform RPC listeners of the change.
+ void SetDevice(WiMaxRefPtr new_device);
+
+ WiMaxRefPtr device_; // Update via SetDevice().
scoped_ptr<WiMaxNetworkProxyInterface> proxy_;
std::string storage_id_;
diff --git a/wimax_service_unittest.cc b/wimax_service_unittest.cc
index 76ce419..1aa906f 100644
--- a/wimax_service_unittest.cc
+++ b/wimax_service_unittest.cc
@@ -18,6 +18,7 @@
#include "shill/mock_wimax.h"
#include "shill/mock_wimax_network_proxy.h"
#include "shill/mock_wimax_provider.h"
+#include "shill/service_property_change_test.h"
using std::string;
using testing::_;
@@ -74,6 +75,14 @@
service_->connectable_ = connectable;
}
+ void SetDevice(WiMaxRefPtr device) {
+ service_->SetDevice(device);
+ }
+
+ ServiceMockAdaptor *GetAdaptor() {
+ return dynamic_cast<ServiceMockAdaptor *>(service_->adaptor());
+ }
+
scoped_ptr<MockWiMaxNetworkProxy> proxy_;
NiceMockControl control_;
MockManager manager_;
@@ -302,4 +311,20 @@
EXPECT_STREQ("", reason);
}
+TEST_F(WiMaxServiceTest, PropertyChanges) {
+ ServiceMockAdaptor *adaptor = GetAdaptor();
+ TestCommonPropertyChanges(service_, adaptor);
+ TestAutoConnectPropertyChange(service_, adaptor);
+
+ EXPECT_CALL(*adaptor,
+ EmitRpcIdentifierChanged(flimflam::kDeviceProperty, _));
+ SetDevice(device_);
+ Mock::VerifyAndClearExpectations(adaptor);
+
+ EXPECT_CALL(*adaptor,
+ EmitRpcIdentifierChanged(flimflam::kDeviceProperty, _));
+ SetDevice(NULL);
+ Mock::VerifyAndClearExpectations(adaptor);
+}
+
} // namespace shill