Remove [Client=] annotation from ServiceProvider

This removes the symmetrical nature of ServiceProvider and consistently
passes and uses a ServiceProvider + ServiceProvider& pair in places that
wish to bidirectionally expose services, such as the view manager.

The
view manager library now deals with InterfaceRequest<ServiceProvider>
and ServiceProviderPtr objects (i.e. c++ wrappers for handles) instead
of a concrete implementation of ServiceProvider to make it easier for
callers.

A number of places that were assuming a particular
ServiceProvider would always exist are updated to reflect the nullability
of the parameters in mojom and places that do not wish to ever look up
or provide services now pass nullptr instead of doomed pipe handles.

The JS application startup classes are reworked a bit to accomodate
exposing services on the third ConnectToApplication/AcceptConnection
parameter.

BUG=449432
R=abarth@chromium.org, sky@chromium.org

Review URL: https://codereview.chromium.org/858103002

engine/core/html/HTMLIFrameElement.cpp

@@ -58,7 +58,7 @@ void HTMLIFrameElement::OnViewDestroyed(mojo::View* view)
 
 ScriptValue HTMLIFrameElement::takeServiceProvider(ScriptState* scriptState)
 {
-    return ScriptValue(scriptState, gin::ConvertToV8(scriptState->isolate(), m_serviceProvider.release()));
+    return ScriptValue(scriptState, gin::ConvertToV8(scriptState->isolate(), m_services.PassMessagePipe().release()));
 }
 
 void HTMLIFrameElement::createView()
@@ -76,10 +76,9 @@ void HTMLIFrameElement::createView()
     if (!m_contentView)
         return;
 
-    mojo::MessagePipe pipe;
-    m_serviceProvider = pipe.handle0.Pass();
     m_contentView->Embed(mojo::String::From(url.string().utf8().data()),
-        mojo::MakeRequest<mojo::ServiceProvider>(pipe.handle1.Pass()));
+        GetProxy(&m_services),
+        nullptr);  // TODO(abarth) Expose the exposedService parameter.
     m_contentView->AddObserver(this);
 }
 

engine/core/html/HTMLIFrameElement.h

@@ -44,7 +44,7 @@ private:
     void createView();
 
     mojo::View* m_contentView;
-    mojo::ScopedMessagePipeHandle m_serviceProvider;
+    mojo::ServiceProviderPtr m_services;
 };
 
 } // namespace blink

engine/v8_inspector/inspector_backend_mojo.cc

@@ -106,13 +106,13 @@ InspectorBackendMojoImpl::~InspectorBackendMojoImpl() {
 
 void InspectorBackendMojoImpl::Connect() {
   mojo::Shell* shell = host_->GetShell();
-  mojo::InterfaceRequest<mojo::ServiceProvider> service_provider_request;
-  mojo::MessagePipe pipe;
-  service_provider_request.Bind(pipe.handle0.Pass());
-  inspector_service_provider_.BindToHandle(pipe.handle1.Pass());
-  shell->ConnectToApplication("mojo:sky_inspector_server",
-                              service_provider_request.Pass(), nullptr);
-  mojo::ConnectToService(&inspector_service_provider_, &frontend_);
+
+  mojo::ServiceProviderPtr services;
+  mojo::ServiceProviderPtr exposed_services;
+  inspector_service_provider_.Bind(GetProxy(&exposed_services));
+  shell->ConnectToApplication("mojo:sky_inspector_server", GetProxy(&services),
+                              exposed_services.Pass());
+  mojo::ConnectToService(services.get(), &frontend_);
 
   // Theoretically we should load our state from the inspector cookie.
   inspector_state_ =

services/inspector/server.cc

@@ -39,12 +39,14 @@ class Server : public mojo::ApplicationDelegate,
       mojo::ApplicationConnection* connection) override {
     connection->AddService<InspectorFrontend>(this);
     connection->AddService<InspectorServer>(this);
-    // The application connecting to us may implement InspectorBackend,
-    // attempt to establish a connection to find out. If it doesn't then this
-    // pipe will close.
-    InspectorBackendPtr backend;
-    connection->ConnectToService(&backend);
-    backends_.AddInterfacePtr(backend.Pass());
+    if (connection->GetServiceProvider()) {
+      // The application connecting to us may implement InspectorBackend,
+      // attempt to establish a connection to find out. If it doesn't then this
+      // pipe will close.
+      InspectorBackendPtr backend;
+      connection->ConnectToService(&backend);
+      backends_.AddInterfacePtr(backend.Pass());
+    }
     return true;
   }
 

tools/debugger/debugger.cc

@@ -15,6 +15,7 @@ SkyDebugger::SkyDebugger()
       content_(nullptr),
       navigator_host_factory_(this),
       weak_factory_(this) {
+  exposed_services_impl_.AddService(&navigator_host_factory_);
 }
 
 SkyDebugger::~SkyDebugger() {
@@ -45,8 +46,8 @@ bool SkyDebugger::ConfigureOutgoingConnection(
 
 void SkyDebugger::OnEmbed(
     mojo::View* root,
-    mojo::ServiceProviderImpl* exported_services,
-    scoped_ptr<mojo::ServiceProvider> imported_services) {
+    mojo::InterfaceRequest<mojo::ServiceProvider> services,
+    mojo::ServiceProviderPtr exposed_services) {
   root_ = root;
   root_->AddObserver(this);
 
@@ -64,13 +65,10 @@ void SkyDebugger::OnEmbed(
     NavigateToURL(pending_url_);
 }
 
-void SkyDebugger::Embed(
-    const mojo::String& url,
-    mojo::InterfaceRequest<mojo::ServiceProvider> service_provider) {
-  scoped_ptr<mojo::ServiceProviderImpl> exported_services(
-      new mojo::ServiceProviderImpl());
-  // exported_services->AddService(TBD) -- no exported services for now.
-  content_->Embed(url, exported_services.Pass());
+void SkyDebugger::Embed(const mojo::String& url,
+                        mojo::InterfaceRequest<mojo::ServiceProvider> services,
+                        mojo::ServiceProviderPtr exposed_services) {
+  content_->Embed(url, nullptr, nullptr);
 }
 
 void SkyDebugger::OnViewManagerDisconnected(mojo::ViewManager* view_manager) {
@@ -97,10 +95,9 @@ void SkyDebugger::NavigateToURL(const mojo::String& url) {
   // embedded into the view and content_ created.
   // Just save the last one.
   if (content_) {
-    scoped_ptr<mojo::ServiceProviderImpl> exported_services(
-      new mojo::ServiceProviderImpl());
-    exported_services->AddService(&navigator_host_factory_);
-    viewer_services_ = content_->Embed(url, exported_services.Pass());
+    mojo::ServiceProviderPtr exposed_services;
+    exposed_services_impl_.Bind(GetProxy(&exposed_services));
+    content_->Embed(url, GetProxy(&viewer_services_), exposed_services.Pass());
   } else {
     pending_url_ = url;
   }

tools/debugger/debugger.h

@@ -48,8 +48,8 @@ class SkyDebugger : public mojo::ApplicationDelegate,
 
   // Overridden from mojo::ViewManagerDelegate:
   void OnEmbed(mojo::View* root,
-               mojo::ServiceProviderImpl* exported_services,
-               scoped_ptr<mojo::ServiceProvider> imported_services) override;
+               mojo::InterfaceRequest<mojo::ServiceProvider> services,
+               mojo::ServiceProviderPtr exposed_services) override;
   void OnViewManagerDisconnected(mojo::ViewManager* view_manager) override;
 
   // Overriden from mojo::ViewObserver:
@@ -63,9 +63,9 @@ class SkyDebugger : public mojo::ApplicationDelegate,
               mojo::InterfaceRequest<Debugger> request) override;
 
   // Overridden from WindowManagerDelegate
-  void Embed(
-      const mojo::String& url,
-      mojo::InterfaceRequest<mojo::ServiceProvider> service_provider) override;
+  void Embed(const mojo::String& url,
+             mojo::InterfaceRequest<mojo::ServiceProvider> services,
+             mojo::ServiceProviderPtr exposed_services) override;
 
   scoped_ptr<window_manager::WindowManagerApp> window_manager_app_;
 
@@ -73,9 +73,10 @@ class SkyDebugger : public mojo::ApplicationDelegate,
   mojo::View* content_;
   std::string pending_url_;
 
-  scoped_ptr<mojo::ServiceProvider> viewer_services_;
+  mojo::ServiceProviderPtr viewer_services_;
 
   NavigatorHostFactory navigator_host_factory_;
+  mojo::ServiceProviderImpl exposed_services_impl_;
 
   base::WeakPtrFactory<SkyDebugger> weak_factory_;
 

tools/tester/test_runner.cc

@@ -7,7 +7,6 @@
 #include <iostream>
 #include "base/bind.h"
 #include "mojo/public/cpp/application/connect.h"
-#include "mojo/public/cpp/application/service_provider_impl.h"
 #include "mojo/services/view_manager/public/cpp/view.h"
 
 namespace sky {
@@ -24,11 +23,11 @@ TestRunner::TestRunner(TestRunnerClient* client, mojo::View* container,
       enable_pixel_dumping_(enable_pixel_dumping) {
   CHECK(client);
 
-  scoped_ptr<mojo::ServiceProviderImpl> exported_services(
-    new mojo::ServiceProviderImpl());
-  exported_services->AddService(&test_harness_factory_);
+  mojo::ServiceProviderPtr test_harness_provider;
+  test_harness_provider_impl_.AddService(&test_harness_factory_);
+  test_harness_provider_impl_.Bind(GetProxy(&test_harness_provider));
 
-  container->Embed(url, exported_services.Pass());
+  container->Embed(url, nullptr, test_harness_provider.Pass());
 }
 
 TestRunner::~TestRunner() {

tools/tester/test_runner.h

@@ -6,6 +6,7 @@
 #define SKY_TOOLS_TESTER_TEST_RUNNER_H_
 
 #include "base/memory/weak_ptr.h"
+#include "mojo/public/cpp/application/service_provider_impl.h"
 #include "sky/tools/tester/test_harness_impl.h"
 
 namespace mojo{
@@ -40,6 +41,7 @@ class TestRunner {
 
  private:
   TestHarnessFactory test_harness_factory_;
+  mojo::ServiceProviderImpl test_harness_provider_impl_;
   TestRunnerClient* client_;
   base::WeakPtrFactory<TestRunner> weak_ptr_factory_;
   bool enable_pixel_dumping_;

tools/tester/tester.cc

@@ -83,10 +83,9 @@ class SkyTester : public mojo::ApplicationDelegate,
   }
 
   // Overridden from mojo::ViewManagerDelegate:
-  virtual void OnEmbed(
-      mojo::View* root,
-      mojo::ServiceProviderImpl* exported_services,
-      scoped_ptr<mojo::ServiceProvider> remote_service_provider) override {
+  virtual void OnEmbed(mojo::View* root,
+                       mojo::InterfaceRequest<mojo::ServiceProvider> services,
+                       mojo::ServiceProviderPtr exposed_services) override {
     root_ = root;
     root_->AddObserver(this);
 
@@ -101,10 +100,9 @@ class SkyTester : public mojo::ApplicationDelegate,
   }
 
   // Overridden from window_manager::WindowManagerDelegate:
-  virtual void Embed(
-      const mojo::String& url,
-      mojo::InterfaceRequest<mojo::ServiceProvider> service_provider) override {
-  }
+  virtual void Embed(const mojo::String& url,
+                     mojo::InterfaceRequest<mojo::ServiceProvider> services,
+                     mojo::ServiceProviderPtr exposed_services) override {}
 
   virtual void OnViewManagerDisconnected(
       mojo::ViewManager* view_manager) override {

viewer/document_view.cc

@@ -81,6 +81,7 @@ DocumentView::DocumentView(
     mojo::URLResponsePtr response,
     mojo::Shell* shell)
     : response_(response.Pass()),
+      exported_services_(services.Pass()),
       shell_(shell),
       web_view_(nullptr),
       root_(nullptr),
@@ -89,9 +90,8 @@ DocumentView::DocumentView(
       bitmap_rasterizer_(nullptr),
       debugger_id_(s_next_debugger_id++),
       weak_factory_(this) {
-  // TODO(jamesr): Is this right?
   exported_services_.AddService(&view_manager_client_factory_);
-  mojo::WeakBindToPipe(&exported_services_, services.PassMessagePipe());
+  inspector_service_provider_impl_.AddService(&inspector_service_factory_);
 }
 
 DocumentView::~DocumentView() {
@@ -107,12 +107,14 @@ base::WeakPtr<DocumentView> DocumentView::GetWeakPtr() {
 
 void DocumentView::OnEmbed(
     mojo::View* root,
-    mojo::ServiceProviderImpl* exported_services,
-    scoped_ptr<mojo::ServiceProvider> imported_services) {
+    mojo::InterfaceRequest<mojo::ServiceProvider> services,
+    mojo::ServiceProviderPtr exposed_services) {
   root_ = root;
-  imported_services_ = imported_services.Pass();
+  imported_services_ = exposed_services.Pass();
   navigator_host_.set_service_provider(imported_services_.get());
-  exported_services->AddService(&inspector_service_factory_);
+
+  if (services.is_pending())
+    inspector_service_provider_impl_.Bind(services.Pass());
 
   Load(response_.Pass());
 

viewer/document_view.h

@@ -103,10 +103,9 @@ class DocumentView : public blink::ServiceProvider,
   mojo::Shell* Shell() override;
 
   // ViewManagerDelegate methods:
-  void OnEmbed(
-      mojo::View* root,
-      mojo::ServiceProviderImpl* exported_services,
-      scoped_ptr<mojo::ServiceProvider> imported_services) override;
+  void OnEmbed(mojo::View* root,
+               mojo::InterfaceRequest<mojo::ServiceProvider> services,
+               mojo::ServiceProviderPtr exposed_services) override;
   void OnViewManagerDisconnected(mojo::ViewManager* view_manager) override;
 
   // ViewObserver methods:
@@ -124,13 +123,14 @@ class DocumentView : public blink::ServiceProvider,
 
   mojo::URLResponsePtr response_;
   mojo::ServiceProviderImpl exported_services_;
-  scoped_ptr<mojo::ServiceProvider> imported_services_;
+  mojo::ServiceProviderPtr imported_services_;
   mojo::Shell* shell_;
   mojo::LazyInterfacePtr<mojo::NavigatorHost> navigator_host_;
   blink::WebView* web_view_;
   mojo::View* root_;
   mojo::ViewManagerClientFactory view_manager_client_factory_;
   InspectorServiceFactory inspector_service_factory_;
+  mojo::ServiceProviderImpl inspector_service_provider_impl_;
   scoped_ptr<LayerHost> layer_host_;
   scoped_refptr<Layer> root_layer_;
   RasterizerBitmap* bitmap_rasterizer_;  // Used for pixel tests.

viewer/internals.cc

@@ -40,7 +40,8 @@ gin::Handle<Internals> Internals::Create(
 Internals::Internals(DocumentView* document_view)
   : document_view_(document_view->GetWeakPtr()),
     shell_binding_(this) {
-  mojo::ConnectToService(document_view->imported_services(), &test_harness_);
+  if (document_view_->imported_services())
+    mojo::ConnectToService(document_view_->imported_services(), &test_harness_);
 }
 
 Internals::~Internals() {
@@ -77,13 +78,15 @@ void Internals::NotifyTestComplete(const std::string& test_result) {
     return;
   std::vector<unsigned char> pixels;
   document_view_->GetPixelsForTesting(&pixels);
-  test_harness_->OnTestComplete(test_result,
-      mojo::Array<uint8_t>::From(pixels));
+  if (test_harness_) {
+    test_harness_->OnTestComplete(test_result,
+        mojo::Array<uint8_t>::From(pixels));
+  }
 }
 
 mojo::Handle Internals::ConnectToEmbedderService(
     const std::string& interface_name) {
-  if (!document_view_)
+  if (!document_view_ || !document_view_->imported_services())
     return mojo::Handle();
 
   mojo::MessagePipe pipe;