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

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.


Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=179809

[deymo, 2013-01-30 05:15:22]

Hi!
Here is the code to implement my suggestion.
haruki should be able to review this, and satorux/keybuk for an OWNER review.

I don't know how to enforce the BusTest.DoubleAddAndRemoveMatch to fail if there
is a LOG(ERROR) issued by the RemoveMatch function, but running the unit test
effectively shows that the message is gone (in this an many other dbus unit
tests).

Thanks!

[Haruki Sato, 2013-01-30 05:33:20]

lgtm

Thank you for the fix and a test!

An idea for the unittest:
We have no effective DBusError for these cases as we actually do not call the
underlying dbus functions.
We can change Add/RemoveMatch to return a bool which represents whether if the
method actually adds/removes a rule so that we can test this fix.

[deymo, 2013-01-30 05:47:27]

On 2013/01/30 05:33:20, Haruki Sato wrote:
> lgtm
> 
> Thank you for the fix and a test!
> 
> An idea for the unittest:
> We have no effective DBusError for these cases as we actually do not call the
> underlying dbus functions.
> We can change Add/RemoveMatch to return a bool which represents whether if the
> method actually adds/removes a rule so that we can test this fix.

The Add function doesn't really have an error value to return since it will go
and add whatever you want. In the other hand, the remove function can fail if we
want to remove a rule that doesn't exist. If you want I can add a bool return
value to the RemoveMatch mainly for testing purposes.

[Haruki Sato, 2013-01-30 06:03:25]

So the returned bool might be just "dbus function is called or not" rather than
success/failure.
Anyway a reasonable return value is telling more than void, so I think having a
bool here is OK.
Could you add it and change the test accordingly?

On 2013/01/30 05:47:27, deymo wrote:
> On 2013/01/30 05:33:20, Haruki Sato wrote:
> > lgtm
> > 
> > Thank you for the fix and a test!
> > 
> > An idea for the unittest:
> > We have no effective DBusError for these cases as we actually do not call
the
> > underlying dbus functions.
> > We can change Add/RemoveMatch to return a bool which represents whether if
the
> > method actually adds/removes a rule so that we can test this fix.
> 
> The Add function doesn't really have an error value to return since it will go
> and add whatever you want. In the other hand, the remove function can fail if
we
> want to remove a rule that doesn't exist. If you want I can add a bool return
> value to the RemoveMatch mainly for testing purposes.

[deymo, 2013-01-30 18:49:09]

Per my talk with Haruki, I added a new bool return value to RemoveMatch and
updated the documentation in the .h file.
RemoveMatch returns false when trying to remove a non-existing rule.
Patch Set 2 is up, with the test case modified to use this value.

[Haruki Sato, 2013-01-31 01:25:29]

lgtm

Thank you!

[satorux1, 2013-01-31 01:43:41]

LGTM

[commit-bot: I haz the power, 2013-01-31 01:56:41]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/deymo@chromium.org/12088068/5001

[commit-bot: I haz the power, 2013-01-31 06:08:08]

Change committed as 179809