shill: add some unit tests for ppp dongle support
This is the first in a series of CLs that will provide unit test coverage
for code introduced or modified by CL:51356, which added support for PPP
dongles.
The primary purpose of this CL is to add unit test coverage for the changes
made to cellular.cc. (Other files will be covered in other CLs.)
While there:
- add a PPPDeviceFactory, and a MockPPPDeviceFactory, so that we
can force the creation of a Mock in Cellular::StartPPP
- add an mm1::MockModemSimpleProxy to CellularTest, so that the PPP
tests can use something similar to what we use at runtime
- mock out SelectService and UpdateIPConfigFromPPP in MockPPPDevice,
so that PPP tests can EXPECT on them
- update L2TPIPSecDriverTest.Notify to reflect that SelectService
and UpdateIPConfigFromPPP are now mock methods
- add some comments in various proxy class headers, to help distinguish
between stuff related to old modemmanager and stuff related to new
modemmanager
- fix a random typo or two
BUG=chromium:246826
TEST=new unit tests
Change-Id: I5b2d0af0f9beb760d05e802531c0c9bc1ac5a040
Reviewed-on: https://gerrit.chromium.org/gerrit/63937
Tested-by: mukesh agrawal <quiche@chromium.org>
Reviewed-by: Thieu Le <thieule@chromium.org>
Commit-Queue: mukesh agrawal <quiche@chromium.org>
diff --git a/Makefile b/Makefile
index c9a075f..c085524 100644
--- a/Makefile
+++ b/Makefile
@@ -270,6 +270,7 @@
power_manager.o \
power_manager_proxy.o \
ppp_device.o \
+ ppp_device_factory.o \
process_killer.o \
profile.o \
profile_dbus_adaptor.o \
@@ -431,6 +432,7 @@
mock_power_manager.o \
mock_power_manager_proxy.o \
mock_ppp_device.o \
+ mock_ppp_device_factory.o \
mock_process_killer.o \
mock_profile.o \
mock_property_store.o \
diff --git a/cellular.cc b/cellular.cc
index b082a0b..af88967 100644
--- a/cellular.cc
+++ b/cellular.cc
@@ -33,6 +33,7 @@
#include "shill/logging.h"
#include "shill/manager.h"
#include "shill/ppp_device.h"
+#include "shill/ppp_device_factory.h"
#include "shill/profile.h"
#include "shill/property_accessor.h"
#include "shill/proxy_factory.h"
@@ -117,6 +118,7 @@
dbus_path_(path),
modem_info_(modem_info),
proxy_factory_(proxy_factory),
+ ppp_device_factory_(PPPDeviceFactory::GetInstance()),
allow_roaming_(false),
explicit_disconnect_(false) {
PropertyStore *store = this->mutable_store();
@@ -886,12 +888,16 @@
}
if (!ppp_device_ || ppp_device_->interface_index() != interface_index) {
- ppp_device_ = new PPPDevice(modem_info_->control_interface(),
- modem_info_->dispatcher(),
- modem_info_->metrics(),
- modem_info_->manager(),
- interface_name,
- interface_index);
+ if (ppp_device_) {
+ ppp_device_->SelectService(NULL); // No longer drives |service_|.
+ }
+ ppp_device_ = ppp_device_factory_->CreatePPPDevice(
+ modem_info_->control_interface(),
+ modem_info_->dispatcher(),
+ modem_info_->metrics(),
+ modem_info_->manager(),
+ interface_name,
+ interface_index);
device_info->RegisterDevice(ppp_device_);
}
diff --git a/cellular.h b/cellular.h
index dcc0f9c..a274855 100644
--- a/cellular.h
+++ b/cellular.h
@@ -29,6 +29,7 @@
class CellularCapability;
class Error;
class ExternalTask;
+class PPPDeviceFactory;
class ProxyFactory;
class Cellular : public Device, public RPCTaskDelegate {
@@ -304,7 +305,9 @@
FRIEND_TEST(CellularTest, ModemStateChangeEnable);
FRIEND_TEST(CellularTest, ModemStateChangeStaleConnected);
FRIEND_TEST(CellularTest, ModemStateChangeValidConnected);
+ FRIEND_TEST(CellularTest, Notify);
FRIEND_TEST(CellularTest, OnConnectionHealthCheckerResult);
+ FRIEND_TEST(CellularTest, OnPPPDied);
FRIEND_TEST(CellularTest, SetAllowRoaming);
FRIEND_TEST(CellularTest, StartModemCallback);
FRIEND_TEST(CellularTest, StartModemCallbackFail);
@@ -314,8 +317,9 @@
FRIEND_TEST(CellularTest, StartCDMARegister);
FRIEND_TEST(CellularTest, StartGSMRegister);
FRIEND_TEST(CellularTest, StartLinked);
+ FRIEND_TEST(CellularTest, StartPPP);
FRIEND_TEST(CellularTest, StartPPPAfterEthernetUp);
- FRIEND_TEST(CellularTest, StartPPPWithoutEthernet);
+ FRIEND_TEST(CellularTest, StartPPPAlreadyStarted);
FRIEND_TEST(Modem1Test, CreateDeviceMM1);
// Names of properties in storage
@@ -372,6 +376,7 @@
ModemInfo *modem_info_;
ProxyFactory *proxy_factory_;
+ PPPDeviceFactory *ppp_device_factory_;
CellularServiceRefPtr service_;
diff --git a/cellular_capability_universal.h b/cellular_capability_universal.h
index 0dd10ce..6b81302 100644
--- a/cellular_capability_universal.h
+++ b/cellular_capability_universal.h
@@ -251,6 +251,7 @@
FRIEND_TEST(CellularTest,
HandleNewRegistrationStateForServiceRequiringActivation);
FRIEND_TEST(CellularTest, ModemStateChangeLostRegistration);
+ FRIEND_TEST(CellularTest, OnPPPDied);
// SimLockStatus represents the fields in the Cellular.SIMLockStatus
// DBUS property of the shill device.
diff --git a/cellular_unittest.cc b/cellular_unittest.cc
index 892da29..bccc31b 100644
--- a/cellular_unittest.cc
+++ b/cellular_unittest.cc
@@ -23,6 +23,7 @@
#include "shill/mock_dhcp_config.h"
#include "shill/mock_dhcp_provider.h"
#include "shill/mock_external_task.h"
+#include "shill/mock_mm1_modem_simple_proxy.h"
#include "shill/mock_modem_cdma_proxy.h"
#include "shill/mock_modem_gsm_card_proxy.h"
#include "shill/mock_modem_gsm_network_proxy.h"
@@ -30,6 +31,7 @@
#include "shill/mock_modem_proxy.h"
#include "shill/mock_modem_simple_proxy.h"
#include "shill/mock_ppp_device.h"
+#include "shill/mock_ppp_device_factory.h"
#include "shill/mock_rtnl_handler.h"
#include "shill/property_store_unittest.h"
#include "shill/proxy_factory.h"
@@ -46,6 +48,7 @@
using std::vector;
using testing::_;
using testing::AnyNumber;
+using testing::DoAll;
using testing::Invoke;
using testing::Mock;
using testing::NiceMock;
@@ -138,6 +141,7 @@
cdma_proxy_(new MockModemCDMAProxy()),
gsm_card_proxy_(new MockModemGSMCardProxy()),
gsm_network_proxy_(new MockModemGSMNetworkProxy()),
+ mm1_simple_proxy_(new mm1::MockModemSimpleProxy()),
proxy_factory_(this),
device_(new Cellular(&modem_info_,
kTestDeviceName,
@@ -286,6 +290,11 @@
if (!callback.is_null())
callback.Run(*error);
}
+ void InvokeDisconnectMM1(const ::DBus::Path &bearer, Error *error,
+ const ResultCallback &callback, int timeout) {
+ if (!callback.is_null())
+ callback.Run(Error());
+ }
void ExpectCdmaStartModem(string network_technology) {
if (!device_->IsUnderlyingDeviceEnabled())
@@ -311,6 +320,18 @@
EXPECT_CALL(*modem_info_.mock_manager(), RegisterService(_));
}
+ void StartPPP(int pid) {
+ MockGLib &mock_glib(*dynamic_cast<MockGLib *>(modem_info_.glib()));
+ EXPECT_CALL(mock_glib, ChildWatchAdd(pid, _, _));
+ EXPECT_CALL(mock_glib, SpawnAsync(_, _, _, _, _, _, _, _))
+ .WillOnce(DoAll(SetArgumentPointee<6>(pid), Return(true)));
+ device_->StartPPP("fake_serial_device");
+ EXPECT_FALSE(device_->ipconfig()); // No DHCP client.
+ EXPECT_FALSE(device_->selected_service());
+ EXPECT_TRUE(device_->ppp_task_);
+ Mock::VerifyAndClearExpectations(&mock_glib);
+ }
+
MOCK_METHOD1(TestCallback, void(const Error &error));
protected:
@@ -424,6 +445,7 @@
scoped_ptr<MockModemCDMAProxy> cdma_proxy_;
scoped_ptr<MockModemGSMCardProxy> gsm_card_proxy_;
scoped_ptr<MockModemGSMNetworkProxy> gsm_network_proxy_;
+ scoped_ptr<mm1::MockModemSimpleProxy> mm1_simple_proxy_;
TestProxyFactory proxy_factory_;
CellularRefPtr device_;
};
@@ -1031,6 +1053,20 @@
device_->LinkEvent(IFF_UP, 0);
}
+TEST_F(CellularTest, StartPPP) {
+ const int kPID = 234;
+ EXPECT_FALSE(device_->ppp_task_);
+ StartPPP(kPID);
+}
+
+TEST_F(CellularTest, StartPPPAlreadyStarted) {
+ const int kPID = 234;
+ StartPPP(kPID);
+
+ const int kPID2 = 235;
+ StartPPP(kPID2);
+}
+
TEST_F(CellularTest, StartPPPAfterEthernetUp) {
CellularService *service(SetService());
device_->state_ = Cellular::kStateLinked;
@@ -1039,24 +1075,12 @@
EXPECT_CALL(*dhcp_config_, ReleaseIP(_))
.Times(AnyNumber())
.WillRepeatedly(Return(true));
- EXPECT_CALL(*dynamic_cast<MockGLib *>(modem_info_.glib()),
- SpawnAsync(_, _, _, _, _, _, _, _));
- device_->StartPPP("fake_serial_device");
- EXPECT_FALSE(device_->ipconfig()); // The DHCP client was stopped.
- EXPECT_FALSE(device_->selected_service());
+ const int kPID = 234;
+ EXPECT_FALSE(device_->ppp_task_);
+ StartPPP(kPID);
EXPECT_EQ(Cellular::kStateLinked, device_->state());
}
-TEST_F(CellularTest, StartPPPWithoutEthernet) {
- EXPECT_CALL(*dynamic_cast<MockGLib *>(modem_info_.glib()),
- SpawnAsync(_, _, _, _, _, _, _, _));
- device_->StartPPP("fake_serial_device");
- EXPECT_FALSE(device_->ipconfig()); // No DHCP client.
- EXPECT_FALSE(device_->selected_service());
-}
-
-// TODO(quiche): test the common bits of StartPPP. crbug.com/246826.
-
TEST_F(CellularTest, GetLogin) {
// Doesn't crash when there is no service.
string username_to_pppd;
@@ -1073,6 +1097,94 @@
device_->GetLogin(&username_to_pppd, &password_to_pppd);
}
+TEST_F(CellularTest, Notify) {
+ // Common setup.
+ MockPPPDeviceFactory *ppp_device_factory =
+ MockPPPDeviceFactory::GetInstance();
+ device_->ppp_device_factory_ = ppp_device_factory;
+ SetMockService();
+
+ // Normal connect.
+ const string kInterfaceName("fake-device");
+ const int kInterfaceIndex = 1;
+ scoped_refptr<MockPPPDevice> ppp_device;
+ map<string, string> ppp_config;
+ ppp_device =
+ new MockPPPDevice(modem_info_.control_interface(),
+ NULL, NULL, NULL, kInterfaceName, kInterfaceIndex);
+ ppp_config[kPPPInterfaceName] = kInterfaceName;
+ EXPECT_CALL(device_info_, GetIndex(kInterfaceName))
+ .WillOnce(Return(kInterfaceIndex));
+ EXPECT_CALL(device_info_, RegisterDevice(_));
+ EXPECT_CALL(*ppp_device_factory,
+ CreatePPPDevice(_, _, _, _, kInterfaceName, kInterfaceIndex))
+ .WillOnce(Return(ppp_device));
+ EXPECT_CALL(*ppp_device, SetEnabled(true));
+ EXPECT_CALL(*ppp_device, SelectService(_));
+ EXPECT_CALL(*ppp_device, UpdateIPConfigFromPPP(ppp_config, false));
+ device_->Notify(kPPPReasonConnect, ppp_config);
+ Mock::VerifyAndClearExpectations(&device_info_);
+ Mock::VerifyAndClearExpectations(ppp_device);
+
+ // Re-connect on same network device: if pppd sends us multiple connect
+ // events, we behave sanely.
+ EXPECT_CALL(device_info_, GetIndex(kInterfaceName))
+ .WillOnce(Return(kInterfaceIndex));
+ EXPECT_CALL(*ppp_device, SetEnabled(true));
+ EXPECT_CALL(*ppp_device, SelectService(_));
+ EXPECT_CALL(*ppp_device, UpdateIPConfigFromPPP(ppp_config, false));
+ device_->Notify(kPPPReasonConnect, ppp_config);
+ Mock::VerifyAndClearExpectations(&device_info_);
+ Mock::VerifyAndClearExpectations(ppp_device);
+
+ // Re-connect on new network device: if we still have the PPPDevice
+ // from a prior connect, this new connect should DTRT. This is
+ // probably an unlikely case.
+ const string kInterfaceName2("fake-device2");
+ const int kInterfaceIndex2 = 2;
+ scoped_refptr<MockPPPDevice> ppp_device2;
+ map<string, string> ppp_config2;
+ ppp_device2 =
+ new MockPPPDevice(modem_info_.control_interface(),
+ NULL, NULL, NULL, kInterfaceName2, kInterfaceIndex2);
+ ppp_config2[kPPPInterfaceName] = kInterfaceName2;
+ EXPECT_CALL(device_info_, GetIndex(kInterfaceName2))
+ .WillOnce(Return(kInterfaceIndex2));
+ EXPECT_CALL(device_info_,
+ RegisterDevice(static_cast<DeviceRefPtr>(ppp_device2)));
+ EXPECT_CALL(*ppp_device_factory,
+ CreatePPPDevice(_, _, _, _, kInterfaceName2, kInterfaceIndex2))
+ .WillOnce(Return(ppp_device2));
+ EXPECT_CALL(*ppp_device, SelectService(ServiceRefPtr(nullptr)));
+ EXPECT_CALL(*ppp_device2, SetEnabled(true));
+ EXPECT_CALL(*ppp_device2, SelectService(_));
+ EXPECT_CALL(*ppp_device2, UpdateIPConfigFromPPP(ppp_config2, false));
+ device_->Notify(kPPPReasonConnect, ppp_config2);
+ Mock::VerifyAndClearExpectations(&device_info_);
+ // Mock::VerifyAndClearExpectations(ppp_device);
+ Mock::VerifyAndClearExpectations(ppp_device2);
+
+ map<string, string> empty_ppp_config;
+ EXPECT_CALL(*ppp_device2, SetServiceFailure(Service::kFailurePPPAuth));
+ device_->Notify(kPPPReasonDisconnect, empty_ppp_config);
+ EXPECT_FALSE(device_->ppp_task_);
+}
+
+TEST_F(CellularTest, OnPPPDied) {
+ const int kPID = 1234;
+ const int kExitStatus = 5;
+ SetCellularType(Cellular::kTypeUniversal);
+ device_->state_ = Cellular::kStateConnected;
+ EXPECT_CALL(*mm1_simple_proxy_, Disconnect(_, _, _, _))
+ .WillOnce(Invoke(this, &CellularTest::InvokeDisconnectMM1));
+ GetCapabilityUniversal()->modem_simple_proxy_.reset(
+ mm1_simple_proxy_.release());
+ GetCapabilityUniversal()->bearer_path_ = "/fake/path";
+
+ device_->OnPPPDied(kPID, kExitStatus);
+ EXPECT_EQ(Cellular::kStateRegistered, device_->state_);
+}
+
TEST_F(CellularTest, DropConnection) {
device_->set_ipconfig(dhcp_config_);
EXPECT_CALL(*dhcp_config_, ReleaseIP(_));
diff --git a/connection_unittest.cc b/connection_unittest.cc
index 6015c18..16f731e 100644
--- a/connection_unittest.cc
+++ b/connection_unittest.cc
@@ -839,7 +839,7 @@
kTestDeviceInterfaceIndex1, RoutingTableEntry());
EXPECT_FALSE(binder->IsBound());
- // Create a mock connection that will be used fo binding.
+ // Create a mock connection that will be used for binding.
scoped_refptr<MockConnection> mock_connection(
new StrictMock<MockConnection>(device_info_.get()));
EXPECT_CALL(*device_info_.get(),
diff --git a/device.h b/device.h
index bfba743..9af4251 100644
--- a/device.h
+++ b/device.h
@@ -246,6 +246,7 @@
FRIEND_TEST(ManagerTest, ConnectedTechnologies);
FRIEND_TEST(ManagerTest, DefaultTechnology);
FRIEND_TEST(ManagerTest, SetEnabledStateForTechnology);
+ FRIEND_TEST(PPPDeviceTest, UpdateIPConfigFromPPP);
FRIEND_TEST(WiFiMainTest, Connect);
FRIEND_TEST(WiFiMainTest, UseArpGateway);
FRIEND_TEST(WiMaxTest, ConnectTimeout);
diff --git a/dhcp_provider.h b/dhcp_provider.h
index 901a858..62534f2 100644
--- a/dhcp_provider.h
+++ b/dhcp_provider.h
@@ -38,7 +38,7 @@
virtual ~DHCPProvider();
- // This is a singleton -- use DHCPProvider::GetInstance()->Foo()
+ // This is a singleton -- use DHCPProvider::GetInstance()->Foo().
static DHCPProvider *GetInstance();
// Initializes the provider singleton. This method hooks up a D-Bus signal
diff --git a/diagnostics_reporter.h b/diagnostics_reporter.h
index 1d21a33..9d80add 100644
--- a/diagnostics_reporter.h
+++ b/diagnostics_reporter.h
@@ -18,7 +18,7 @@
public:
virtual ~DiagnosticsReporter();
- // This is a singleton -- use DiagnosticsReporter::GetInstance()->Foo()
+ // This is a singleton -- use DiagnosticsReporter::GetInstance()->Foo().
static DiagnosticsReporter *GetInstance();
// Handle a connectivity event -- collect and stash diagnostics data, possibly
diff --git a/dns_client_factory.h b/dns_client_factory.h
index 685bbab..034ce00 100644
--- a/dns_client_factory.h
+++ b/dns_client_factory.h
@@ -20,7 +20,7 @@
public:
virtual ~DNSClientFactory();
- // This is a singleton. Use DNSClientFactory::GetInstance()->Foo()
+ // This is a singleton. Use DNSClientFactory::GetInstance()->Foo().
static DNSClientFactory *GetInstance();
virtual DNSClient *CreateDNSClient(
diff --git a/file_io.h b/file_io.h
index 8b654ab..63bc6e8 100644
--- a/file_io.h
+++ b/file_io.h
@@ -14,7 +14,7 @@
public:
virtual ~FileIO();
- // This is a singleton -- use FileIO::GetInstance()->Foo()
+ // This is a singleton -- use FileIO::GetInstance()->Foo().
static FileIO *GetInstance();
virtual ssize_t Write(int fd, const void *buf, size_t count);
diff --git a/l2tp_ipsec_driver_unittest.cc b/l2tp_ipsec_driver_unittest.cc
index 7de16b0..2f9a8b0 100644
--- a/l2tp_ipsec_driver_unittest.cc
+++ b/l2tp_ipsec_driver_unittest.cc
@@ -529,8 +529,10 @@
EXPECT_CALL(device_info_, GetIndex(kInterfaceName))
.WillOnce(Return(kInterfaceIndex));
EXPECT_CALL(*device_, SetEnabled(true));
- EXPECT_CALL(*device_, UpdateIPConfig(_));
+ EXPECT_CALL(*device_, SelectService(static_cast<ServiceRefPtr>(service_)));
+ EXPECT_CALL(*device_, UpdateIPConfigFromPPP(config, _));
SetDevice(device_);
+ SetService(service_);
FilePath psk_file = SetupPSKFile();
StartConnectTimeout(0);
diff --git a/minijail.h b/minijail.h
index 9f5d390..9a668c0 100644
--- a/minijail.h
+++ b/minijail.h
@@ -21,7 +21,7 @@
public:
virtual ~Minijail();
- // This is a singleton -- use Minijail::GetInstance()->Foo()
+ // This is a singleton -- use Minijail::GetInstance()->Foo().
static Minijail *GetInstance();
// minijail_new
diff --git a/mm1_bearer_proxy.h b/mm1_bearer_proxy.h
index 67e78cb..07f96cf 100644
--- a/mm1_bearer_proxy.h
+++ b/mm1_bearer_proxy.h
@@ -14,6 +14,7 @@
namespace shill {
namespace mm1 {
+// A proxy to org.freedesktop.ModemManager1.Bearer.
class BearerProxy : public BearerProxyInterface {
public:
// Constructs an org.freedesktop.ModemManager1.Bearer DBus object
diff --git a/mm1_modem_location_proxy.h b/mm1_modem_location_proxy.h
index f48b2e0..d69bf44 100644
--- a/mm1_modem_location_proxy.h
+++ b/mm1_modem_location_proxy.h
@@ -14,8 +14,11 @@
namespace shill {
namespace mm1 {
+// A proxy to org.freedesktop.ModemManager1.Bearer.
class ModemLocationProxy : public ModemLocationProxyInterface {
public:
+ // Constructs an org.freedesktop.ModemManager1.Modem.Location DBus
+ // object proxy at |path| owned by |service|.
ModemLocationProxy(DBus::Connection *connection,
const std::string &path,
const std::string &service);
diff --git a/mm1_modem_modem3gpp_proxy.h b/mm1_modem_modem3gpp_proxy.h
index 17c015f..16f0526 100644
--- a/mm1_modem_modem3gpp_proxy.h
+++ b/mm1_modem_modem3gpp_proxy.h
@@ -14,9 +14,10 @@
namespace shill {
namespace mm1 {
+// A proxy to org.freedesktop.ModemManager1.Modem.Modem3gpp.
class ModemModem3gppProxy : public ModemModem3gppProxyInterface {
public:
- // Constructs a org.freedesktop.ModemManager1.Modem.Modem3gpp DBus
+ // Constructs an org.freedesktop.ModemManager1.Modem.Modem3gpp DBus
// object proxy at |path| owned by |service|.
ModemModem3gppProxy(DBus::Connection *connection,
const std::string &path,
diff --git a/mm1_modem_modemcdma_proxy.h b/mm1_modem_modemcdma_proxy.h
index f20d96f..acbedba 100644
--- a/mm1_modem_modemcdma_proxy.h
+++ b/mm1_modem_modemcdma_proxy.h
@@ -14,6 +14,7 @@
namespace shill {
namespace mm1 {
+// A proxy to org.freedesktop.ModemManager1.Modem.ModemCdma.
class ModemModemCdmaProxy : public ModemModemCdmaProxyInterface {
public:
// Constructs a org.freedesktop.ModemManager1.Modem.ModemCdma DBus
diff --git a/mm1_modem_proxy.h b/mm1_modem_proxy.h
index c646326..ffab77f 100644
--- a/mm1_modem_proxy.h
+++ b/mm1_modem_proxy.h
@@ -14,6 +14,7 @@
namespace shill {
namespace mm1 {
+// A proxy to org.freedesktop.ModemManager1.Modem.
class ModemProxy : public ModemProxyInterface {
public:
// Constructs a org.freedesktop.ModemManager1.Modem DBus object
diff --git a/mm1_modem_simple_proxy.h b/mm1_modem_simple_proxy.h
index 8136855..37d2bf5 100644
--- a/mm1_modem_simple_proxy.h
+++ b/mm1_modem_simple_proxy.h
@@ -14,6 +14,7 @@
namespace shill {
namespace mm1 {
+// A proxy to org.freedesktop.ModemManager1.Modem.Simple.
class ModemSimpleProxy : public ModemSimpleProxyInterface {
public:
// Constructs a org.freedesktop.ModemManager1.Modem.Simple DBus
diff --git a/mm1_modem_time_proxy.h b/mm1_modem_time_proxy.h
index d36614a..0cae12a 100644
--- a/mm1_modem_time_proxy.h
+++ b/mm1_modem_time_proxy.h
@@ -14,8 +14,11 @@
namespace shill {
namespace mm1 {
+// A proxy to org.freedesktop.ModemManager1.Modem.Time.
class ModemTimeProxy : public ModemTimeProxyInterface {
public:
+ // Constructs an org.freedesktop.ModemManager1.Modem.Time DBus object
+ // proxy at |path| owned by |service|.
ModemTimeProxy(DBus::Connection *connection,
const std::string &path,
const std::string &service);
diff --git a/mm1_sim_proxy.h b/mm1_sim_proxy.h
index 15bb096..e9472f0 100644
--- a/mm1_sim_proxy.h
+++ b/mm1_sim_proxy.h
@@ -14,9 +14,10 @@
namespace shill {
namespace mm1 {
+// A proxy to org.freedesktop.ModemManager1.Sim.
class SimProxy : public SimProxyInterface {
public:
- // Constructs a org.freedesktop.ModemManager1.Modem DBus object
+ // Constructs an org.freedesktop.ModemManager1.Sim DBus object
// proxy at |path| owned by |service|.
SimProxy(DBus::Connection *connection,
const std::string &path,
diff --git a/mock_dns_client_factory.h b/mock_dns_client_factory.h
index aa9a541..39fe203 100644
--- a/mock_dns_client_factory.h
+++ b/mock_dns_client_factory.h
@@ -21,7 +21,7 @@
public:
virtual ~MockDNSClientFactory();
- // This is a singleton. Use MockDNSClientFactory::GetInstance()->Foo()
+ // This is a singleton. Use MockDNSClientFactory::GetInstance()->Foo().
static MockDNSClientFactory *GetInstance();
MOCK_METHOD6(CreateDNSClient,
diff --git a/mock_ppp_device.h b/mock_ppp_device.h
index 0618895..57a75aa 100644
--- a/mock_ppp_device.h
+++ b/mock_ppp_device.h
@@ -25,10 +25,14 @@
void(Error *error, const EnabledStateChangedCallback &callback));
MOCK_METHOD1(UpdateIPConfig, void(const IPConfig::Properties &properties));
MOCK_METHOD0(DropConnection, void());
+ MOCK_METHOD1(SelectService, void(const ServiceRefPtr &service));
MOCK_METHOD1(SetServiceState, void(Service::ConnectState));
MOCK_METHOD1(SetServiceFailure, void(Service::ConnectFailure));
MOCK_METHOD1(SetServiceFailureSilent, void(Service::ConnectFailure));
MOCK_METHOD1(SetEnabled, void(bool));
+ MOCK_METHOD2(UpdateIPConfigFromPPP, void(
+ const std::map<std::string, std::string> &config,
+ bool blackhole_ipv6));
private:
DISALLOW_COPY_AND_ASSIGN(MockPPPDevice);
diff --git a/mock_ppp_device_factory.cc b/mock_ppp_device_factory.cc
new file mode 100644
index 0000000..9f04625
--- /dev/null
+++ b/mock_ppp_device_factory.cc
@@ -0,0 +1,23 @@
+// Copyright (c) 2013 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_ppp_device_factory.h"
+
+namespace shill {
+
+namespace {
+
+base::LazyInstance<MockPPPDeviceFactory> g_mock_ppp_device_factory
+ = LAZY_INSTANCE_INITIALIZER;
+
+} // namespace
+
+MockPPPDeviceFactory::MockPPPDeviceFactory() {}
+MockPPPDeviceFactory::~MockPPPDeviceFactory() {}
+
+MockPPPDeviceFactory *MockPPPDeviceFactory::GetInstance() {
+ return g_mock_ppp_device_factory.Pointer();
+}
+
+} // namespace shill
diff --git a/mock_ppp_device_factory.h b/mock_ppp_device_factory.h
new file mode 100644
index 0000000..10ed851
--- /dev/null
+++ b/mock_ppp_device_factory.h
@@ -0,0 +1,41 @@
+// Copyright (c) 2013 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_PPP_DEVICE_FACTORY_H_
+#define SHILL_MOCK_PPP_DEVICE_FACTORY_H_
+
+#include <base/lazy_instance.h>
+#include <gmock/gmock.h>
+
+#include "shill/ppp_device_factory.h"
+
+namespace shill {
+
+class MockPPPDeviceFactory : public PPPDeviceFactory {
+ public:
+ virtual ~MockPPPDeviceFactory() override;
+
+ // This is a singleton. Use MockPPPDeviceFactory::GetInstance()->Foo().
+ static MockPPPDeviceFactory *GetInstance();
+
+ MOCK_METHOD6(CreatePPPDevice,
+ PPPDevice *(ControlInterface *control,
+ EventDispatcher *dispatcher,
+ Metrics *metrics,
+ Manager *manager,
+ const std::string &link_name,
+ int interface_index));
+
+ protected:
+ MockPPPDeviceFactory();
+
+ private:
+ friend struct base::DefaultLazyInstanceTraits<MockPPPDeviceFactory>;
+
+ DISALLOW_COPY_AND_ASSIGN(MockPPPDeviceFactory);
+};
+
+} // namespace shill
+
+#endif // SHILL_MOCK_PPP_DEVICE_FACTORY_H_
diff --git a/modem_cdma_proxy.h b/modem_cdma_proxy.h
index 1c9922d..12477ed 100644
--- a/modem_cdma_proxy.h
+++ b/modem_cdma_proxy.h
@@ -13,6 +13,7 @@
namespace shill {
+// A proxy to (old) ModemManager.Modem.CDMA.
class ModemCDMAProxy : public ModemCDMAProxyInterface {
public:
// Constructs a ModemManager.Modem.CDMA DBus object proxy at |path| owned by
diff --git a/modem_gobi_proxy.h b/modem_gobi_proxy.h
index be122b7..c46a6ef 100644
--- a/modem_gobi_proxy.h
+++ b/modem_gobi_proxy.h
@@ -12,6 +12,7 @@
namespace shill {
+// A proxy to (old) ModemManager.Modem.Gobi.
class ModemGobiProxy : public ModemGobiProxyInterface {
public:
// Constructs a ModemManager.Modem.Gobi DBus object proxy at |path| owned by
diff --git a/modem_gsm_card_proxy.h b/modem_gsm_card_proxy.h
index b6196d1..b203d2e 100644
--- a/modem_gsm_card_proxy.h
+++ b/modem_gsm_card_proxy.h
@@ -14,6 +14,7 @@
namespace shill {
+// A proxy to (old) ModemManager.Modem.Gsm.Card.
class ModemGSMCardProxy : public ModemGSMCardProxyInterface {
public:
// Constructs a ModemManager.Modem.Gsm.Card DBus
diff --git a/modem_gsm_network_proxy.h b/modem_gsm_network_proxy.h
index f714585..a200584 100644
--- a/modem_gsm_network_proxy.h
+++ b/modem_gsm_network_proxy.h
@@ -12,6 +12,7 @@
namespace shill {
+// A proxy to (old) ModemManager.Modem.Gsm.Network.
class ModemGSMNetworkProxy : public ModemGSMNetworkProxyInterface {
public:
// Constructs a ModemManager.Modem.Gsm.Network DBus object proxy at |path|
diff --git a/modem_manager_proxy.h b/modem_manager_proxy.h
index 2b261a6..68097e0 100644
--- a/modem_manager_proxy.h
+++ b/modem_manager_proxy.h
@@ -17,8 +17,8 @@
class ModemManagerClassic;
-// There's a single proxy per ModemManager service identified by its DBus |path|
-// and owner name |service|.
+// There's a single proxy per (old) ModemManager service identified by
+// its DBus |path| and owner name |service|.
class ModemManagerProxy : public ModemManagerProxyInterface {
public:
ModemManagerProxy(DBus::Connection *connection,
diff --git a/modem_proxy.h b/modem_proxy.h
index 8e21bf0..10a046e 100644
--- a/modem_proxy.h
+++ b/modem_proxy.h
@@ -14,7 +14,7 @@
namespace shill {
-// A proxy to ModemManager.Modem.
+// A proxy to (old) ModemManager.Modem.
class ModemProxy : public ModemProxyInterface {
public:
// Constructs a ModemManager.Modem DBus object proxy at |path| owned by
diff --git a/modem_simple_proxy.h b/modem_simple_proxy.h
index ff99d65..758258a 100644
--- a/modem_simple_proxy.h
+++ b/modem_simple_proxy.h
@@ -14,9 +14,11 @@
namespace shill {
-// A proxy to ModemManager.Modem.Simple.
+// A proxy to (old) ModemManager.Modem.Simple.
class ModemSimpleProxy : public ModemSimpleProxyInterface {
public:
+ // Constructs a ModemManager.Modem.Simple DBus object proxy at
+ // |path| owned by |service|.
ModemSimpleProxy(DBus::Connection *connection,
const std::string &path,
const std::string &service);
diff --git a/nss.h b/nss.h
index 75f5e5f..21fe77d 100644
--- a/nss.h
+++ b/nss.h
@@ -20,7 +20,7 @@
public:
virtual ~NSS();
- // This is a singleton -- use NSS::GetInstance()->Foo()
+ // This is a singleton -- use NSS::GetInstance()->Foo().
static NSS *GetInstance();
// Returns an empty path on failure.
diff --git a/ppp_device_factory.cc b/ppp_device_factory.cc
new file mode 100644
index 0000000..c4366fa
--- /dev/null
+++ b/ppp_device_factory.cc
@@ -0,0 +1,38 @@
+// Copyright (c) 2013 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/ppp_device_factory.h"
+
+#include "shill/ppp_device.h"
+
+using std::string;
+
+namespace shill {
+
+namespace {
+
+base::LazyInstance<PPPDeviceFactory> g_ppp_device_factory
+ = LAZY_INSTANCE_INITIALIZER;
+
+} // namespace
+
+PPPDeviceFactory::PPPDeviceFactory() {}
+PPPDeviceFactory::~PPPDeviceFactory() {}
+
+PPPDeviceFactory *PPPDeviceFactory::GetInstance() {
+ return g_ppp_device_factory.Pointer();
+}
+
+PPPDevice *PPPDeviceFactory::CreatePPPDevice(
+ ControlInterface *control,
+ EventDispatcher *dispatcher,
+ Metrics *metrics,
+ Manager *manager,
+ const string &link_name,
+ int interface_index) {
+ return new PPPDevice(control, dispatcher, metrics, manager, link_name,
+ interface_index);
+}
+
+} // namespace shill
diff --git a/ppp_device_factory.h b/ppp_device_factory.h
new file mode 100644
index 0000000..62c9783
--- /dev/null
+++ b/ppp_device_factory.h
@@ -0,0 +1,46 @@
+// Copyright (c) 2013 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_PPP_DEVICE_FACTORY_H_
+#define SHILL_PPP_DEVICE_FACTORY_H_
+
+#include <string>
+
+#include <base/lazy_instance.h>
+
+namespace shill {
+
+class ControlInterface;
+class EventDispatcher;
+class Manager;
+class Metrics;
+class PPPDevice;
+
+class PPPDeviceFactory {
+ public:
+ virtual ~PPPDeviceFactory();
+
+ // This is a singleton. Use PPPDeviceFactory::GetInstance()->Foo().
+ static PPPDeviceFactory *GetInstance();
+
+ virtual PPPDevice *CreatePPPDevice(
+ ControlInterface *control,
+ EventDispatcher *dispatcher,
+ Metrics *metrics,
+ Manager *manager,
+ const std::string &link_name,
+ int interface_index);
+
+ protected:
+ PPPDeviceFactory();
+
+ private:
+ friend struct base::DefaultLazyInstanceTraits<PPPDeviceFactory>;
+
+ DISALLOW_COPY_AND_ASSIGN(PPPDeviceFactory);
+};
+
+} // namespace shill
+
+#endif // SHILL_PPP_DEVICE_FACTORY_H_
diff --git a/ppp_device_unittest.cc b/ppp_device_unittest.cc
index a73fa62..bf3b970 100644
--- a/ppp_device_unittest.cc
+++ b/ppp_device_unittest.cc
@@ -16,6 +16,8 @@
namespace shill {
+// TODO(quiche): Add test for UpdateIPConfigFromPPP. crbug.com/266404
+
TEST(PPPDeviceTest, GetInterfaceName) {
map<string, string> config;
config[kPPPInterfaceName] = "ppp0";
diff --git a/process_killer.h b/process_killer.h
index 70c367c..7aceb70 100644
--- a/process_killer.h
+++ b/process_killer.h
@@ -20,7 +20,7 @@
public:
virtual ~ProcessKiller();
- // This is a singleton -- use ProcessKiller::GetInstance()->Foo()
+ // This is a singleton -- use ProcessKiller::GetInstance()->Foo().
static ProcessKiller *GetInstance();
// Uses GLib to wait asynchronously for the process to exit and reap it. GLib
diff --git a/proxy_factory.h b/proxy_factory.h
index 8fd45cb..b400f7a 100644
--- a/proxy_factory.h
+++ b/proxy_factory.h
@@ -57,7 +57,7 @@
public:
virtual ~ProxyFactory();
- // Since this is a singleton, use ProxyFactory::GetInstance()->Foo()
+ // Since this is a singleton, use ProxyFactory::GetInstance()->Foo().
static ProxyFactory *GetInstance();
virtual void Init();
diff --git a/resolver.h b/resolver.h
index a52d7c5..d50d33f 100644
--- a/resolver.h
+++ b/resolver.h
@@ -30,7 +30,7 @@
virtual ~Resolver();
- // Since this is a singleton, use Resolver::GetInstance()->Foo()
+ // Since this is a singleton, use Resolver::GetInstance()->Foo().
static Resolver *GetInstance();
virtual void set_path(const base::FilePath &path) { path_ = path; }
diff --git a/rtnl_handler.h b/rtnl_handler.h
index 6691531..69a591e 100644
--- a/rtnl_handler.h
+++ b/rtnl_handler.h
@@ -47,7 +47,7 @@
virtual ~RTNLHandler();
- // Since this is a singleton, use RTNHandler::GetInstance()->Foo()
+ // Since this is a singleton, use RTNHandler::GetInstance()->Foo().
static RTNLHandler *GetInstance();
// This starts the event-monitoring function of the RTNL handler. This
diff --git a/shims/environment.h b/shims/environment.h
index 60acd3b..3ee4352 100644
--- a/shims/environment.h
+++ b/shims/environment.h
@@ -19,7 +19,7 @@
public:
virtual ~Environment();
- // This is a singleton -- use Environment::GetInstance()->Foo()
+ // This is a singleton -- use Environment::GetInstance()->Foo().
static Environment *GetInstance();
// Sets |value| to the value of environment variable |name| and returns
diff --git a/shims/ppp.h b/shims/ppp.h
index 6989cb6..c6741ac 100644
--- a/shims/ppp.h
+++ b/shims/ppp.h
@@ -26,7 +26,7 @@
public:
~PPP();
- // This is a singleton -- use PPP::GetInstance()->Foo()
+ // This is a singleton -- use PPP::GetInstance()->Foo().
static PPP *GetInstance();
void Init();