Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(10)

Issue 2280483002: Add FlushForTesting to InterfacePtr and Binding. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 year ago by Sam McNally
Modified:
1 year ago
CC:
Aaron Boodman, abarth-chromium, chrome-apps-syd-reviews_chromium.org, chromium-reviews, darin (slow to review), qsr+mojo_chromium.org, viettrungluu+watch_chromium.org, yzshen+watch_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Add FlushForTesting to InterfacePtr and Binding. Previously, when unit testing a class that is a client of a mojo interface, there was no reliable way to detect that no message was sent. This adds a FlushForTesting() method to mojo message pipe bindings, which can be used to wait for a round trip, after which any messages already sent will have been delivered; this can be used to test for the absence of messages sent in response to some stimulus. Committed: https://crrev.com/462458f4b533d44bfad95ea07dc99257eaf028cb Cr-Commit-Position: refs/heads/master@{#415920}

Patch Set 1 : #

Total comments: 2

Patch Set 2 : rebase #

Patch Set 3 : #

Total comments: 14

Patch Set 4 : #

Total comments: 20

Patch Set 5 : #

Total comments: 2

Patch Set 6 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+646 lines, -203 lines) Patch
M mojo/public/cpp/bindings/associated_binding.h View 1 2 3 3 chunks +10 lines, -1 line 0 comments Download
M mojo/public/cpp/bindings/associated_interface_ptr.h View 1 chunk +6 lines, -0 lines 0 comments Download
M mojo/public/cpp/bindings/binding.h View 1 chunk +6 lines, -0 lines 0 comments Download
M mojo/public/cpp/bindings/binding_set.h View 1 2 3 2 chunks +8 lines, -0 lines 0 comments Download
M mojo/public/cpp/bindings/interface_endpoint_client.h View 1 2 4 chunks +11 lines, -1 line 0 comments Download
M mojo/public/cpp/bindings/interface_ptr.h View 1 chunk +6 lines, -0 lines 0 comments Download
M mojo/public/cpp/bindings/lib/associated_interface_ptr_state.h View 1 2 3 4 4 chunks +14 lines, -10 lines 0 comments Download
M mojo/public/cpp/bindings/lib/binding_state.h View 1 2 6 chunks +10 lines, -4 lines 0 comments Download
M mojo/public/cpp/bindings/lib/binding_state.cc View 1 2 3 6 chunks +18 lines, -4 lines 0 comments Download
M mojo/public/cpp/bindings/lib/control_message_handler.cc View 1 2 3 4 4 chunks +46 lines, -0 lines 0 comments Download
M mojo/public/cpp/bindings/lib/control_message_proxy.h View 1 2 3 2 chunks +12 lines, -2 lines 0 comments Download
M mojo/public/cpp/bindings/lib/control_message_proxy.cc View 1 2 3 4 5 5 chunks +57 lines, -0 lines 0 comments Download
M mojo/public/cpp/bindings/lib/interface_endpoint_client.cc View 1 2 3 6 chunks +15 lines, -7 lines 0 comments Download
M mojo/public/cpp/bindings/lib/interface_ptr_state.h View 1 2 3 4 7 chunks +28 lines, -21 lines 0 comments Download
M mojo/public/cpp/bindings/lib/router.h View 1 2 4 chunks +10 lines, -1 line 0 comments Download
M mojo/public/cpp/bindings/lib/router.cc View 1 2 3 6 chunks +15 lines, -7 lines 0 comments Download
M mojo/public/cpp/bindings/lib/validation_util.h View 1 2 3 1 chunk +0 lines, -7 lines 0 comments Download
M mojo/public/cpp/bindings/lib/validation_util.cc View 1 2 3 1 chunk +0 lines, -32 lines 0 comments Download
M mojo/public/cpp/bindings/strong_binding.h View 1 chunk +6 lines, -0 lines 0 comments Download
M mojo/public/cpp/bindings/tests/associated_interface_unittest.cc View 1 2 3 4 3 chunks +202 lines, -0 lines 0 comments Download
M mojo/public/cpp/bindings/tests/binding_unittest.cc View 1 2 3 4 3 chunks +84 lines, -1 line 0 comments Download
M mojo/public/cpp/bindings/tests/interface_ptr_unittest.cc View 1 2 3 4 2 chunks +37 lines, -0 lines 0 comments Download
M mojo/public/cpp/bindings/tests/multiplex_router_unittest.cc View 1 2 3 6 chunks +19 lines, -45 lines 0 comments Download
M mojo/public/cpp/bindings/tests/router_unittest.cc View 1 2 3 5 chunks +12 lines, -38 lines 0 comments Download
M mojo/public/interfaces/bindings/interface_control_messages.mojom View 2 chunks +4 lines, -0 lines 0 comments Download
M mojo/public/tools/bindings/generators/cpp_templates/interface_definition.tmpl View 1 2 6 chunks +8 lines, -19 lines 0 comments Download
M mojo/public/tools/bindings/generators/cpp_templates/interface_proxy_declaration.tmpl View 1 2 2 chunks +2 lines, -2 lines 0 comments Download
M mojo/public/tools/bindings/generators/cpp_templates/interface_stub_declaration.tmpl View 1 2 1 chunk +0 lines, -1 line 0 comments Download
Commit queue not available (can’t edit this change).

Messages

Total messages: 76 (63 generated)
Sam McNally
1 year ago (2016-08-26 20:23:19 UTC) #38
yzshen1
https://codereview.chromium.org/2280483002/diff/180001/mojo/public/cpp/bindings/lib/binding_state.cc File mojo/public/cpp/bindings/lib/binding_state.cc (right): https://codereview.chromium.org/2280483002/diff/180001/mojo/public/cpp/bindings/lib/binding_state.cc#newcode129 mojo/public/cpp/bindings/lib/binding_state.cc:129: ControlMessageProxy control_message_proxy(endpoint_client_.get()); Previously we only handled interface control messages ...
1 year ago (2016-08-26 21:07:07 UTC) #39
Sam McNally
https://codereview.chromium.org/2280483002/diff/180001/mojo/public/cpp/bindings/lib/binding_state.cc File mojo/public/cpp/bindings/lib/binding_state.cc (right): https://codereview.chromium.org/2280483002/diff/180001/mojo/public/cpp/bindings/lib/binding_state.cc#newcode129 mojo/public/cpp/bindings/lib/binding_state.cc:129: ControlMessageProxy control_message_proxy(endpoint_client_.get()); On 2016/08/26 21:07:07, yzshen1 wrote: > Previously ...
1 year ago (2016-08-29 06:39:24 UTC) #45
yzshen1
https://codereview.chromium.org/2280483002/diff/240001/mojo/public/cpp/bindings/binding_set.h File mojo/public/cpp/bindings/binding_set.h (right): https://codereview.chromium.org/2280483002/diff/240001/mojo/public/cpp/bindings/binding_set.h#newcode101 mojo/public/cpp/bindings/binding_set.h:101: binding->FlushForTesting(); I guess this won't work because bindings_ is ...
1 year ago (2016-08-29 23:24:58 UTC) #48
Sam McNally
https://codereview.chromium.org/2280483002/diff/240001/mojo/public/cpp/bindings/binding_set.h File mojo/public/cpp/bindings/binding_set.h (right): https://codereview.chromium.org/2280483002/diff/240001/mojo/public/cpp/bindings/binding_set.h#newcode101 mojo/public/cpp/bindings/binding_set.h:101: binding->FlushForTesting(); On 2016/08/29 23:24:58, yzshen1 wrote: > I guess ...
1 year ago (2016-08-30 03:05:52 UTC) #53
yzshen1
LGTM with a few nits. Thanks for making this change! https://codereview.chromium.org/2280483002/diff/300001/mojo/public/cpp/bindings/lib/associated_interface_ptr_state.h File mojo/public/cpp/bindings/lib/associated_interface_ptr_state.h (right): https://codereview.chromium.org/2280483002/diff/300001/mojo/public/cpp/bindings/lib/associated_interface_ptr_state.h#newcode94 ...
1 year ago (2016-08-31 17:06:47 UTC) #56
Ken Rockot(use gerrit already)
lgtm % yuzhu's nits
1 year ago (2016-08-31 19:05:00 UTC) #57
Sam McNally
+dcheng for the mojom https://codereview.chromium.org/2280483002/diff/300001/mojo/public/cpp/bindings/lib/associated_interface_ptr_state.h File mojo/public/cpp/bindings/lib/associated_interface_ptr_state.h (right): https://codereview.chromium.org/2280483002/diff/300001/mojo/public/cpp/bindings/lib/associated_interface_ptr_state.h#newcode94 mojo/public/cpp/bindings/lib/associated_interface_ptr_state.h:94: // The version is only ...
1 year ago (2016-09-01 00:44:37 UTC) #61
dcheng
mojom lgtm I looked over the other files as well, and the changes seem reasonable. ...
1 year ago (2016-09-01 08:19:00 UTC) #64
Sam McNally
https://codereview.chromium.org/2280483002/diff/320001/mojo/public/cpp/bindings/lib/control_message_proxy.cc File mojo/public/cpp/bindings/lib/control_message_proxy.cc (right): https://codereview.chromium.org/2280483002/diff/320001/mojo/public/cpp/bindings/lib/control_message_proxy.cc#newcode167 mojo/public/cpp/bindings/lib/control_message_proxy.cc:167: run_loop_quit_closure_.Run(); On 2016/09/01 08:19:00, dcheng wrote: > This could ...
1 year ago (2016-09-01 08:35:34 UTC) #67
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2280483002/340001
1 year ago (2016-09-01 09:48:33 UTC) #72
commit-bot: I haz the power
Committed patchset #6 (id:340001)
1 year ago (2016-09-01 09:53:18 UTC) #74
commit-bot: I haz the power
1 year ago (2016-09-01 09:55:25 UTC) #76
Message was sent while issue was closed.
Patchset 6 (id:??) landed as
https://crrev.com/462458f4b533d44bfad95ea07dc99257eaf028cb
Cr-Commit-Position: refs/heads/master@{#415920}
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld b40b6558b