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
CrOS-Libchrome-Original-Commit: bae0f88d93c59922d7b249d1c7dedc772f33b147
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 @@
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 @@
// 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 @@
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 @@
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 @@
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 @@
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,