shill: Convert ProxyFactory singleton to LazyInstance.
This makes it consistent with the rest of shill's singletons (DHCPProvider,
Resolver, RoutingTable, RTNLHandler) and improves static object's memory
management.
BUG=chromium-os:19178
TEST=unit tests, tested in a VM
Change-Id: Ib403d31473360b46dd25d12508c799cbc73368fd
Reviewed-on: http://gerrit.chromium.org/gerrit/8861
Tested-by: Darin Petkov <petkov@chromium.org>
Reviewed-by: Paul Stewart <pstew@chromium.org>
Reviewed-by: mukesh agrawal <quiche@chromium.org>
Commit-Ready: Darin Petkov <petkov@chromium.org>
diff --git a/cellular.cc b/cellular.cc
index b3142d5..81a3212 100644
--- a/cellular.cc
+++ b/cellular.cc
@@ -111,6 +111,7 @@
link_name,
address,
interface_index),
+ proxy_factory_(ProxyFactory::GetInstance()),
type_(type),
state_(kStateDisabled),
modem_state_(kModemStateUnknown),
@@ -329,24 +330,21 @@
void Cellular::InitProxies() {
VLOG(2) << __func__;
- proxy_.reset(
- ProxyFactory::factory()->CreateModemProxy(this, dbus_path_, dbus_owner_));
+ proxy_.reset(proxy_factory_->CreateModemProxy(this, dbus_path_, dbus_owner_));
simple_proxy_.reset(
- ProxyFactory::factory()->CreateModemSimpleProxy(
- dbus_path_, dbus_owner_));
+ proxy_factory_->CreateModemSimpleProxy(dbus_path_, dbus_owner_));
switch (type_) {
case kTypeGSM:
gsm_card_proxy_.reset(
- ProxyFactory::factory()->CreateModemGSMCardProxy(
+ proxy_factory_->CreateModemGSMCardProxy(
this, dbus_path_, dbus_owner_));
gsm_network_proxy_.reset(
- ProxyFactory::factory()->CreateModemGSMNetworkProxy(
+ proxy_factory_->CreateModemGSMNetworkProxy(
this, dbus_path_, dbus_owner_));
break;
case kTypeCDMA:
cdma_proxy_.reset(
- ProxyFactory::factory()->CreateModemCDMAProxy(
- this, dbus_path_, dbus_owner_));
+ proxy_factory_->CreateModemCDMAProxy(this, dbus_path_, dbus_owner_));
break;
default: NOTREACHED();
}
diff --git a/cellular.h b/cellular.h
index c633bea..07bdfe9 100644
--- a/cellular.h
+++ b/cellular.h
@@ -24,6 +24,7 @@
class Error;
class ModemSimpleProxyInterface;
+class ProxyFactory;
class Cellular : public Device,
public ModemCDMAProxyListener,
@@ -151,6 +152,7 @@
Error *error);
private:
+ friend class CellularTest;
FRIEND_TEST(CellularTest, Activate);
FRIEND_TEST(CellularTest, ActivateError);
FRIEND_TEST(CellularTest, CreateService);
@@ -314,6 +316,9 @@
uint32 new_state,
uint32 reason);
+ // Store cached copies of singletons for speed/ease of testing.
+ ProxyFactory *proxy_factory_;
+
Type type_;
State state_;
ModemState modem_state_;
diff --git a/cellular_unittest.cc b/cellular_unittest.cc
index 3b8637f..19142dc 100644
--- a/cellular_unittest.cc
+++ b/cellular_unittest.cc
@@ -131,14 +131,14 @@
virtual ~CellularTest() {}
virtual void SetUp() {
- ProxyFactory::set_factory(&proxy_factory_);
+ device_->proxy_factory_ = &proxy_factory_;
static_cast<Device *>(device_)->rtnl_handler_ = &rtnl_handler_;
device_->set_dhcp_provider(&dhcp_provider_);
EXPECT_CALL(manager_, device_info()).WillRepeatedly(Return(&device_info_));
}
virtual void TearDown() {
- ProxyFactory::set_factory(NULL);
+ device_->proxy_factory_ = NULL;
device_->Stop();
device_->set_dhcp_provider(NULL);
}
diff --git a/dhcp_config.cc b/dhcp_config.cc
index 5561b1e..0ba8485 100644
--- a/dhcp_config.cc
+++ b/dhcp_config.cc
@@ -50,6 +50,7 @@
const string &device_name,
GLib *glib)
: IPConfig(control_interface, device_name, kType),
+ proxy_factory_(ProxyFactory::GetInstance()),
provider_(provider),
pid_(0),
child_watch_tag_(0),
@@ -117,7 +118,7 @@
void DHCPConfig::InitProxyTask(const string &service) {
if (!proxy_.get()) {
VLOG(2) << "Init DHCP Proxy: " << device_name() << " at " << service;
- proxy_.reset(ProxyFactory::factory()->CreateDHCPProxy(service));
+ proxy_.reset(proxy_factory_->CreateDHCPProxy(service));
}
}
diff --git a/dhcp_config.h b/dhcp_config.h
index 7b8e39d..8b81ddd 100644
--- a/dhcp_config.h
+++ b/dhcp_config.h
@@ -21,6 +21,7 @@
class DHCPProxyInterface;
class EventDispatcher;
class GLib;
+class ProxyFactory;
class DHCPConfig : public IPConfig {
public:
@@ -115,6 +116,9 @@
// its GPid, exit watch callback, and state files.
void CleanupClientState();
+ // Store cached copies of singletons for speed/ease of testing.
+ ProxyFactory *proxy_factory_;
+
DHCPProvider *provider_;
// The PID of the spawned DHCP client. May be 0 if no client has been spawned
diff --git a/dhcp_config_unittest.cc b/dhcp_config_unittest.cc
index d6d3929..a8b4512 100644
--- a/dhcp_config_unittest.cc
+++ b/dhcp_config_unittest.cc
@@ -44,11 +44,11 @@
glib())) {}
virtual void SetUp() {
- ProxyFactory::set_factory(&proxy_factory_);
+ config_->proxy_factory_ = &proxy_factory_;
}
virtual void TearDown() {
- ProxyFactory::set_factory(NULL);
+ config_->proxy_factory_ = NULL;
}
protected:
diff --git a/dhcp_provider.cc b/dhcp_provider.cc
index e632328..f8f4398 100644
--- a/dhcp_provider.cc
+++ b/dhcp_provider.cc
@@ -19,7 +19,8 @@
base::LINKER_INITIALIZED);
DHCPProvider::DHCPProvider()
- : control_interface_(NULL),
+ : proxy_factory_(ProxyFactory::GetInstance()),
+ control_interface_(NULL),
dispatcher_(NULL),
glib_(NULL) {
VLOG(2) << __func__;
@@ -37,8 +38,7 @@
EventDispatcher *dispatcher,
GLib *glib) {
VLOG(2) << __func__;
- listener_.reset(
- new DHCPCDListener(ProxyFactory::factory()->connection(), this));
+ listener_.reset(new DHCPCDListener(proxy_factory_->connection(), this));
glib_ = glib;
control_interface_ = control_interface;
dispatcher_ = dispatcher;
diff --git a/dhcp_provider.h b/dhcp_provider.h
index eb03946..dea1f77 100644
--- a/dhcp_provider.h
+++ b/dhcp_provider.h
@@ -20,6 +20,7 @@
class DHCPCDListener;
class EventDispatcher;
class GLib;
+class ProxyFactory;
// DHCPProvider is a singleton providing the main DHCP configuration
// entrypoint. Once the provider is initialized through its Init method, DHCP
@@ -72,6 +73,9 @@
typedef std::map<int, DHCPConfigRefPtr> PIDConfigMap;
+ // Store cached copies of singletons for speed/ease of testing.
+ ProxyFactory *proxy_factory_;
+
// A single listener is used to catch signals from all DHCP clients and
// dispatch them to the appropriate DHCP configuration instance.
scoped_ptr<DHCPCDListener> listener_;
diff --git a/modem.cc b/modem.cc
index 5c5a77c..f1fc98e 100644
--- a/modem.cc
+++ b/modem.cc
@@ -31,7 +31,8 @@
ControlInterface *control_interface,
EventDispatcher *dispatcher,
Manager *manager)
- : owner_(owner),
+ : proxy_factory_(ProxyFactory::GetInstance()),
+ owner_(owner),
path_(path),
task_factory_(this),
control_interface_(control_interface),
@@ -56,7 +57,7 @@
CHECK(!device_.get());
dbus_properties_proxy_.reset(
- ProxyFactory::factory()->CreateDBusPropertiesProxy(this, path_, owner_));
+ proxy_factory_->CreateDBusPropertiesProxy(this, path_, owner_));
// TODO(petkov): Switch to asynchronous calls (crosbug.com/17583).
DBusPropertiesMap properties =
diff --git a/modem.h b/modem.h
index ea5c20d..766e7d5 100644
--- a/modem.h
+++ b/modem.h
@@ -20,6 +20,7 @@
class ControlInterface;
class EventDispatcher;
class Manager;
+class ProxyFactory;
// Handles an instance of ModemManager.Modem and an instance of a Cellular
// device.
@@ -71,6 +72,9 @@
const std::string &interface,
const DBusPropertiesMap &properties);
+ // Store cached copies of singletons for speed/ease of testing.
+ ProxyFactory *proxy_factory_;
+
// A proxy to the org.freedesktop.DBus.Properties interface used to obtain
// ModemManager.Modem properties and watch for property changes.
scoped_ptr<DBusPropertiesProxyInterface> dbus_properties_proxy_;
diff --git a/modem_manager.cc b/modem_manager.cc
index 090a5b1..91cad69 100644
--- a/modem_manager.cc
+++ b/modem_manager.cc
@@ -23,7 +23,8 @@
EventDispatcher *dispatcher,
Manager *manager,
GLib *glib)
- : service_(service),
+ : proxy_factory_(ProxyFactory::GetInstance()),
+ service_(service),
path_(path),
watcher_id_(0),
control_interface_(control_interface),
@@ -59,8 +60,7 @@
void ModemManager::Connect(const string &owner) {
owner_ = owner;
- proxy_.reset(
- ProxyFactory::factory()->CreateModemManagerProxy(this, path_, owner_));
+ proxy_.reset(proxy_factory_->CreateModemManagerProxy(this, path_, owner_));
// TODO(petkov): Switch to asynchronous calls (crosbug.com/17583).
vector<DBus::Path> devices = proxy_->EnumerateDevices();
diff --git a/modem_manager.h b/modem_manager.h
index 037bba9..f706b2b 100644
--- a/modem_manager.h
+++ b/modem_manager.h
@@ -21,6 +21,7 @@
class Manager;
class Modem;
class ModemManagerProxyInterface;
+class ProxyFactory;
// Handles a modem manager service and creates and destroys modem instances.
class ModemManager {
@@ -74,6 +75,9 @@
const gchar *name,
gpointer user_data);
+ // Store cached copies of singletons for speed/ease of testing.
+ ProxyFactory *proxy_factory_;
+
const std::string service_;
const std::string path_;
guint watcher_id_;
diff --git a/modem_manager_unittest.cc b/modem_manager_unittest.cc
index 6b415a4..0eebd34 100644
--- a/modem_manager_unittest.cc
+++ b/modem_manager_unittest.cc
@@ -36,9 +36,9 @@
&glib_),
proxy_(new MockModemManagerProxy()),
proxy_factory_(this) {
- ProxyFactory::set_factory(&proxy_factory_);
}
+ virtual void SetUp();
virtual void TearDown();
protected:
@@ -76,9 +76,13 @@
const char ModemManagerTest::kOwner[] = ":1.17";
const char ModemManagerTest::kModemPath[] = "/org/chromium/ModemManager/Gobi/0";
+void ModemManagerTest::SetUp() {
+ modem_manager_.proxy_factory_ = &proxy_factory_;
+}
+
void ModemManagerTest::TearDown() {
modem_manager_.watcher_id_ = 0;
- ProxyFactory::set_factory(NULL);
+ modem_manager_.proxy_factory_ = NULL;
}
TEST_F(ModemManagerTest, Start) {
diff --git a/modem_unittest.cc b/modem_unittest.cc
index 2ad19c1..e51fb5b 100644
--- a/modem_unittest.cc
+++ b/modem_unittest.cc
@@ -96,12 +96,12 @@
void ModemTest::SetUp() {
EXPECT_EQ(kOwner, modem_.owner_);
EXPECT_EQ(kPath, modem_.path_);
- ProxyFactory::set_factory(&proxy_factory_);
+ modem_.proxy_factory_ = &proxy_factory_;
SetSockets(&sockets_);
}
void ModemTest::TearDown() {
- ProxyFactory::set_factory(NULL);
+ modem_.proxy_factory_ = NULL;
SetSockets(NULL);
}
diff --git a/proxy_factory.cc b/proxy_factory.cc
index 29c71d2..efa3eb9 100644
--- a/proxy_factory.cc
+++ b/proxy_factory.cc
@@ -21,12 +21,17 @@
namespace shill {
-ProxyFactory *ProxyFactory::factory_ = NULL;
+static base::LazyInstance<ProxyFactory> g_proxy_factory(
+ base::LINKER_INITIALIZED);
ProxyFactory::ProxyFactory() {}
ProxyFactory::~ProxyFactory() {}
+ProxyFactory *ProxyFactory::GetInstance() {
+ return g_proxy_factory.Pointer();
+}
+
void ProxyFactory::Init() {
CHECK(DBus::default_dispatcher); // Initialized in DBusControl::Init.
CHECK(!connection_.get());
diff --git a/proxy_factory.h b/proxy_factory.h
index 6205c77..a55c392 100644
--- a/proxy_factory.h
+++ b/proxy_factory.h
@@ -8,6 +8,7 @@
#include <string>
#include <base/basictypes.h>
+#include <base/lazy_instance.h>
#include <base/memory/scoped_ptr.h>
#include <dbus-c++/dbus.h>
@@ -35,9 +36,11 @@
// Global DBus proxy factory that can be mocked out in tests.
class ProxyFactory {
public:
- ProxyFactory();
virtual ~ProxyFactory();
+ // Since this is a singleton, use ProxyFactory::GetInstance()->Foo()
+ static ProxyFactory *GetInstance();
+
void Init();
virtual DBusPropertiesProxyInterface *CreateDBusPropertiesProxy(
@@ -84,13 +87,13 @@
virtual DHCPProxyInterface *CreateDHCPProxy(const std::string &service);
- static ProxyFactory *factory() { return factory_; }
- static void set_factory(ProxyFactory *factory) { factory_ = factory; }
+ DBus::Connection *connection() const { return connection_.get(); }
- DBus::Connection *connection() { return connection_.get(); }
+ protected:
+ ProxyFactory();
private:
- static ProxyFactory *factory_;
+ friend struct base::DefaultLazyInstanceTraits<ProxyFactory>;
scoped_ptr<DBus::Connection> connection_;
diff --git a/shill_daemon.cc b/shill_daemon.cc
index 323f285..0e6782a 100644
--- a/shill_daemon.cc
+++ b/shill_daemon.cc
@@ -12,6 +12,7 @@
#include <base/logging.h>
#include "shill/dhcp_provider.h"
+#include "shill/proxy_factory.h"
#include "shill/rtnl_handler.h"
#include "shill/shill_config.h"
@@ -40,8 +41,7 @@
void Daemon::Start() {
glib_.TypeInit();
- proxy_factory_.Init();
- ProxyFactory::set_factory(&proxy_factory_);
+ ProxyFactory::GetInstance()->Init();
RTNLHandler::GetInstance()->Start(&dispatcher_, &sockets_);
DHCPProvider::GetInstance()->Init(control_, &dispatcher_, &glib_);
manager_.Start();
diff --git a/shill_daemon.h b/shill_daemon.h
index 37906dd..bef04cf 100644
--- a/shill_daemon.h
+++ b/shill_daemon.h
@@ -9,7 +9,6 @@
#include "shill/glib.h"
#include "shill/manager.h"
-#include "shill/proxy_factory.h"
#include "shill/shill_event.h"
#include "shill/sockets.h"
@@ -38,7 +37,6 @@
EventDispatcher dispatcher_;
Sockets sockets_;
GLib glib_;
- ProxyFactory proxy_factory_;
};
} // namespace shill
diff --git a/wifi.cc b/wifi.cc
index 9d6ef2d..e22bb9f 100644
--- a/wifi.cc
+++ b/wifi.cc
@@ -70,6 +70,7 @@
link,
address,
interface_index),
+ proxy_factory_(ProxyFactory::GetInstance()),
task_factory_(this),
bgscan_short_interval_(0),
bgscan_signal_threshold_(0),
@@ -98,7 +99,7 @@
::DBus::Path interface_path;
supplicant_process_proxy_.reset(
- ProxyFactory::factory()->CreateSupplicantProcessProxy(
+ proxy_factory_->CreateSupplicantProcessProxy(
wpa_supplicant::kDBusPath, wpa_supplicant::kDBusAddr));
try {
std::map<string, DBus::Variant> create_interface_args;
@@ -121,7 +122,7 @@
}
supplicant_interface_proxy_.reset(
- ProxyFactory::factory()->CreateSupplicantInterfaceProxy(
+ proxy_factory_->CreateSupplicantInterfaceProxy(
this, interface_path, wpa_supplicant::kDBusAddr));
// TODO(quiche) set ApScan=1 and BSSExpireAge=190, like flimflam does?
diff --git a/wifi.h b/wifi.h
index 4fd8eca..313ed6f 100644
--- a/wifi.h
+++ b/wifi.h
@@ -19,6 +19,7 @@
class Error;
class KeyValueStore;
+class ProxyFactory;
class SupplicantInterfaceProxyInterface;
class SupplicantProcessProxyInterface;
class WiFiService;
@@ -70,6 +71,9 @@
void ScanDoneTask();
void ScanTask();
+ // Store cached copies of singletons for speed/ease of testing.
+ ProxyFactory *proxy_factory_;
+
ScopedRunnableMethodFactory<WiFi> task_factory_;
scoped_ptr<SupplicantProcessProxyInterface> supplicant_process_proxy_;
scoped_ptr<SupplicantInterfaceProxyInterface> supplicant_interface_proxy_;
diff --git a/wifi_unittest.cc b/wifi_unittest.cc
index a3f3bdd..2da6820 100644
--- a/wifi_unittest.cc
+++ b/wifi_unittest.cc
@@ -124,16 +124,17 @@
kDeviceName,
&glib_)),
proxy_factory_(this) {
- ProxyFactory::set_factory(&proxy_factory_);
::testing::DefaultValue< ::DBus::Path>::Set("/default/path");
}
virtual void SetUp() {
+ wifi_->proxy_factory_ = &proxy_factory_;
static_cast<Device *>(wifi_)->rtnl_handler_ = &rtnl_handler_;
wifi_->set_dhcp_provider(&dhcp_provider_);
}
virtual void TearDown() {
+ wifi_->proxy_factory_ = NULL;
// must Stop WiFi instance, to clear its list of services.
// otherwise, the WiFi instance will not be deleted. (because
// services reference a WiFi instance, creating a cycle.)