commit b6b6ccbfee5a04be973a67c0360bf35bb25d9da0
author David Rochberg <rochberg@chromium.org> Fri Mar 16 13:40:36 2012 -0400
committer Gerrit <chrome-bot@google.com> Mon Mar 19 09:25:41 2012 -0700
tree 166b1cb0353892f2f1ad00de2180820752974794
parent 273028abb353311da8b63c86ae8d5dbb6638afea

Remove indirection task for creating a modem object.

Creating a modem object causes a DBus call (to register the object on
the bus).  This used to cause a deadlock, but ers fixed the deadlock,
so now we don't need the indirection.

BUG=chromium-os:27936
TEST=creates a cromo modem

Change-Id: I226248001989781c6098cc620b30584aabb59e4c
Reviewed-on: https://gerrit.chromium.org/gerrit/18399
Reviewed-by: Eric Shienbrood <ers@chromium.org>
Tested-by: David Rochberg <rochberg@chromium.org>
Commit-Ready: David Rochberg <rochberg@chromium.org>

modem.cc

diff --git a/modem.cc b/modem.cc
index b36b5c6..7b938ed 100644
--- a/modem.cc
+++ b/modem.cc
@@ -53,16 +53,6 @@
 void Modem::Init() {
   VLOG(2) << __func__;
   CHECK(!device_.get());
-
-  // Defer device creation because dbus-c++ doesn't allow registration of new
-  // D-Bus objects in the context of a D-Bus signal handler.
-  dispatcher_->PostTask(Bind(&Modem::InitTask, AsWeakPtr()));
-}
-
-void Modem::InitTask() {
-  VLOG(2) << __func__;
-  CHECK(!device_.get());
-
   dbus_properties_proxy_.reset(
       proxy_factory_->CreateDBusPropertiesProxy(this, path_, owner_));
   CreateDevice();

modem.h

diff --git a/modem.h b/modem.h
index df031d6..e92ba88 100644
--- a/modem.h
+++ b/modem.h
@@ -28,8 +28,7 @@
 
 // Handles an instance of ModemManager.Modem and an instance of a Cellular
 // device.
-class Modem : public DBusPropertiesProxyDelegate,
-              public base::SupportsWeakPtr<Modem> {
+class Modem : public DBusPropertiesProxyDelegate {
  public:
   // |owner| is the ModemManager DBus service owner (e.g., ":1.17"). |path| is
   // the ModemManager.Modem DBus object path (e.g.,
@@ -61,8 +60,6 @@
   static const char kPropertyState[];
   static const char kPropertyType[];
 
-  void InitTask();
-
   // Creates and registers a Cellular device in |device_| based on
   // ModemManager.Modem's |properties|. The device may not be created if the
   // properties are invalid.

modem_manager_unittest.cc

diff --git a/modem_manager_unittest.cc b/modem_manager_unittest.cc
index d48f4f3..22ba368 100644
--- a/modem_manager_unittest.cc
+++ b/modem_manager_unittest.cc
@@ -22,6 +22,7 @@
 using std::vector;
 using testing::_;
 using testing::Invoke;
+using testing::Pointee;
 using testing::Return;
 using testing::StrEq;
 using testing::Test;
@@ -164,6 +165,26 @@
   EXPECT_EQ(0, modem_manager_.modems_.size());
 }
 
+class ModemManagerClassicMockInit : public ModemManagerClassic {
+ public:
+  ModemManagerClassicMockInit(const string &service,
+                              const string &path,
+                              ControlInterface *control_interface,
+                              EventDispatcher *dispatcher,
+                              Metrics *metrics,
+                              Manager *manager,
+                              GLib *glib,
+                              mobile_provider_db *provider_db) :
+      ModemManagerClassic(service,
+                          path,
+                          control_interface,
+                          dispatcher,
+                          metrics,
+                          manager,
+                          glib,
+                          provider_db) {}
+  MOCK_METHOD1(InitModem, void(shared_ptr<Modem> modem));
+};
 
 class ModemManagerClassicTest : public ModemManagerTest {
  public:
@@ -205,7 +226,7 @@
     modem_manager_.proxy_factory_ = NULL;
   }
 
-  ModemManagerClassic modem_manager_;
+  ModemManagerClassicMockInit modem_manager_;
   scoped_ptr<MockModemManagerProxy> proxy_;
   TestProxyFactory proxy_factory_;
 };
@@ -216,6 +237,9 @@
   EXPECT_CALL(*proxy_, EnumerateDevices())
       .WillOnce(Return(vector<DBus::Path>(1, kModemPath)));
 
+  EXPECT_CALL(modem_manager_,
+              InitModem(Pointee(Field(&Modem::path_, StrEq(kModemPath)))));
+
   modem_manager_.Connect(kOwner);
   EXPECT_EQ(kOwner, modem_manager_.owner_);
   EXPECT_EQ(1, modem_manager_.modems_.size());

modem_unittest.cc

diff --git a/modem_unittest.cc b/modem_unittest.cc
index d3a6c98..a716e74 100644
--- a/modem_unittest.cc
+++ b/modem_unittest.cc
@@ -120,20 +120,6 @@
   SetSockets(NULL);
 }
 
-TEST_F(ModemTest, Init) {
-  DBusPropertiesMap props;
-
-  SetSockets(&sockets_);
-  props[Modem::kPropertyIPMethod].writer().append_uint32(
-      MM_MODEM_IP_METHOD_DHCP);
-  props[Modem::kPropertyLinkName].writer().append_string("usb1");
-  EXPECT_CALL(*proxy_, GetAll(MM_MODEM_INTERFACE)).WillOnce(Return(props));
-  modem_->Init();
-
-  EXPECT_CALL(sockets_, Socket(PF_INET, SOCK_DGRAM, 0)).WillOnce(Return(-1));
-  dispatcher_.DispatchPendingEvents();
-}
-
 TEST_F(ModemTest, CreateDeviceFromProperties) {
   DBusPropertiesMap props;