NetworkSmsHandler: Use the ModemManager1 dbus interfaces

Adapt NetworkSmsHandler to monitor both the ModemManager and
ModemManager1 dbus interfaces for SMS messages.

BUG=chromium-os:28421
TEST=chromeos_unittests

Change-Id: Ifbbce68192a360240f519608d4bea46c30997493


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

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

chromeos/network/network_sms_handler.cc

@@ -4,13 +4,18 @@
 
 #include "chromeos/network/network_sms_handler.h"
 
+#include <algorithm>
+#include <deque>
 #include <string>
+#include <vector>
 
 #include "base/bind.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "chromeos/dbus/flimflam_device_client.h"
 #include "chromeos/dbus/flimflam_manager_client.h"
 #include "chromeos/dbus/gsm_sms_client.h"
+#include "chromeos/dbus/modem_messaging_client.h"
+#include "chromeos/dbus/sms_client.h"
 #include "dbus/object_path.h"
 #include "third_party/cros_system_api/dbus/service_constants.h"
 
@@ -20,6 +25,11 @@ const char kSmscKey[] = "smsc";
 const char kValidityKey[] = "validity";
 const char kClassKey[] = "class";
 const char kIndexKey[] = "index";
+
+// Keys from ModemManager1
+const char kModemManager1NumberKey[] = "Number";
+const char kModemManager1TextKey[] = "Text";
+const char kModemManager1TimestampKey[] = "Timestamp";
 }  // namespace
 
 namespace chromeos {
@@ -31,9 +41,18 @@ const char NetworkSmsHandler::kTimestampKey[] = "timestamp";
 
 class NetworkSmsHandler::NetworkSmsDeviceHandler {
  public:
-  NetworkSmsDeviceHandler(NetworkSmsHandler* host,
-                          std::string dbus_connection,
-                          dbus::ObjectPath object_path);
+  NetworkSmsDeviceHandler() {}
+  virtual ~NetworkSmsDeviceHandler() {}
+
+  virtual void RequestUpdate() = 0;
+};
+
+class NetworkSmsHandler::ModemManagerNetworkSmsDeviceHandler
+    : public NetworkSmsHandler::NetworkSmsDeviceHandler {
+ public:
+  ModemManagerNetworkSmsDeviceHandler(NetworkSmsHandler* host,
+                                      std::string dbus_connection,
+                                      dbus::ObjectPath object_path);
 
   void RequestUpdate();
 
@@ -48,13 +67,14 @@ class NetworkSmsHandler::NetworkSmsDeviceHandler {
   std::string dbus_connection_;
   dbus::ObjectPath object_path_;
   bool deleting_messages_;
-  base::WeakPtrFactory<NetworkSmsDeviceHandler> weak_ptr_factory_;
+  base::WeakPtrFactory<ModemManagerNetworkSmsDeviceHandler> weak_ptr_factory_;
   std::vector<uint32> delete_queue_;
 
-  DISALLOW_COPY_AND_ASSIGN(NetworkSmsDeviceHandler);
+  DISALLOW_COPY_AND_ASSIGN(ModemManagerNetworkSmsDeviceHandler);
 };
 
-NetworkSmsHandler::NetworkSmsDeviceHandler::NetworkSmsDeviceHandler(
+NetworkSmsHandler::
+ModemManagerNetworkSmsDeviceHandler::ModemManagerNetworkSmsDeviceHandler(
     NetworkSmsHandler* host,
     std::string dbus_connection,
     dbus::ObjectPath object_path)
@@ -66,22 +86,23 @@ NetworkSmsHandler::NetworkSmsDeviceHandler::NetworkSmsDeviceHandler(
   // Set the handler for received Sms messaages.
   DBusThreadManager::Get()->GetGsmSMSClient()->SetSmsReceivedHandler(
       dbus_connection_, object_path_,
-      base::Bind(&NetworkSmsDeviceHandler::SmsReceivedCallback,
+      base::Bind(&ModemManagerNetworkSmsDeviceHandler::SmsReceivedCallback,
                  weak_ptr_factory_.GetWeakPtr()));
 
   // List the existing messages.
   DBusThreadManager::Get()->GetGsmSMSClient()->List(
       dbus_connection_, object_path_,
-      base::Bind(&NetworkSmsDeviceHandler::ListCallback,
+      base::Bind(&NetworkSmsHandler::
+                 ModemManagerNetworkSmsDeviceHandler::ListCallback,
                  weak_ptr_factory_.GetWeakPtr()));
 }
 
-void NetworkSmsHandler::NetworkSmsDeviceHandler::RequestUpdate() {
+void NetworkSmsHandler::ModemManagerNetworkSmsDeviceHandler::RequestUpdate() {
   DBusThreadManager::Get()->GetGsmSMSClient()->RequestUpdate(
       dbus_connection_, object_path_);
 }
 
-void NetworkSmsHandler::NetworkSmsDeviceHandler::ListCallback(
+void NetworkSmsHandler::ModemManagerNetworkSmsDeviceHandler::ListCallback(
     const base::ListValue& message_list) {
   // This receives all messages, so clear any pending deletes.
   delete_queue_.clear();
@@ -98,10 +119,10 @@ void NetworkSmsHandler::NetworkSmsDeviceHandler::ListCallback(
   DeleteMessages();
 }
 
-// Messages must be deleted one at a time, since we can not gaurantee the order
-// the deletion will be executed in. Delete messages from the back of the list
-// so that the indices are valid.
-void NetworkSmsHandler::NetworkSmsDeviceHandler::DeleteMessages() {
+// Messages must be deleted one at a time, since we can not guarantee
+// the order the deletion will be executed in. Delete messages from
+// the back of the list so that the indices are valid.
+void NetworkSmsHandler::ModemManagerNetworkSmsDeviceHandler::DeleteMessages() {
   if (delete_queue_.empty()) {
     deleting_messages_ = false;
     return;
@@ -111,11 +132,13 @@ void NetworkSmsHandler::NetworkSmsDeviceHandler::DeleteMessages() {
   delete_queue_.pop_back();
   DBusThreadManager::Get()->GetGsmSMSClient()->Delete(
       dbus_connection_, object_path_, index,
-      base::Bind(&NetworkSmsDeviceHandler::DeleteMessages,
+      base::Bind(&NetworkSmsHandler::
+                 ModemManagerNetworkSmsDeviceHandler::DeleteMessages,
                  weak_ptr_factory_.GetWeakPtr()));
 }
 
-void NetworkSmsHandler::NetworkSmsDeviceHandler::SmsReceivedCallback(
+void NetworkSmsHandler::
+ModemManagerNetworkSmsDeviceHandler::SmsReceivedCallback(
     uint32 index,
     bool complete) {
   // Only handle complete messages.
@@ -123,11 +146,12 @@ void NetworkSmsHandler::NetworkSmsDeviceHandler::SmsReceivedCallback(
     return;
   DBusThreadManager::Get()->GetGsmSMSClient()->Get(
       dbus_connection_, object_path_, index,
-      base::Bind(&NetworkSmsDeviceHandler::GetCallback,
+      base::Bind(&NetworkSmsHandler::
+                 ModemManagerNetworkSmsDeviceHandler::GetCallback,
                  weak_ptr_factory_.GetWeakPtr(), index));
 }
 
-void NetworkSmsHandler::NetworkSmsDeviceHandler::GetCallback(
+void NetworkSmsHandler::ModemManagerNetworkSmsDeviceHandler::GetCallback(
     uint32 index,
     const base::DictionaryValue& dictionary) {
   NotifyMessageReceived(dictionary);
@@ -136,11 +160,169 @@ void NetworkSmsHandler::NetworkSmsDeviceHandler::GetCallback(
     DeleteMessages();
 }
 
-void NetworkSmsHandler::NetworkSmsDeviceHandler::NotifyMessageReceived(
+void NetworkSmsHandler::
+ModemManagerNetworkSmsDeviceHandler::NotifyMessageReceived(
     const base::DictionaryValue& dictionary) {
+  // The keys of the ModemManager.Modem.Gsm.SMS interface match the
+  // exported keys, so the dictionary used as a notification argument
+  // unchanged.
   host_->NotifyMessageReceived(dictionary);
 }
 
+class NetworkSmsHandler::ModemManager1NetworkSmsDeviceHandler
+    : public NetworkSmsHandler::NetworkSmsDeviceHandler {
+ public:
+  ModemManager1NetworkSmsDeviceHandler(NetworkSmsHandler* host,
+                                       std::string dbus_connection,
+                                       dbus::ObjectPath object_path);
+
+  void RequestUpdate();
+
+ private:
+  void ListCallback(const std::vector<dbus::ObjectPath>& paths);
+  void SmsReceivedCallback(const dbus::ObjectPath& path, bool complete);
+  void GetCallback(const base::DictionaryValue& dictionary);
+  void DeleteMessages();
+  void GetMessages();
+  void NotifyMessageReceived(const base::DictionaryValue& dictionary);
+
+  NetworkSmsHandler* host_;
+  std::string dbus_connection_;
+  dbus::ObjectPath object_path_;
+  bool deleting_messages_;
+  bool retrieving_messages_;
+  base::WeakPtrFactory<ModemManager1NetworkSmsDeviceHandler> weak_ptr_factory_;
+  std::vector<dbus::ObjectPath> delete_queue_;
+  std::deque<dbus::ObjectPath> retrieval_queue_;
+
+  DISALLOW_COPY_AND_ASSIGN(ModemManager1NetworkSmsDeviceHandler);
+};
+
+NetworkSmsHandler::
+ModemManager1NetworkSmsDeviceHandler::ModemManager1NetworkSmsDeviceHandler(
+    NetworkSmsHandler* host,
+    std::string dbus_connection,
+    dbus::ObjectPath object_path)
+    : host_(host),
+      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_connection_, object_path_,
+      base::Bind(
+          &NetworkSmsHandler::
+          ModemManager1NetworkSmsDeviceHandler::SmsReceivedCallback,
+          weak_ptr_factory_.GetWeakPtr()));
+
+  // List the existing messages.
+  DBusThreadManager::Get()->GetModemMessagingClient()->List(
+      dbus_connection_, object_path_,
+      base::Bind(&NetworkSmsHandler::
+                 ModemManager1NetworkSmsDeviceHandler::ListCallback,
+                 weak_ptr_factory_.GetWeakPtr()));
+}
+
+void NetworkSmsHandler::ModemManager1NetworkSmsDeviceHandler::RequestUpdate() {
+  // Calling List using the service "AddSMS" causes the stub
+  // implementation to deliver new sms messages.
+  DBusThreadManager::Get()->GetModemMessagingClient()->List(
+      std::string("AddSMS"), dbus::ObjectPath("/"),
+      base::Bind(&NetworkSmsHandler::
+                 ModemManager1NetworkSmsDeviceHandler::ListCallback,
+                 weak_ptr_factory_.GetWeakPtr()));
+}
+
+void NetworkSmsHandler::ModemManager1NetworkSmsDeviceHandler::ListCallback(
+    const std::vector<dbus::ObjectPath>& paths) {
+  // This receives all messages, so clear any pending gets and deletes.
+  retrieval_queue_.clear();
+  delete_queue_.clear();
+
+  retrieval_queue_.resize(paths.size());
+  std::copy(paths.begin(), paths.end(), retrieval_queue_.begin());
+  if (!retrieving_messages_)
+    GetMessages();
+}
+
+// Messages must be deleted one at a time, since we can not guarantee
+// the order the deletion will be executed in. Delete messages from
+// the back of the list so that the indices are valid.
+void NetworkSmsHandler::ModemManager1NetworkSmsDeviceHandler::DeleteMessages() {
+  if (delete_queue_.empty()) {
+    deleting_messages_ = false;
+    return;
+  }
+  deleting_messages_ = true;
+  dbus::ObjectPath sms_path = delete_queue_.back();
+  delete_queue_.pop_back();
+  DBusThreadManager::Get()->GetModemMessagingClient()->Delete(
+      dbus_connection_, object_path_, sms_path,
+      base::Bind(&NetworkSmsHandler::
+                 ModemManager1NetworkSmsDeviceHandler::DeleteMessages,
+                 weak_ptr_factory_.GetWeakPtr()));
+}
+
+// Messages must be fetched one at a time, so that we do not queue too
+// many requests to a single threaded server.
+void NetworkSmsHandler::ModemManager1NetworkSmsDeviceHandler::GetMessages() {
+  if (retrieval_queue_.empty()) {
+    retrieving_messages_ = false;
+    if (!deleting_messages_)
+      DeleteMessages();
+    return;
+  }
+  retrieving_messages_ = true;
+  dbus::ObjectPath sms_path = retrieval_queue_.front();
+  retrieval_queue_.pop_front();
+  DBusThreadManager::Get()->GetSMSClient()->GetAll(
+      dbus_connection_, sms_path,
+      base::Bind(&NetworkSmsHandler::
+                 ModemManager1NetworkSmsDeviceHandler::GetCallback,
+                 weak_ptr_factory_.GetWeakPtr()));
+  delete_queue_.push_back(sms_path);
+}
+
+void NetworkSmsHandler::
+ModemManager1NetworkSmsDeviceHandler::SmsReceivedCallback(
+    const dbus::ObjectPath& sms_path,
+    bool complete) {
+  // Only handle complete messages.
+  if (!complete)
+    return;
+  retrieval_queue_.push_back(sms_path);
+  if (!retrieving_messages_)
+    GetMessages();
+}
+
+void NetworkSmsHandler::ModemManager1NetworkSmsDeviceHandler::GetCallback(
+    const base::DictionaryValue& dictionary) {
+  NotifyMessageReceived(dictionary);
+  GetMessages();
+}
+
+void NetworkSmsHandler::
+ModemManager1NetworkSmsDeviceHandler::NotifyMessageReceived(
+    const base::DictionaryValue& dictionary) {
+  // The keys of the ModemManager1.SMS interface do not match the
+  // exported keys, so a new dictionary is created with the expected
+  // key namaes.
+  base::DictionaryValue new_dictionary;
+  std::string text, number, timestamp;
+  if (dictionary.GetStringWithoutPathExpansion(kModemManager1NumberKey,
+                                               &number))
+    new_dictionary.SetString(kNumberKey, number);
+  if (dictionary.GetStringWithoutPathExpansion(kModemManager1TextKey, &text))
+    new_dictionary.SetString(kTextKey, text);
+  // TODO(jglasgow): consider normalizing timestamp.
+  if (dictionary.GetStringWithoutPathExpansion(kModemManager1TimestampKey,
+                                               &timestamp))
+    new_dictionary.SetString(kTimestampKey, timestamp);
+  host_->NotifyMessageReceived(new_dictionary);
+}
+
 ///////////////////////////////////////////////////////////////////////////////
 // NetworkSmsHandler
 
@@ -188,7 +370,7 @@ void NetworkSmsHandler::ManagerPropertiesCallback(
   base::Value* value;
   if (!properties.GetWithoutPathExpansion(flimflam::kDevicesProperty, &value) ||
       value->GetType() != base::Value::TYPE_LIST) {
-    LOG(ERROR) << "NetworkSmsDeviceHandler: No list value for: "
+    LOG(ERROR) << "NetworkSmsHandler: No list value for: "
                << flimflam::kDevicesProperty;
     return;
   }
@@ -218,7 +400,7 @@ void NetworkSmsHandler::DevicePropertiesCallback(
   std::string device_type;
   if (!properties.GetStringWithoutPathExpansion(
           flimflam::kTypeProperty, &device_type)) {
-    LOG(ERROR) << "NetworkSmsDeviceHandler: No type for: " << device_path;
+    LOG(ERROR) << "NetworkSmsHandler: No type for: " << device_path;
     return;
   }
   if (device_type != flimflam::kTypeCellular)
@@ -238,8 +420,17 @@ void NetworkSmsHandler::DevicePropertiesCallback(
     return;
   }
   dbus::ObjectPath object_path(object_path_string);
-  device_handlers_.push_back(
-      new NetworkSmsDeviceHandler(this, dbus_connection, object_path));
+  if (object_path_string.compare(
+          0, sizeof(modemmanager::kModemManager1ServicePath) - 1,
+          modemmanager::kModemManager1ServicePath) == 0) {
+    device_handlers_.push_back(
+        new ModemManager1NetworkSmsDeviceHandler(
+            this, dbus_connection, object_path));
+  } else {
+    device_handlers_.push_back(
+        new ModemManagerNetworkSmsDeviceHandler(
+            this, dbus_connection, object_path));
+  }
 }
 
 

chromeos/network/network_sms_handler.h

@@ -5,6 +5,8 @@
 #ifndef CHROMEOS_NETWORK_NETWORK_SMS_HANDLER_H_
 #define CHROMEOS_NETWORK_NETWORK_SMS_HANDLER_H_
 
+#include <string>
+
 #include "base/memory/scoped_vector.h"
 #include "base/memory/weak_ptr.h"
 #include "base/observer_list.h"
@@ -46,6 +48,8 @@ class CHROMEOS_EXPORT NetworkSmsHandler {
 
  private:
   class NetworkSmsDeviceHandler;
+  class ModemManagerNetworkSmsDeviceHandler;
+  class ModemManager1NetworkSmsDeviceHandler;
 
   // Called from NetworkSmsDeviceHandler when a message is received.
   void NotifyMessageReceived(const base::DictionaryValue& message);

chromeos/network/network_sms_handler_unittest.cc

@@ -4,6 +4,9 @@
 
 #include "chromeos/network/network_sms_handler.h"
 
+#include <set>
+#include <string>
+
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
@@ -15,17 +18,28 @@ namespace {
 
 class TestObserver : public NetworkSmsHandler::Observer {
  public:
-  TestObserver() : message_count_(0) {}
+  TestObserver() {}
   virtual ~TestObserver() {}
 
   virtual void MessageReceived(const base::DictionaryValue& message) OVERRIDE {
-    ++message_count_;
+    std::string text;
+    if (message.GetStringWithoutPathExpansion(
+            NetworkSmsHandler::kTextKey, &text)) {
+      messages_.insert(text);
+    }
+  }
+
+  void ClearMessages() {
+    messages_.clear();
   }
 
-  int message_count() { return message_count_; }
+  int message_count() { return messages_.size(); }
+  const std::set<std::string>& messages() const {
+    return messages_;
+  }
 
  private:
-  int message_count_;
+  std::set<std::string> messages_;
 };
 
 }  // namespace
@@ -50,17 +64,29 @@ class NetworkSmsHandlerTest : public testing::Test {
 
 TEST_F(NetworkSmsHandlerTest, SmsHandlerDbusStub) {
   // This relies on the stub dbus implementations for FlimflamManagerClient,
-  // FlimflamDeviceClient, and GsmSMSClient.
-  // Initialize a sms handler. The stub dbus clients will send the first test
-  // message when Gsm.SMS.List is called in NetworkSmsHandler::Init.
+  // FlimflamDeviceClient, GsmSMSClient, ModemMessagingClient and SMSClient.
+  // Initialize a sms handler. The stub dbus clients will not send the
+  // first test message until RequestUpdate has been called.
   scoped_ptr<NetworkSmsHandler> sms_handler(new NetworkSmsHandler());
   scoped_ptr<TestObserver> test_observer(new TestObserver());
   sms_handler->AddObserver(test_observer.get());
   sms_handler->Init();
   message_loop_.RunAllPending();
+  EXPECT_EQ(test_observer->message_count(), 0);
+
+  // Test that no messages have been received yet
+  const std::set<std::string>& messages(test_observer->messages());
+  // Note: The following string corresponds to values in
+  // ModemMessagingClientStubImpl and SmsClientStubImpl.
+  const char kMessage1[] = "SMSClientStubImpl: Test Message: /SMS/0";
+  EXPECT_EQ(messages.find(kMessage1), messages.end());
+
+  // Test for messages delivered by signals.
+  test_observer->ClearMessages();
   sms_handler->RequestUpdate();
   message_loop_.RunAllPending();
   EXPECT_GE(test_observer->message_count(), 1);
+  EXPECT_NE(messages.find(kMessage1), messages.end());
 }
 
 }  // namespace chromeos