dbus: verify object path of incoming signals

dbus: verify object path of incoming signals

The existing behavior, while convenient for debugging, is wrong.

D-Bus will not call any further filter functions once one returns
DBUS_HANDLER_RESULT_HANDLED, in order for the next to be called a
filter must return DBUS_HANDLER_RESULT_NOT_YET_HANDLED if it does
not handle the incoming signal.

We also can't defer this to the signal function since we have to
post that to a different thread, and return values get hard.

Since object proxies are constructed per-path, and match common
interfaces and members, this means signals must be matched on an
object otherwise only the first registered object proxy for any
client will be called, and will be called for all signals.

BUG=chromium-os:27113
TEST=ran unit tests, and manually verified existing code that uses ConnectToSignal

Change-Id: Ia4cbc064dff0421a37fe4c4b7c719acf25eb630c


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=124357

[satorux1, 2012-02-29 06:18:23]

http://codereview.chromium.org/9508005/diff/1/dbus/end_to_end_async_unittest.cc
File dbus/end_to_end_async_unittest.cc (left):

http://codereview.chromium.org/9508005/diff/1/dbus/end_to_end_async_unittest....
dbus/end_to_end_async_unittest.cc:312: }
Rather than removing it, can you write a test to make sure this is *not*
handled?

[keybuk, 2012-03-01 00:10:45]

http://codereview.chromium.org/9508005/diff/1/dbus/end_to_end_async_unittest.cc
File dbus/end_to_end_async_unittest.cc (left):

http://codereview.chromium.org/9508005/diff/1/dbus/end_to_end_async_unittest....
dbus/end_to_end_async_unittest.cc:312: }
On 2012/02/29 06:18:24, satorux1 wrote:
> Rather than removing it, can you write a test to make sure this is *not*
> handled?

Done.

[satorux1, 2012-03-01 00:14:32]

http://codereview.chromium.org/9508005/diff/6001/dbus/end_to_end_async_unitte...
File dbus/end_to_end_async_unittest.cc (right):

http://codereview.chromium.org/9508005/diff/6001/dbus/end_to_end_async_unitte...
dbus/end_to_end_async_unittest.cc:322: WaitForTestSignalOrTimeout(timeout_ms);
Does this mean the test takes 5 seconds to perform? It's a bit too long for a
unit test. We should probably use action_timeout_ms() in
base/test/test_timeouts.h

[keybuk, 2012-03-01 00:15:22]

http://codereview.chromium.org/9508005/diff/6001/dbus/end_to_end_async_unitte...
File dbus/end_to_end_async_unittest.cc (right):

http://codereview.chromium.org/9508005/diff/6001/dbus/end_to_end_async_unitte...
dbus/end_to_end_async_unittest.cc:322: WaitForTestSignalOrTimeout(timeout_ms);
yes.

action_timeout_ms is 10s, which is even longer

but tiny_timeout_ms was too short to guarantee the signal got through dbus (the
test failed 3/7 times I tried)

[satorux1, 2012-03-01 00:23:46]

http://codereview.chromium.org/9508005/diff/6001/dbus/end_to_end_async_unitte...
File dbus/end_to_end_async_unittest.cc (right):

http://codereview.chromium.org/9508005/diff/6001/dbus/end_to_end_async_unitte...
dbus/end_to_end_async_unittest.cc:322: WaitForTestSignalOrTimeout(timeout_ms);
On 2012/03/01 00:15:22, keybuk wrote:
> yes.
> 
> action_timeout_ms is 10s, which is even longer
> 
> but tiny_timeout_ms was too short to guarantee the signal got through dbus
(the
> test failed 3/7 times I tried)

I see. Then let's remove this test. 

A better way to test this is to check if ObjectProxy rejects  signals from wrong
senders, but we don't have ability to observe the behavior from outside.

[keybuk, 2012-03-01 00:25:35]

I have a better idea...

[satorux1, 2012-03-01 00:36:16]

LGTM

https://chromiumcodereview.appspot.com/9508005/diff/7004/dbus/end_to_end_asyn...
File dbus/end_to_end_async_unittest.cc (right):

https://chromiumcodereview.appspot.com/9508005/diff/7004/dbus/end_to_end_asyn...
dbus/end_to_end_async_unittest.cc:336: // arrives from a different object path
like "/" the proxy should not
the proxy -> the 1st object proxy (object_proxy_)

https://chromiumcodereview.appspot.com/9508005/diff/7004/dbus/end_to_end_asyn...
dbus/end_to_end_async_unittest.cc:337: // handle it, and should leave it for the
other.
the other (root_object_proxy_)

[commit-bot: I haz the power, 2012-03-01 00:39:23]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/keybuk@chromium.org/9508005/7006

[commit-bot: I haz the power, 2012-03-01 04:01:06]

Change committed as 124357