shill: cellular: Add Network.Shill.Cellular.DropsPerHour.
BUG=chromium-os:38557
TEST=Unit tests, manually lose LTE signal and check chrome://histograms
Change-Id: I82aa1edabd21c5dfde4834cd36a34a12b629ba8f
Reviewed-on: https://gerrit.chromium.org/gerrit/42813
Reviewed-by: Arman Uguray <armansito@chromium.org>
Commit-Queue: Thieu Le <thieule@chromium.org>
Tested-by: Thieu Le <thieule@chromium.org>
diff --git a/cellular.cc b/cellular.cc
index 4e5f8eb..b1d86de 100644
--- a/cellular.cc
+++ b/cellular.cc
@@ -368,7 +368,9 @@
(state_ == kStateLinked || state_ == kStateConnected) &&
service_.get())
metrics()->NotifyCellularDeviceDrop(
- capability_->GetNetworkTechnologyString(), service_->strength());
+ interface_index(),
+ capability_->GetNetworkTechnologyString(),
+ service_->strength());
DestroyService();
if (state_ == kStateLinked ||
state_ == kStateConnected ||
diff --git a/cellular_capability_cdma_unittest.cc b/cellular_capability_cdma_unittest.cc
index 0d06f97..a0baa2d 100644
--- a/cellular_capability_cdma_unittest.cc
+++ b/cellular_capability_cdma_unittest.cc
@@ -37,6 +37,7 @@
public:
CellularCapabilityCDMATest()
: manager_(&control_, &dispatcher_, &metrics_, &glib_),
+ metrics_(&dispatcher_),
cellular_(new MockCellular(&control_,
&dispatcher_,
&metrics_,
diff --git a/cellular_capability_classic_unittest.cc b/cellular_capability_classic_unittest.cc
index d5f18fe..ec5c58c 100644
--- a/cellular_capability_classic_unittest.cc
+++ b/cellular_capability_classic_unittest.cc
@@ -48,7 +48,8 @@
class CellularCapabilityTest : public testing::Test {
public:
CellularCapabilityTest()
- : manager_(&control_, &dispatcher_, &metrics_, &glib_),
+ : metrics_(&dispatcher_),
+ manager_(&control_, &dispatcher_, &metrics_, &glib_),
create_gsm_card_proxy_from_factory_(false),
proxy_(new MockModemProxy()),
simple_proxy_(new MockModemSimpleProxy()),
diff --git a/cellular_capability_gsm_unittest.cc b/cellular_capability_gsm_unittest.cc
index df3b06a..18593bb 100644
--- a/cellular_capability_gsm_unittest.cc
+++ b/cellular_capability_gsm_unittest.cc
@@ -55,7 +55,8 @@
class CellularCapabilityGSMTest : public testing::Test {
public:
CellularCapabilityGSMTest()
- : create_card_proxy_from_factory_(false),
+ : metrics_(&dispatcher_),
+ create_card_proxy_from_factory_(false),
proxy_(new MockModemProxy()),
simple_proxy_(new MockModemSimpleProxy()),
card_proxy_(new MockModemGSMCardProxy()),
diff --git a/cellular_capability_universal_unittest.cc b/cellular_capability_universal_unittest.cc
index e7c2b31..8d095e4 100644
--- a/cellular_capability_universal_unittest.cc
+++ b/cellular_capability_universal_unittest.cc
@@ -70,7 +70,8 @@
class CellularCapabilityUniversalTest : public testing::Test {
public:
CellularCapabilityUniversalTest()
- : manager_(&control_, &dispatcher_, &metrics_, &glib_),
+ : metrics_(&dispatcher_),
+ manager_(&control_, &dispatcher_, &metrics_, &glib_),
bearer_proxy_(new mm1::MockBearerProxy()),
modem_3gpp_proxy_(new mm1::MockModemModem3gppProxy()),
modem_cdma_proxy_(new mm1::MockModemModemCdmaProxy()),
diff --git a/cellular_service_unittest.cc b/cellular_service_unittest.cc
index cbec9ad..8563d86 100644
--- a/cellular_service_unittest.cc
+++ b/cellular_service_unittest.cc
@@ -29,7 +29,8 @@
class CellularServiceTest : public testing::Test {
public:
CellularServiceTest()
- : manager_(&control_, &dispatcher_, &metrics_, NULL),
+ : metrics_(&dispatcher_),
+ manager_(&control_, &dispatcher_, &metrics_, NULL),
device_(new Cellular(&control_,
NULL,
&metrics_,
diff --git a/cellular_unittest.cc b/cellular_unittest.cc
index 5062388..5af0c2b 100644
--- a/cellular_unittest.cc
+++ b/cellular_unittest.cc
@@ -122,7 +122,8 @@
class CellularTest : public testing::Test {
public:
CellularTest()
- : manager_(&control_interface_, &dispatcher_, &metrics_, &glib_),
+ : metrics_(&dispatcher_),
+ manager_(&control_interface_, &dispatcher_, &metrics_, &glib_),
device_info_(&control_interface_, &dispatcher_, &metrics_, &manager_),
dhcp_config_(new MockDHCPConfig(&control_interface_,
kTestDeviceName)),
diff --git a/device_info_unittest.cc b/device_info_unittest.cc
index 985b65d..7739811 100644
--- a/device_info_unittest.cc
+++ b/device_info_unittest.cc
@@ -71,7 +71,8 @@
class DeviceInfoTest : public Test {
public:
DeviceInfoTest()
- : manager_(&control_interface_, &dispatcher_, &metrics_, &glib_),
+ : metrics_(&dispatcher_),
+ manager_(&control_interface_, &dispatcher_, &metrics_, &glib_),
device_info_(&control_interface_, &dispatcher_, &metrics_, &manager_) {
}
virtual ~DeviceInfoTest() {}
diff --git a/device_unittest.cc b/device_unittest.cc
index 42be15f..f9ae59b 100644
--- a/device_unittest.cc
+++ b/device_unittest.cc
@@ -92,7 +92,8 @@
kDeviceAddress,
kDeviceInterfaceIndex,
Technology::kUnknown)),
- device_info_(control_interface(), NULL, NULL, NULL) {
+ device_info_(control_interface(), NULL, NULL, NULL),
+ metrics_(dispatcher()) {
DHCPProvider::GetInstance()->glib_ = glib();
DHCPProvider::GetInstance()->control_interface_ = control_interface();
DHCPProvider::GetInstance()->dispatcher_ = dispatcher();
diff --git a/l2tp_ipsec_driver_unittest.cc b/l2tp_ipsec_driver_unittest.cc
index b27b6c2..9f2b9b6 100644
--- a/l2tp_ipsec_driver_unittest.cc
+++ b/l2tp_ipsec_driver_unittest.cc
@@ -41,6 +41,7 @@
public:
L2TPIPSecDriverTest()
: device_info_(&control_, &dispatcher_, &metrics_, &manager_),
+ metrics_(&dispatcher_),
manager_(&control_, &dispatcher_, &metrics_, &glib_),
driver_(new L2TPIPSecDriver(&control_, &dispatcher_, &metrics_,
&manager_, &device_info_, &glib_)),
diff --git a/link_monitor_unittest.cc b/link_monitor_unittest.cc
index f360fa0..8c5f249 100644
--- a/link_monitor_unittest.cc
+++ b/link_monitor_unittest.cc
@@ -78,7 +78,8 @@
class LinkMonitorTest : public Test {
public:
LinkMonitorTest()
- : device_info_(
+ : metrics_(&dispatcher_),
+ device_info_(
&control_,
reinterpret_cast<EventDispatcher*>(NULL),
reinterpret_cast<Metrics*>(NULL),
diff --git a/manager.h b/manager.h
index d4989bd..5d9a32f 100644
--- a/manager.h
+++ b/manager.h
@@ -83,7 +83,7 @@
void AddDeviceToBlackList(const std::string &device_name);
virtual void Start();
- void Stop();
+ virtual void Stop();
bool running() const { return running_; }
const ProfileRefPtr &ActiveProfile() const;
diff --git a/manager_unittest.cc b/manager_unittest.cc
index 74c0983..0c8b981 100644
--- a/manager_unittest.cc
+++ b/manager_unittest.cc
@@ -1904,7 +1904,7 @@
}
TEST_F(ManagerTest, SortServicesWithConnection) {
- MockMetrics mock_metrics;
+ MockMetrics mock_metrics(dispatcher());
SetMetrics(&mock_metrics);
scoped_refptr<MockService> mock_service0(
@@ -1983,7 +1983,7 @@
EXPECT_EQ(0, manager()->default_service_callback_tag_);
EXPECT_TRUE(manager()->default_service_callbacks_.empty());
- MockMetrics mock_metrics;
+ MockMetrics mock_metrics(dispatcher());
SetMetrics(&mock_metrics);
scoped_refptr<MockService> mock_service(
@@ -2627,7 +2627,7 @@
EXPECT_FALSE(manager()->IsOnline());
EXPECT_EQ("offline", manager()->CalculateState(NULL));
- MockMetrics mock_metrics;
+ MockMetrics mock_metrics(dispatcher());
SetMetrics(&mock_metrics);
EXPECT_CALL(mock_metrics, NotifyDefaultServiceChanged(_))
.Times(AnyNumber());
@@ -2659,7 +2659,7 @@
}
TEST_F(ManagerTest, CalculateStateOnline) {
- MockMetrics mock_metrics;
+ MockMetrics mock_metrics(dispatcher());
SetMetrics(&mock_metrics);
EXPECT_CALL(mock_metrics, NotifyDefaultServiceChanged(_))
.Times(AnyNumber());
diff --git a/metrics.cc b/metrics.cc
index 9987e21..2c10a83 100644
--- a/metrics.cc
+++ b/metrics.cc
@@ -4,6 +4,7 @@
#include "shill/metrics.h"
+#include <base/bind.h>
#include <base/string_util.h>
#include <base/stringprintf.h>
#include <chromeos/dbus/service_constants.h>
@@ -14,6 +15,8 @@
#include "shill/logging.h"
#include "shill/wifi_service.h"
+using base::Bind;
+using base::Unretained;
using std::string;
using std::tr1::shared_ptr;
@@ -193,6 +196,11 @@
// static
const char Metrics::kMetricCellularDrop[] =
"Network.Shill.Cellular.Drop";
+const char Metrics::kMetricCellularDropsPerHour[] =
+ "Network.Shill.Cellular.DropsPerHour";
+const int Metrics::kMetricCellularDropsPerHourMax = 60;
+const int Metrics::kMetricCellularDropsPerHourMin = 1;
+const int Metrics::kMetricCellularDropsPerHourNumBuckets = 10;
// The format of FailureReason is different to other metrics because this
// name is prepended to the error message before the entire string is sent
// via SendUserActionToUMA.
@@ -215,9 +223,13 @@
const int Metrics::kMetricCellularAutoConnectTotalTimeMin = 0;
const int Metrics::kMetricCellularAutoConnectTotalTimeNumBuckets = 60;
+// static
+const int Metrics::kHourlyTimeoutMilliseconds = 3600 * 1000; // One hour
-Metrics::Metrics()
- : library_(&metrics_library_),
+
+Metrics::Metrics(EventDispatcher *dispatcher)
+ : dispatcher_(dispatcher),
+ library_(&metrics_library_),
last_default_technology_(Technology::kUnknown),
was_online_(false),
time_online_timer_(new chromeos_metrics::Timer),
@@ -363,6 +375,19 @@
return retval;
}
+void Metrics::Start() {
+ SLOG(Metrics, 2) << __func__;
+ hourly_timeout_handler_.Reset(
+ Bind(&Metrics::HourlyTimeoutHandler, Unretained(this)));
+ dispatcher_->PostDelayedTask(hourly_timeout_handler_.callback(),
+ kHourlyTimeoutMilliseconds);
+}
+
+void Metrics::Stop() {
+ SLOG(Metrics, 2) << __func__;
+ hourly_timeout_handler_.Cancel();
+}
+
void Metrics::RegisterService(const Service *service) {
shared_ptr<ServiceMetrics> service_metrics(new ServiceMetrics);
services_metrics_[service] = service_metrics;
@@ -665,7 +690,6 @@
kMetricTimeToConnectMillisecondsMin,
kMetricTimeToConnectMillisecondsMax,
kMetricTimeToConnectMillisecondsNumBuckets));
- device_metrics->auto_connect_tries = 0;
device_metrics->auto_connect_timer.reset(
new chromeos_metrics::TimerReporter(
kMetricCellularAutoConnectTotalTime,
@@ -794,7 +818,8 @@
}
}
-void Metrics::NotifyCellularDeviceDrop(const string &network_technology,
+void Metrics::NotifyCellularDeviceDrop(int interface_index,
+ const string &network_technology,
uint16 signal_strength) {
SLOG(Metrics, 2) << __func__ << ": " << network_technology
<< ", " << signal_strength;
@@ -826,6 +851,11 @@
kMetricCellularSignalStrengthBeforeDropMin,
kMetricCellularSignalStrengthBeforeDropMax,
kMetricCellularSignalStrengthBeforeDropNumBuckets);
+
+ DeviceMetrics *device_metrics = GetDeviceMetrics(interface_index);
+ if (device_metrics == NULL)
+ return;
+ device_metrics->num_drops++;
}
void Metrics::NotifyCellularDeviceFailure(const Error &error) {
@@ -902,6 +932,24 @@
device_metrics->auto_connect_timer->Reset();
}
+void Metrics::HourlyTimeoutHandler() {
+ SLOG(Metrics, 2) << __func__;
+ DeviceMetricsLookupMap::iterator it;
+ for (it = devices_metrics_.begin(); it != devices_metrics_.end(); ++it) {
+ if (it->second->technology != Technology::kCellular ||
+ it->second->num_drops == 0)
+ continue;
+ SendToUMA(kMetricCellularDropsPerHour,
+ it->second->num_drops,
+ kMetricCellularDropsPerHourMin,
+ kMetricCellularDropsPerHourMax,
+ kMetricCellularDropsPerHourNumBuckets);
+ it->second->num_drops = 0;
+ }
+ dispatcher_->PostDelayedTask(hourly_timeout_handler_.callback(),
+ kHourlyTimeoutMilliseconds);
+}
+
void Metrics::set_library(MetricsLibraryInterface *library) {
chromeos_metrics::TimerReporter::set_metrics_lib(library);
library_ = library;
diff --git a/metrics.h b/metrics.h
index 7bec934..2fdec24 100644
--- a/metrics.h
+++ b/metrics.h
@@ -11,6 +11,7 @@
#include <metrics/metrics_library.h>
#include <metrics/timer.h>
+#include "shill/event_dispatcher.h"
#include "shill/ieee80211.h"
#include "shill/portal_detector.h"
#include "shill/power_manager.h"
@@ -278,6 +279,10 @@
// Cellular specific statistics.
static const char kMetricCellularDrop[];
+ static const char kMetricCellularDropsPerHour[];
+ static const int kMetricCellularDropsPerHourMax;
+ static const int kMetricCellularDropsPerHourMin;
+ static const int kMetricCellularDropsPerHourNumBuckets;
static const char kMetricCellularFailureReason[];
static const char kMetricCellularSignalStrengthBeforeDrop[];
static const int kMetricCellularSignalStrengthBeforeDropMax;
@@ -292,7 +297,7 @@
static const int kMetricCellularAutoConnectTotalTimeMin;
static const int kMetricCellularAutoConnectTotalTimeNumBuckets;
- Metrics();
+ explicit Metrics(EventDispatcher *dispatcher);
virtual ~Metrics();
// Converts the WiFi frequency into the associated UMA channel enumerator.
@@ -305,6 +310,12 @@
static PortalResult PortalDetectionResultToEnum(
const PortalDetector::Result &result);
+ // Starts this object. Call this during initialization.
+ virtual void Start();
+
+ // Stops this object. Call this during cleanup.
+ virtual void Stop();
+
// Registers a service with this object so it can use the timers to track
// state transition metrics.
void RegisterService(const Service *service);
@@ -407,7 +418,8 @@
// Notifies this object that a cellular device has been dropped by the
// network.
- void NotifyCellularDeviceDrop(const std::string &network_technology,
+ void NotifyCellularDeviceDrop(int interface_index,
+ const std::string &network_technology,
uint16 signal_strength);
// Notifies this object about a cellular device failure code.
@@ -422,6 +434,7 @@
private:
friend class MetricsTest;
+ FRIEND_TEST(MetricsTest, CellularDropsPerHour);
FRIEND_TEST(MetricsTest, FrequencyToChannel);
FRIEND_TEST(MetricsTest, ServiceFailure);
FRIEND_TEST(MetricsTest, TimeOnlineTimeToDrop);
@@ -453,15 +466,16 @@
ServiceMetricsLookupMap;
struct DeviceMetrics {
- DeviceMetrics() {}
+ DeviceMetrics() : auto_connect_tries(0), num_drops(0) {}
Technology::Identifier technology;
scoped_ptr<chromeos_metrics::TimerReporter> initialization_timer;
scoped_ptr<chromeos_metrics::TimerReporter> enable_timer;
scoped_ptr<chromeos_metrics::TimerReporter> disable_timer;
scoped_ptr<chromeos_metrics::TimerReporter> scan_timer;
scoped_ptr<chromeos_metrics::TimerReporter> connect_timer;
- int auto_connect_tries;
scoped_ptr<chromeos_metrics::TimerReporter> auto_connect_timer;
+ int auto_connect_tries;
+ int num_drops;
};
typedef std::map<const int, std::tr1::shared_ptr<DeviceMetrics> >
DeviceMetricsLookupMap;
@@ -481,6 +495,8 @@
static const uint16 kWiFiFrequency5745;
static const uint16 kWiFiFrequency5825;
+ static const int kHourlyTimeoutMilliseconds;
+
void InitializeCommonServiceMetrics(const Service *service);
void UpdateServiceStateTransitionMetrics(ServiceMetrics *service_metrics,
Service::ConnectState new_state);
@@ -489,6 +505,8 @@
DeviceMetrics *GetDeviceMetrics (int interface_index) const;
void AutoConnectMetricsReset(DeviceMetrics *device_metrics);
+ void HourlyTimeoutHandler();
+
// For unit test purposes.
void set_library(MetricsLibraryInterface *library);
void set_time_online_timer(chromeos_metrics::Timer *timer) {
@@ -513,6 +531,7 @@
// |library_| points to |metrics_library_| when shill runs normally.
// However, in order to allow for unit testing, we point |library_| to a
// MetricsLibraryMock object instead.
+ EventDispatcher *dispatcher_;
MetricsLibrary metrics_library_;
MetricsLibraryInterface *library_;
ServiceMetricsLookupMap services_metrics_;
@@ -524,6 +543,7 @@
scoped_ptr<chromeos_metrics::Timer> time_termination_actions_timer;
bool collect_bootstats_;
DeviceMetricsLookupMap devices_metrics_;
+ base::CancelableClosure hourly_timeout_handler_;
DISALLOW_COPY_AND_ASSIGN(Metrics);
};
diff --git a/metrics_unittest.cc b/metrics_unittest.cc
index da9236e..9f44e82 100644
--- a/metrics_unittest.cc
+++ b/metrics_unittest.cc
@@ -10,9 +10,12 @@
#include <metrics/metrics_library_mock.h>
#include <metrics/timer_mock.h>
+#include "shill/mock_control.h"
+#include "shill/mock_event_dispatcher.h"
+#include "shill/mock_glib.h"
+#include "shill/mock_manager.h"
#include "shill/mock_service.h"
#include "shill/mock_wifi_service.h"
-#include "shill/property_store_unittest.h"
using std::string;
@@ -26,24 +29,29 @@
namespace shill {
-class MetricsTest : public PropertyStoreTest {
+class MetricsTest : public Test {
public:
MetricsTest()
- : service_(new MockService(control_interface(),
- dispatcher(),
+ : manager_(&control_interface_,
+ &dispatcher_,
+ &metrics_,
+ &glib_),
+ metrics_(&dispatcher_),
+ service_(new MockService(&control_interface_,
+ &dispatcher_,
&metrics_,
- manager())),
- wifi_(new WiFi(control_interface(),
- dispatcher(),
+ &manager_)),
+ wifi_(new WiFi(&control_interface_,
+ &dispatcher_,
&metrics_,
- manager(),
+ &manager_,
"wlan0",
"000102030405",
0)),
- wifi_service_(new MockWiFiService(control_interface(),
- dispatcher(),
+ wifi_service_(new MockWiFiService(&control_interface_,
+ &dispatcher_,
&metrics_,
- manager(),
+ &manager_,
wifi_,
ssid_,
flimflam::kModeManaged,
@@ -80,6 +88,10 @@
Metrics::kMetricNetworkSignalStrengthNumBuckets));
}
+ MockControl control_interface_;
+ MockEventDispatcher dispatcher_;
+ MockGLib glib_;
+ MockManager manager_;
Metrics metrics_; // This must be destroyed after service_ and wifi_service_
MetricsLibraryMock library_;
scoped_refptr<MockService> service_;
@@ -457,6 +469,8 @@
"Unknown" };
const uint16 signal_strength = 100;
+ const int kInterfaceIndex = 1;
+ metrics_.RegisterDevice(kInterfaceIndex, Technology::kCellular);
for (size_t index = 0; index < arraysize(kUMATechnologyStrings); ++index) {
EXPECT_CALL(library_,
SendEnumToUMA(Metrics::kMetricCellularDrop,
@@ -468,12 +482,49 @@
Metrics::kMetricCellularSignalStrengthBeforeDropMin,
Metrics::kMetricCellularSignalStrengthBeforeDropMax,
Metrics::kMetricCellularSignalStrengthBeforeDropNumBuckets));
- metrics_.NotifyCellularDeviceDrop(kUMATechnologyStrings[index],
+ metrics_.NotifyCellularDeviceDrop(kInterfaceIndex,
+ kUMATechnologyStrings[index],
signal_strength);
Mock::VerifyAndClearExpectations(&library_);
}
}
+TEST_F(MetricsTest, CellularDropsPerHour) {
+ const int kInterfaceIndex = 1;
+ const int kSignalStrength = 33;
+ const int kNumDrops = 3;
+ metrics_.RegisterDevice(kInterfaceIndex, Technology::kCellular);
+ EXPECT_CALL(library_,
+ SendEnumToUMA(Metrics::kMetricCellularDrop,
+ Metrics::kCellularDropTechnologyLte,
+ Metrics::kCellularDropTechnologyMax))
+ .Times(kNumDrops);
+ EXPECT_CALL(library_,
+ SendToUMA(Metrics::kMetricCellularSignalStrengthBeforeDrop,
+ kSignalStrength,
+ Metrics::kMetricCellularSignalStrengthBeforeDropMin,
+ Metrics::kMetricCellularSignalStrengthBeforeDropMax,
+ Metrics::kMetricCellularSignalStrengthBeforeDropNumBuckets))
+ .Times(kNumDrops);
+ EXPECT_CALL(library_,
+ SendToUMA(Metrics::kMetricCellularDropsPerHour,
+ kNumDrops,
+ Metrics::kMetricCellularDropsPerHourMin,
+ Metrics::kMetricCellularDropsPerHourMax,
+ Metrics::kMetricCellularDropsPerHourNumBuckets));
+ for (int count = 0; count < kNumDrops; ++count)
+ metrics_.NotifyCellularDeviceDrop(kInterfaceIndex,
+ flimflam::kNetworkTechnologyLte,
+ kSignalStrength);
+ metrics_.HourlyTimeoutHandler();
+
+ // Make sure the number of drops gets resetted after each hour.
+ EXPECT_CALL(library_,
+ SendToUMA(Metrics::kMetricCellularDropsPerHour, _, _, _, _))
+ .Times(0);
+ metrics_.HourlyTimeoutHandler();
+}
+
TEST_F(MetricsTest, CellularDeviceFailure) {
const char kErrorMessage[] =
"org.chromium.flimflam.Error.Failure:"
diff --git a/mock_manager.h b/mock_manager.h
index 8b1ef7e..1101e22 100644
--- a/mock_manager.h
+++ b/mock_manager.h
@@ -28,6 +28,7 @@
MOCK_CONST_METHOD0(store, const PropertyStore &());
MOCK_CONST_METHOD0(run_path, const FilePath &());
MOCK_METHOD0(Start, void());
+ MOCK_METHOD0(Stop, void());
MOCK_METHOD1(RegisterDevice, void(const DeviceRefPtr &to_manage));
MOCK_METHOD1(DeregisterDevice, void(const DeviceRefPtr &to_forget));
MOCK_METHOD1(HasService, bool(const ServiceRefPtr &to_manage));
diff --git a/mock_metrics.cc b/mock_metrics.cc
index 330fc91..1caa74a 100644
--- a/mock_metrics.cc
+++ b/mock_metrics.cc
@@ -6,7 +6,8 @@
namespace shill {
-MockMetrics::MockMetrics() {}
+MockMetrics::MockMetrics(EventDispatcher *dispatcher)
+ : Metrics(dispatcher) {}
MockMetrics::~MockMetrics() {}
diff --git a/mock_metrics.h b/mock_metrics.h
index 129fda3..1ca3bf5 100644
--- a/mock_metrics.h
+++ b/mock_metrics.h
@@ -13,9 +13,11 @@
class MockMetrics : public Metrics {
public:
- MockMetrics();
+ MockMetrics(EventDispatcher *dispatcher);
virtual ~MockMetrics();
+ MOCK_METHOD0(Start, void());
+ MOCK_METHOD0(Stop, void());
MOCK_METHOD1(NotifyDefaultServiceChanged, void(const Service *service));
MOCK_METHOD2(NotifyServiceStateChanged,
void(const Service *service, Service::ConnectState new_state));
diff --git a/modem_1_unittest.cc b/modem_1_unittest.cc
index c5b95ee..d99b346 100644
--- a/modem_1_unittest.cc
+++ b/modem_1_unittest.cc
@@ -48,7 +48,8 @@
class Modem1Test : public Test {
public:
Modem1Test()
- : manager_(&control_interface_, &dispatcher_, &metrics_, &glib_),
+ : metrics_(&dispatcher_),
+ manager_(&control_interface_, &dispatcher_, &metrics_, &glib_),
info_(&control_interface_, &dispatcher_, &metrics_, &manager_),
proxy_(new MockDBusPropertiesProxy()),
proxy_factory_(this),
diff --git a/modem_info_unittest.cc b/modem_info_unittest.cc
index 885d243..d4ccbb7 100644
--- a/modem_info_unittest.cc
+++ b/modem_info_unittest.cc
@@ -23,7 +23,8 @@
class ModemInfoTest : public Test {
public:
ModemInfoTest()
- : manager_(&control_interface_, &dispatcher_, &metrics_, &glib_),
+ : metrics_(&dispatcher_),
+ manager_(&control_interface_, &dispatcher_, &metrics_, &glib_),
modem_info_(&control_interface_, &dispatcher_, &metrics_, &manager_,
&glib_) {}
diff --git a/modem_manager_unittest.cc b/modem_manager_unittest.cc
index 4266737..2014b13 100644
--- a/modem_manager_unittest.cc
+++ b/modem_manager_unittest.cc
@@ -61,7 +61,8 @@
class ModemManagerTest : public Test {
public:
ModemManagerTest()
- : manager_(&control_interface_, &dispatcher_, &metrics_, &glib_) {
+ : metrics_(&dispatcher_),
+ manager_(&control_interface_, &dispatcher_, &metrics_, &glib_) {
}
virtual void SetUp() {
diff --git a/modem_unittest.cc b/modem_unittest.cc
index 39f08f6..79393ff 100644
--- a/modem_unittest.cc
+++ b/modem_unittest.cc
@@ -56,7 +56,8 @@
class ModemTest : public Test {
public:
ModemTest()
- : manager_(&control_interface_, &dispatcher_, &metrics_, &glib_),
+ : metrics_(&dispatcher_),
+ manager_(&control_interface_, &dispatcher_, &metrics_, &glib_),
info_(&control_interface_, &dispatcher_, &metrics_, &manager_),
proxy_(new MockDBusPropertiesProxy()),
proxy_factory_(this),
diff --git a/openvpn_driver_unittest.cc b/openvpn_driver_unittest.cc
index 7a049d1..900dfa6 100644
--- a/openvpn_driver_unittest.cc
+++ b/openvpn_driver_unittest.cc
@@ -55,6 +55,7 @@
public:
OpenVPNDriverTest()
: device_info_(&control_, &dispatcher_, &metrics_, &manager_),
+ metrics_(&dispatcher_),
manager_(&control_, &dispatcher_, &metrics_, &glib_),
driver_(new OpenVPNDriver(&control_, &dispatcher_, &metrics_, &manager_,
&device_info_, &glib_)),
diff --git a/property_store_unittest.cc b/property_store_unittest.cc
index 8fa059a..b4b29f8 100644
--- a/property_store_unittest.cc
+++ b/property_store_unittest.cc
@@ -69,6 +69,7 @@
invalid_args_(Error::GetName(Error::kInvalidArguments)),
invalid_prop_(Error::GetName(Error::kInvalidProperty)),
path_(dir_.CreateUniqueTempDir() ? dir_.path().value() : ""),
+ metrics_(dispatcher()),
manager_(control_interface(),
dispatcher(),
metrics(),
diff --git a/rtnl_handler_unittest.cc b/rtnl_handler_unittest.cc
index e325fb3..5a23456 100644
--- a/rtnl_handler_unittest.cc
+++ b/rtnl_handler_unittest.cc
@@ -61,7 +61,8 @@
class RTNLHandlerTest : public Test {
public:
RTNLHandlerTest()
- : manager_(&control_interface_, &dispatcher_, &metrics_, &glib_),
+ : metrics_(&dispatcher_),
+ manager_(&control_interface_, &dispatcher_, &metrics_, &glib_),
callback_(Bind(&RTNLHandlerTest::HandlerCallback, Unretained(this))) {
}
diff --git a/service_unittest.cc b/service_unittest.cc
index b43e5c4..3c12033 100644
--- a/service_unittest.cc
+++ b/service_unittest.cc
@@ -170,7 +170,8 @@
class AllMockServiceTest : public testing::Test {
public:
AllMockServiceTest()
- : manager_(&control_interface_, &dispatcher_, &metrics_, &glib_),
+ : metrics_(&dispatcher_),
+ manager_(&control_interface_, &dispatcher_, &metrics_, &glib_),
service_(new ServiceUnderTest(&control_interface_,
&dispatcher_,
&metrics_,
diff --git a/shill_daemon.cc b/shill_daemon.cc
index 6df6f10..33efa52 100644
--- a/shill_daemon.cc
+++ b/shill_daemon.cc
@@ -34,6 +34,7 @@
Daemon::Daemon(Config *config, ControlInterface *control)
: config_(config),
control_(control),
+ metrics_(new Metrics(&dispatcher_)),
nss_(NSS::GetInstance()),
proxy_factory_(ProxyFactory::GetInstance()),
rtnl_handler_(RTNLHandler::GetInstance()),
@@ -42,13 +43,13 @@
config80211_(Config80211::GetInstance()),
manager_(new Manager(control_,
&dispatcher_,
- &metrics_,
+ metrics_.get(),
&glib_,
config->GetRunDirectory(),
config->GetStorageDirectory(),
config->GetUserStorageDirectoryFormat())),
callback80211_output_(),
- callback80211_metrics_(&metrics_) {
+ callback80211_metrics_(metrics_.get()) {
}
Daemon::~Daemon() { }
@@ -88,7 +89,7 @@
void Daemon::TerminationActionsCompleted(const Error &error) {
SLOG(Daemon, 1) << "Finished termination actions. Result: " << error;
- metrics_.NotifyTerminationActionsCompleted(
+ metrics_->NotifyTerminationActionsCompleted(
Metrics::kTerminationActionReasonTerminate, error.IsSuccess());
dispatcher_.PostTask(MessageLoop::QuitClosure());
}
@@ -96,6 +97,7 @@
void Daemon::Start() {
glib_.TypeInit();
proxy_factory_->Init();
+ metrics_->Start();
rtnl_handler_->Start(&dispatcher_, &sockets_);
routing_table_->Start();
dhcp_provider_->Init(control_, &dispatcher_, &glib_);
@@ -123,6 +125,7 @@
void Daemon::Stop() {
manager_->Stop();
+ metrics_->Stop();
}
} // namespace shill
diff --git a/shill_daemon.h b/shill_daemon.h
index 8f2df30..ab9d4c5 100644
--- a/shill_daemon.h
+++ b/shill_daemon.h
@@ -12,7 +12,6 @@
#include "shill/event_dispatcher.h"
#include "shill/glib.h"
#include "shill/manager.h"
-#include "shill/metrics.h"
#include "shill/sockets.h"
#include "shill/callback80211_object.h"
#include "shill/callback80211_metrics.h"
@@ -25,6 +24,7 @@
class DHCPProvider;
class Error;
class GLib;
+class Metrics;
class NSS;
class ProxyFactory;
class RoutingTable;
@@ -56,7 +56,7 @@
Config *config_;
ControlInterface *control_;
- Metrics metrics_;
+ scoped_ptr<Metrics> metrics_;
NSS *nss_;
ProxyFactory *proxy_factory_;
RTNLHandler *rtnl_handler_;
diff --git a/shill_unittest.cc b/shill_unittest.cc
index d6d029b..10756ab 100644
--- a/shill_unittest.cc
+++ b/shill_unittest.cc
@@ -17,6 +17,7 @@
#include "shill/mock_control.h"
#include "shill/mock_dhcp_provider.h"
#include "shill/mock_manager.h"
+#include "shill/mock_metrics.h"
#include "shill/mock_proxy_factory.h"
#include "shill/mock_rtnl_handler.h"
#include "shill/mock_routing_table.h"
@@ -31,6 +32,7 @@
using ::testing::Expectation;
using ::testing::Gt;
+using ::testing::Mock;
using ::testing::NotNull;
using ::testing::Return;
using ::testing::StrictMock;
@@ -179,11 +181,12 @@
public:
ShillDaemonTest()
: daemon_(&config_, new MockControl()),
+ metrics_(new MockMetrics(&daemon_.dispatcher_)),
manager_(new MockManager(daemon_.control_,
&daemon_.dispatcher_,
- &daemon_.metrics_,
+ metrics_,
&daemon_.glib_)),
- device_info_(daemon_.control_, dispatcher_, &daemon_.metrics_,
+ device_info_(daemon_.control_, dispatcher_, metrics_,
daemon_.manager_.get()),
dispatcher_(&daemon_.dispatcher_),
dispatcher_test_(dispatcher_) {
@@ -197,6 +200,7 @@
daemon_.rtnl_handler_ = &rtnl_handler_;
daemon_.routing_table_ = &routing_table_;
daemon_.dhcp_provider_ = &dhcp_provider_;
+ daemon_.metrics_.reset(metrics_); // Passes ownership
daemon_.manager_.reset(manager_); // Passes ownership
dispatcher_test_.ScheduleFailSafe();
}
@@ -204,6 +208,10 @@
daemon_.Start();
}
+ void StopDaemon() {
+ daemon_.Stop();
+ }
+
void ResetConfig80211() {
daemon_.config80211_->Reset(true);
}
@@ -217,6 +225,7 @@
MockRTNLHandler rtnl_handler_;
MockRoutingTable routing_table_;
MockDHCPProvider dhcp_provider_;
+ MockMetrics *metrics_;
MockManager *manager_;
DeviceInfo device_info_;
EventDispatcher *dispatcher_;
@@ -224,7 +233,7 @@
};
-TEST_F(ShillDaemonTest, Start) {
+TEST_F(ShillDaemonTest, StartStop) {
// To ensure we do not have any stale routes, we flush a device's routes
// when it is started. This requires that the routing table is fully
// populated before we create and start devices. So test to make sure that
@@ -234,11 +243,18 @@
// completes, we request the dump of the links. For each link found, we
// create and start the device.
EXPECT_CALL(proxy_factory_, Init());
+ EXPECT_CALL(*metrics_, Start());
EXPECT_CALL(rtnl_handler_, Start(_, _));
Expectation routing_table_started = EXPECT_CALL(routing_table_, Start());
EXPECT_CALL(dhcp_provider_, Init(_, _, _));
EXPECT_CALL(*manager_, Start()).After(routing_table_started);
StartDaemon();
+ Mock::VerifyAndClearExpectations(metrics_);
+ Mock::VerifyAndClearExpectations(manager_);
+
+ EXPECT_CALL(*manager_, Stop());
+ EXPECT_CALL(*metrics_, Stop());
+ StopDaemon();
}
TEST_F(ShillDaemonTest, EventDispatcherTimer) {
diff --git a/vpn_driver_unittest.cc b/vpn_driver_unittest.cc
index af52ebb..951e17b 100644
--- a/vpn_driver_unittest.cc
+++ b/vpn_driver_unittest.cc
@@ -87,6 +87,7 @@
public:
VPNDriverTest()
: device_info_(&control_, &dispatcher_, &metrics_, &manager_),
+ metrics_(&dispatcher_),
manager_(&control_, &dispatcher_, &metrics_, &glib_),
driver_(&dispatcher_, &manager_) {}
diff --git a/vpn_provider_unittest.cc b/vpn_provider_unittest.cc
index 61777be..bce5926 100644
--- a/vpn_provider_unittest.cc
+++ b/vpn_provider_unittest.cc
@@ -30,7 +30,8 @@
class VPNProviderTest : public testing::Test {
public:
VPNProviderTest()
- : manager_(&control_, NULL, &metrics_, NULL),
+ : metrics_(NULL),
+ manager_(&control_, NULL, &metrics_, NULL),
device_info_(&control_, NULL, &metrics_, &manager_),
provider_(&control_, NULL, &metrics_, &manager_) {}
diff --git a/vpn_service_unittest.cc b/vpn_service_unittest.cc
index f464c6c..dab06fe 100644
--- a/vpn_service_unittest.cc
+++ b/vpn_service_unittest.cc
@@ -33,6 +33,7 @@
: interface_name_("test-interface"),
driver_(new MockVPNDriver()),
manager_(&control_, NULL, NULL, NULL),
+ metrics_(NULL),
device_info_(&control_, NULL, NULL, NULL),
connection_(new NiceMock<MockConnection>(&device_info_)),
sockets_(new MockSockets()),
diff --git a/vpn_unittest.cc b/vpn_unittest.cc
index 97de62d..4d71e63 100644
--- a/vpn_unittest.cc
+++ b/vpn_unittest.cc
@@ -17,7 +17,8 @@
class VPNTest : public testing::Test {
public:
VPNTest()
- : manager_(&control_, &dispatcher_, &metrics_, &glib_),
+ : metrics_(&dispatcher_),
+ manager_(&control_, &dispatcher_, &metrics_, &glib_),
vpn_(new VPN(&control_,
&dispatcher_,
&metrics_,
diff --git a/wifi_provider_unittest.cc b/wifi_provider_unittest.cc
index ccef30e..21b9fd8 100644
--- a/wifi_provider_unittest.cc
+++ b/wifi_provider_unittest.cc
@@ -15,7 +15,8 @@
class WiFiProviderTest : public testing::Test {
public:
WiFiProviderTest()
- : manager_(&control_, NULL, &metrics_, NULL),
+ : metrics_(NULL),
+ manager_(&control_, NULL, &metrics_, NULL),
provider_(&control_, NULL, &metrics_, &manager_) {}
virtual ~WiFiProviderTest() {}
diff --git a/wifi_unittest.cc b/wifi_unittest.cc
index 2edcf76..62b362e 100644
--- a/wifi_unittest.cc
+++ b/wifi_unittest.cc
@@ -185,6 +185,7 @@
public:
WiFiObjectTest(EventDispatcher *dispatcher)
: event_dispatcher_(dispatcher),
+ metrics_(NULL),
manager_(&control_interface_, NULL, &metrics_, &glib_),
device_info_(&control_interface_, dispatcher, &metrics_, &manager_),
wifi_(new WiFi(&control_interface_,
diff --git a/wimax_provider_unittest.cc b/wimax_provider_unittest.cc
index b528ad8..abe06de 100644
--- a/wimax_provider_unittest.cc
+++ b/wimax_provider_unittest.cc
@@ -56,6 +56,7 @@
: wimax_manager_proxy_(new MockWiMaxManagerProxy()),
network_proxy_(new MockWiMaxNetworkProxy()),
proxy_factory_(this),
+ metrics_(NULL),
manager_(&control_, NULL, &metrics_, NULL),
device_info_(&control_, NULL, &metrics_, &manager_),
dbus_manager_(new MockDBusManager()),
diff --git a/wimax_service_unittest.cc b/wimax_service_unittest.cc
index a91eff4..43df3b2 100644
--- a/wimax_service_unittest.cc
+++ b/wimax_service_unittest.cc
@@ -44,6 +44,7 @@
WiMaxServiceTest()
: proxy_(new MockWiMaxNetworkProxy()),
manager_(&control_, NULL, NULL, NULL),
+ metrics_(static_cast<EventDispatcher *>(NULL)),
device_(new MockWiMax(&control_, NULL, &metrics_, &manager_,
kTestLinkName, kTestAddress, kTestInterfaceIndex,
kTestPath)),
diff --git a/wimax_unittest.cc b/wimax_unittest.cc
index 2719c18..9184a9d 100644
--- a/wimax_unittest.cc
+++ b/wimax_unittest.cc
@@ -44,6 +44,7 @@
WiMaxTest()
: proxy_(new MockWiMaxDeviceProxy()),
proxy_factory_(this),
+ metrics_(&dispatcher_),
manager_(&control_, &dispatcher_, &metrics_, NULL),
dhcp_config_(new MockDHCPConfig(&control_,
kTestLinkName)),