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

mojo/apps/js/application_delegate_impl.h

Index: mojo/apps/js/application_delegate_impl.h
diff --git a/mojo/apps/js/application_delegate_impl.h b/mojo/apps/js/application_delegate_impl.h
index 81a481cd356bc3a76be815561d7a17358c8693af..d6f62c884895b6eb9fe6c98d8c086e5179b58a66 100644
--- a/mojo/apps/js/application_delegate_impl.h
+++ b/mojo/apps/js/application_delegate_impl.h
@@ -37,9 +37,9 @@ class ApplicationDelegateImpl : public ApplicationDelegate {
   // Remove app from the AppVector; destroys the app.
   void QuitJSApp(JSApp *app);
 
-  void ConnectToService(ScopedMessagePipeHandle pipe_handle,
-                        const std::string& application_url,
-                        const std::string& interface_name);
+  // Use the shell to connect to a ServiceProvider for application_url.

[Aaron Boodman, 2014/10/24 22:17:18]

Why is the first param a bare pipe rather than a ServiceProviderPtr or
something?

[hansmuller, 2014/10/27 22:43:22]

JSApp::ConnectToApplication() calls this method with one end of the MessagePipe
it's created. It returns the other end of the MessagePipe to JS. Using
ServiceProviderPtr would only make sense if the JS bindings based on
InterfacePtrs, since InterfacePtr encapsulates a Mojo handle.

[Aaron Boodman, 2014/10/29 17:12:51]

You turn it into an InterfaceRequest in the impl of this method file. Passing an
InterfaceRequest here instead of a bare handle would make the code more
self-documenting.

[hansmuller, 2014/10/29 22:43:40]

Done.

+  void ConnectToApplication(ScopedMessagePipeHandle pipe_handle,
+                            const std::string& application_url);

[Aaron Boodman, 2014/10/24 22:17:18]

Nit: it makes slightly more sense to me for application_url to go first in the
param list, but nbd.

[hansmuller, 2014/10/27 22:43:22]

I agree, reversing the order matches the ServiceProvider Mojom.

 
  protected:
   // ApplicationDelegate:
@@ -47,7 +47,7 @@ class ApplicationDelegateImpl : public ApplicationDelegate {
 
  private:
   typedef ScopedVector<JSApp> AppVector;
-  ApplicationImpl* application_impl_;
+  ApplicationImpl* application_impl_;  // Owns the shell used by all JSApps.
   AppVector app_vector_;
 };