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}

1 files changed, 47 insertions, 4 deletions

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.