Pass ServiceProvider thru ViewManagerService::Embed() allowing embedder & embeddee to expose servic…

mojo/public/cpp/application/exported_service_registry.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_PUBLIC_APPLICATION_EXPORTED_SERVICE_REGISTRY_H_
+#define MOJO_PUBLIC_APPLICATION_EXPORTED_SERVICE_REGISTRY_H_
+
+#include "mojo/public/cpp/application/lib/service_connector.h"
+#include "mojo/public/interfaces/application/service_provider.mojom.h"
+
+namespace mojo {
+namespace internal {
+class ServiceConnectorBase;
+}
+

[DaveMoore, 2014/08/06 21:34:59]

Nit: Please provide a comment. On quick inspection it looks like the type is
pointless.

+class RemoteServiceProvider : public ServiceProvider {

[darin (slow to review), 2014/08/06 22:13:29]

nit: break this class out into its own .h and .cc files

+ public:
+  explicit RemoteServiceProvider(ServiceProvider* service_provider)

[darin (slow to review), 2014/08/06 22:13:29]

technically, this is more of a ServiceProvider thunk or weakly bound
ServiceProvider. I wonder if its name shouldn't be based on that.
WeakServiceProvider or ServiceProviderThunk.

+      : service_provider_(service_provider) {}
+  virtual ~RemoteServiceProvider() {}
+
+  void Clear() {
+    service_provider_ = NULL;
+  }
+
+  virtual void ConnectToService(
+      const String& service_name,
+      ScopedMessagePipeHandle client_handle) MOJO_OVERRIDE {
+    if (service_provider_)
+      service_provider_->ConnectToService(service_name, client_handle.Pass());
+  }
+
+ private:
+  ServiceProvider* service_provider_;
+
+  MOJO_DISALLOW_COPY_AND_ASSIGN(RemoteServiceProvider);
+};
+
+class ExportedServiceRegistry : public InterfaceImpl<ServiceProvider> {

[darin (slow to review), 2014/08/06 22:13:29]

nit: an Impl suffix might be nice here just to preserve the notion of it being
an interface impl. also not sure about Registry versus Provider here. why not
just call this ServiceProviderImpl or ExportedServiceProviderImpl?

+ public:
+  ExportedServiceRegistry();
+  virtual ~ExportedServiceRegistry();
+
+  template <typename Interface>
+  void AddService(InterfaceFactory<Interface>* factory) {
+    AddServiceConnector(
+        new internal::InterfaceFactoryConnector<Interface>(factory));
+  }
+
+  void set_remote(RemoteServiceProvider* remote) { remote_ = remote; }

[darin (slow to review), 2014/08/06 22:13:29]

hmm, it's not clear what the remote service provider has to do with this service
provider impl.

instead of the user of this class having to allocate and set a RemoteSP on the
instance, wouldn't it make more sense for the ExportedServiceRegistry to vend a
RemoteSP pointer? then, you might even completely hide the implementation
details of RemoteServiceProvider.

+
+ private:
+  typedef std::map<std::string, internal::ServiceConnectorBase*>
+      NameToServiceConnectorMap;
+
+  // Overridden from ServiceProvider:
+  virtual void ConnectToService(
+      const String& service_name,
+      ScopedMessagePipeHandle client_handle) MOJO_OVERRIDE;
+
+  // Overridden from InterfaceImpl:
+  virtual void OnConnectionError() MOJO_OVERRIDE {
+    if (remote_)

[darin (slow to review), 2014/08/06 22:13:29]

nit:

if (remote_) {
  remote_->Clear();
  remote_ = NULL;
}

+      remote_->Clear();
+    remote_ = NULL;
+  }
+
+  void AddServiceConnector(
+      internal::ServiceConnectorBase* service_connector);
+  void RemoveServiceConnector(
+      internal::ServiceConnectorBase* service_connector);
+
+  NameToServiceConnectorMap service_connectors_;
+
+  RemoteServiceProvider* remote_;
+
+  MOJO_DISALLOW_COPY_AND_ASSIGN(ExportedServiceRegistry);
+};
+
+}  // namespace mojo
+
+#endif  // MOJO_PUBLIC_APPLICATION_EXPORTED_SERVICE_REGISTRY_H_