shill: cellular: Handle vanish of bearer when getting its properties.
This CL changes CellularCapabilityUniversal::OnListBearersReply() to get
the properties of a bearer via a single invocation of
DBusPropertiesProxy::GetAll() instead of multiple invocations of
different property getters in mm1::BearerProxy. This allows us to handle
the vanish of a bearer more gracefully when getting its properties over
DBus.
BUG=chromium:244496
TEST=Tested the following:
1. Build and run unit tests.
2. Run network_3GSmokeTest test on platforms with Y3400 and E362 modem.
3. Run network_3GModemControl test with pseudomodem.
4. Test a few 3G dongles that use PPP.
Change-Id: I7d594d59dae5191bc0db1defcdaf4663d929a529
Reviewed-on: https://gerrit.chromium.org/gerrit/61587
Commit-Queue: Ben Chan <benchan@chromium.org>
Reviewed-by: Ben Chan <benchan@chromium.org>
Tested-by: Ben Chan <benchan@chromium.org>
diff --git a/cellular_capability_universal.cc b/cellular_capability_universal.cc
index 2ca66fc..482d89c 100644
--- a/cellular_capability_universal.cc
+++ b/cellular_capability_universal.cc
@@ -1063,22 +1063,50 @@
string network_device;
for (size_t i = 0; i < paths.size(); ++i) {
const DBus::Path &path = paths[i];
- scoped_ptr<mm1::BearerProxyInterface> bearer_proxy(
- proxy_factory()->CreateBearerProxy(path, cellular()->dbus_owner()));
- if (!bearer_proxy->Connected()) {
+
+ scoped_ptr<DBusPropertiesProxyInterface> properties_proxy(
+ proxy_factory()->CreateDBusPropertiesProxy(path,
+ cellular()->dbus_owner()));
+ DBusPropertiesMap properties(
+ properties_proxy->GetAll(MM_DBUS_INTERFACE_BEARER));
+ if (properties.empty()) {
+ LOG(WARNING) << "Could not get properties of bearer \"" << path
+ << "\". Bearer is likely gone and thus ignored.";
continue;
}
- CHECK(new_bearer_path.empty()) << "Found more than one active bearer.";
- network_device = bearer_proxy->Interface();
+
+ bool connected = false;
+ if (!DBusProperties::GetBool(
+ properties, MM_BEARER_PROPERTY_CONNECTED, &connected)) {
+ SLOG(Cellular, 2) << "Bearer does not indicate whether it is connected "
+ "or not. Assume it is not connected.";
+ continue;
+ }
+
+ if (!connected) {
+ continue;
+ }
+
SLOG(Cellular, 2) << "Found active bearer \"" << path << "\".";
- SLOG(Cellular, 2) << "Bearer uses interface \"" << network_device << "\".";
+ CHECK(new_bearer_path.empty()) << "Found more than one active bearer.";
+
+ if (DBusProperties::GetString(
+ properties, MM_BEARER_PROPERTY_INTERFACE, &network_device)) {
+ SLOG(Cellular, 2) << "Bearer uses network interface \"" << network_device
+ << "\".";
+ } else {
+ SLOG(Cellular, 2) << "Bearer does not specify network interface.";
+ }
+
// TODO(quiche): Add support for scenarios where the bearer is
// IPv6 only, or where there are conflicting configuration methods
// for IPv4 and IPv6. crbug.com/248360.
- DBusPropertiesMap bearer_ip4config = bearer_proxy->Ip4Config();
- if (ContainsKey(bearer_ip4config, kIpConfigPropertyMethod)) {
- ipconfig_method = bearer_ip4config[kIpConfigPropertyMethod].reader().
- get_uint32();
+ DBusPropertiesMap bearer_ip4config;
+ DBusProperties::GetDBusPropertiesMap(
+ properties, MM_BEARER_PROPERTY_IP4CONFIG, &bearer_ip4config);
+
+ if (DBusProperties::GetUint32(
+ bearer_ip4config, kIpConfigPropertyMethod, &ipconfig_method)) {
SLOG(Cellular, 2) << "Bearer has IPv4 config method " << ipconfig_method;
} else {
SLOG(Cellular, 2) << "Bearer does not specify IPv4 config method.";
diff --git a/cellular_capability_universal_cdma_unittest.cc b/cellular_capability_universal_cdma_unittest.cc
index ec6622f..e9e707f 100644
--- a/cellular_capability_universal_cdma_unittest.cc
+++ b/cellular_capability_universal_cdma_unittest.cc
@@ -22,7 +22,6 @@
#include "shill/mock_glib.h"
#include "shill/mock_manager.h"
#include "shill/mock_metrics.h"
-#include "shill/mock_mm1_bearer_proxy.h"
#include "shill/mock_mm1_modem_modem3gpp_proxy.h"
#include "shill/mock_mm1_modem_modemcdma_proxy.h"
#include "shill/mock_mm1_modem_proxy.h"
@@ -51,7 +50,6 @@
: capability_(NULL),
device_adaptor_(NULL),
modem_info_(NULL, dispatcher, NULL, NULL, NULL),
- bearer_proxy_(new mm1::MockBearerProxy()),
modem_3gpp_proxy_(new mm1::MockModemModem3gppProxy()),
modem_cdma_proxy_(new mm1::MockModemModemCdmaProxy()),
modem_proxy_(new mm1::MockModemProxy()),
@@ -117,12 +115,6 @@
// TODO(armansito): Some of these methods won't be necessary after 3GPP
// gets refactored out of CellularCapabilityUniversal.
- virtual mm1::BearerProxyInterface *CreateBearerProxy(
- const std::string &path,
- const std::string &/*service*/) {
- return test_->bearer_proxy_.release();
- }
-
virtual mm1::ModemModem3gppProxyInterface *CreateMM1ModemModem3gppProxy(
const std::string &/*path*/,
const std::string &/*service*/) {
@@ -168,7 +160,6 @@
NiceMock<DeviceMockAdaptor> *device_adaptor_;
MockModemInfo modem_info_;
MockGLib glib_;
- scoped_ptr<mm1::MockBearerProxy> bearer_proxy_;
// TODO(armansito): Remove |modem_3gpp_proxy_| after refactor.
scoped_ptr<mm1::MockModemModem3gppProxy> modem_3gpp_proxy_;
scoped_ptr<mm1::MockModemModemCdmaProxy> modem_cdma_proxy_;
diff --git a/cellular_capability_universal_unittest.cc b/cellular_capability_universal_unittest.cc
index d8d05ec..98a4179 100644
--- a/cellular_capability_universal_unittest.cc
+++ b/cellular_capability_universal_unittest.cc
@@ -26,7 +26,6 @@
#include "shill/mock_cellular_service.h"
#include "shill/mock_dbus_properties_proxy.h"
#include "shill/mock_event_dispatcher.h"
-#include "shill/mock_mm1_bearer_proxy.h"
#include "shill/mock_mm1_modem_modem3gpp_proxy.h"
#include "shill/mock_mm1_modem_modemcdma_proxy.h"
#include "shill/mock_mm1_modem_proxy.h"
@@ -72,7 +71,6 @@
public:
CellularCapabilityUniversalTest(EventDispatcher *dispatcher)
: modem_info_(NULL, dispatcher, NULL, NULL, NULL),
- bearer_proxy_(new mm1::MockBearerProxy()),
modem_3gpp_proxy_(new mm1::MockModemModem3gppProxy()),
modem_cdma_proxy_(new mm1::MockModemModemCdmaProxy()),
modem_proxy_(new mm1::MockModemProxy()),
@@ -195,25 +193,20 @@
public:
explicit TestProxyFactory(CellularCapabilityUniversalTest *test) :
test_(test) {
- ::DBus::Variant ip_method_dhcp;
- ip_method_dhcp.writer().append_uint32(MM_BEARER_IP_METHOD_DHCP);
- bearer_ip4config_dhcp_[
- CellularCapabilityUniversal::kIpConfigPropertyMethod] =
- ip_method_dhcp;
- }
+ active_bearer_properties_[MM_BEARER_PROPERTY_CONNECTED].writer()
+ .append_bool(true);
+ active_bearer_properties_[MM_BEARER_PROPERTY_INTERFACE].writer()
+ .append_string("/dev/fake");
- virtual mm1::BearerProxyInterface *CreateBearerProxy(
- const std::string &path,
- const std::string &/*service*/) {
- mm1::MockBearerProxy *bearer_proxy = test_->bearer_proxy_.release();
- if (path.find(kActiveBearerPathPrefix) != std::string::npos)
- ON_CALL(*bearer_proxy, Connected()).WillByDefault(Return(true));
- else
- ON_CALL(*bearer_proxy, Connected()).WillByDefault(Return(false));
- ON_CALL(*bearer_proxy, Ip4Config()).WillByDefault(Return(
- bearer_ip4config_dhcp_));
- test_->bearer_proxy_.reset(new mm1::MockBearerProxy());
- return bearer_proxy;
+ DBusPropertiesMap ip4config;
+ ip4config[CellularCapabilityUniversal::kIpConfigPropertyMethod].writer()
+ .append_uint32(MM_BEARER_IP_METHOD_DHCP);
+ DBus::MessageIter writer =
+ active_bearer_properties_[MM_BEARER_PROPERTY_IP4CONFIG].writer();
+ writer << ip4config;
+
+ inactive_bearer_properties_[MM_BEARER_PROPERTY_CONNECTED].writer()
+ .append_bool(false);
}
virtual mm1::ModemModem3gppProxyInterface *CreateMM1ModemModem3gppProxy(
@@ -248,21 +241,42 @@
return sim_proxy;
}
virtual DBusPropertiesProxyInterface *CreateDBusPropertiesProxy(
- const std::string &/*path*/,
+ const std::string &path,
const std::string &/*service*/) {
MockDBusPropertiesProxy *properties_proxy =
test_->properties_proxy_.release();
+ if (path.find(kActiveBearerPathPrefix) != std::string::npos) {
+ ON_CALL(*properties_proxy, GetAll(MM_DBUS_INTERFACE_BEARER))
+ .WillByDefault(Return(active_bearer_properties_));
+ } else {
+ ON_CALL(*properties_proxy, GetAll(MM_DBUS_INTERFACE_BEARER))
+ .WillByDefault(Return(inactive_bearer_properties_));
+ }
test_->properties_proxy_.reset(new MockDBusPropertiesProxy());
return properties_proxy;
}
private:
+ DBusPropertiesMap GetActiveBearerProperties(
+ const string &/*interface_name*/) {
+ DBusPropertiesMap properties;
+ properties[MM_BEARER_PROPERTY_CONNECTED].writer().append_bool(true);
+ return properties;
+ }
+
+ DBusPropertiesMap GetInactiveBearerProperties(
+ const string &/*interface_name*/) {
+ DBusPropertiesMap properties;
+ properties[MM_BEARER_PROPERTY_CONNECTED].writer().append_bool(false);
+ return properties;
+ }
+
CellularCapabilityUniversalTest *test_;
- DBusPropertiesMap bearer_ip4config_dhcp_;
+ DBusPropertiesMap active_bearer_properties_;
+ DBusPropertiesMap inactive_bearer_properties_;
};
MockModemInfo modem_info_;
- scoped_ptr<mm1::MockBearerProxy> bearer_proxy_;
scoped_ptr<mm1::MockModemModem3gppProxy> modem_3gpp_proxy_;
scoped_ptr<mm1::MockModemModemCdmaProxy> modem_cdma_proxy_;
scoped_ptr<mm1::MockModemProxy> modem_proxy_;
