DBus: Bus::AddMatch and RemoveMatch support repeated rules.

This fix counts the number of times the same rule is added to a dbus::Bus
and removes the rule from the real DBus connection when all the copies of
the same match rule have been removed.

BUG=chromium:173054
TEST=BusTest.DoubleAddAndRemoveMatch passes.


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

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

4 files changed, 68 insertions, 12 deletions

diff --git a/dbus/bus.cc b/dbus/bus.cc
index c7584fd..4096049 100644
--- a/dbus/bus.cc
+++ b/dbus/bus.cc
@@ -617,26 +617,38 @@ void Bus::AddMatch(const std::string& match_rule, DBusError* error) {
   DCHECK(connection_);
   AssertOnDBusThread();
 
-  if (match_rules_added_.find(match_rule) != match_rules_added_.end()) {
+  std::map<std::string, int>::iterator iter =
+      match_rules_added_.find(match_rule);
+  if (iter != match_rules_added_.end()) {
+    // The already existing rule's counter is incremented.
+    iter->second++;
+
     VLOG(1) << "Match rule already exists: " << match_rule;
     return;
   }
 
   dbus_bus_add_match(connection_, match_rule.c_str(), error);
-  match_rules_added_.insert(match_rule);
+  match_rules_added_[match_rule] = 1;
 }
 
-void Bus::RemoveMatch(const std::string& match_rule, DBusError* error) {
+bool Bus::RemoveMatch(const std::string& match_rule, DBusError* error) {
   DCHECK(connection_);
   AssertOnDBusThread();
 
-  if (match_rules_added_.find(match_rule) == match_rules_added_.end()) {
+  std::map<std::string, int>::iterator iter =
+      match_rules_added_.find(match_rule);
+  if (iter == match_rules_added_.end()) {
     LOG(ERROR) << "Requested to remove an unknown match rule: " << match_rule;
-    return;
+    return false;
   }
 
-  dbus_bus_remove_match(connection_, match_rule.c_str(), error);
-  match_rules_added_.erase(match_rule);
+  // The rule's counter is decremented and the rule is deleted when reachs 0.
+  iter->second--;
+  if (iter->second == 0) {
+    dbus_bus_remove_match(connection_, match_rule.c_str(), error);
+    match_rules_added_.erase(match_rule);
+  }
+  return true;
 }
 
 bool Bus::TryRegisterObjectPath(const ObjectPath& object_path,
diff --git a/dbus/bus.h b/dbus/bus.h
index 0545258..e6a9b20 100644
--- a/dbus/bus.h
+++ b/dbus/bus.h
@@ -405,8 +405,8 @@ class CHROME_DBUS_EXPORT Bus : public base::RefCountedThreadSafe<Bus> {
   // Instead, you should check if an incoming message is what you are
   // interested in, in the filter functions.
   //
-  // The same match rule can be added more than once, but ignored from the
-  // second time.
+  // The same match rule can be added more than once and should be removed
+  // as many times as it was added.
   //
   // The match rule looks like:
   // "type='signal', interface='org.chromium.SomeInterface'".
@@ -419,9 +419,11 @@ class CHROME_DBUS_EXPORT Bus : public base::RefCountedThreadSafe<Bus> {
   virtual void AddMatch(const std::string& match_rule, DBusError* error);
 
   // Removes the match rule previously added by AddMatch().
+  // Returns false if the requested match rule is unknown or has already been
+  // removed. Otherwise, returns true and sets |error| accordingly.
   //
   // BLOCKING CALL.
-  virtual void RemoveMatch(const std::string& match_rule, DBusError* error);
+  virtual bool RemoveMatch(const std::string& match_rule, DBusError* error);
 
   // Tries to register the object path. Returns true on success.
   // Returns false if the object path is already registered.
@@ -554,7 +556,9 @@ class CHROME_DBUS_EXPORT Bus : public base::RefCountedThreadSafe<Bus> {
   std::set<std::string> owned_service_names_;
   // The following sets are used to check if rules/object_paths/filters
   // are properly cleaned up before destruction of the bus object.
-  std::set<std::string> match_rules_added_;
+  // Since it's not an error to add the same match rule twice, the repeated
+  // match rules are counted in a map.
+  std::map<std::string, int> match_rules_added_;
   std::set<ObjectPath> registered_object_paths_;
   std::set<std::pair<DBusHandleMessageFunction, void*> >
       filter_functions_added_;
diff --git a/dbus/bus_unittest.cc b/dbus/bus_unittest.cc
index 0f57db1..6c002a8 100644
--- a/dbus/bus_unittest.cc
+++ b/dbus/bus_unittest.cc
@@ -11,6 +11,7 @@
 #include "dbus/exported_object.h"
 #include "dbus/object_path.h"
 #include "dbus/object_proxy.h"
+#include "dbus/scoped_dbus_error.h"
 
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -256,3 +257,42 @@ TEST(BusTest, AddFilterFunction) {
 
   bus->ShutdownAndBlock();
 }
+
+TEST(BusTest, DoubleAddAndRemoveMatch) {
+  dbus::Bus::Options options;
+  scoped_refptr<dbus::Bus> bus = new dbus::Bus(options);
+  dbus::ScopedDBusError error;
+
+  bus->Connect();
+
+  // Adds the same rule twice.
+  bus->AddMatch(
+      "type='signal',interface='org.chromium.TestService',path='/'",
+      error.get());
+  ASSERT_FALSE(error.is_set());
+
+  bus->AddMatch(
+      "type='signal',interface='org.chromium.TestService',path='/'",
+      error.get());
+  ASSERT_FALSE(error.is_set());
+
+  // Removes the same rule twice.
+  ASSERT_TRUE(bus->RemoveMatch(
+      "type='signal',interface='org.chromium.TestService',path='/'",
+      error.get()));
+  ASSERT_FALSE(error.is_set());
+
+  // The rule should be still in the bus since it was removed only once.
+  // A second removal shouldn't give an error.
+  ASSERT_TRUE(bus->RemoveMatch(
+      "type='signal',interface='org.chromium.TestService',path='/'",
+      error.get()));
+  ASSERT_FALSE(error.is_set());
+
+  // A third attemp to remove the same rule should fail.
+  ASSERT_FALSE(bus->RemoveMatch(
+      "type='signal',interface='org.chromium.TestService',path='/'",
+      error.get()));
+
+  bus->ShutdownAndBlock();
+}
diff --git a/dbus/mock_bus.h b/dbus/mock_bus.h
index 6894682..33a62e0 100644
--- a/dbus/mock_bus.h
+++ b/dbus/mock_bus.h
@@ -49,7 +49,7 @@ class MockBus : public Bus {
                                   void* user_data));
   MOCK_METHOD2(AddMatch, void(const std::string& match_rule,
                               DBusError* error));
-  MOCK_METHOD2(RemoveMatch, void(const std::string& match_rule,
+  MOCK_METHOD2(RemoveMatch, bool(const std::string& match_rule,
                                  DBusError* error));
   MOCK_METHOD4(TryRegisterObjectPath, bool(const ObjectPath& object_path,
                                            const DBusObjectPathVTable* vtable,