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

Issue 2779203003: [Media Router] Export Mojom definitions in media_router_bindings.js. (Closed)

Created:
3 years, 8 months ago by imcheng
Modified:
3 years, 8 months ago
Reviewers:
mark a. foltz
CC:
chromium-apps-reviews_chromium.org, chromium-reviews, extensions-reviews_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

[Media Router] Export Mojom definitions in media_router_bindings.js. In media_router_bindings, instead of just exporting the mediaRouter instance to the component, also export definitions of generated Mojom classes that will be needed by the component. This lets the component work with Mojo objects directly rather than relying on bindings as an intermediate layer. BUG=687365 Review-Url: https://codereview.chromium.org/2779203003 Cr-Commit-Position: refs/heads/master@{#461194} Committed: https://chromium.googlesource.com/chromium/src/+/532ac77836647714a0ed93c871b4a0b75159fcaa

Patch Set 1 #

Patch Set 2 : Add MediaController / MediaStatusObsever into bindigns #

Total comments: 1

Patch Set 3 : backwards compatible #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+87 lines, -2 lines) Patch
M extensions/renderer/resources/media_router_bindings.js View 1 2 5 chunks +87 lines, -2 lines 1 comment Download

Messages

Total messages: 11 (5 generated)
imcheng
PTAL
3 years, 8 months ago (2017-03-29 23:01:46 UTC) #3
imcheng
https://codereview.chromium.org/2779203003/diff/20001/extensions/renderer/resources/media_router_bindings.js File extensions/renderer/resources/media_router_bindings.js (right): https://codereview.chromium.org/2779203003/diff/20001/extensions/renderer/resources/media_router_bindings.js#newcode9 extensions/renderer/resources/media_router_bindings.js:9: 'chrome/browser/media/router/mojo/media_controller.mojom', Note: I will change this if Takumi's patch ...
3 years, 8 months ago (2017-03-29 23:37:33 UTC) #4
mark a. foltz
Thanks, this looks cleaner. Looking forward to the day when we can eliminate the trampoline ...
3 years, 8 months ago (2017-03-30 23:21:05 UTC) #5
mark a. foltz
lgtm
3 years, 8 months ago (2017-03-30 23:21:07 UTC) #6
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/2779203003/40001
3 years, 8 months ago (2017-03-31 19:33:40 UTC) #8
commit-bot: I haz the power
3 years, 8 months ago (2017-03-31 20:01:44 UTC) #11
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/532ac77836647714a0ed93c871b4...

Powered by Google App Engine
This is Rietveld 408576698