Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(479)

Issue 2820783002: Add associated interfaces & bindings. (Closed)

Created:
3 years, 8 months ago by wangjimmy
Modified:
3 years, 8 months ago
CC:
Aaron Boodman, abarth-chromium, blink-reviews, chromium-reviews, darin (slow to review), qsr+mojo_chromium.org, viettrungluu+watch_chromium.org, yzshen+watch_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Add associated interfaces & bindings. Add message serialize and deserialize associated endpoint handles. Add new MessageV2Builder, add encode and decode for associated endpoint handles. Modify tmpl to generate AssociatedInterfacePtr and use MessageV2Builder. Add associated_interface_ptr.html layout test to test encoding/decoding. BUG=695635 Review-Url: https://codereview.chromium.org/2820783002 Cr-Commit-Position: refs/heads/master@{#466531} Committed: https://chromium.googlesource.com/chromium/src/+/6aabe530831aad419602e06ba5ac364ca18dc862

Patch Set 1 #

Patch Set 2 : Add encode/decode & test for AssociatedInterfacePtrInfo. Generate MessageV2Builder in mojom only if… #

Patch Set 3 : Add associated interface connection error tests. Code clean up. #

Patch Set 4 : Add passInterface and unBind for Associated Interfaces. #

Patch Set 5 : Add test_associated_interfaces mojom as dependency for webkit_layout_tests. #

Patch Set 6 : Add documentation in associated_binding.js #

Patch Set 7 : Add associated_interface_ptr.html test interfaces at both ends #

Total comments: 6

Patch Set 8 : Remove changes to vibration-iframe-expected.txt because it was removed in master. #

Total comments: 45

Patch Set 9 : Add mojom interface IntegerSenderConnectionAtBothEnds for associated_interface_ptr.html test, '… #

Patch Set 10 : Add to method isNullable, types NullableAssociatedInterfacePtrInfo & NullableAssociatedInterfaceReq… #

Patch Set 11 : Formatting. Simplify tmpl code. Address code review. #

Patch Set 12 : Change Router.prototype.accept. Add a TODO for endpoint client not attached. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+859 lines, -102 lines) Patch
M BUILD.gn View 1 2 3 4 1 chunk +1 line, -0 lines 0 comments Download
M mojo/public/interfaces/bindings/tests/test_associated_interfaces.mojom View 1 2 3 4 5 6 7 8 1 chunk +5 lines, -0 lines 0 comments Download
M mojo/public/js/associated_bindings.js View 1 2 3 4 5 6 7 8 9 10 1 chunk +224 lines, -0 lines 0 comments Download
M mojo/public/js/bindings.js View 1 2 5 chunks +11 lines, -11 lines 0 comments Download
M mojo/public/js/codec.js View 1 2 3 4 5 6 7 8 9 10 15 chunks +194 lines, -20 lines 0 comments Download
M mojo/public/js/lib/control_message_handler.js View 1 2 3 4 5 6 7 8 9 10 1 chunk +1 line, -1 line 0 comments Download
M mojo/public/js/lib/control_message_proxy.js View 1 2 3 4 5 6 7 8 9 10 2 chunks +2 lines, -2 lines 0 comments Download
M mojo/public/js/lib/interface_endpoint_client.js View 1 2 3 4 5 6 7 8 9 10 5 chunks +15 lines, -8 lines 0 comments Download
M mojo/public/js/lib/interface_endpoint_handle.js View 1 2 3 4 5 6 7 8 9 10 3 chunks +29 lines, -0 lines 0 comments Download
M mojo/public/js/lib/pipe_control_message_proxy.js View 1 2 3 4 5 6 7 8 9 10 1 chunk +1 line, -1 line 0 comments Download
M mojo/public/js/new_bindings/codec.js View 1 2 3 4 5 6 7 8 9 10 5 chunks +13 lines, -13 lines 0 comments Download
M mojo/public/js/new_bindings/lib/control_message_handler.js View 1 2 3 4 5 6 7 8 9 10 1 chunk +1 line, -1 line 0 comments Download
M mojo/public/js/new_bindings/lib/control_message_proxy.js View 1 2 3 4 5 6 7 8 9 10 2 chunks +2 lines, -2 lines 0 comments Download
M mojo/public/js/router.js View 1 2 3 4 5 6 7 8 9 10 11 2 chunks +56 lines, -15 lines 0 comments Download
M mojo/public/js/validator.js View 1 2 3 4 5 6 7 8 9 1 chunk +2 lines, -0 lines 0 comments Download
M mojo/public/tools/bindings/generators/js_templates/interface_definition.tmpl View 1 2 3 4 5 6 7 8 9 10 11 3 chunks +38 lines, -3 lines 0 comments Download
M mojo/public/tools/bindings/generators/js_templates/module_definition.tmpl View 1 chunk +1 line, -0 lines 0 comments Download
M mojo/public/tools/bindings/generators/mojom_js_generator.py View 1 1 chunk +1 line, -0 lines 0 comments Download
M mojo/public/tools/bindings/pylib/mojom/generate/module.py View 1 2 3 4 5 6 7 8 9 10 2 chunks +23 lines, -12 lines 0 comments Download
A third_party/WebKit/LayoutTests/mojo/associated_interface_ptr.html View 1 2 3 4 5 6 7 8 9 10 1 chunk +226 lines, -0 lines 0 comments Download
M third_party/WebKit/LayoutTests/mojo/codec.html View 1 2 3 4 5 6 7 8 9 10 9 chunks +9 lines, -9 lines 0 comments Download
M third_party/WebKit/LayoutTests/mojo/struct.html View 1 2 3 4 5 6 7 8 9 10 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/LayoutTests/mojo/union.html View 1 2 3 4 5 6 7 8 9 10 2 chunks +2 lines, -2 lines 0 comments Download
M third_party/WebKit/LayoutTests/vibration/vibration-expected.txt View 1 2 3 4 5 6 7 8 9 10 1 chunk +1 line, -1 line 0 comments Download

Dependent Patchsets:

Messages

Total messages: 58 (44 generated)
wangjimmy
Hi Yuzhu, PTAL at mojo/. Thank you!
3 years, 8 months ago (2017-04-18 16:16:18 UTC) #16
wangjimmy
ERROR:mojo_context_state.cc(209)] Failed to fetch source for module "mojo/public/interfaces/bindings/tests/test_associated_interfaces.mojom" 20:53:38.022 25381 [8356/9384] mojo/associated_interface_ptr.html failed unexpectedly (test ...
3 years, 8 months ago (2017-04-18 16:18:20 UTC) #17
yzshen1
First part of comments, I haven't reviewed the tests. Thanks! https://codereview.chromium.org/2820783002/diff/120001/mojo/public/js/associated_bindings.js File mojo/public/js/associated_bindings.js (right): https://codereview.chromium.org/2820783002/diff/120001/mojo/public/js/associated_bindings.js#newcode17 ...
3 years, 8 months ago (2017-04-19 21:11:35 UTC) #32
wangjimmy
https://codereview.chromium.org/2820783002/diff/120001/mojo/public/js/associated_bindings.js File mojo/public/js/associated_bindings.js (right): https://codereview.chromium.org/2820783002/diff/120001/mojo/public/js/associated_bindings.js#newcode17 mojo/public/js/associated_bindings.js:17: var {handle0, handle1} = On 2017/04/19 21:11:33, yzshen1 wrote: ...
3 years, 8 months ago (2017-04-20 15:36:44 UTC) #37
yzshen1
https://codereview.chromium.org/2820783002/diff/140001/mojo/public/js/router.js File mojo/public/js/router.js (right): https://codereview.chromium.org/2820783002/diff/140001/mojo/public/js/router.js#newcode192 mojo/public/js/router.js:192: return false; > Since RaiseErrorInNonTestingMode() isn't called when you ...
3 years, 8 months ago (2017-04-20 18:12:13 UTC) #38
wangjimmy
Hi Ken, could you also PTAL at this patch as the secondary reviewer. Thank you!
3 years, 8 months ago (2017-04-20 21:54:28 UTC) #42
yzshen1
lgtm
3 years, 8 months ago (2017-04-21 00:43:25 UTC) #45
Ken Rockot(use gerrit already)
lgtm
3 years, 8 months ago (2017-04-21 19:46:42 UTC) #46
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/2820783002/220001
3 years, 8 months ago (2017-04-21 20:30:37 UTC) #48
commit-bot: I haz the power
Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presubmit/builds/417867)
3 years, 8 months ago (2017-04-21 20:40:05 UTC) #50
wangjimmy
Hi John PTAL at BUILD.gn Thank you
3 years, 8 months ago (2017-04-21 21:07:02 UTC) #52
jam
lgtm
3 years, 8 months ago (2017-04-21 22:42:33 UTC) #53
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/2820783002/220001
3 years, 8 months ago (2017-04-22 01:25:44 UTC) #55
commit-bot: I haz the power
3 years, 8 months ago (2017-04-22 03:06:30 UTC) #58
Message was sent while issue was closed.
Committed patchset #12 (id:220001) as
https://chromium.googlesource.com/chromium/src/+/6aabe530831aad419602e06ba5ac...

Powered by Google App Engine
This is Rietveld 408576698