Revert 142765 - Use DBus.ServiceProperty to watch for SMS signals

Use DBus.ServiceProperty instead of DBus.ConnectionProperty to watch
for SMS signals from ModemManager. The allows Chrome to continue
monitoring for signals even if ModemManager is restarted.

Contributed by jglasgow@chromium.org

Requires updates to shill in https://gerrit.chromium.org/gerrit/#change,24613

BUG=chromium-os:31575
TEST=unittests, run /usr/local/lib/flimflam/test/fake-modem

Change-Id: Ia811eff852ac02328e1b105e2378ce4400c60315


Review URL: https://chromiumcodereview.appspot.com/10536104

TBR=jglasgow@chromium.org
Review URL: https://chromiumcodereview.appspot.com/10563031

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@142780 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 45 insertions, 56 deletions

diff --git a/chrome/browser/chromeos/cros/cros_network_functions_unittest.cc b/chrome/browser/chromeos/cros/cros_network_functions_unittest.cc
index e103b33..e2fc342 100644
--- a/chrome/browser/chromeos/cros/cros_network_functions_unittest.cc
+++ b/chrome/browser/chromeos/cros/cros_network_functions_unittest.cc
@@ -1260,16 +1260,12 @@ TEST_F(CrosNetworkFunctionsTest, CrosMonitorCellularDataPlan) {
 
 TEST_F(CrosNetworkFunctionsTest, CrosMonitorSMS) {
   const std::string dbus_connection = ":1.1";
-  const std::string dbus_service_name = "org.freedesktop.ModemManager";
   const dbus::ObjectPath object_path("/object/path");
   base::DictionaryValue device_properties;
   device_properties.SetWithoutPathExpansion(
       flimflam::kDBusConnectionProperty,
       base::Value::CreateStringValue(dbus_connection));
   device_properties.SetWithoutPathExpansion(
-      flimflam::kDBusServiceProperty,
-      base::Value::CreateStringValue(dbus_service_name));
-  device_properties.SetWithoutPathExpansion(
       flimflam::kDBusObjectProperty,
       base::Value::CreateStringValue(object_path.value()));
 
diff --git a/chrome/browser/chromeos/cros/sms_watcher.cc b/chrome/browser/chromeos/cros/sms_watcher.cc
index f2c94c9..d7327d0 100644
--- a/chrome/browser/chromeos/cros/sms_watcher.cc
+++ b/chrome/browser/chromeos/cros/sms_watcher.cc
@@ -72,12 +72,12 @@ class SMSWatcher::WatcherBase {
   WatcherBase(const std::string& device_path,
               MonitorSMSCallback callback,
               void* object,
-              const std::string& dbus_service_name,
+              const std::string& dbus_connection,
               const dbus::ObjectPath& object_path) :
       device_path_(device_path),
       callback_(callback),
       object_(object),
-      dbus_service_name_(dbus_service_name),
+      dbus_connection_(dbus_connection),
       object_path_(object_path) {}
 
   virtual ~WatcherBase() {}
@@ -86,7 +86,7 @@ class SMSWatcher::WatcherBase {
   const std::string device_path_;
   MonitorSMSCallback callback_;
   void* object_;
-  const std::string dbus_service_name_;
+  const std::string dbus_connection_;
   const dbus::ObjectPath object_path_;
 
   DISALLOW_COPY_AND_ASSIGN(WatcherBase);
@@ -99,25 +99,25 @@ class GsmWatcher : public SMSWatcher::WatcherBase {
   GsmWatcher(const std::string& device_path,
              MonitorSMSCallback callback,
              void* object,
-             const std::string& dbus_service_name,
+             const std::string& dbus_connection,
              const dbus::ObjectPath& object_path)
-      : WatcherBase(device_path, callback, object, dbus_service_name,
+      : WatcherBase(device_path, callback, object, dbus_connection,
                     object_path),
         weak_ptr_factory_(this) {
     DBusThreadManager::Get()->GetGsmSMSClient()->SetSmsReceivedHandler(
-        dbus_service_name_,
+        dbus_connection_,
         object_path_,
         base::Bind(&GsmWatcher::OnSmsReceived, weak_ptr_factory_.GetWeakPtr()));
 
     DBusThreadManager::Get()->GetGsmSMSClient()->List(
-        dbus_service_name_, object_path_,
+        dbus_connection_, object_path_,
         base::Bind(&GsmWatcher::ListSMSCallback,
                    weak_ptr_factory_.GetWeakPtr()));
   }
 
   virtual ~GsmWatcher() {
     DBusThreadManager::Get()->GetGsmSMSClient()->ResetSmsReceivedHandler(
-        dbus_service_name_, object_path_);
+        dbus_connection_, object_path_);
   }
 
  private:
@@ -127,7 +127,7 @@ class GsmWatcher : public SMSWatcher::WatcherBase {
     if (!complete)
       return;
     DBusThreadManager::Get()->GetGsmSMSClient()->Get(
-        dbus_service_name_, object_path_, index,
+        dbus_connection_, object_path_, index,
         base::Bind(&GsmWatcher::GetSMSCallback,
                    weak_ptr_factory_.GetWeakPtr(),
                    index));
@@ -191,8 +191,7 @@ class GsmWatcher : public SMSWatcher::WatcherBase {
     RunCallbackWithSMS(sms_dictionary);
 
     DBusThreadManager::Get()->GetGsmSMSClient()->Delete(
-        dbus_service_name_, object_path_, index,
-        base::Bind(&DeleteSMSCallback));
+        dbus_connection_, object_path_, index, base::Bind(&DeleteSMSCallback));
   }
 
   // Callback for List() method.
@@ -219,7 +218,7 @@ class GsmWatcher : public SMSWatcher::WatcherBase {
   void DeleteSMSInChain() {
     if (!delete_queue_.empty()) {
       DBusThreadManager::Get()->GetGsmSMSClient()->Delete(
-          dbus_service_name_, object_path_, delete_queue_.back(),
+          dbus_connection_, object_path_, delete_queue_.back(),
           base::Bind(&GsmWatcher::DeleteSMSInChain,
                      weak_ptr_factory_.GetWeakPtr()));
       delete_queue_.pop_back();
@@ -237,28 +236,28 @@ class ModemManager1Watcher : public SMSWatcher::WatcherBase {
   ModemManager1Watcher(const std::string& device_path,
                        MonitorSMSCallback callback,
                        void *object,
-                       const std::string& dbus_service_name,
+                       const std::string& dbus_connection,
                        const dbus::ObjectPath& object_path)
-      : WatcherBase(device_path, callback, object, dbus_service_name,
+      : WatcherBase(device_path, callback, object, dbus_connection,
                     object_path),
         weak_ptr_factory_(this),
         deleting_messages_(false),
         retrieving_messages_(false) {
     DBusThreadManager::Get()->GetModemMessagingClient()->SetSmsReceivedHandler(
-        dbus_service_name_,
+        dbus_connection_,
         object_path_,
         base::Bind(&ModemManager1Watcher::OnSmsReceived,
                    weak_ptr_factory_.GetWeakPtr()));
 
     DBusThreadManager::Get()->GetModemMessagingClient()->List(
-        dbus_service_name_, object_path_,
+        dbus_connection_, object_path_,
         base::Bind(&ModemManager1Watcher::ListSMSCallback,
                    weak_ptr_factory_.GetWeakPtr()));
   }
 
   virtual ~ModemManager1Watcher() {
     DBusThreadManager::Get()->GetModemMessagingClient()
-        ->ResetSmsReceivedHandler(dbus_service_name_, object_path_);
+        ->ResetSmsReceivedHandler(dbus_connection_, object_path_);
   }
 
  private:
@@ -287,7 +286,7 @@ class ModemManager1Watcher : public SMSWatcher::WatcherBase {
     dbus::ObjectPath sms_path = delete_queue_.back();
     delete_queue_.pop_back();
     DBusThreadManager::Get()->GetModemMessagingClient()->Delete(
-        dbus_service_name_, object_path_, sms_path,
+        dbus_connection_, object_path_, sms_path,
         base::Bind(&ModemManager1Watcher::DeleteMessages,
                    weak_ptr_factory_.GetWeakPtr()));
   }
@@ -305,7 +304,7 @@ class ModemManager1Watcher : public SMSWatcher::WatcherBase {
     dbus::ObjectPath sms_path = retrieval_queue_.front();
     retrieval_queue_.pop_front();
     DBusThreadManager::Get()->GetSMSClient()->GetAll(
-        dbus_service_name_, sms_path,
+        dbus_connection_, sms_path,
         base::Bind(&ModemManager1Watcher::GetCallback,
                    weak_ptr_factory_.GetWeakPtr()));
     delete_queue_.push_back(sms_path);
@@ -411,10 +410,10 @@ void SMSWatcher::DevicePropertiesCallback(
   if (call_status != DBUS_METHOD_CALL_SUCCESS)
     return;
 
-  std::string dbus_service_name;
+  std::string dbus_connection;
   if (!properties.GetStringWithoutPathExpansion(
-          flimflam::kDBusServiceProperty, &dbus_service_name)) {
-    LOG(WARNING) << "Modem device properties do not include DBus service.";
+          flimflam::kDBusConnectionProperty, &dbus_connection)) {
+    LOG(WARNING) << "Modem device properties do not include DBus connection.";
     return;
   }
 
@@ -430,12 +429,12 @@ void SMSWatcher::DevicePropertiesCallback(
           modemmanager::kModemManager1ServicePath) == 0) {
     watcher_.reset(
         new ModemManager1Watcher(device_path_,
-                                 callback_, object_, dbus_service_name,
+                                 callback_, object_, dbus_connection,
                                  dbus::ObjectPath(object_path_string)));
   } else {
     watcher_.reset(
         new GsmWatcher(device_path_,
-                       callback_, object_, dbus_service_name,
+                       callback_, object_, dbus_connection,
                        dbus::ObjectPath(object_path_string)));
   }
 }
diff --git a/chromeos/dbus/flimflam_device_client.cc b/chromeos/dbus/flimflam_device_client.cc
index efee09b..f16189f 100644
--- a/chromeos/dbus/flimflam_device_client.cc
+++ b/chromeos/dbus/flimflam_device_client.cc
@@ -222,9 +222,6 @@ class FlimflamDeviceClientStubImpl : public FlimflamDeviceClient {
         flimflam::kDBusConnectionProperty,
         base::Value::CreateStringValue("/stub"));
     cellular_properties->SetWithoutPathExpansion(
-        flimflam::kDBusServiceProperty,
-        base::Value::CreateStringValue("org.freedesktop.ModemManager"));
-    cellular_properties->SetWithoutPathExpansion(
         flimflam::kDBusObjectProperty,
         base::Value::CreateStringValue("/device/cellular1"));
     stub_devices_.Set(kStubCellular1, cellular_properties);
@@ -241,9 +238,6 @@ class FlimflamDeviceClientStubImpl : public FlimflamDeviceClient {
         flimflam::kDBusConnectionProperty,
         base::Value::CreateStringValue(":stub.0"));
     cellular_properties->SetWithoutPathExpansion(
-        flimflam::kDBusServiceProperty,
-        base::Value::CreateStringValue("org.freedesktop.ModemManager1"));
-    cellular_properties->SetWithoutPathExpansion(
         flimflam::kDBusObjectProperty,
         base::Value::CreateStringValue(
                 "/org/freedesktop/ModemManager1/stub/0"));
diff --git a/chromeos/network/network_sms_handler.cc b/chromeos/network/network_sms_handler.cc
index aa3ef5d..72d188a 100644
--- a/chromeos/network/network_sms_handler.cc
+++ b/chromeos/network/network_sms_handler.cc
@@ -51,7 +51,7 @@ class NetworkSmsHandler::ModemManagerNetworkSmsDeviceHandler
     : public NetworkSmsHandler::NetworkSmsDeviceHandler {
  public:
   ModemManagerNetworkSmsDeviceHandler(NetworkSmsHandler* host,
-                                      std::string dbus_service_name,
+                                      std::string dbus_connection,
                                       dbus::ObjectPath object_path);
 
   void RequestUpdate();
@@ -64,7 +64,7 @@ class NetworkSmsHandler::ModemManagerNetworkSmsDeviceHandler
   void NotifyMessageReceived(const base::DictionaryValue& dictionary);
 
   NetworkSmsHandler* host_;
-  std::string dbus_service_name_;
+  std::string dbus_connection_;
   dbus::ObjectPath object_path_;
   bool deleting_messages_;
   base::WeakPtrFactory<ModemManagerNetworkSmsDeviceHandler> weak_ptr_factory_;
@@ -76,22 +76,22 @@ class NetworkSmsHandler::ModemManagerNetworkSmsDeviceHandler
 NetworkSmsHandler::
 ModemManagerNetworkSmsDeviceHandler::ModemManagerNetworkSmsDeviceHandler(
     NetworkSmsHandler* host,
-    std::string dbus_service_name,
+    std::string dbus_connection,
     dbus::ObjectPath object_path)
     : host_(host),
-      dbus_service_name_(dbus_service_name),
+      dbus_connection_(dbus_connection),
       object_path_(object_path),
       deleting_messages_(false),
       weak_ptr_factory_(this) {
   // Set the handler for received Sms messaages.
   DBusThreadManager::Get()->GetGsmSMSClient()->SetSmsReceivedHandler(
-      dbus_service_name_, object_path_,
+      dbus_connection_, object_path_,
       base::Bind(&ModemManagerNetworkSmsDeviceHandler::SmsReceivedCallback,
                  weak_ptr_factory_.GetWeakPtr()));
 
   // List the existing messages.
   DBusThreadManager::Get()->GetGsmSMSClient()->List(
-      dbus_service_name_, object_path_,
+      dbus_connection_, object_path_,
       base::Bind(&NetworkSmsHandler::
                  ModemManagerNetworkSmsDeviceHandler::ListCallback,
                  weak_ptr_factory_.GetWeakPtr()));
@@ -99,7 +99,7 @@ ModemManagerNetworkSmsDeviceHandler::ModemManagerNetworkSmsDeviceHandler(
 
 void NetworkSmsHandler::ModemManagerNetworkSmsDeviceHandler::RequestUpdate() {
   DBusThreadManager::Get()->GetGsmSMSClient()->RequestUpdate(
-      dbus_service_name_, object_path_);
+      dbus_connection_, object_path_);
 }
 
 void NetworkSmsHandler::ModemManagerNetworkSmsDeviceHandler::ListCallback(
@@ -131,7 +131,7 @@ void NetworkSmsHandler::ModemManagerNetworkSmsDeviceHandler::DeleteMessages() {
   uint32 index = delete_queue_.back();
   delete_queue_.pop_back();
   DBusThreadManager::Get()->GetGsmSMSClient()->Delete(
-      dbus_service_name_, object_path_, index,
+      dbus_connection_, object_path_, index,
       base::Bind(&NetworkSmsHandler::
                  ModemManagerNetworkSmsDeviceHandler::DeleteMessages,
                  weak_ptr_factory_.GetWeakPtr()));
@@ -145,7 +145,7 @@ ModemManagerNetworkSmsDeviceHandler::SmsReceivedCallback(
   if (!complete)
     return;
   DBusThreadManager::Get()->GetGsmSMSClient()->Get(
-      dbus_service_name_, object_path_, index,
+      dbus_connection_, object_path_, index,
       base::Bind(&NetworkSmsHandler::
                  ModemManagerNetworkSmsDeviceHandler::GetCallback,
                  weak_ptr_factory_.GetWeakPtr(), index));
@@ -173,7 +173,7 @@ class NetworkSmsHandler::ModemManager1NetworkSmsDeviceHandler
     : public NetworkSmsHandler::NetworkSmsDeviceHandler {
  public:
   ModemManager1NetworkSmsDeviceHandler(NetworkSmsHandler* host,
-                                       std::string dbus_service_name,
+                                       std::string dbus_connection,
                                        dbus::ObjectPath object_path);
 
   void RequestUpdate();
@@ -187,7 +187,7 @@ class NetworkSmsHandler::ModemManager1NetworkSmsDeviceHandler
   void NotifyMessageReceived(const base::DictionaryValue& dictionary);
 
   NetworkSmsHandler* host_;
-  std::string dbus_service_name_;
+  std::string dbus_connection_;
   dbus::ObjectPath object_path_;
   bool deleting_messages_;
   bool retrieving_messages_;
@@ -201,17 +201,17 @@ class NetworkSmsHandler::ModemManager1NetworkSmsDeviceHandler
 NetworkSmsHandler::
 ModemManager1NetworkSmsDeviceHandler::ModemManager1NetworkSmsDeviceHandler(
     NetworkSmsHandler* host,
-    std::string dbus_service_name,
+    std::string dbus_connection,
     dbus::ObjectPath object_path)
     : host_(host),
-      dbus_service_name_(dbus_service_name),
+      dbus_connection_(dbus_connection),
       object_path_(object_path),
       deleting_messages_(false),
       retrieving_messages_(false),
       weak_ptr_factory_(this) {
   // Set the handler for received Sms messaages.
   DBusThreadManager::Get()->GetModemMessagingClient()->SetSmsReceivedHandler(
-      dbus_service_name_, object_path_,
+      dbus_connection_, object_path_,
       base::Bind(
           &NetworkSmsHandler::
           ModemManager1NetworkSmsDeviceHandler::SmsReceivedCallback,
@@ -219,7 +219,7 @@ ModemManager1NetworkSmsDeviceHandler::ModemManager1NetworkSmsDeviceHandler(
 
   // List the existing messages.
   DBusThreadManager::Get()->GetModemMessagingClient()->List(
-      dbus_service_name_, object_path_,
+      dbus_connection_, object_path_,
       base::Bind(&NetworkSmsHandler::
                  ModemManager1NetworkSmsDeviceHandler::ListCallback,
                  weak_ptr_factory_.GetWeakPtr()));
@@ -259,7 +259,7 @@ void NetworkSmsHandler::ModemManager1NetworkSmsDeviceHandler::DeleteMessages() {
   dbus::ObjectPath sms_path = delete_queue_.back();
   delete_queue_.pop_back();
   DBusThreadManager::Get()->GetModemMessagingClient()->Delete(
-      dbus_service_name_, object_path_, sms_path,
+      dbus_connection_, object_path_, sms_path,
       base::Bind(&NetworkSmsHandler::
                  ModemManager1NetworkSmsDeviceHandler::DeleteMessages,
                  weak_ptr_factory_.GetWeakPtr()));
@@ -278,7 +278,7 @@ void NetworkSmsHandler::ModemManager1NetworkSmsDeviceHandler::GetMessages() {
   dbus::ObjectPath sms_path = retrieval_queue_.front();
   retrieval_queue_.pop_front();
   DBusThreadManager::Get()->GetSMSClient()->GetAll(
-      dbus_service_name_, sms_path,
+      dbus_connection_, sms_path,
       base::Bind(&NetworkSmsHandler::
                  ModemManager1NetworkSmsDeviceHandler::GetCallback,
                  weak_ptr_factory_.GetWeakPtr()));
@@ -406,10 +406,10 @@ void NetworkSmsHandler::DevicePropertiesCallback(
   if (device_type != flimflam::kTypeCellular)
     return;
 
-  std::string dbus_service_name;
+  std::string dbus_connection;
   if (!properties.GetStringWithoutPathExpansion(
-          flimflam::kDBusServiceProperty, &dbus_service_name)) {
-    LOG(ERROR) << "Device has no DBusService Property: " << device_path;
+          flimflam::kDBusConnectionProperty, &dbus_connection)) {
+    LOG(ERROR) << "Device has no DBusConnection Property: " << device_path;
     return;
   }
 
@@ -425,11 +425,11 @@ void NetworkSmsHandler::DevicePropertiesCallback(
           modemmanager::kModemManager1ServicePath) == 0) {
     device_handlers_.push_back(
         new ModemManager1NetworkSmsDeviceHandler(
-            this, dbus_service_name, object_path));
+            this, dbus_connection, object_path));
   } else {
     device_handlers_.push_back(
         new ModemManagerNetworkSmsDeviceHandler(
-            this, dbus_service_name, object_path));
+            this, dbus_connection, object_path));
   }
 }