shill: Manager: Create and emit signals for the ConnectedState property
As opposed to the "State" property of the Manager which is strictly
"offline" or "online", this new property returns the connected state
of the highest-ranked service. This allows observers to tell whether
or not to expect to have wider network connectivity by waiting for
this property to be "online", with the assurance that portal detection
has completed successfully.
CQ-DEPEND=CL:170701
BUG=chromium:298568
TEST=New unit test + manual: run "dbus-monitor --system" and restart
shill. Expect to see updates to the "ConnectionState" of the manager;
initially "idle" as the manager starts:
signal sender=:1.20 -> dest=(null destination) serial=11 path=/; interface=org.chromium.flimflam.Manager; member=PropertyChanged
string "ConnectionState"
variant string "idle"
then ultimately if the connection succeeds:
signal sender=:1.20 -> dest=(null destination) serial=62 path=/; interface=org.c
hromium.flimflam.Manager; member=PropertyChanged
string "ConnectionState"
variant string "online"
Change-Id: I2c34e33c6742cbe7516af7208131e76344b2f690
Reviewed-on: https://chromium-review.googlesource.com/170704
Reviewed-by: Paul Stewart <pstew@chromium.org>
Commit-Queue: Paul Stewart <pstew@chromium.org>
Tested-by: Paul Stewart <pstew@chromium.org>
diff --git a/doc/manager-api.txt b/doc/manager-api.txt
index 659ed7f..0acf7d1 100644
--- a/doc/manager-api.txt
+++ b/doc/manager-api.txt
@@ -501,6 +501,14 @@
The list of connected technologies. The strings
are the same as the ones from the service type.
+ string ConnectionState [readonly]
+
+ The state of the highest ranked connected service.
+ The value of this property is "idle" if there are
+ no connected services. Otherwise this can be any
+ of the connected service states, e.g, "portal" or
+ "online".
+
string Country [readwrite]
The ISO 3166 country code. This may not be defined;
diff --git a/manager.cc b/manager.cc
index 943a580..79c1bf0 100644
--- a/manager.cc
+++ b/manager.cc
@@ -132,6 +132,7 @@
&Manager::SetCheckPortalList);
HelpRegisterConstDerivedStrings(kConnectedTechnologiesProperty,
&Manager::ConnectedTechnologies);
+ store_.RegisterConstString(kConnectionStateProperty, &connection_state_);
store_.RegisterString(kCountryProperty, &props_.country);
HelpRegisterDerivedString(kDefaultTechnologyProperty,
&Manager::DefaultTechnology,
@@ -1394,6 +1395,7 @@
}
}
NotifyDefaultServiceChanged(default_service);
+ RefreshConnectionState();
AutoConnect();
}
@@ -1523,6 +1525,16 @@
return IsOnline() ? kStateOnline : kStateOffline;
}
+void Manager::RefreshConnectionState() {
+ const ServiceRefPtr &service = GetDefaultService();
+ string connection_state = service ? service->GetStateString() : kStateIdle;
+ if (connection_state_ == connection_state) {
+ return;
+ }
+ connection_state_ = connection_state;
+ adaptor_->EmitStringChanged(kConnectionStateProperty, connection_state_);
+}
+
vector<string> Manager::AvailableTechnologies(Error */*error*/) {
set<string> unique_technologies;
for (vector<DeviceRefPtr>::iterator it = devices_.begin();
diff --git a/manager.h b/manager.h
index 7884cd1..6ec7827 100644
--- a/manager.h
+++ b/manager.h
@@ -251,6 +251,10 @@
virtual bool IsOnline() const;
std::string CalculateState(Error *error);
+ // Recalculate the |connected_state_| string and emit a singal if it has
+ // changed.
+ void RefreshConnectionState();
+
virtual int GetPortalCheckInterval() const {
return props_.portal_check_interval_seconds;
}
@@ -594,6 +598,9 @@
// Stores the most recent copy of geolocation information for each
// technology type.
std::map<std::string, GeolocationInfos> networks_for_geolocation_;
+
+ // Stores the state of the highest ranked connected service.
+ std::string connection_state_;
};
} // namespace shill
diff --git a/manager_unittest.cc b/manager_unittest.cc
index 1dc9849..1d36318 100644
--- a/manager_unittest.cc
+++ b/manager_unittest.cc
@@ -269,6 +269,10 @@
EXPECT_TRUE(manager()->sort_services_task_.IsCancelled());
}
+ void RefreshConnectionState() {
+ manager()->RefreshConnectionState();
+ }
+
RpcIdentifier GetDefaultServiceRpcIdentifier() {
return manager()->GetDefaultServiceRpcIdentifier(NULL);
}
@@ -3223,6 +3227,41 @@
manager()->DeregisterService(mock_service1);
}
+TEST_F(ManagerTest, RefreshConnectionState) {
+ EXPECT_CALL(*manager_adaptor_,
+ EmitStringChanged(kConnectionStateProperty, kStateIdle));
+ RefreshConnectionState();
+ Mock::VerifyAndClearExpectations(manager_adaptor_);
+
+ scoped_refptr<MockService> mock_service(
+ new NiceMock<MockService>(control_interface(),
+ dispatcher(),
+ metrics(),
+ manager()));
+ EXPECT_CALL(*manager_adaptor_,
+ EmitStringChanged(kConnectionStateProperty, _)).Times(0);
+ manager()->RegisterService(mock_service);
+ RefreshConnectionState();
+
+ scoped_refptr<MockConnection> mock_connection(
+ new NiceMock<MockConnection>(device_info_.get()));
+ mock_service->set_mock_connection(mock_connection);
+ EXPECT_CALL(*mock_service, state())
+ .WillOnce(Return(Service::kStateIdle));
+ RefreshConnectionState();
+
+ Mock::VerifyAndClearExpectations(manager_adaptor_);
+ EXPECT_CALL(*mock_service, state())
+ .WillOnce(Return(Service::kStatePortal));
+ EXPECT_CALL(*manager_adaptor_,
+ EmitStringChanged(kConnectionStateProperty, kStatePortal));
+ RefreshConnectionState();
+ Mock::VerifyAndClearExpectations(manager_adaptor_);
+
+ mock_service->set_mock_connection(NULL);
+ manager()->DeregisterService(mock_service);
+}
+
TEST_F(ManagerTest, StartupPortalList) {
// Simulate loading value from the default profile.
const string kProfileValue("wifi,vpn");
diff --git a/service.cc b/service.cc
index 4a5c494..cbc05eb 100644
--- a/service.cc
+++ b/service.cc
@@ -1017,7 +1017,7 @@
string Service::GetStateString() const {
// TODO(benchan): We may want to rename shill::kState* to avoid name clashing
// with Service::kState*.
- switch (state_) {
+ switch (state()) {
case kStateIdle:
return shill::kStateIdle;
case kStateAssociating: