shill: Initiate scan on resume, if appropriate
When a power resume event occurs, start a scan if the WiFi device is idle.
BUG=chromium-os:24885
TEST=Added new unit tests to WiFiMainTest. Ran all shill unit tests.
autotest WiFiRoaming.009ConnectOnResume should pass but currently doesn't
because shill crashes on suspend-resume.
Change-Id: I2730b0f27c0d85e72c0add57a3e9a5a2c995a04f
Reviewed-on: https://gerrit.chromium.org/gerrit/16107
Reviewed-by: mukesh agrawal <quiche@chromium.org>
Commit-Ready: Gary Morain <gmorain@chromium.org>
Tested-by: Gary Morain <gmorain@chromium.org>
diff --git a/Makefile b/Makefile
index 357776a..1a11252 100644
--- a/Makefile
+++ b/Makefile
@@ -239,6 +239,7 @@
mock_modem_manager_proxy.o \
mock_modem_proxy.o \
mock_modem_simple_proxy.o \
+ mock_power_manager.o \
mock_power_manager_proxy.o \
mock_profile.o \
mock_property_store.o \
diff --git a/manager.cc b/manager.cc
index dba8137..2abe65b 100644
--- a/manager.cc
+++ b/manager.cc
@@ -31,6 +31,7 @@
#include "shill/metrics.h"
#include "shill/profile.h"
#include "shill/property_accessor.h"
+#include "shill/proxy_factory.h"
#include "shill/resolver.h"
#include "shill/service.h"
#include "shill/service_sorter.h"
@@ -130,6 +131,7 @@
void Manager::Start() {
LOG(INFO) << "Manager started.";
+ power_manager_.reset(new PowerManager(ProxyFactory::GetInstance()));
CHECK(file_util::CreateDirectory(run_path_)) << run_path_.value();
Resolver::GetInstance()->set_path(run_path_.Append("resolv.conf"));
diff --git a/manager.h b/manager.h
index 92f3294..1de0f5b 100644
--- a/manager.h
+++ b/manager.h
@@ -18,6 +18,7 @@
#include "shill/device_info.h"
#include "shill/event_dispatcher.h"
#include "shill/modem_info.h"
+#include "shill/power_manager.h"
#include "shill/property_store.h"
#include "shill/service.h"
#include "shill/wifi.h"
@@ -52,6 +53,7 @@
virtual ~Manager();
void AddDeviceToBlackList(const std::string &device_name);
+
void Start();
void Stop();
@@ -137,9 +139,12 @@
}
virtual const std::string &GetHostName() { return props_.host_name; }
+ PowerManager *power_manager() const { return power_manager_.get(); }
+
private:
friend class ManagerAdaptorInterface;
friend class ManagerTest;
+ friend class WiFiMainTest;
FRIEND_TEST(ManagerTest, AvailableTechnologies);
FRIEND_TEST(ManagerTest, ConnectedTechnologies);
FRIEND_TEST(ManagerTest, DefaultTechnology);
@@ -185,6 +190,12 @@
// For unit testing.
void set_metrics(Metrics *metrics) { metrics_ = metrics; }
+ // Used by tests to set a mock PowerManager. Takes ownership of
+ // power_manager.
+ void set_power_manager(PowerManager *power_manager) {
+ power_manager_.reset(power_manager);
+ }
+
EventDispatcher *dispatcher_;
ScopedRunnableMethodFactory<Manager> task_factory_;
const FilePath run_path_;
@@ -206,6 +217,7 @@
ControlInterface *control_interface_;
Metrics *metrics_;
GLib *glib_;
+ scoped_ptr<PowerManager> power_manager_;
// The priority order of technologies
std::vector<Technology::Identifier> technology_order_;
diff --git a/mock_power_manager.cc b/mock_power_manager.cc
new file mode 100644
index 0000000..172681d
--- /dev/null
+++ b/mock_power_manager.cc
@@ -0,0 +1,14 @@
+// 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_power_manager.h"
+
+namespace shill {
+
+MockPowerManager::MockPowerManager(ProxyFactory *proxy_factory)
+ : PowerManager(proxy_factory) {}
+
+MockPowerManager::~MockPowerManager() {}
+
+} // namespace shill
diff --git a/mock_power_manager.h b/mock_power_manager.h
new file mode 100644
index 0000000..5284fd5
--- /dev/null
+++ b/mock_power_manager.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_POWER_MANAGER_
+#define SHILL_MOCK_POWER_MANAGER_
+
+#include <base/basictypes.h>
+#include <gmock/gmock.h>
+
+#include "shill/power_manager.h"
+
+namespace shill {
+
+class ProxyFactory;
+
+class MockPowerManager : public PowerManager {
+ public:
+ explicit MockPowerManager(ProxyFactory *proxy_factory);
+ virtual ~MockPowerManager();
+ MOCK_METHOD2(AddStateChangeCallback,
+ void(const std::string &key, PowerStateCallback *callback));
+ MOCK_METHOD1(RemoveStateChangeCallback, void(const std::string &key));
+
+ private:
+ DISALLOW_COPY_AND_ASSIGN(MockPowerManager);
+};
+
+} // namespace shill
+
+#endif // SHILL_MOCK_POWER_MANAGER_
diff --git a/power_manager.h b/power_manager.h
index f77f49b..f6428eb 100644
--- a/power_manager.h
+++ b/power_manager.h
@@ -69,12 +69,12 @@
// power state change occurs, this callback will be called with the new power
// state as its argument. |key| must be unique. Ownership of |callback| is
// passed to this class.
- void AddStateChangeCallback(const std::string &key,
- PowerStateCallback *callback);
+ virtual void AddStateChangeCallback(const std::string &key,
+ PowerStateCallback *callback);
// Unregisters a callback identified by |key|. The callback must have been
// previously registered and not yet removed. The callback is deleted.
- void RemoveStateChangeCallback(const std::string &key);
+ virtual void RemoveStateChangeCallback(const std::string &key);
// TODO(gmorain): Add registration for the OnSuspendDelay event.
diff --git a/wifi.cc b/wifi.cc
index 7a421d0..78381a2 100644
--- a/wifi.cc
+++ b/wifi.cc
@@ -32,6 +32,7 @@
#include "shill/ieee80211.h"
#include "shill/manager.h"
#include "shill/metrics.h"
+#include "shill/power_manager.h"
#include "shill/profile.h"
#include "shill/property_accessor.h"
#include "shill/proxy_factory.h"
@@ -173,6 +174,12 @@
<< "May be running an older version of wpa_supplicant.";
}
+ // Register for power state changes. HandlePowerStateChange() will be called
+ // when the power state changes.
+ PowerManager::PowerStateCallback *cb =
+ NewCallback(this, &WiFi::HandlePowerStateChange);
+ manager()->power_manager()->AddStateChangeCallback(UniqueName(), cb);
+
Scan(NULL);
Device::Start();
}
@@ -1111,7 +1118,7 @@
size_t i;
bool changed = false;
- for (i=0; i < ssid_len; ++i) {
+ for (i = 0; i < ssid_len; ++i) {
if (!g_ascii_isprint((*ssid)[i])) {
(*ssid)[i] = '?';
changed = true;
@@ -1151,4 +1158,10 @@
Uint16Accessor(new CustomAccessor<WiFi, uint16>(this, get, set)));
}
+void WiFi::HandlePowerStateChange(PowerManager::SuspendState new_state) {
+ if ((new_state == PowerManagerProxyDelegate::kOn) && IsIdle()) {
+ Scan(NULL);
+ }
+}
+
} // namespace shill
diff --git a/wifi.h b/wifi.h
index a98c306..1d0275d 100644
--- a/wifi.h
+++ b/wifi.h
@@ -14,6 +14,7 @@
#include "shill/device.h"
#include "shill/event_dispatcher.h"
+#include "shill/power_manager.h"
#include "shill/refptr_types.h"
namespace shill {
@@ -147,6 +148,10 @@
uint16(WiFi::*get)(Error *error),
void(WiFi::*set)(const uint16 &value, Error *error));
+ // If this WiFi device is idle and |new_state| indicates a resume event, a
+ // scan is initiated.
+ void HandlePowerStateChange(PowerManager::SuspendState new_state);
+
// Store cached copies of singletons for speed/ease of testing.
ProxyFactory *proxy_factory_;
diff --git a/wifi_unittest.cc b/wifi_unittest.cc
index bd5e2a4..a196641 100644
--- a/wifi_unittest.cc
+++ b/wifi_unittest.cc
@@ -35,6 +35,8 @@
#include "shill/mock_dhcp_provider.h"
#include "shill/mock_manager.h"
#include "shill/mock_metrics.h"
+#include "shill/mock_power_manager.h"
+#include "shill/mock_power_manager_proxy.h"
#include "shill/mock_rtnl_handler.h"
#include "shill/mock_store.h"
#include "shill/mock_supplicant_bss_proxy.h"
@@ -45,10 +47,10 @@
#include "shill/property_store_unittest.h"
#include "shill/proxy_factory.h"
#include "shill/wifi_endpoint.h"
-#include "shill/wifi.h"
#include "shill/wifi_service.h"
#include "shill/wpa_supplicant.h"
+
using std::map;
using std::set;
using std::string;
@@ -62,6 +64,7 @@
using ::testing::Mock;
using ::testing::NiceMock;
using ::testing::Return;
+using ::testing::SaveArg;
using ::testing::SetArgumentPointee;
using ::testing::StrEq;
using ::testing::StrictMock;
@@ -176,7 +179,9 @@
kDeviceName,
kHostName,
&glib_)),
- proxy_factory_(this) {
+ proxy_factory_(this),
+ power_manager_(new MockPowerManager(&proxy_factory_)),
+ power_state_callback_(NULL) {
::testing::DefaultValue< ::DBus::Path>::Set("/default/path");
// Except for WiFiServices created via WiFi::GetService, we expect
// that any WiFiService has been registered with the Manager. So
@@ -189,6 +194,9 @@
WillByDefault(Return(dhcp_config_));
ON_CALL(*dhcp_config_.get(), RequestIP()).
WillByDefault(Return(true));
+
+ // |manager_| takes ownership of |power_manager_|.
+ manager_.set_power_manager(power_manager_);
}
virtual void SetUp() {
@@ -239,6 +247,11 @@
const DBus::Path &object_path,
const char *dbus_addr));
+ virtual PowerManagerProxyInterface *CreatePowerManagerProxy(
+ PowerManagerProxyDelegate */*delegate*/) {
+ return new MockPowerManagerProxy();
+ }
+
private:
SupplicantBSSProxyInterface *CreateSupplicantBSSProxyInternal(
WiFiEndpoint */*wifi_endpoint*/,
@@ -329,12 +342,15 @@
wifi_->StateChanged(new_state);
}
void StartWiFi() {
+ EXPECT_CALL(*power_manager_, AddStateChangeCallback(wifi_->UniqueName(), _))
+ .WillOnce(SaveArg<1>(&power_state_callback_));
wifi_->Start();
}
void StopWiFi() {
wifi_->Stop();
}
- void GetOpenService(const char *service_type,
+
+ void GetOpenService(const char *service_type,
const char *ssid,
const char *mode,
Error *result) {
@@ -415,6 +431,10 @@
return &dhcp_provider_;
}
+ PowerManager::PowerStateCallback *power_state_callback() const {
+ return power_state_callback_;
+ }
+
const WiFiConstRefPtr wifi() const {
return wifi_;
}
@@ -451,6 +471,8 @@
private:
TestProxyFactory proxy_factory_;
+ MockPowerManager *power_manager_;
+ PowerManager::PowerStateCallback *power_state_callback_;
};
const char WiFiMainTest::kDeviceName[] = "wlan0";
@@ -541,6 +563,53 @@
StartWiFi();
}
+TEST_F(WiFiMainTest, PowerChangeToResumeStartsScanWhenIdle) {
+ EXPECT_CALL(*supplicant_interface_proxy_, Scan(_));
+ StartWiFi();
+ dispatcher_.DispatchPendingEvents();
+ Mock::VerifyAndClearExpectations(&supplicant_interface_proxy_);
+ ASSERT_TRUE(power_state_callback() != NULL);
+ ASSERT_TRUE(wifi()->IsIdle());
+ EXPECT_CALL(*GetSupplicantInterfaceProxy(), Scan(_));
+ power_state_callback()->Run(PowerManagerProxyDelegate::kOn);
+ dispatcher_.DispatchPendingEvents();
+}
+
+TEST_F(WiFiMainTest, PowerChangeToSuspendDoesNotStartScan) {
+ EXPECT_CALL(*supplicant_interface_proxy_, Scan(_));
+ StartWiFi();
+ dispatcher_.DispatchPendingEvents();
+ Mock::VerifyAndClearExpectations(&supplicant_interface_proxy_);
+ ASSERT_TRUE(power_state_callback() != NULL);
+ ASSERT_TRUE(wifi()->IsIdle());
+ EXPECT_CALL(*GetSupplicantInterfaceProxy(), Scan(_)).Times(0);
+ power_state_callback()->Run(PowerManagerProxyDelegate::kStandby);
+ dispatcher_.DispatchPendingEvents();
+ power_state_callback()->Run(PowerManagerProxyDelegate::kMem);
+ dispatcher_.DispatchPendingEvents();
+ power_state_callback()->Run(PowerManagerProxyDelegate::kDisk);
+ dispatcher_.DispatchPendingEvents();
+}
+
+TEST_F(WiFiMainTest, PowerChangeDoesNotStartScanWhenNotIdle) {
+ EXPECT_CALL(*supplicant_interface_proxy_, Scan(_));
+ StartWiFi();
+
+ WiFiEndpointRefPtr ap = MakeEndpoint("an_ssid", "00:01:02:03:04:05");
+ WiFiServiceRefPtr service = CreateServiceForEndpoint(*ap);
+ Error error;
+ service->AddEndpoint(ap);
+ service->AutoConnect();
+ EXPECT_FALSE(wifi()->IsIdle());
+ dispatcher_.DispatchPendingEvents();
+ Mock::VerifyAndClearExpectations(&supplicant_interface_proxy_);
+ ASSERT_TRUE(power_state_callback() != NULL);
+ ASSERT_FALSE(wifi()->IsIdle());
+ EXPECT_CALL(*GetSupplicantInterfaceProxy(), Scan(_)).Times(0);
+ power_state_callback()->Run(PowerManagerProxyDelegate::kOn);
+ dispatcher_.DispatchPendingEvents();
+}
+
TEST_F(WiFiMainTest, ScanResults) {
EXPECT_CALL(*manager(), RegisterService(_)).Times(AnyNumber());
StartWiFi();
@@ -1550,6 +1619,7 @@
EXPECT_CALL(*manager(), device_info()).Times(AnyNumber());
EXPECT_CALL(*dhcp_provider(), CreateConfig(_, _)).Times(AnyNumber());
EXPECT_CALL(*manager(), HasService(_)).Times(AnyNumber());
+ EXPECT_CALL(*manager(), IsPortalDetectionEnabled(_)).Times(AnyNumber());
WiFiEndpointRefPtr ap = MakeEndpoint("an_ssid", "00:01:02:03:04:05");
WiFiServiceRefPtr service = CreateServiceForEndpoint(*ap);