Mojo JS Bindings: Simplify sharing services for content-provided JS applications

Mojo JS Bindings: Simplify sharing services for content-provided JS applications

This CL was originally here: https://codereview.chromium.org/665743003

BUG=425684
R=aa@chromium.org

Committed: https://chromium.googlesource.com/external/mojo/+/7144debbae398519178e3dcba9cbae58277f2361

[hansmuller, 2014-10-29 22:42:03]

hansmuller@chromium.org changed reviewers:
+ aa@chromium.org

[Aaron Boodman, 2014-10-29 23:06:29]

lgtm w/ nits

Just fix these and go ahead and land without another round.

https://codereview.chromium.org/686203004/diff/1/mojo/apps/js/content_handler...
File mojo/apps/js/content_handler_impl.cc (right):

https://codereview.chromium.org/686203004/diff/1/mojo/apps/js/content_handler...
mojo/apps/js/content_handler_impl.cc:18: ScopedMessagePipeHandle sp)
You could at least make this InterfaceRequest<ServiceProvider>, and then move
the PassMessagePipe() to initializing requestor_message_pipe_hanlde_.

https://codereview.chromium.org/686203004/diff/1/mojo/apps/js/mojo_bridge_mod...
File mojo/apps/js/mojo_bridge_module.cc (right):

https://codereview.chromium.org/686203004/diff/1/mojo/apps/js/mojo_bridge_mod...
mojo/apps/js/mojo_bridge_module.cc:1: 
remove blank line

https://codereview.chromium.org/686203004/diff/1/mojo/bindings/js/handle.cc
File mojo/bindings/js/handle.cc (right):

https://codereview.chromium.org/686203004/diff/1/mojo/bindings/js/handle.cc#n...
mojo/bindings/js/handle.cc:89: v8::Handle<v8::Value>
Converter<mojo::MessagePipeHandle>::ToV8(
You can just delegate to the converter for mojo::Handle:

return Converter<mojo::Handle>::ToV8(isolate, val);

Same for FromV8.

[hansmuller, 2014-10-29 23:48:18]

Committed patchset #1 (id:1) manually as
7144debbae398519178e3dcba9cbae58277f2361 (presubmit successful).

[hansmuller, 2014-10-30 00:23:38]

Things that remain to be done:
- One JSApp() per OnConnect() response URL. This implies that requestor() can no
long be a global, each JSApp() may have many requestors. 
- Integration tests for a complete content-handler JS app, as well as unit tests
for the mojo module.
- Revise the tutorial/description from the original CL
https://codereview.chromium.org/665743003 and share it.
- Export the JS ServiceProvider class or something like it. It's what you get
from connectToApplication() so calling it a mojo Application might make more
sense.
- Add more handle types to js/handle.cc

https://codereview.chromium.org/686203004/diff/1/mojo/apps/js/content_handler...
File mojo/apps/js/content_handler_impl.cc (right):

https://codereview.chromium.org/686203004/diff/1/mojo/apps/js/content_handler...
mojo/apps/js/content_handler_impl.cc:18: ScopedMessagePipeHandle sp)
On 2014/10/29 23:06:28, Aaron Boodman wrote:
> You could at least make this InterfaceRequest<ServiceProvider>, and then move
> the PassMessagePipe() to initializing requestor_message_pipe_hanlde_.

Done.

https://codereview.chromium.org/686203004/diff/1/mojo/apps/js/mojo_bridge_mod...
File mojo/apps/js/mojo_bridge_module.cc (right):

https://codereview.chromium.org/686203004/diff/1/mojo/apps/js/mojo_bridge_mod...
mojo/apps/js/mojo_bridge_module.cc:1: 
On 2014/10/29 23:06:28, Aaron Boodman wrote:
> remove blank line

Done.

https://codereview.chromium.org/686203004/diff/1/mojo/bindings/js/handle.cc
File mojo/bindings/js/handle.cc (right):

https://codereview.chromium.org/686203004/diff/1/mojo/bindings/js/handle.cc#n...
mojo/bindings/js/handle.cc:89: v8::Handle<v8::Value>
Converter<mojo::MessagePipeHandle>::ToV8(
On 2014/10/29 23:06:29, Aaron Boodman wrote:
> You can just delegate to the converter for mojo::Handle:
> 
> return Converter<mojo::Handle>::ToV8(isolate, val);
> 
> Same for FromV8.

Good point!