shill: inject WakeOnWiFi into WiFi
Having the WiFi object instantiate WakeOnWiFi unconditionally
forces it to instantiate SimpleAlarmTimers, which try to grab the
current sequenced task runner handle. This is a problem when there
is no task runner, i.e. during tests with no message loop.
In the new libchrome, this causes a DCHECK in WiFi unit tests.
We should inject a mock WakeOnWiFi instead.
BUG=b:37434548,chromium:509138
TEST=unit tests
Change-Id: If11d737650691c44b36f8e389a524e61705b3f03
Reviewed-on: https://chromium-review.googlesource.com/1072731
Commit-Ready: Kirtika Ruchandani <kirtika@chromium.org>
Tested-by: Kirtika Ruchandani <kirtika@chromium.org>
Reviewed-by: Kirtika Ruchandani <kirtika@chromium.org>
diff --git a/device_info.cc b/device_info.cc
index 6b8767a..2539d4a 100644
--- a/device_info.cc
+++ b/device_info.cc
@@ -31,6 +31,7 @@
#include <unistd.h>
#include <string>
+#include <utility>
#include <base/bind.h>
#include <base/files/file_enumerator.h>
@@ -1334,9 +1335,20 @@
<< " at interface index "
<< interface_index;
string address = info->mac_address.HexEncode();
- DeviceRefPtr device =
- new WiFi(control_interface_, dispatcher_, metrics_, manager_,
- info->name, address, interface_index);
+ auto wake_on_wifi = std::make_unique<WakeOnWiFi>(
+ netlink_manager_,
+ dispatcher_,
+ metrics_,
+ address,
+ base::Bind(&Manager::RecordDarkResumeWakeReason, manager_->AsWeakPtr()));
+ DeviceRefPtr device = new WiFi(control_interface_,
+ dispatcher_,
+ metrics_,
+ manager_,
+ info->name,
+ address,
+ interface_index,
+ std::move(wake_on_wifi));
device->EnableIPv6Privacy();
RegisterDevice(device);
}
diff --git a/wifi/mock_wifi.cc b/wifi/mock_wifi.cc
index 3ff7e43..a2f090d 100644
--- a/wifi/mock_wifi.cc
+++ b/wifi/mock_wifi.cc
@@ -16,6 +16,7 @@
#include "shill/wifi/mock_wifi.h"
+#include <memory>
#include <string>
namespace shill {
@@ -28,14 +29,16 @@
Manager* manager,
const string& link_name,
const string& address,
- int interface_index)
+ int interface_index,
+ WakeOnWiFiInterface* wake_on_wifi)
: WiFi(control_interface,
dispatcher,
metrics,
manager,
link_name,
address,
- interface_index) {}
+ interface_index,
+ std::unique_ptr<WakeOnWiFiInterface>(wake_on_wifi)) {}
MockWiFi::~MockWiFi() {}
diff --git a/wifi/mock_wifi.h b/wifi/mock_wifi.h
index e59c102..d6941ef 100644
--- a/wifi/mock_wifi.h
+++ b/wifi/mock_wifi.h
@@ -24,6 +24,7 @@
#include "shill/key_value_store.h"
#include "shill/refptr_types.h"
+#include "shill/wifi/wake_on_wifi.h"
#include "shill/wifi/wifi.h"
#include "shill/wifi/wifi_endpoint.h"
#include "shill/wifi/wifi_service.h"
@@ -36,13 +37,17 @@
class MockWiFi : public WiFi {
public:
+ // MockWiFi takes ownership of the wake_on_wifi pointer passed to it.
+ // This is not exposed in the constructor type because gmock doesn't
+ // provide the ability to forward arguments that aren't const &...
MockWiFi(ControlInterface* control_interface,
EventDispatcher* dispatcher,
Metrics* metrics,
Manager* manager,
const std::string& link_name,
const std::string& address,
- int interface_index);
+ int interface_index,
+ WakeOnWiFiInterface* wake_on_wifi);
~MockWiFi() override;
MOCK_METHOD2(Start, void(Error* error,
diff --git a/wifi/wifi.cc b/wifi/wifi.cc
index fa1667d..2b9efa1 100644
--- a/wifi/wifi.cc
+++ b/wifi/wifi.cc
@@ -130,7 +130,8 @@
Manager* manager,
const string& link,
const string& address,
- int interface_index)
+ int interface_index,
+ std::unique_ptr<WakeOnWiFiInterface> wake_on_wifi)
: Device(control_interface,
dispatcher,
metrics,
@@ -143,10 +144,9 @@
weak_ptr_factory_(this),
time_(Time::GetInstance()),
supplicant_present_(false),
- supplicant_process_proxy_(
- control_interface->CreateSupplicantProcessProxy(
- Bind(&WiFi::OnSupplicantAppear, Unretained(this)),
- Bind(&WiFi::OnSupplicantVanish, Unretained(this)))),
+ supplicant_process_proxy_(control_interface->CreateSupplicantProcessProxy(
+ Bind(&WiFi::OnSupplicantAppear, Unretained(this)),
+ Bind(&WiFi::OnSupplicantVanish, Unretained(this)))),
supplicant_state_(kInterfaceStateUnknown),
supplicant_bss_("(unknown)"),
supplicant_disconnect_reason_(kDefaultDisconnectReason),
@@ -158,13 +158,13 @@
is_roaming_in_progress_(false),
is_debugging_connection_(false),
eap_state_handler_(new SupplicantEAPStateHandler()),
- mac80211_monitor_(new Mac80211Monitor(
- dispatcher,
- link,
- kStuckQueueLengthThreshold,
- base::Bind(&WiFi::RestartFastScanAttempts,
- weak_ptr_factory_.GetWeakPtr()),
- metrics)),
+ mac80211_monitor_(
+ new Mac80211Monitor(dispatcher,
+ link,
+ kStuckQueueLengthThreshold,
+ base::Bind(&WiFi::RestartFastScanAttempts,
+ weak_ptr_factory_.GetWeakPtr()),
+ metrics)),
bgscan_short_interval_seconds_(kDefaultBgscanShortIntervalSeconds),
bgscan_signal_threshold_dbm_(kDefaultBgscanSignalThresholdDbm),
roam_threshold_db_(kDefaultRoamThresholdDb),
@@ -177,12 +177,7 @@
scan_method_(kScanMethodNone),
receive_byte_count_at_connect_(0),
wiphy_index_(kDefaultWiphyIndex),
- wake_on_wifi_(new WakeOnWiFi(netlink_manager_,
- dispatcher,
- metrics,
- address,
- Bind(&Manager::RecordDarkResumeWakeReason,
- manager->AsWeakPtr()))) {
+ wake_on_wifi_(std::move(wake_on_wifi)) {
PropertyStore* store = this->mutable_store();
store->RegisterDerivedString(kBgscanMethodProperty,
StringAccessor(new CustomAccessor<WiFi, string>(
diff --git a/wifi/wifi.h b/wifi/wifi.h
index 8032bc8..dc74ce4 100644
--- a/wifi/wifi.h
+++ b/wifi/wifi.h
@@ -136,7 +136,8 @@
Manager* manager,
const std::string& link,
const std::string& address,
- int interface_index);
+ int interface_index,
+ std::unique_ptr<WakeOnWiFiInterface> wake_on_wifi);
~WiFi() override;
void Start(Error* error,
diff --git a/wifi/wifi_endpoint_unittest.cc b/wifi/wifi_endpoint_unittest.cc
index 1089362..1517d54 100644
--- a/wifi/wifi_endpoint_unittest.cc
+++ b/wifi/wifi_endpoint_unittest.cc
@@ -30,10 +30,12 @@
#include "shill/mock_log.h"
#include "shill/net/ieee80211.h"
+#include "shill/net/mock_netlink_manager.h"
#include "shill/property_store_unittest.h"
#include "shill/refptr_types.h"
#include "shill/supplicant/wpa_supplicant.h"
#include "shill/tethering.h"
+#include "shill/wifi/mock_wake_on_wifi.h"
#include "shill/wifi/mock_wifi.h"
using std::map;
@@ -47,17 +49,21 @@
namespace shill {
+// Fake MAC address.
+constexpr char kDeviceAddress[] = "aabbccddeeff";
+
class WiFiEndpointTest : public PropertyStoreTest {
public:
- WiFiEndpointTest() : wifi_(
- new NiceMock<MockWiFi>(
- control_interface(),
- dispatcher(),
- metrics(),
- manager(),
- "wifi",
- "aabbccddeeff", // fake mac
- 0)) {}
+ WiFiEndpointTest()
+ : wifi_(new NiceMock<MockWiFi>(
+ control_interface(),
+ dispatcher(),
+ metrics(),
+ manager(),
+ "wifi",
+ kDeviceAddress,
+ 0,
+ new MockWakeOnWiFi())) {}
virtual ~WiFiEndpointTest() {}
protected:
@@ -230,6 +236,7 @@
scoped_refptr<MockWiFi> wifi() { return wifi_; }
private:
+ MockNetlinkManager netlink_manager_;
scoped_refptr<MockWiFi> wifi_;
};
diff --git a/wifi/wifi_service_unittest.cc b/wifi/wifi_service_unittest.cc
index 52ee8d6..edcebf0 100644
--- a/wifi/wifi_service_unittest.cc
+++ b/wifi/wifi_service_unittest.cc
@@ -40,12 +40,14 @@
#include "shill/mock_profile.h"
#include "shill/mock_service.h"
#include "shill/mock_store.h"
+#include "shill/net/mock_netlink_manager.h"
#include "shill/property_store_unittest.h"
#include "shill/refptr_types.h"
#include "shill/service_property_change_test.h"
#include "shill/supplicant/wpa_supplicant.h"
#include "shill/technology.h"
#include "shill/tethering.h"
+#include "shill/wifi/mock_wake_on_wifi.h"
#include "shill/wifi/mock_wifi.h"
#include "shill/wifi/mock_wifi_provider.h"
#include "shill/wifi/wifi_endpoint.h"
@@ -73,14 +75,15 @@
public:
WiFiServiceTest()
: mock_manager_(control_interface(), dispatcher(), metrics()),
- wifi_(
- new NiceMock<MockWiFi>(control_interface(),
- dispatcher(),
- metrics(),
- manager(),
- "wifi",
- fake_mac,
- 0)),
+ wifi_(new NiceMock<MockWiFi>(
+ control_interface(),
+ dispatcher(),
+ metrics(),
+ manager(),
+ "wifi",
+ fake_mac,
+ 0,
+ new MockWakeOnWiFi())),
simple_ssid_(1, 'a'),
simple_ssid_string_("a") {}
virtual ~WiFiServiceTest() {}
@@ -176,7 +179,8 @@
manager(),
link_name,
fake_mac,
- 0);
+ 0,
+ new MockWakeOnWiFi());
}
ServiceMockAdaptor* GetAdaptor(WiFiService* service) {
return static_cast<ServiceMockAdaptor*>(service->adaptor());
@@ -207,6 +211,7 @@
private:
MockManager mock_manager_;
+ MockNetlinkManager netlink_manager_;
scoped_refptr<MockWiFi> wifi_;
MockWiFiProvider provider_;
const vector<uint8_t> simple_ssid_;
diff --git a/wifi/wifi_unittest.cc b/wifi/wifi_unittest.cc
index fdd5a2e..b656348 100644
--- a/wifi/wifi_unittest.cc
+++ b/wifi/wifi_unittest.cc
@@ -463,14 +463,20 @@
public:
WiFiPropertyTest()
: metrics_(nullptr),
- device_(
- new WiFi(control_interface(), dispatcher(), &metrics_,
- manager(), "wifi", "", kInterfaceIndex)) {
+ device_(new WiFi(control_interface(),
+ dispatcher(),
+ &metrics_,
+ manager(),
+ "wifi",
+ "",
+ kInterfaceIndex,
+ std::make_unique<MockWakeOnWiFi>())) {
}
virtual ~WiFiPropertyTest() {}
protected:
MockMetrics metrics_;
+ MockNetlinkManager netlink_manager_;
WiFiRefPtr device_;
};
@@ -568,7 +574,8 @@
&manager_,
kDeviceName,
kDeviceAddress,
- kInterfaceIndex)),
+ kInterfaceIndex,
+ std::make_unique<MockWakeOnWiFi>())),
bss_counter_(0),
mac80211_monitor_(new StrictMock<MockMac80211Monitor>(
event_dispatcher_.get(),
@@ -641,6 +648,8 @@
wifi_->all_scan_frequencies_.insert(kRandomScanFrequency1);
wifi_->all_scan_frequencies_.insert(kRandomScanFrequency2);
wifi_->all_scan_frequencies_.insert(kRandomScanFrequency3);
+
+ wake_on_wifi_ = static_cast<MockWakeOnWiFi*>(wifi_->wake_on_wifi_.get());
}
void SetUp() override {
@@ -652,10 +661,6 @@
ON_CALL(manager_, device_info()).WillByDefault(Return(&device_info_));
EXPECT_CALL(manager_, UpdateEnabledTechnologies()).Times(AnyNumber());
EXPECT_CALL(*supplicant_bss_proxy_, Die()).Times(AnyNumber());
- // Must be called here instead of in the constructor so that the destructor
- // of SimpleAlarmTimer will not be invoked before the EventDispatcher is
- // properly constructed (crbug.com/509138).
- InstallMockWakeOnWiFi();
}
void TearDown() override {
@@ -690,11 +695,6 @@
return wifi_->all_scan_frequencies_.size();
}
- void InstallMockWakeOnWiFi() {
- wake_on_wifi_ = new MockWakeOnWiFi();
- wifi_->wake_on_wifi_.reset(wake_on_wifi_);
- }
-
void SetScanState(WiFi::ScanState new_state,
WiFi::ScanMethod new_method,
const char* reason) {
@@ -1307,6 +1307,7 @@
MockWakeOnWiFi* wake_on_wifi_; // Owned by |wifi_|.
NiceMock<MockRTNLHandler> rtnl_handler_;
MockTime time_;
+ MockNetlinkManager netlink_manager_;
private:
NiceMockControl control_interface_;
@@ -1338,7 +1339,6 @@
// and manager so we can perform expectations against them.
DeviceMockAdaptor* adaptor_;
MockSupplicantEAPStateHandler* eap_state_handler_;
- MockNetlinkManager netlink_manager_;
private:
std::unique_ptr<SupplicantInterfaceProxyInterface>
