shill: portal: Recheck portal state
Introduce a retry interval for automatically retrying portal
checks. Also provide a Manager API method for immediately
re-checking portal status.
BUG=chromium-os:27335
TEST=New unit tests, tested on real machine, including setting
PortaCheckInterval over DBus, and using Jason's addition to
test-flimflam for 'recheck-portal'.
Change-Id: Idc7def18c6f863859e94f4d4e9f266ab2670679c
Reviewed-on: https://gerrit.chromium.org/gerrit/17367
Reviewed-by: mukesh agrawal <quiche@chromium.org>
Commit-Ready: Paul Stewart <pstew@chromium.org>
Tested-by: Paul Stewart <pstew@chromium.org>
diff --git a/Makefile b/Makefile
index b068a48..e3448f4 100644
--- a/Makefile
+++ b/Makefile
@@ -247,6 +247,7 @@
mock_modem_manager_proxy.o \
mock_modem_proxy.o \
mock_modem_simple_proxy.o \
+ mock_portal_detector.o \
mock_power_manager.o \
mock_power_manager_proxy.o \
mock_profile.o \
diff --git a/connection.cc b/connection.cc
index bfd1962..c40fc60 100644
--- a/connection.cc
+++ b/connection.cc
@@ -87,11 +87,15 @@
routing_table_->SetDefaultMetric(interface_index_, GetMetric(is_default));
+ is_default_ = is_default;
+
if (is_default) {
resolver_->SetDNSFromLists(dns_servers_, dns_domain_search_);
+ DeviceRefPtr device = device_info_->GetDevice(interface_index_);
+ if (device) {
+ device->RequestPortalDetection();
+ }
}
-
- is_default_ = is_default;
}
void Connection::RequestRouting() {
diff --git a/connection.h b/connection.h
index 93a903a..0e7b5ab 100644
--- a/connection.h
+++ b/connection.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium OS Authors. All rights reserved.
+// Copyright (c) 2012 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.
@@ -35,7 +35,7 @@
// Sets the current connection as "default", i.e., routes and DNS entries
// should be used by all system components that don't select explicitly.
- bool is_default() const { return is_default_; }
+ virtual bool is_default() const { return is_default_; }
virtual void SetIsDefault(bool is_default);
virtual const std::string &interface_name() const { return interface_name_; }
diff --git a/connection_unittest.cc b/connection_unittest.cc
index ec9807a..16e0f78 100644
--- a/connection_unittest.cc
+++ b/connection_unittest.cc
@@ -117,6 +117,18 @@
ipconfig_->properties().dns_servers,
ipconfig_->properties().domain_search));
+ scoped_refptr<MockDevice> device(new StrictMock<MockDevice>(
+ &control_,
+ reinterpret_cast<EventDispatcher *>(NULL),
+ reinterpret_cast<Metrics *>(NULL),
+ reinterpret_cast<Manager *>(NULL),
+ kTestDeviceName0,
+ string(),
+ kTestDeviceInterfaceIndex0));
+ EXPECT_CALL(*device_info_, GetDevice(kTestDeviceInterfaceIndex0))
+ .WillOnce(Return(device));
+ EXPECT_CALL(*device.get(), RequestPortalDetection())
+ .WillOnce(Return(true));
connection_->SetIsDefault(true);
EXPECT_TRUE(connection_->is_default());
@@ -133,6 +145,18 @@
Connection::kDefaultMetric));
vector<string> empty_list;
EXPECT_CALL(resolver_, SetDNSFromLists(empty_list, empty_list));
+ scoped_refptr<MockDevice> device(new StrictMock<MockDevice>(
+ &control_,
+ reinterpret_cast<EventDispatcher *>(NULL),
+ reinterpret_cast<Metrics *>(NULL),
+ reinterpret_cast<Manager *>(NULL),
+ kTestDeviceName0,
+ string(),
+ kTestDeviceInterfaceIndex0));
+ EXPECT_CALL(*device_info_, GetDevice(kTestDeviceInterfaceIndex0))
+ .WillOnce(Return(device));
+ EXPECT_CALL(*device.get(), RequestPortalDetection())
+ .WillOnce(Return(true));
connection_->SetIsDefault(true);
EXPECT_CALL(rtnl_handler_,
diff --git a/dbus_bindings/org.chromium.flimflam.Manager.xml b/dbus_bindings/org.chromium.flimflam.Manager.xml
index 0047c5c..ff4c041 100644
--- a/dbus_bindings/org.chromium.flimflam.Manager.xml
+++ b/dbus_bindings/org.chromium.flimflam.Manager.xml
@@ -34,6 +34,7 @@
<arg type="s" direction="in"/>
</method>
<method name="PopAnyProfile"/>
+ <method name="RecheckPortal"/>
<method name="RequestScan">
<arg type="s" direction="in"/>
</method>
diff --git a/default_profile.cc b/default_profile.cc
index 280856a..bc28386 100644
--- a/default_profile.cc
+++ b/default_profile.cc
@@ -7,6 +7,7 @@
#include <vector>
#include <base/file_path.h>
+#include <base/string_number_conversions.h>
#include <base/stringprintf.h>
#include <chromeos/dbus/service_constants.h>
@@ -33,6 +34,9 @@
const char DefaultProfile::kStorageOfflineMode[] = "OfflineMode";
// static
const char DefaultProfile::kStoragePortalURL[] = "PortalURL";
+// static
+const char DefaultProfile::kStoragePortalCheckInterval[] =
+ "PortalCheckInterval";
DefaultProfile::DefaultProfile(ControlInterface *control,
Manager *manager,
@@ -50,6 +54,8 @@
&manager_props.offline_mode);
store->RegisterConstString(flimflam::kPortalURLProperty,
&manager_props.portal_url);
+ store->RegisterConstInt32(shill::kPortalCheckIntervalProperty,
+ &manager_props.portal_check_interval_seconds);
}
DefaultProfile::~DefaultProfile() {}
@@ -65,6 +71,14 @@
&manager_props->portal_url)) {
manager_props->portal_url = PortalDetector::kDefaultURL;
}
+ std::string check_interval;
+ if (!storage()->GetString(kStorageId, kStoragePortalCheckInterval,
+ &check_interval) ||
+ !base::StringToInt(check_interval,
+ &manager_props->portal_check_interval_seconds)) {
+ manager_props->portal_check_interval_seconds =
+ PortalDetector::kDefaultCheckIntervalSeconds;
+ }
return true;
}
@@ -78,6 +92,9 @@
storage()->SetString(kStorageId,
kStoragePortalURL,
props_.portal_url);
+ storage()->SetString(kStorageId,
+ kStoragePortalCheckInterval,
+ base::IntToString(props_.portal_check_interval_seconds));
vector<DeviceRefPtr>::iterator it;
for (it = manager()->devices_begin(); it != manager()->devices_end(); ++it) {
if (!(*it)->Save(storage())) {
diff --git a/default_profile.h b/default_profile.h
index 58f619b..9c5bfb6 100644
--- a/default_profile.h
+++ b/default_profile.h
@@ -59,6 +59,7 @@
static const char kStorageHostName[];
static const char kStorageName[];
static const char kStorageOfflineMode[];
+ static const char kStoragePortalCheckInterval[];
static const char kStoragePortalURL[];
const FilePath storage_path_;
diff --git a/default_profile_unittest.cc b/default_profile_unittest.cc
index e66a469..a60d37b 100644
--- a/default_profile_unittest.cc
+++ b/default_profile_unittest.cc
@@ -120,6 +120,11 @@
DefaultProfile::kStoragePortalURL,
""))
.WillOnce(Return(true));
+ EXPECT_CALL(*storage.get(),
+ SetString(DefaultProfile::kStorageId,
+ DefaultProfile::kStoragePortalCheckInterval,
+ "0"))
+ .WillOnce(Return(true));
EXPECT_CALL(*storage.get(), Flush()).WillOnce(Return(true));
EXPECT_CALL(*device_.get(), Save(storage.get())).WillOnce(Return(true));
@@ -148,6 +153,11 @@
DefaultProfile::kStoragePortalURL,
_))
.WillOnce(Return(false));
+ EXPECT_CALL(*storage.get(),
+ GetString(DefaultProfile::kStorageId,
+ DefaultProfile::kStoragePortalCheckInterval,
+ _))
+ .WillOnce(Return(false));
profile_->set_storage(storage.release());
Manager::Properties manager_props;
@@ -156,6 +166,8 @@
EXPECT_FALSE(manager_props.offline_mode);
EXPECT_EQ("", manager_props.check_portal_list);
EXPECT_EQ(PortalDetector::kDefaultURL, manager_props.portal_url);
+ EXPECT_EQ(PortalDetector::kDefaultCheckIntervalSeconds,
+ manager_props.portal_check_interval_seconds);
}
TEST_F(DefaultProfileTest, LoadManagerProperties) {
@@ -179,6 +191,14 @@
DefaultProfile::kStoragePortalURL,
_))
.WillOnce(DoAll(SetArgumentPointee<2>(portal_url), Return(true)));
+ const string portal_check_interval_string("10");
+ const int portal_check_interval_int = 10;
+ EXPECT_CALL(*storage.get(),
+ GetString(DefaultProfile::kStorageId,
+ DefaultProfile::kStoragePortalCheckInterval,
+ _))
+ .WillOnce(DoAll(SetArgumentPointee<2>(portal_check_interval_string),
+ Return(true)));
profile_->set_storage(storage.release());
Manager::Properties manager_props;
@@ -187,6 +207,8 @@
EXPECT_TRUE(manager_props.offline_mode);
EXPECT_EQ(portal_list, manager_props.check_portal_list);
EXPECT_EQ(portal_url, manager_props.portal_url);
+ EXPECT_EQ(portal_check_interval_int,
+ manager_props.portal_check_interval_seconds);
}
TEST_F(DefaultProfileTest, GetStoragePath) {
diff --git a/device.cc b/device.cc
index 38db2da..721163c 100644
--- a/device.cc
+++ b/device.cc
@@ -454,13 +454,45 @@
return true;
}
+bool Device::RequestPortalDetection() {
+ if (!selected_service_) {
+ VLOG(2) << FriendlyName()
+ << ": No selected service, so no need for portal check.";
+ return false;
+ }
+
+ if (!connection_.get()) {
+ VLOG(2) << FriendlyName()
+ << ": No connection, so no need for portal check.";
+ return false;
+ }
+
+ if (selected_service_->state() != Service::kStatePortal) {
+ VLOG(2) << FriendlyName()
+ << ": Service is not in portal state. No need to start check.";
+ return false;
+ }
+
+ if (!connection_->is_default()) {
+ VLOG(2) << FriendlyName()
+ << ": Service is not the default connection. Don't start check.";
+ return false;
+ }
+
+ if (portal_detector_.get() && portal_detector_->IsInProgress()) {
+ VLOG(2) << FriendlyName() << ": Portal detection is already running.";
+ return true;
+ }
+
+ return StartPortalDetection();
+}
+
bool Device::StartPortalDetection() {
if (!manager_->IsPortalDetectionEnabled(technology())) {
// If portal detection is disabled for this technology, immediately set
// the service state to "Online".
VLOG(2) << "Device " << FriendlyName()
<< ": portal detection is disabled; marking service online.";
- portal_detector_.reset();
SetServiceConnectedState(Service::kStateOnline);
return false;
}
@@ -472,7 +504,6 @@
// arbitrary proxy configs and their possible credentials.
VLOG(2) << "Device " << FriendlyName()
<< ": service has proxy config; marking it online.";
- portal_detector_.reset();
SetServiceConnectedState(Service::kStateOnline);
return false;
}
@@ -514,6 +545,25 @@
return;
}
+ if (state == Service::kStatePortal && connection_->is_default() &&
+ manager_->GetPortalCheckInterval() != 0) {
+ CHECK(portal_detector_.get());
+ if (!portal_detector_->StartAfterDelay(
+ manager_->GetPortalCheckURL(),
+ manager_->GetPortalCheckInterval())) {
+ LOG(ERROR) << "Device " << FriendlyName()
+ << ": Portal detection failed to restart: likely bad URL: "
+ << manager_->GetPortalCheckURL();
+ SetServiceState(Service::kStateOnline);
+ portal_detector_.reset();
+ return;
+ }
+ VLOG(2) << "Device " << FriendlyName() << ": portal detection retrying.";
+ } else {
+ VLOG(2) << "Device " << FriendlyName() << ": portal will not retry.";
+ portal_detector_.reset();
+ }
+
SetServiceState(state);
}
diff --git a/device.h b/device.h
index 8036cd1..130eabe 100644
--- a/device.h
+++ b/device.h
@@ -97,6 +97,10 @@
// is not connected.
bool IsConnected() const;
+ // Requests that portal detection be done, if this device has the default
+ // connection. Returns true if portal detection was started.
+ virtual bool RequestPortalDetection();
+
std::string GetRpcIdentifier();
std::string GetStorageIdentifier();
@@ -139,6 +143,7 @@
FRIEND_TEST(DeviceTest, GetProperties);
FRIEND_TEST(DeviceTest, Save);
FRIEND_TEST(DeviceTest, SelectedService);
+ FRIEND_TEST(DeviceTest, SetServiceConnectedState);
FRIEND_TEST(DeviceTest, Stop);
FRIEND_TEST(ManagerTest, DeviceRegistrationAndStart);
FRIEND_TEST(ManagerTest, ConnectedTechnologies);
@@ -203,6 +208,7 @@
private:
friend class DeviceAdaptorInterface;
+ friend class DevicePortalDetectionTest;
friend class DeviceTest;
friend class CellularTest;
friend class WiFiMainTest;
diff --git a/device_unittest.cc b/device_unittest.cc
index f47895d..90dadf0 100644
--- a/device_unittest.cc
+++ b/device_unittest.cc
@@ -28,6 +28,7 @@
#include "shill/mock_glib.h"
#include "shill/mock_ipconfig.h"
#include "shill/mock_manager.h"
+#include "shill/mock_portal_detector.h"
#include "shill/mock_rtnl_handler.h"
#include "shill/mock_service.h"
#include "shill/mock_store.h"
@@ -59,7 +60,8 @@
kDeviceName,
kDeviceAddress,
0,
- Technology::kUnknown)) {
+ Technology::kUnknown)),
+ device_info_(control_interface(), NULL, NULL, NULL) {
DHCPProvider::GetInstance()->glib_ = glib();
DHCPProvider::GetInstance()->control_interface_ = control_interface();
}
@@ -81,23 +83,13 @@
device_->SelectService(service);
}
- bool StartPortalDetection() { return device_->StartPortalDetection(); }
- void StopPortalDetection() { device_->StopPortalDetection(); }
-
- void PortalDetectorCallback(const PortalDetector::Result &result) {
- device_->PortalDetectorCallback(result);
- }
-
- void SetManager(Manager *manager) {
- device_->manager_ = manager;
- }
-
void SetConnection(ConnectionRefPtr connection) {
device_->connection_ = connection;
}
MockControl control_interface_;
DeviceRefPtr device_;
+ MockDeviceInfo device_info_;
StrictMock<MockRTNLHandler> rtnl_handler_;
};
@@ -328,111 +320,111 @@
EXPECT_FALSE(device_->selected_service_.get());
}
-TEST_F(DeviceTest, PortalDetectionDisabled) {
- scoped_refptr<MockService> service(
- new StrictMock<MockService>(control_interface(),
- dispatcher(),
- metrics(),
- manager()));
- EXPECT_CALL(*service.get(), IsConnected())
+
+class DevicePortalDetectionTest : public DeviceTest {
+ public:
+ DevicePortalDetectionTest()
+ : connection_(new StrictMock<MockConnection>(&device_info_)),
+ manager_(control_interface(),
+ dispatcher(),
+ metrics(),
+ glib()),
+ service_(new StrictMock<MockService>(control_interface(),
+ dispatcher(),
+ metrics(),
+ &manager_)),
+ portal_detector_(new StrictMock<MockPortalDetector>(connection_)) {}
+ virtual ~DevicePortalDetectionTest() {}
+ virtual void SetUp() {
+ DeviceTest::SetUp();
+ SelectService(service_);
+ SetConnection(connection_.get());
+ device_->portal_detector_.reset(portal_detector_); // Passes ownership.
+ device_->manager_ = &manager_;
+ }
+
+ protected:
+ bool StartPortalDetection() { return device_->StartPortalDetection(); }
+ void StopPortalDetection() { device_->StopPortalDetection(); }
+
+ void PortalDetectorCallback(const PortalDetector::Result &result) {
+ device_->PortalDetectorCallback(result);
+ }
+ bool RequestPortalDetection() {
+ return device_->RequestPortalDetection();
+ }
+ void SetServiceConnectedState(Service::ConnectState state) {
+ device_->SetServiceConnectedState(state);
+ }
+ void ExpectPortalDetectorReset() {
+ EXPECT_FALSE(device_->portal_detector_.get());
+ }
+ void ExpectPortalDetectorSet() {
+ EXPECT_TRUE(device_->portal_detector_.get());
+ }
+ void ExpectPortalDetectorIsMock() {
+ EXPECT_EQ(portal_detector_, device_->portal_detector_.get());
+ }
+ scoped_refptr<MockConnection> connection_;
+ StrictMock<MockManager> manager_;
+ scoped_refptr<MockService> service_;
+
+ // Used only for EXPECT_CALL(). Object is owned by device.
+ MockPortalDetector *portal_detector_;
+};
+
+TEST_F(DevicePortalDetectionTest, PortalDetectionDisabled) {
+ EXPECT_CALL(*service_.get(), IsConnected())
.WillRepeatedly(Return(true));
- StrictMock<MockManager> manager(control_interface(),
- dispatcher(),
- metrics(),
- glib());
- SetManager(&manager);
- EXPECT_CALL(manager, IsPortalDetectionEnabled(device_->technology()))
+ EXPECT_CALL(manager_, IsPortalDetectionEnabled(device_->technology()))
.WillOnce(Return(false));
- SelectService(service);
- EXPECT_CALL(*service.get(), SetState(Service::kStateOnline));
+ EXPECT_CALL(*service_.get(), SetState(Service::kStateOnline));
EXPECT_FALSE(StartPortalDetection());
}
-TEST_F(DeviceTest, PortalDetectionProxyConfig) {
- scoped_refptr<MockService> service(
- new StrictMock<MockService>(control_interface(),
- dispatcher(),
- metrics(),
- manager()));
- EXPECT_CALL(*service.get(), IsConnected())
+TEST_F(DevicePortalDetectionTest, PortalDetectionProxyConfig) {
+ EXPECT_CALL(*service_.get(), IsConnected())
.WillRepeatedly(Return(true));
- EXPECT_CALL(*service.get(), HasProxyConfig())
+ EXPECT_CALL(*service_.get(), HasProxyConfig())
.WillOnce(Return(true));
- SelectService(service);
- StrictMock<MockManager> manager(control_interface(),
- dispatcher(),
- metrics(),
- glib());
- EXPECT_CALL(manager, IsPortalDetectionEnabled(device_->technology()))
+ EXPECT_CALL(manager_, IsPortalDetectionEnabled(device_->technology()))
.WillOnce(Return(true));
- SetManager(&manager);
- EXPECT_CALL(*service.get(), SetState(Service::kStateOnline));
+ EXPECT_CALL(*service_.get(), SetState(Service::kStateOnline));
EXPECT_FALSE(StartPortalDetection());
}
-TEST_F(DeviceTest, PortalDetectionBadUrl) {
- scoped_refptr<MockService> service(
- new StrictMock<MockService>(control_interface(),
- dispatcher(),
- metrics(),
- manager()));
- EXPECT_CALL(*service.get(), IsConnected())
+TEST_F(DevicePortalDetectionTest, PortalDetectionBadUrl) {
+ EXPECT_CALL(*service_.get(), IsConnected())
.WillRepeatedly(Return(true));
- EXPECT_CALL(*service.get(), HasProxyConfig())
+ EXPECT_CALL(*service_.get(), HasProxyConfig())
.WillOnce(Return(false));
- SelectService(service);
- StrictMock<MockManager> manager(control_interface(),
- dispatcher(),
- metrics(),
- glib());
- EXPECT_CALL(manager, IsPortalDetectionEnabled(device_->technology()))
+ EXPECT_CALL(manager_, IsPortalDetectionEnabled(device_->technology()))
.WillOnce(Return(true));
const string portal_url;
- EXPECT_CALL(manager, GetPortalCheckURL())
+ EXPECT_CALL(manager_, GetPortalCheckURL())
.WillRepeatedly(ReturnRef(portal_url));
- SetManager(&manager);
- EXPECT_CALL(*service.get(), SetState(Service::kStateOnline));
+ EXPECT_CALL(*service_.get(), SetState(Service::kStateOnline));
EXPECT_FALSE(StartPortalDetection());
}
-TEST_F(DeviceTest, PortalDetectionStart) {
- scoped_refptr<MockService> service(
- new StrictMock<MockService>(control_interface(),
- dispatcher(),
- metrics(),
- manager()));
- EXPECT_CALL(*service.get(), IsConnected())
+TEST_F(DevicePortalDetectionTest, PortalDetectionStart) {
+ EXPECT_CALL(*service_.get(), IsConnected())
.WillRepeatedly(Return(true));
- EXPECT_CALL(*service.get(), HasProxyConfig())
+ EXPECT_CALL(*service_.get(), HasProxyConfig())
.WillOnce(Return(false));
- SelectService(service);
- StrictMock<MockManager> manager(control_interface(),
- dispatcher(),
- metrics(),
- glib());
- EXPECT_CALL(manager, IsPortalDetectionEnabled(device_->technology()))
+ EXPECT_CALL(manager_, IsPortalDetectionEnabled(device_->technology()))
.WillOnce(Return(true));
const string portal_url(PortalDetector::kDefaultURL);
- EXPECT_CALL(manager, GetPortalCheckURL())
+ EXPECT_CALL(manager_, GetPortalCheckURL())
.WillRepeatedly(ReturnRef(portal_url));
- SetManager(&manager);
- EXPECT_CALL(*service.get(), SetState(Service::kStateOnline))
+ EXPECT_CALL(*service_.get(), SetState(Service::kStateOnline))
.Times(0);
- scoped_ptr<MockDeviceInfo> device_info(
- new NiceMock<MockDeviceInfo>(
- control_interface(),
- reinterpret_cast<EventDispatcher *>(NULL),
- reinterpret_cast<Metrics *>(NULL),
- reinterpret_cast<Manager *>(NULL)));
- scoped_refptr<MockConnection> connection(
- new NiceMock<MockConnection>(device_info.get()));
const string kInterfaceName("int0");
- EXPECT_CALL(*connection.get(), interface_name())
- .WillRepeatedly(ReturnRef(kInterfaceName));
+ EXPECT_CALL(*connection_.get(), interface_name())
+ .WillRepeatedly(ReturnRef(kInterfaceName));
const vector<string> kDNSServers;
- EXPECT_CALL(*connection.get(), dns_servers())
- .WillRepeatedly(ReturnRef(kDNSServers));
- SetConnection(connection.get());
+ EXPECT_CALL(*connection_.get(), dns_servers())
+ .WillRepeatedly(ReturnRef(kDNSServers));
EXPECT_TRUE(StartPortalDetection());
// Drop all references to device_info before it falls out of scope.
@@ -440,53 +432,133 @@
StopPortalDetection();
}
-TEST_F(DeviceTest, PortalDetectionNonFinal) {
- scoped_refptr<MockService> service(
- new StrictMock<MockService>(control_interface(),
- dispatcher(),
- metrics(),
- manager()));
- EXPECT_CALL(*service.get(), IsConnected())
+TEST_F(DevicePortalDetectionTest, PortalDetectionNonFinal) {
+ EXPECT_CALL(*service_.get(), IsConnected())
.Times(0);
- EXPECT_CALL(*service.get(), SetState(_))
+ EXPECT_CALL(*service_.get(), SetState(_))
.Times(0);
- SelectService(service);
PortalDetectorCallback(PortalDetector::Result(
PortalDetector::kPhaseUnknown,
PortalDetector::kStatusFailure,
false));
}
-TEST_F(DeviceTest, PortalDetectionFailure) {
- scoped_refptr<MockService> service(
- new StrictMock<MockService>(control_interface(),
- dispatcher(),
- metrics(),
- manager()));
- EXPECT_CALL(*service.get(), IsConnected())
+TEST_F(DevicePortalDetectionTest, PortalDetectionFailure) {
+ EXPECT_CALL(*service_.get(), IsConnected())
.WillOnce(Return(true));
- SelectService(service);
- EXPECT_CALL(*service.get(), SetState(Service::kStatePortal));
+ EXPECT_CALL(*service_.get(), SetState(Service::kStatePortal));
+ EXPECT_CALL(*connection_.get(), is_default())
+ .WillOnce(Return(false));
PortalDetectorCallback(PortalDetector::Result(
PortalDetector::kPhaseUnknown,
PortalDetector::kStatusFailure,
true));
}
-TEST_F(DeviceTest, PortalDetectionSuccess) {
- scoped_refptr<MockService> service(
- new StrictMock<MockService>(control_interface(),
- dispatcher(),
- metrics(),
- manager()));
- EXPECT_CALL(*service.get(), IsConnected())
+TEST_F(DevicePortalDetectionTest, PortalDetectionSuccess) {
+ EXPECT_CALL(*service_.get(), IsConnected())
.WillOnce(Return(true));
- SelectService(service);
- EXPECT_CALL(*service.get(), SetState(Service::kStateOnline));
+ EXPECT_CALL(*service_.get(), SetState(Service::kStateOnline));
PortalDetectorCallback(PortalDetector::Result(
PortalDetector::kPhaseUnknown,
PortalDetector::kStatusSuccess,
true));
}
+TEST_F(DevicePortalDetectionTest, RequestPortalDetection) {
+ EXPECT_CALL(*service_.get(), state())
+ .WillOnce(Return(Service::kStateOnline))
+ .WillRepeatedly(Return(Service::kStatePortal));
+ EXPECT_FALSE(RequestPortalDetection());
+
+ EXPECT_CALL(*connection_.get(), is_default())
+ .WillOnce(Return(false))
+ .WillRepeatedly(Return(true));
+ EXPECT_FALSE(RequestPortalDetection());
+
+ EXPECT_CALL(*portal_detector_, IsInProgress())
+ .WillOnce(Return(true));
+ // Portal detection already running.
+ EXPECT_TRUE(RequestPortalDetection());
+
+ // Make sure our running mock portal detector was not replaced.
+ ExpectPortalDetectorIsMock();
+
+ // Throw away our pre-fabricated portal detector, and have the device create
+ // a new one.
+ StopPortalDetection();
+ EXPECT_CALL(manager_, IsPortalDetectionEnabled(device_->technology()))
+ .WillRepeatedly(Return(true));
+ EXPECT_CALL(*service_.get(), HasProxyConfig())
+ .WillRepeatedly(Return(false));
+ const string kPortalCheckURL("http://portal");
+ EXPECT_CALL(manager_, GetPortalCheckURL())
+ .WillOnce(ReturnRef(kPortalCheckURL));
+ const string kInterfaceName("int0");
+ EXPECT_CALL(*connection_.get(), interface_name())
+ .WillRepeatedly(ReturnRef(kInterfaceName));
+ const vector<string> kDNSServers;
+ EXPECT_CALL(*connection_.get(), dns_servers())
+ .WillRepeatedly(ReturnRef(kDNSServers));
+ EXPECT_TRUE(RequestPortalDetection());
+}
+
+TEST_F(DevicePortalDetectionTest, NotConnected) {
+ EXPECT_CALL(*service_.get(), IsConnected())
+ .WillOnce(Return(false));
+ SetServiceConnectedState(Service::kStatePortal);
+ // We don't check for the portal detector to be reset here, because
+ // it would have been reset as a part of disconnection.
+}
+
+TEST_F(DevicePortalDetectionTest, NotPortal) {
+ EXPECT_CALL(*service_.get(), IsConnected())
+ .WillOnce(Return(true));
+ EXPECT_CALL(*service_.get(), SetState(Service::kStateOnline));
+ SetServiceConnectedState(Service::kStateOnline);
+ ExpectPortalDetectorReset();
+}
+
+TEST_F(DevicePortalDetectionTest, NotDefault) {
+ EXPECT_CALL(*service_.get(), IsConnected())
+ .WillOnce(Return(true));
+ EXPECT_CALL(*connection_.get(), is_default())
+ .WillOnce(Return(false));
+ EXPECT_CALL(*service_.get(), SetState(Service::kStatePortal));
+ SetServiceConnectedState(Service::kStatePortal);
+ ExpectPortalDetectorReset();
+}
+
+TEST_F(DevicePortalDetectionTest, PortalIntervalIsZero) {
+ EXPECT_CALL(*service_.get(), IsConnected())
+ .WillOnce(Return(true));
+ EXPECT_CALL(*connection_.get(), is_default())
+ .WillOnce(Return(true));
+ EXPECT_CALL(manager_, GetPortalCheckInterval())
+ .WillOnce(Return(0));
+ EXPECT_CALL(*service_.get(), SetState(Service::kStatePortal));
+ SetServiceConnectedState(Service::kStatePortal);
+ ExpectPortalDetectorReset();
+}
+
+TEST_F(DevicePortalDetectionTest, RestartPortalDetection) {
+ EXPECT_CALL(*service_.get(), IsConnected())
+ .WillOnce(Return(true));
+ EXPECT_CALL(*connection_.get(), is_default())
+ .WillOnce(Return(true));
+ const int kPortalDetectionInterval = 10;
+ EXPECT_CALL(manager_, GetPortalCheckInterval())
+ .Times(AtLeast(1))
+ .WillRepeatedly(Return(kPortalDetectionInterval));
+ const string kPortalCheckURL("http://portal");
+ EXPECT_CALL(manager_, GetPortalCheckURL())
+ .WillOnce(ReturnRef(kPortalCheckURL));
+ EXPECT_CALL(*portal_detector_, StartAfterDelay(kPortalCheckURL,
+ kPortalDetectionInterval))
+ .WillOnce(Return(true));
+ EXPECT_CALL(*service_.get(), SetState(Service::kStatePortal));
+ SetServiceConnectedState(Service::kStatePortal);
+ ExpectPortalDetectorSet();
+}
+
} // namespace shill
diff --git a/manager.cc b/manager.cc
index a833f9c..1902e4c 100644
--- a/manager.cc
+++ b/manager.cc
@@ -96,10 +96,12 @@
NULL);
store_.RegisterBool(flimflam::kOfflineModeProperty, &props_.offline_mode);
store_.RegisterString(flimflam::kPortalURLProperty, &props_.portal_url);
+ store_.RegisterInt32(kPortalCheckIntervalProperty,
+ &props_.portal_check_interval_seconds);
HelpRegisterDerivedStrings(flimflam::kProfilesProperty,
&Manager::EnumerateProfiles,
NULL);
- store_.RegisterString(shill::kHostNameProperty, &props_.host_name);
+ store_.RegisterString(kHostNameProperty, &props_.host_name);
HelpRegisterDerivedString(flimflam::kStateProperty,
&Manager::CalculateState,
NULL);
@@ -780,6 +782,18 @@
}
}
+void Manager::RecheckPortal(Error */*error*/) {
+ for (vector<DeviceRefPtr>::iterator it = devices_.begin();
+ it != devices_.end(); ++it) {
+ if ((*it)->RequestPortalDetection()) {
+ // Only start Portal Detection on the device with the default connection.
+ // We will get a "true" return value when we've found that device, and
+ // can end our loop early as a result.
+ break;
+ }
+ }
+}
+
// called via RPC (e.g., from ManagerDBusAdaptor)
void Manager::RequestScan(const string &technology, Error *error) {
if (technology == flimflam::kTypeWifi || technology == "") {
diff --git a/manager.h b/manager.h
index c76732f..faf055d 100644
--- a/manager.h
+++ b/manager.h
@@ -36,10 +36,12 @@
public:
struct Properties {
public:
- Properties() : offline_mode(false) {}
+ Properties()
+ : offline_mode(false), portal_check_interval_seconds(0) {}
bool offline_mode;
std::string check_portal_list;
std::string country;
+ int32 portal_check_interval_seconds;
std::string portal_url;
std::string host_name;
};
@@ -88,6 +90,9 @@
// called via RPC (e.g., from ManagerDBusAdaptor)
ServiceRefPtr GetService(const KeyValueStore &args, Error *error);
+ // Request portal detection checks on each registered device until a portal
+ // detection attempt starts on one of them.
+ void RecheckPortal(Error *error);
void RequestScan(const std::string &technology, Error *error);
std::string GetTechnologyOrder();
void SetTechnologyOrder(const std::string &order, Error *error);
@@ -122,6 +127,9 @@
// Return whether a technology is marked as enabled for portal detection.
virtual bool IsPortalDetectionEnabled(Technology::Identifier tech);
+ virtual int GetPortalCheckInterval() const {
+ return props_.portal_check_interval_seconds;
+ }
virtual const std::string &GetPortalCheckURL() const {
return props_.portal_url;
}
diff --git a/manager_dbus_adaptor.cc b/manager_dbus_adaptor.cc
index b7c283b..a86367c 100644
--- a/manager_dbus_adaptor.cc
+++ b/manager_dbus_adaptor.cc
@@ -135,6 +135,12 @@
e.ToDBusError(&error);
}
+void ManagerDBusAdaptor::RecheckPortal(::DBus::Error &error) {
+ Error e;
+ manager_->RecheckPortal(&e);
+ e.ToDBusError(&error);
+}
+
void ManagerDBusAdaptor::RequestScan(const string &technology,
::DBus::Error &error) {
Error e;
diff --git a/manager_dbus_adaptor.h b/manager_dbus_adaptor.h
index c9e0390..7377e2b 100644
--- a/manager_dbus_adaptor.h
+++ b/manager_dbus_adaptor.h
@@ -58,6 +58,7 @@
::DBus::Path PushProfile(const std::string &, ::DBus::Error &error);
void PopProfile(const std::string &, ::DBus::Error &error);
void PopAnyProfile(::DBus::Error &error);
+ void RecheckPortal(::DBus::Error &error);
void RequestScan(const std::string &, ::DBus::Error &error);
void EnableTechnology(const std::string &, ::DBus::Error &error);
diff --git a/manager_unittest.cc b/manager_unittest.cc
index 9ce7177..8b4269e 100644
--- a/manager_unittest.cc
+++ b/manager_unittest.cc
@@ -1353,4 +1353,19 @@
dispatcher()->DispatchPendingEvents();
}
+TEST_F(ManagerTest, RecheckPortal) {
+ EXPECT_CALL(*mock_devices_[0].get(), RequestPortalDetection())
+ .WillOnce(Return(false));
+ EXPECT_CALL(*mock_devices_[1].get(), RequestPortalDetection())
+ .WillOnce(Return(true));
+ EXPECT_CALL(*mock_devices_[2].get(), RequestPortalDetection())
+ .Times(0);
+
+ manager()->RegisterDevice(mock_devices_[0]);
+ manager()->RegisterDevice(mock_devices_[1]);
+ manager()->RegisterDevice(mock_devices_[2]);
+
+ manager()->RecheckPortal(NULL);
+}
+
} // namespace shill
diff --git a/mock_connection.h b/mock_connection.h
index 2d8af12..fdc1601 100644
--- a/mock_connection.h
+++ b/mock_connection.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium OS Authors. All rights reserved.
+// Copyright (c) 2012 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.
@@ -18,6 +18,7 @@
virtual ~MockConnection();
MOCK_METHOD1(UpdateFromIPConfig, void(const IPConfigRefPtr &config));
+ MOCK_CONST_METHOD0(is_default, bool());
MOCK_METHOD1(SetIsDefault, void(bool is_default));
MOCK_METHOD0(RequestRouting, void());
MOCK_METHOD0(ReleaseRouting, void());
diff --git a/mock_device.h b/mock_device.h
index 86d4fa7..96a522c 100644
--- a/mock_device.h
+++ b/mock_device.h
@@ -37,6 +37,7 @@
MOCK_METHOD0(EnableIPv6Privacy, void());
MOCK_METHOD0(DisableReversePathFilter, void());
MOCK_METHOD0(EnableReversePathFilter, void());
+ MOCK_METHOD0(RequestPortalDetection, bool());
MOCK_CONST_METHOD0(technology, Technology::Identifier());
private:
diff --git a/mock_manager.h b/mock_manager.h
index 5b97d96..32f0e45 100644
--- a/mock_manager.h
+++ b/mock_manager.h
@@ -32,6 +32,7 @@
const std::string &entry_name));
MOCK_METHOD1(IsPortalDetectionEnabled, bool(Technology::Identifier tech));
MOCK_CONST_METHOD0(GetPortalCheckURL, const std::string &());
+ MOCK_CONST_METHOD0(GetPortalCheckInterval, int());
private:
DISALLOW_COPY_AND_ASSIGN(MockManager);
diff --git a/mock_portal_detector.cc b/mock_portal_detector.cc
new file mode 100644
index 0000000..1d2475a
--- /dev/null
+++ b/mock_portal_detector.cc
@@ -0,0 +1,16 @@
+// Copyright (c) 2012 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/mock_portal_detector.h"
+
+#include "shill/connection.h"
+
+namespace shill {
+
+MockPortalDetector::MockPortalDetector(ConnectionRefPtr connection)
+ : PortalDetector(connection, NULL, NULL) {}
+
+MockPortalDetector::~MockPortalDetector() {}
+
+} // namespace shill
diff --git a/mock_portal_detector.h b/mock_portal_detector.h
new file mode 100644
index 0000000..9a15eb9
--- /dev/null
+++ b/mock_portal_detector.h
@@ -0,0 +1,31 @@
+// Copyright (c) 2012 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_MOCK_PORTAL_DETECTOR_H_
+#define SHILL_MOCK_PORTAL_DETECTOR_H_
+
+#include <base/basictypes.h>
+#include <gmock/gmock.h>
+
+#include "shill/portal_detector.h"
+
+namespace shill {
+
+class MockPortalDetector : public PortalDetector {
+ public:
+ MockPortalDetector(ConnectionRefPtr connection);
+ virtual ~MockPortalDetector();
+
+ MOCK_METHOD1(Start, bool(const std::string &));
+ MOCK_METHOD2(StartAfterDelay, bool(const std::string &, int delay_seconds));
+ MOCK_METHOD0(Stop, void());
+ MOCK_METHOD0(IsInProgress, bool());
+
+ private:
+ DISALLOW_COPY_AND_ASSIGN(MockPortalDetector);
+};
+
+} // namespace shill
+
+#endif // SHILL_MOCK_PORTAL_DETECTOR_H_
diff --git a/portal_detector.cc b/portal_detector.cc
index 60e98ab..938c50a 100644
--- a/portal_detector.cc
+++ b/portal_detector.cc
@@ -24,6 +24,7 @@
namespace shill {
+const int PortalDetector::kDefaultCheckIntervalSeconds = 30;
const char PortalDetector::kDefaultURL[] =
"http://clients3.google.com/generate_204";
const char PortalDetector::kResponseExpected[] = "HTTP/1.1 204";
@@ -61,7 +62,12 @@
Stop();
}
-bool PortalDetector::Start(const std::string &url_string) {
+bool PortalDetector::Start(const string &url_string) {
+ return StartAfterDelay(url_string, 0);
+}
+
+bool PortalDetector::StartAfterDelay(const string &url_string,
+ int delay_seconds) {
VLOG(3) << "In " << __func__;
DCHECK(!request_.get());
@@ -73,7 +79,7 @@
request_.reset(new HTTPRequest(connection_, dispatcher_, &sockets_));
attempt_count_ = 0;
- StartAttempt();
+ StartAttempt(delay_seconds);
return true;
}
@@ -89,6 +95,10 @@
request_.reset();
}
+bool PortalDetector::IsInProgress() {
+ return attempt_count_ != 0;
+}
+
// static
const string PortalDetector::PhaseToString(Phase phase) {
switch (phase) {
@@ -127,7 +137,7 @@
StatusToString(result.status).c_str());
StopAttempt();
if (result.status != kStatusSuccess && attempt_count_ < kMaxRequestAttempts) {
- StartAttempt();
+ StartAttempt(0);
} else {
result.final = true;
Stop();
@@ -192,7 +202,7 @@
CompleteAttempt(GetPortalResultForRequestResult(result));
}
-void PortalDetector::StartAttempt() {
+void PortalDetector::StartAttempt(int init_delay_seconds) {
int64 next_attempt_delay = 0;
if (attempt_count_ > 0) {
// Ensure that attempts are spaced at least by a minimal interval.
@@ -206,6 +216,8 @@
next_attempt_delay =
remaining_time.tv_sec * 1000 + remaining_time.tv_usec / 1000;
}
+ } else {
+ next_attempt_delay = init_delay_seconds * 1000;
}
dispatcher_->PostDelayedTask(
task_factory_.NewRunnableMethod(&PortalDetector::StartAttemptTask),
diff --git a/portal_detector.h b/portal_detector.h
index 2b4148c..ff27e3a 100644
--- a/portal_detector.h
+++ b/portal_detector.h
@@ -69,6 +69,7 @@
bool final;
};
+ static const int kDefaultCheckIntervalSeconds;
static const char kDefaultURL[];
static const char kResponseExpected[];
@@ -87,11 +88,20 @@
// this is the last attempt, the "final" flag in the Result structure will
// be true, otherwise it will be false, and the PortalDetector will
// schedule the next attempt.
- bool Start(const std::string &url_string);
+ virtual bool Start(const std::string &url_string);
+ virtual bool StartAfterDelay(const std::string &url_string,
+ int delay_seconds);
// End the current portal detection process if one exists, and do not call
// the callback.
- void Stop();
+ virtual void Stop();
+
+ // Returns whether portal request is "in progress": whether the portal
+ // detector is in the progress of making attempts. Returns true if
+ // attempts are in progress, false otherwise. Notably, this function
+ // returns false during the period of time between calling "Start" or
+ // "StartAfterDelay" and the actual start of the first attempt.
+ virtual bool IsInProgress();
static const std::string PhaseToString(Phase phase);
static const std::string StatusToString(Status status);
@@ -101,6 +111,7 @@
friend class PortalDetectorTest;
FRIEND_TEST(PortalDetectorTest, StartAttemptFailed);
FRIEND_TEST(PortalDetectorTest, StartAttemptRepeated);
+ FRIEND_TEST(PortalDetectorTest, StartAttemptAfterDelay);
FRIEND_TEST(PortalDetectorTest, AttemptCount);
// Number of times to attempt connection.
@@ -124,7 +135,7 @@
void RequestReadCallback(const ByteString &response_data);
void RequestResultCallback(HTTPRequest::Result result,
const ByteString &response_data);
- void StartAttempt();
+ void StartAttempt(int init_delay_seconds);
void StartAttemptTask();
void StopAttempt();
void TimeoutAttemptTask();
diff --git a/portal_detector_unittest.cc b/portal_detector_unittest.cc
index 7119cc3..589336e 100644
--- a/portal_detector_unittest.cc
+++ b/portal_detector_unittest.cc
@@ -115,6 +115,17 @@
return ret;
}
+ void StartAttemptTask() {
+ AssignHTTPRequest();
+ EXPECT_CALL(*http_request(), Start(_, _, _))
+ .WillOnce(Return(HTTPRequest::kResultInProgress));
+ EXPECT_CALL(
+ dispatcher(),
+ PostDelayedTask(
+ _, PortalDetector::kRequestTimeoutSeconds * 1000));
+ portal_detector()->StartAttemptTask();
+ }
+
void TimeoutAttempt() {
portal_detector_->TimeoutAttemptTask();
}
@@ -217,31 +228,44 @@
TEST_F(PortalDetectorTest, StartAttemptRepeated) {
EXPECT_CALL(dispatcher(), PostDelayedTask(_, 0));
- portal_detector()->StartAttempt();
+ portal_detector()->StartAttempt(0);
- AssignHTTPRequest();
- EXPECT_CALL(*http_request(), Start(_, _, _))
- .WillOnce(Return(HTTPRequest::kResultInProgress));
- EXPECT_CALL(
- dispatcher(),
- PostDelayedTask(
- _, PortalDetector::kRequestTimeoutSeconds * 1000));
- portal_detector()->StartAttemptTask();
+ StartAttemptTask();
// A second attempt should be delayed by kMinTimeBetweenAttemptsSeconds.
EXPECT_CALL(
dispatcher(),
PostDelayedTask(
_, PortalDetector::kMinTimeBetweenAttemptsSeconds * 1000));
- portal_detector()->StartAttempt();
+ portal_detector()->StartAttempt(0);
+}
+
+TEST_F(PortalDetectorTest, StartAttemptAfterDelay) {
+ const int kDelaySeconds = 123;
+ // The first attempt should be delayed by kDelaySeconds.
+ EXPECT_CALL(dispatcher(), PostDelayedTask(_, kDelaySeconds * 1000));
+ EXPECT_TRUE(portal_detector()->StartAfterDelay(kURL, kDelaySeconds));
+
+ AdvanceTime(kDelaySeconds * 1000);
+ StartAttemptTask();
+
+ // A second attempt should be delayed by kMinTimeBetweenAttemptsSeconds.
+ EXPECT_CALL(
+ dispatcher(),
+ PostDelayedTask(
+ _, PortalDetector::kMinTimeBetweenAttemptsSeconds * 1000));
+ portal_detector()->StartAttempt(0);
}
TEST_F(PortalDetectorTest, AttemptCount) {
+ EXPECT_FALSE(portal_detector()->IsInProgress());
// Expect the PortalDetector to immediately post a task for the each attempt.
EXPECT_CALL(dispatcher(), PostDelayedTask(_, 0))
.Times(PortalDetector::kMaxRequestAttempts);
EXPECT_TRUE(StartPortalRequest(kURL));
+ EXPECT_FALSE(portal_detector()->IsInProgress());
+
// Expect that the request will be started -- return failure.
EXPECT_CALL(*http_request(), Start(_, _, _))
.Times(PortalDetector::kMaxRequestAttempts)
@@ -282,11 +306,13 @@
for (int i = 0; i < PortalDetector::kMaxRequestAttempts; i++) {
portal_detector()->StartAttemptTask();
+ EXPECT_TRUE(portal_detector()->IsInProgress());
AdvanceTime(PortalDetector::kMinTimeBetweenAttemptsSeconds * 1000);
portal_detector()->RequestResultCallback(HTTPRequest::kResultDNSFailure,
response_data());
}
+ EXPECT_FALSE(portal_detector()->IsInProgress());
ExpectReset();
}
@@ -349,6 +375,7 @@
AppendReadData(response_expected.substr(partial_size));
}
+
struct ResultMapping {
ResultMapping() : http_result(HTTPRequest::kResultUnknown), portal_result() {}
ResultMapping(HTTPRequest::Result in_http_result,