dbus::ObjectManager: Add a match rule for properties before GetManagedObjects.

There is a race condition in the way that match rules get set up for object
proxies created in response to GetManagedObjects that may cause us the miss
PropertiesChanged signals if they're received before the match rule and filter
function get added by ObjectProxy.

This patch changes this to work the "intended" way: ObjectManager now adds a
single match rule for its corresponding service name, and specifically for the
org.freedesktop.DBus.Properties.PropertiesChanged signal. Once it receives the
signal, ObjectManager dispatches the signal to the corresponding PropertySet.

BUG=407109,400768
TEST=dbus_unittests

Review URL: https://codereview.chromium.org/510863002

Cr-Commit-Position: refs/heads/master@{#293551}

13 files changed, 453 insertions, 53 deletions

diff --git a/dbus/BUILD.gn b/dbus/BUILD.gn
index c1694b0..a17f1f1 100644
--- a/dbus/BUILD.gn
+++ b/dbus/BUILD.gn
@@ -30,6 +30,8 @@ component("dbus") {
     "string_util.h",
     "values_util.cc",
     "values_util.h",
+    "util.cc",
+    "util.h",
   ]
 
   defines = [
diff --git a/dbus/bus.cc b/dbus/bus.cc
index 28257f8..3cad3c8 100644
--- a/dbus/bus.cc
+++ b/dbus/bus.cc
@@ -267,7 +267,7 @@ bool Bus::RemoveObjectProxyWithOptions(const std::string& service_name,
   if (iter != object_proxy_table_.end()) {
     scoped_refptr<ObjectProxy> object_proxy = iter->second;
     object_proxy_table_.erase(iter);
-    // Object is present. Remove it now and Detach in the DBus thread.
+    // Object is present. Remove it now and Detach on the DBus thread.
     GetDBusTaskRunner()->PostTask(
         FROM_HERE,
         base::Bind(&Bus::RemoveObjectProxyInternal,
@@ -350,17 +350,54 @@ ObjectManager* Bus::GetObjectManager(const std::string& service_name,
   return object_manager.get();
 }
 
-void Bus::RemoveObjectManager(const std::string& service_name,
-                              const ObjectPath& object_path) {
+bool Bus::RemoveObjectManager(const std::string& service_name,
+                              const ObjectPath& object_path,
+                              const base::Closure& callback) {
   AssertOnOriginThread();
+  DCHECK(!callback.is_null());
 
   const ObjectManagerTable::key_type key(service_name + object_path.value());
   ObjectManagerTable::iterator iter = object_manager_table_.find(key);
   if (iter == object_manager_table_.end())
-    return;
+    return false;
 
+  // ObjectManager is present. Remove it now and CleanUp on the DBus thread.
   scoped_refptr<ObjectManager> object_manager = iter->second;
   object_manager_table_.erase(iter);
+
+  GetDBusTaskRunner()->PostTask(
+      FROM_HERE,
+      base::Bind(&Bus::RemoveObjectManagerInternal,
+                 this, object_manager, callback));
+
+  return true;
+}
+
+void Bus::RemoveObjectManagerInternal(
+      scoped_refptr<dbus::ObjectManager> object_manager,
+      const base::Closure& callback) {
+  AssertOnDBusThread();
+  DCHECK(object_manager.get());
+
+  object_manager->CleanUp();
+
+  // The ObjectManager has to be deleted on the origin thread since it was
+  // created there.
+  GetOriginTaskRunner()->PostTask(
+      FROM_HERE,
+      base::Bind(&Bus::RemoveObjectManagerInternalHelper,
+                 this, object_manager, callback));
+}
+
+void Bus::RemoveObjectManagerInternalHelper(
+      scoped_refptr<dbus::ObjectManager> object_manager,
+      const base::Closure& callback) {
+  AssertOnOriginThread();
+  DCHECK(object_manager.get());
+
+  // Release the object manager and run the callback.
+  object_manager = NULL;
+  callback.Run();
 }
 
 void Bus::GetManagedObjects() {
@@ -460,6 +497,12 @@ void Bus::ShutdownAndBlock() {
     iter->second->Detach();
   }
 
+  // Clean up the object managers.
+  for (ObjectManagerTable::iterator iter = object_manager_table_.begin();
+       iter != object_manager_table_.end(); ++iter) {
+    iter->second->CleanUp();
+  }
+
   // Release object proxies and exported objects here. We should do this
   // here rather than in the destructor to avoid memory leaks due to
   // cyclic references.
diff --git a/dbus/bus.h b/dbus/bus.h
index 647b1b7..544d31f 100644
--- a/dbus/bus.h
+++ b/dbus/bus.h
@@ -354,9 +354,15 @@ class CHROME_DBUS_EXPORT Bus : public base::RefCountedThreadSafe<Bus> {
   // will return a new object, method calls on any remaining copies of the
   // previous object are not permitted.
   //
+  // This method will asynchronously clean up any match rules that have been
+  // added for the object manager and invoke |callback| when the operation is
+  // complete. If this method returns false, then |callback| is never called.
+  // The |callback| argument must not be null.
+  //
   // Must be called in the origin thread.
-  virtual void RemoveObjectManager(const std::string& service_name,
-                                   const ObjectPath& object_path);
+  virtual bool RemoveObjectManager(const std::string& service_name,
+                                   const ObjectPath& object_path,
+                                   const base::Closure& callback);
 
   // Instructs all registered object managers to retrieve their set of managed
   // objects from their respective remote objects. There is no need to call this
@@ -601,6 +607,14 @@ class CHROME_DBUS_EXPORT Bus : public base::RefCountedThreadSafe<Bus> {
   void RemoveObjectProxyInternal(scoped_refptr<dbus::ObjectProxy> object_proxy,
                                  const base::Closure& callback);
 
+  // Helper functions used for RemoveObjectManager().
+  void RemoveObjectManagerInternal(
+      scoped_refptr<dbus::ObjectManager> object_manager,
+      const base::Closure& callback);
+  void RemoveObjectManagerInternalHelper(
+      scoped_refptr<dbus::ObjectManager> object_manager,
+      const base::Closure& callback);
+
   // Helper function used for UnregisterExportedObject().
   void UnregisterExportedObjectInternal(
       scoped_refptr<dbus::ExportedObject> exported_object);
diff --git a/dbus/dbus.gyp b/dbus/dbus.gyp
index 4d90497..3882df8 100644
--- a/dbus/dbus.gyp
+++ b/dbus/dbus.gyp
@@ -44,6 +44,8 @@
         'scoped_dbus_error.h',
         'string_util.cc',
         'string_util.h',
+        'util.cc',
+        'util.h',
         'values_util.cc',
         'values_util.h',
       ],
diff --git a/dbus/exported_object.cc b/dbus/exported_object.cc
index 1a4fbb5..107d2e5 100644
--- a/dbus/exported_object.cc
+++ b/dbus/exported_object.cc
@@ -15,6 +15,7 @@
 #include "dbus/message.h"
 #include "dbus/object_path.h"
 #include "dbus/scoped_dbus_error.h"
+#include "dbus/util.h"
 
 namespace dbus {
 
@@ -23,15 +24,6 @@ namespace {
 // Used for success ratio histograms. 1 for success, 0 for failure.
 const int kSuccessRatioHistogramMaxValue = 2;
 
-// Gets the absolute method name by concatenating the interface name and
-// the method name. Used for building keys for method_table_ in
-// ExportedObject.
-std::string GetAbsoluteMethodName(
-    const std::string& interface_name,
-    const std::string& method_name) {
-  return interface_name + "." + method_name;
-}
-
 }  // namespace
 
 ExportedObject::ExportedObject(Bus* bus,
@@ -53,7 +45,7 @@ bool ExportedObject::ExportMethodAndBlock(
 
   // Check if the method is already exported.
   const std::string absolute_method_name =
-      GetAbsoluteMethodName(interface_name, method_name);
+      GetAbsoluteMemberName(interface_name, method_name);
   if (method_table_.find(absolute_method_name) != method_table_.end()) {
     LOG(ERROR) << absolute_method_name << " is already exported";
     return false;
@@ -203,7 +195,7 @@ DBusHandlerResult ExportedObject::HandleMessage(
   }
 
   // Check if we know about the method.
-  const std::string absolute_method_name = GetAbsoluteMethodName(
+  const std::string absolute_method_name = GetAbsoluteMemberName(
       interface, member);
   MethodTable::const_iterator iter = method_table_.find(absolute_method_name);
   if (iter == method_table_.end()) {
diff --git a/dbus/object_manager.cc b/dbus/object_manager.cc
index d8eb569..f754bb6 100644
--- a/dbus/object_manager.cc
+++ b/dbus/object_manager.cc
@@ -5,11 +5,18 @@
 #include "dbus/object_manager.h"
 
 #include "base/bind.h"
+#include "base/location.h"
 #include "base/logging.h"
+#include "base/metrics/histogram.h"
+#include "base/strings/stringprintf.h"
+#include "base/task_runner_util.h"
 #include "dbus/bus.h"
+#include "dbus/dbus_statistics.h"
 #include "dbus/message.h"
 #include "dbus/object_proxy.h"
 #include "dbus/property.h"
+#include "dbus/scoped_dbus_error.h"
+#include "dbus/util.h"
 
 namespace dbus {
 
@@ -26,33 +33,27 @@ ObjectManager::ObjectManager(Bus* bus,
     : bus_(bus),
       service_name_(service_name),
       object_path_(object_path),
+      setup_success_(false),
+      cleanup_called_(false),
       weak_ptr_factory_(this) {
   DVLOG(1) << "Creating ObjectManager for " << service_name_
            << " " << object_path_.value();
-
   DCHECK(bus_);
+  bus_->AssertOnOriginThread();
   object_proxy_ = bus_->GetObjectProxy(service_name_, object_path_);
   object_proxy_->SetNameOwnerChangedCallback(
       base::Bind(&ObjectManager::NameOwnerChanged,
                  weak_ptr_factory_.GetWeakPtr()));
 
-  object_proxy_->ConnectToSignal(
-      kObjectManagerInterface,
-      kObjectManagerInterfacesAdded,
-      base::Bind(&ObjectManager::InterfacesAddedReceived,
-                 weak_ptr_factory_.GetWeakPtr()),
-      base::Bind(&ObjectManager::InterfacesAddedConnected,
-                 weak_ptr_factory_.GetWeakPtr()));
-
-  object_proxy_->ConnectToSignal(
-      kObjectManagerInterface,
-      kObjectManagerInterfacesRemoved,
-      base::Bind(&ObjectManager::InterfacesRemovedReceived,
-                 weak_ptr_factory_.GetWeakPtr()),
-      base::Bind(&ObjectManager::InterfacesRemovedConnected,
-                 weak_ptr_factory_.GetWeakPtr()));
-
-  GetManagedObjects();
+  // Set up a match rule and a filter function to handle PropertiesChanged
+  // signals from the service. This is important to avoid any race conditions
+  // that might cause us to miss PropertiesChanged signals once all objects are
+  // initialized via GetManagedObjects.
+  base::PostTaskAndReplyWithResult(
+      bus_->GetDBusTaskRunner(),
+      FROM_HERE,
+      base::Bind(&ObjectManager::SetupMatchRuleAndFilter, this),
+      base::Bind(&ObjectManager::OnSetupMatchRuleAndFilterComplete, this));
 }
 
 ObjectManager::~ObjectManager() {
@@ -144,6 +145,205 @@ void ObjectManager::GetManagedObjects() {
                  weak_ptr_factory_.GetWeakPtr()));
 }
 
+void ObjectManager::CleanUp() {
+  DCHECK(bus_);
+  bus_->AssertOnDBusThread();
+  DCHECK(!cleanup_called_);
+
+  cleanup_called_ = true;
+
+  if (!setup_success_)
+    return;
+
+  if (!bus_->RemoveFilterFunction(&ObjectManager::HandleMessageThunk, this))
+    LOG(ERROR) << "Failed to remove filter function";
+
+  ScopedDBusError error;
+  bus_->RemoveMatch(match_rule_, error.get());
+  if (error.is_set())
+    LOG(ERROR) << "Failed to remove match rule: " << match_rule_;
+
+  match_rule_.clear();
+}
+
+void ObjectManager::InitializeObjects() {
+  DCHECK(bus_);
+  DCHECK(object_proxy_);
+  DCHECK(setup_success_);
+
+  // |object_proxy_| is no longer valid if the Bus was shut down before this
+  // call. Don't initiate any other action from the origin thread.
+  if (cleanup_called_)
+    return;
+
+  object_proxy_->ConnectToSignal(
+      kObjectManagerInterface,
+      kObjectManagerInterfacesAdded,
+      base::Bind(&ObjectManager::InterfacesAddedReceived,
+                 weak_ptr_factory_.GetWeakPtr()),
+      base::Bind(&ObjectManager::InterfacesAddedConnected,
+                 weak_ptr_factory_.GetWeakPtr()));
+
+  object_proxy_->ConnectToSignal(
+      kObjectManagerInterface,
+      kObjectManagerInterfacesRemoved,
+      base::Bind(&ObjectManager::InterfacesRemovedReceived,
+                 weak_ptr_factory_.GetWeakPtr()),
+      base::Bind(&ObjectManager::InterfacesRemovedConnected,
+                 weak_ptr_factory_.GetWeakPtr()));
+
+  GetManagedObjects();
+}
+
+bool ObjectManager::SetupMatchRuleAndFilter() {
+  DCHECK(bus_);
+  DCHECK(!setup_success_);
+  bus_->AssertOnDBusThread();
+
+  if (cleanup_called_)
+    return false;
+
+  if (!bus_->Connect() || !bus_->SetUpAsyncOperations())
+    return false;
+
+  service_name_owner_ =
+      bus_->GetServiceOwnerAndBlock(service_name_, Bus::SUPPRESS_ERRORS);
+
+  const std::string match_rule =
+      base::StringPrintf(
+          "type='signal', sender='%s', interface='%s', member='%s'",
+          service_name_.c_str(),
+          kPropertiesInterface,
+          kPropertiesChanged);
+
+  if (!bus_->AddFilterFunction(&ObjectManager::HandleMessageThunk, this)) {
+    LOG(ERROR) << "ObjectManager failed to add filter function";
+    return false;
+  }
+
+  ScopedDBusError error;
+  bus_->AddMatch(match_rule, error.get());
+  if (error.is_set()) {
+    LOG(ERROR) << "ObjectManager failed to add match rule \"" << match_rule
+               << "\". Got " << error.name() << ": " << error.message();
+    bus_->RemoveFilterFunction(&ObjectManager::HandleMessageThunk, this);
+    return false;
+  }
+
+  match_rule_ = match_rule;
+  setup_success_ = true;
+
+  return true;
+}
+
+void ObjectManager::OnSetupMatchRuleAndFilterComplete(bool success) {
+  LOG_IF(WARNING, !success) << service_name_ << " " << object_path_.value()
+                            << ": Failed to set up match rule.";
+  if (success)
+    InitializeObjects();
+}
+
+// static
+DBusHandlerResult ObjectManager::HandleMessageThunk(DBusConnection* connection,
+                                                    DBusMessage* raw_message,
+                                                    void* user_data) {
+  ObjectManager* self = reinterpret_cast<ObjectManager*>(user_data);
+  return self->HandleMessage(connection, raw_message);
+}
+
+DBusHandlerResult ObjectManager::HandleMessage(DBusConnection* connection,
+                                               DBusMessage* raw_message) {
+  DCHECK(bus_);
+  bus_->AssertOnDBusThread();
+
+  if (dbus_message_get_type(raw_message) != DBUS_MESSAGE_TYPE_SIGNAL)
+    return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
+
+  // raw_message will be unrefed on exit of the function. Increment the
+  // reference so we can use it in Signal.
+  dbus_message_ref(raw_message);
+  scoped_ptr<Signal> signal(
+      Signal::FromRawMessage(raw_message));
+
+  const std::string interface = signal->GetInterface();
+  const std::string member = signal->GetMember();
+
+  statistics::AddReceivedSignal(service_name_, interface, member);
+
+  // Only handle the PropertiesChanged signal.
+  const std::string absolute_signal_name =
+      GetAbsoluteMemberName(interface, member);
+  const std::string properties_changed_signal_name =
+      GetAbsoluteMemberName(kPropertiesInterface, kPropertiesChanged);
+  if (absolute_signal_name != properties_changed_signal_name)
+    return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
+
+  VLOG(1) << "Signal received: " << signal->ToString();
+
+  // Make sure that the signal originated from the correct sender.
+  std::string sender = signal->GetSender();
+  if (service_name_owner_ != sender) {
+    LOG(ERROR) << "Rejecting a message from a wrong sender.";
+    UMA_HISTOGRAM_COUNTS("DBus.RejectedSignalCount", 1);
+    return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
+  }
+
+  const ObjectPath path = signal->GetPath();
+
+  if (bus_->HasDBusThread()) {
+    // Post a task to run the method in the origin thread. Transfer ownership of
+    // |signal| to NotifyPropertiesChanged, which will handle the clean up.
+    Signal* released_signal = signal.release();
+    bus_->GetOriginTaskRunner()->PostTask(
+        FROM_HERE,
+        base::Bind(&ObjectManager::NotifyPropertiesChanged,
+                   this, path,
+                   released_signal));
+  } else {
+    // If the D-Bus thread is not used, just call the callback on the
+    // current thread. Transfer the ownership of |signal| to
+    // NotifyPropertiesChanged.
+    NotifyPropertiesChanged(path, signal.release());
+  }
+
+  // We don't return DBUS_HANDLER_RESULT_HANDLED for signals because other
+  // objects may be interested in them. (e.g. Signals from org.freedesktop.DBus)
+  return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
+}
+
+void ObjectManager::NotifyPropertiesChanged(
+    const dbus::ObjectPath object_path,
+    Signal* signal) {
+  DCHECK(bus_);
+  bus_->AssertOnOriginThread();
+
+  NotifyPropertiesChangedHelper(object_path, signal);
+
+  // Delete the message on the D-Bus thread. See comments in HandleMessage.
+  bus_->GetDBusTaskRunner()->PostTask(
+      FROM_HERE,
+      base::Bind(&base::DeletePointer<Signal>, signal));
+}
+
+void ObjectManager::NotifyPropertiesChangedHelper(
+    const dbus::ObjectPath object_path,
+    Signal* signal) {
+  DCHECK(bus_);
+  bus_->AssertOnOriginThread();
+
+  MessageReader reader(signal);
+  std::string interface;
+  if (!reader.PopString(&interface)) {
+    LOG(WARNING) << "Property changed signal has wrong parameters: "
+                 << "expected interface name: " << signal->ToString();
+    return;
+  }
+
+  PropertySet* properties = GetProperties(object_path, interface);
+  if (properties)
+    properties->ChangedReceived(signal);
+}
+
 void ObjectManager::OnGetManagedObjects(Response* response) {
   if (response != NULL) {
     MessageReader reader(response);
@@ -257,7 +457,6 @@ void ObjectManager::AddInterface(const ObjectPath& object_path,
     property_set = object->properties_map[interface_name] =
         interface->CreateProperties(object->object_proxy,
                                     object_path, interface_name);
-    property_set->ConnectSignals();
   } else
     property_set = piter->second;
 
@@ -297,6 +496,8 @@ void ObjectManager::RemoveInterface(const ObjectPath& object_path,
 
 void ObjectManager::NameOwnerChanged(const std::string& old_owner,
                                      const std::string& new_owner) {
+  service_name_owner_ = new_owner;
+
   if (!old_owner.empty()) {
     ObjectMap::iterator iter = object_map_.begin();
     while (iter != object_map_.end()) {
diff --git a/dbus/object_manager.h b/dbus/object_manager.h
index 0d3ae5d0..30dfc42 100644
--- a/dbus/object_manager.h
+++ b/dbus/object_manager.h
@@ -223,12 +223,47 @@ public:
   // a need to call this manually.
   void GetManagedObjects();
 
+  // Cleans up any match rules and filter functions added by this ObjectManager.
+  // The Bus object will take care of this so you don't have to do it manually.
+  //
+  // BLOCKING CALL.
+  void CleanUp();
+
  protected:
   virtual ~ObjectManager();
 
  private:
   friend class base::RefCountedThreadSafe<ObjectManager>;
 
+  // Connects the InterfacesAdded and InterfacesRemoved signals and calls
+  // GetManagedObjects. Called from OnSetupMatchRuleAndFilterComplete.
+  void InitializeObjects();
+
+  // Called from the constructor to add a match rule for PropertiesChanged
+  // signals on the DBus thread and set up a corresponding filter function.
+  bool SetupMatchRuleAndFilter();
+
+  // Called on the origin thread once the match rule and filter have been set
+  // up. |success| is false, if an error occurred during set up; it's true
+  // otherwise.
+  void OnSetupMatchRuleAndFilterComplete(bool success);
+
+  // Called by dbus:: when a message is received. This is used to filter
+  // PropertiesChanged signals from the correct sender and relay the event to
+  // the correct PropertySet.
+  static DBusHandlerResult HandleMessageThunk(DBusConnection* connection,
+                                              DBusMessage* raw_message,
+                                              void* user_data);
+  DBusHandlerResult HandleMessage(DBusConnection* connection,
+                                  DBusMessage* raw_message);
+
+  // Called when a PropertiesChanged signal is received from the sender.
+  // This method notifies the relevant PropertySet that it should update its
+  // properties based on the received signal. Called from HandleMessage.
+  void NotifyPropertiesChanged(const dbus::ObjectPath object_path,
+                               Signal* signal);
+  void NotifyPropertiesChangedHelper(const dbus::ObjectPath object_path,
+                                     Signal* signal);
 
   // Called by dbus:: in response to the GetManagedObjects() method call.
   void OnGetManagedObjects(Response* response);
@@ -281,8 +316,12 @@ public:
 
   Bus* bus_;
   std::string service_name_;
+  std::string service_name_owner_;
+  std::string match_rule_;
   ObjectPath object_path_;
   ObjectProxy* object_proxy_;
+  bool setup_success_;
+  bool cleanup_called_;
 
   // Maps the name of an interface to the implementation class used for
   // instantiating PropertySet structures for that interface's properties.
diff --git a/dbus/object_manager_unittest.cc b/dbus/object_manager_unittest.cc
index 10730e1..54ddeac 100644
--- a/dbus/object_manager_unittest.cc
+++ b/dbus/object_manager_unittest.cc
@@ -28,7 +28,7 @@ class ObjectManagerTest
     : public testing::Test,
       public ObjectManager::Interface {
  public:
-  ObjectManagerTest() {
+  ObjectManagerTest() : timeout_expired_(false) {
   }
 
   struct Properties : public PropertySet {
@@ -90,7 +90,6 @@ class ObjectManagerTest
         ObjectPath("/org/chromium/TestService"));
     object_manager_->RegisterInterface("org.chromium.TestInterface", this);
 
-    object_manager_->GetManagedObjects();
     WaitForObject();
   }
 
@@ -115,6 +114,16 @@ class ObjectManagerTest
     run_loop_->Quit();
   }
 
+  // Called from the PropertiesChangedAsObjectsReceived test case. The test will
+  // not run the message loop if it receives the expected PropertiesChanged
+  // signal before the timeout. This method immediately fails the test.
+  void PropertiesChangedTestTimeout() {
+    timeout_expired_ = true;
+    run_loop_->Quit();
+
+    FAIL() << "Never received PropertiesChanged";
+  }
+
  protected:
   // Called when an object is added.
   virtual void ObjectAdded(const ObjectPath& object_path,
@@ -133,6 +142,16 @@ class ObjectManagerTest
   // Called when a property value is updated.
   void OnPropertyChanged(const ObjectPath& object_path,
                          const std::string& name) {
+    // Store the value of the "Name" property if that's the one that
+    // changed.
+    Properties* properties = static_cast<Properties*>(
+        object_manager_->GetProperties(
+            object_path,
+            "org.chromium.TestInterface"));
+    if (name == properties->name.name())
+      last_name_value_ = properties->name.value();
+
+    // Store the updated property.
     updated_properties_.push_back(name);
     run_loop_->Quit();
   }
@@ -191,6 +210,9 @@ class ObjectManagerTest
   ObjectManager* object_manager_;
   scoped_ptr<TestService> test_service_;
 
+  std::string last_name_value_;
+  bool timeout_expired_;
+
   std::vector<std::pair<ObjectPath, std::string> > added_objects_;
   std::vector<std::pair<ObjectPath, std::string> > removed_objects_;
   std::vector<std::string> updated_properties_;
@@ -359,4 +381,38 @@ TEST_F(ObjectManagerTest, OwnershipLostAndRegained) {
   ASSERT_EQ(1U, object_paths.size());
 }
 
+TEST_F(ObjectManagerTest, PropertiesChangedAsObjectsReceived) {
+  // Remove the existing object manager.
+  object_manager_->UnregisterInterface("org.chromium.TestInterface");
+  run_loop_.reset(new base::RunLoop);
+  EXPECT_TRUE(bus_->RemoveObjectManager(
+      "org.chromium.TestService",
+      ObjectPath("/org/chromium/TestService"),
+      run_loop_->QuitClosure()));
+  run_loop_->Run();
+
+  PerformAction("SetSendImmediatePropertiesChanged",
+                ObjectPath("/org/chromium/TestService"));
+
+  object_manager_ = bus_->GetObjectManager(
+      "org.chromium.TestService",
+      ObjectPath("/org/chromium/TestService"));
+  object_manager_->RegisterInterface("org.chromium.TestInterface", this);
+
+  // The newly created object manager should call GetManagedObjects immediately
+  // after setting up the match rule for PropertiesChanged. We should process
+  // the PropertiesChanged event right after that. If we don't receive it within
+  // 2 seconds, then fail the test.
+  message_loop_.PostDelayedTask(
+      FROM_HERE,
+      base::Bind(&ObjectManagerTest::PropertiesChangedTestTimeout,
+                 base::Unretained(this)),
+      base::TimeDelta::FromSeconds(2));
+
+  while (last_name_value_ != "ChangedTestServiceName" && !timeout_expired_) {
+    run_loop_.reset(new base::RunLoop);
+    run_loop_->Run();
+  }
+}
+
 }  // namespace dbus
diff --git a/dbus/object_proxy.cc b/dbus/object_proxy.cc
index d0b660b..016425d 100644
--- a/dbus/object_proxy.cc
+++ b/dbus/object_proxy.cc
@@ -18,6 +18,7 @@
 #include "dbus/object_path.h"
 #include "dbus/object_proxy.h"
 #include "dbus/scoped_dbus_error.h"
+#include "dbus/util.h"
 
 namespace dbus {
 
@@ -41,15 +42,6 @@ const char kDBusSystemObjectAddress[] = "org.freedesktop.DBus";
 // The NameOwnerChanged member in |kDBusSystemObjectInterface|.
 const char kNameOwnerChangedMember[] = "NameOwnerChanged";
 
-// Gets the absolute signal name by concatenating the interface name and
-// the signal name. Used for building keys for method_table_ in
-// ObjectProxy.
-std::string GetAbsoluteSignalName(
-    const std::string& interface_name,
-    const std::string& signal_name) {
-  return interface_name + "." + signal_name;
-}
-
 // An empty function used for ObjectProxy::EmptyResponseCallback().
 void EmptyResponseCallbackBody(Response* /*response*/) {
 }
@@ -421,7 +413,7 @@ bool ObjectProxy::ConnectToSignalInternal(const std::string& interface_name,
     return false;
 
   const std::string absolute_signal_name =
-      GetAbsoluteSignalName(interface_name, signal_name);
+      GetAbsoluteMemberName(interface_name, signal_name);
 
   // Add a match rule so the signal goes through HandleMessage().
   const std::string match_rule =
@@ -488,7 +480,7 @@ DBusHandlerResult ObjectProxy::HandleMessage(
   statistics::AddReceivedSignal(service_name_, interface, member);
 
   // Check if we know about the signal.
-  const std::string absolute_signal_name = GetAbsoluteSignalName(
+  const std::string absolute_signal_name = GetAbsoluteMemberName(
       interface, member);
   MethodTable::const_iterator iter = method_table_.find(absolute_signal_name);
   if (iter == method_table_.end()) {
diff --git a/dbus/test_service.cc b/dbus/test_service.cc
index 96fa8bc..c976c37 100644
--- a/dbus/test_service.cc
+++ b/dbus/test_service.cc
@@ -24,7 +24,7 @@ void EmptyCallback(bool /* success */) {
 namespace dbus {
 
 // Echo, SlowEcho, AsyncEcho, BrokenMethod, GetAll, Get, Set, PerformAction,
-// GetManagedObjects.
+// GetManagedObjects
 const int TestService::kNumMethodsToExport = 9;
 
 TestService::Options::Options()
@@ -160,6 +160,10 @@ void TestService::ReleaseOwnershipInternal(
       callback);
 }
 
+void TestService::SetSendImmediatePropertiesChanged() {
+  send_immediate_properties_changed_ = true;
+}
+
 void TestService::OnExported(const std::string& interface_name,
                              const std::string& method_name,
                              bool success) {
@@ -471,11 +475,13 @@ void TestService::PerformAction(
     return;
   }
 
-  if (action == "AddObject")
+  if (action == "AddObject") {
     AddObject(object_path);
-  else if (action == "RemoveObject")
+  } else if (action == "RemoveObject") {
     RemoveObject(object_path);
-  else if (action == "ReleaseOwnership") {
+  } else if (action == "SetSendImmediatePropertiesChanged") {
+    SetSendImmediatePropertiesChanged();
+  } if (action == "ReleaseOwnership") {
     ReleaseOwnership(base::Bind(&TestService::PerformActionResponse,
                                 base::Unretained(this),
                                 method_call, response_sender));
@@ -556,6 +562,9 @@ void TestService::GetManagedObjects(
   writer.CloseContainer(&array_writer);
 
   response_sender.Run(response.Pass());
+
+  if (send_immediate_properties_changed_)
+    SendPropertyChangedSignal("ChangedTestServiceName");
 }
 
 void TestService::AddPropertiesToWriter(MessageWriter* writer) {
diff --git a/dbus/test_service.h b/dbus/test_service.h
index 523864c..8039fe8 100644
--- a/dbus/test_service.h
+++ b/dbus/test_service.h
@@ -173,6 +173,10 @@ class TestService : public base::Thread {
   // Helper function for ReleaseOwnership().
   void ReleaseOwnershipInternal(base::Closure callback);
 
+  // Configures the test service to send a PropertiesChanged signal for the
+  // "Name" property immediately after a call to GetManagedObjects.
+  void SetSendImmediatePropertiesChanged();
+
   // Sends the response on completion of the performed action.
   void PerformActionResponse(
       MethodCall* method_call,
@@ -197,6 +201,10 @@ class TestService : public base::Thread {
   // The number of methods actually exported.
   int num_exported_methods_;
 
+  // True if a PropertiesChanged signal for the "Name" property should be sent
+  // immediately following a call to GetManagedObjects.
+  bool send_immediate_properties_changed_;
+
   // True iff this instance has successfully acquired the name ownership.
   bool has_ownership_;
 
diff --git a/dbus/util.cc b/dbus/util.cc
new file mode 100644
index 0000000..26e5c71
--- /dev/null
+++ b/dbus/util.cc
@@ -0,0 +1,14 @@
+// Copyright 2014 The Chromium 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 "dbus/util.h"
+
+namespace dbus {
+
+std::string GetAbsoluteMemberName(const std::string& interface_name,
+                                  const std::string& member_name) {
+  return interface_name + "." + member_name;
+}
+
+}  // namespace dbus
diff --git a/dbus/util.h b/dbus/util.h
new file mode 100644
index 0000000..b983b6f
--- /dev/null
+++ b/dbus/util.h
@@ -0,0 +1,28 @@
+// Copyright 2014 The Chromium 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 DBUS_UTIL_H_
+#define DBUS_UTIL_H_
+
+#include <string>
+
+#include "dbus/dbus_export.h"
+
+namespace dbus {
+
+// Returns the absolute name of a member by concatanating |interface_name| and
+// |member_name|. e.g.:
+//    GetAbsoluteMemberName(
+//        "org.freedesktop.DBus.Properties",
+//        "PropertiesChanged")
+//
+//    => "org.freedesktop.DBus.Properties.PropertiesChanged"
+//
+CHROME_DBUS_EXPORT std::string GetAbsoluteMemberName(
+    const std::string& interface_name,
+    const std::string& member_name);
+
+}  // namespace dbus
+
+#endif  // DBUS_UTIL_H_