Remove [Client=] annotation from ServiceProvider

mojo/services/view_manager/public/interfaces/view_manager.mojom

Index: mojo/services/view_manager/public/interfaces/view_manager.mojom
diff --git a/mojo/services/view_manager/public/interfaces/view_manager.mojom b/mojo/services/view_manager/public/interfaces/view_manager.mojom
index 0f5c51d6cbfd604f161c17aad8ee31b7ba627a46..ba5944453b6a7278ee980eab2d4617d3714cad95 100644
--- a/mojo/services/view_manager/public/interfaces/view_manager.mojom
+++ b/mojo/services/view_manager/public/interfaces/view_manager.mojom
@@ -124,15 +124,16 @@ interface ViewManagerService {
   // children the children are removed. The one exception is the root
   // connection.
   //
-  // |service_provider| encapsulates services offered by the embedder to the
-  // embedded app alongside this Embed() call. It also provides a means for
-  // the embedder to connect to services symmetrically exposed by the embedded
-  // app. Note that if a different app is subsequently embedded at |view_id|
-  // the |service_provider|'s connection to its client in the embedded app and
-  // any services it provided are not broken and continue to be valid.
+  // |services| encapsulates services offered by the embedder to the embedded

[sky, 2015/01/21 16:59:00]

The names services and exposed_services are confusing. Additionally you don't
use the names consistently; ViewManagerClientImpl uses parent_services and
parent_exposed_services. Is there better names we can use? Some ideas:

parent_to_client_services and client_to_parent_services: long, but better
conveys the directionality.
embedder_to_embedded_services and embedded_to_embedder_services: longer, and I
think 'embedder' and 'embedded' are so similar that it's easy to confuse the
two.

WDYT?

My vote is for parent_client_services and client_to_parent_services.

No matter what you go with, please use it consistently.

[jamesr, 2015/01/21 18:51:40]

Done.

[jamesr, 2015/01/21 18:51:40]

These are the same that //mojo/public/interfaces/application/application.mojom
uses, which had a lot of debate already.  This is the same thing and I expect
everyone working on mojo will need to be fairly familiar with the concepts and
names.  Do you think there's an inherent difference between this usage and that
one, or do you think the discussion on the application names needs to be
reopened?

+  // app alongside this Embed() call. |exposed_services| provides a means for
+  // the embedder to connect to services exposed by the embedded app. Note that
+  // if a different app is subsequently embedded at |view_id| the
+  // ServiceProvider connections to its client in the embedded app and any
+  // services it provided are not broken and continue to be valid.
   Embed(string url,
         uint32 view_id,
-        ServiceProvider& service_provider) => (bool success);
+        ServiceProvider&? services,

[sky, 2015/01/21 16:59:00]

We recently made service_provider required. Could you make the pair required?

[jamesr, 2015/01/21 18:51:40]

That was a mistake
No.  In many cases (most cases, in the current codebase) there are no services
to expose in one or both directions.  In this case, allocating a message pipe
and passing a handle is overhead with no benefit.  If you have nothing to
provide or have nothing to request, you can leave these null.  The one-parameter
View::Embed() function is an example of needing neither.

+        ServiceProvider? exposed_services) => (bool success);
 };
 
 // Changes to views are not sent to the connection that originated the
@@ -146,7 +147,8 @@ interface ViewManagerClient {
   OnEmbed(uint16 connection_id,
           string embedder_url,
           ViewData root,
-          ServiceProvider& parent_service_provider,
+          ServiceProvider&? parent_services,
+          ServiceProvider? parent_exposed_services,
           handle<message_pipe> window_manager_pipe);
 
   // Invoked when the application embedded at |view| is disconnected.