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

mojo/apps/js/application_delegate_impl.h

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef MOJO_APPS_JS_CONTENT_HANDLER_H_
 #define MOJO_APPS_JS_CONTENT_HANDLER_H_
 
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/scoped_vector.h"
 #include "mojo/public/cpp/application/application_delegate.h"
@@ skipping 19 matching lines @@
  public:
   ApplicationDelegateImpl();
   ~ApplicationDelegateImpl() override;
 
   // Add app to the AppVector and call its Start() method.
   void StartJSApp(scoped_ptr<JSApp> app);
 
   // 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:
   void Initialize(ApplicationImpl* app) override;
 
  private:
   typedef ScopedVector<JSApp> AppVector;
-  ApplicationImpl* application_impl_;
+  ApplicationImpl* application_impl_;  // Owns the shell used by all JSApps.
   AppVector app_vector_;
 };
 
 }  // namespace apps
 }  // namespace mojo
 
 #endif  // MOJO_APPS_JS_CONTENT_HANDLER_H_