From 375d0f44e0075c6d3f4efbe077651db2dfbd937f Mon Sep 17 00:00:00 2001 From: James Robinson Date: Fri, 9 Jan 2015 14:50:39 -0800 Subject: [PATCH] Remove client relationship between sky::Inspector{Front,Back}end This removes the client relationship between the sky::InspectorFrontend and sky::InspectorBackend mojo interfaces. Instead, the two are now unrelated (in the mojom) and just happen to be often used together. The inspector service provides the InspectorFrontend interface to connecting applications and optimistically tries to connect to the InspectorBackend interface of applications that connect, in case they want to receive messages. The front and back end interfaces are both broadcast APIs, so no attempt is made to keep track of which binding is which. If they did need to be correlated this could be easily accomplished by allocating a new object to keep a mojo::Binding and the corresponding sky::InspectorBackendPtr. R=abarth@chromium.org, eseidel@chromium.org Review URL: https://codereview.chromium.org/835463003 --- engine/v8_inspector/inspector_backend_mojo.cc | 30 +++++++++++++++---- framework/inspector/inspector.sky | 21 ++++++------- services/inspector/inspector.mojom | 2 -- services/inspector/server.cc | 21 ++++++++----- 4 files changed, 49 insertions(+), 25 deletions(-) diff --git a/engine/v8_inspector/inspector_backend_mojo.cc b/engine/v8_inspector/inspector_backend_mojo.cc index 136d34ef3ec..df8a08833a2 100644 --- a/engine/v8_inspector/inspector_backend_mojo.cc +++ b/engine/v8_inspector/inspector_backend_mojo.cc @@ -24,13 +24,15 @@ namespace blink { class InspectorBackendMojoImpl : public InspectorFrontendChannel, - public mojo::InterfaceImpl { + public sky::InspectorBackend, + public mojo::InterfaceFactory { public: explicit InspectorBackendMojoImpl(inspector::InspectorHost*); ~InspectorBackendMojoImpl(); void Connect(); + private: // InspectorBackend: void OnConnect(); void OnMessage(const mojo::String& message) override; @@ -41,8 +43,13 @@ class InspectorBackendMojoImpl // TODO(eseidel): Unclear if flush is needed. void flush() override {} + // mojo::InterfaceFactory + void Create(mojo::ApplicationConnection* connection, + mojo::InterfaceRequest request) override; + inspector::InspectorHost* host_; sky::InspectorFrontendPtr frontend_; + mojo::ServiceProviderImpl inspector_service_provider_; OwnPtr old_frontend_; RefPtr dispatcher_; @@ -51,6 +58,8 @@ class InspectorBackendMojoImpl OwnPtr inspector_state_; OwnPtr agents_; + mojo::Binding binding_; + DISALLOW_COPY_AND_ASSIGN(InspectorBackendMojoImpl); }; @@ -88,7 +97,8 @@ class InspectorHostResolverImpl : public PageScriptDebugServer::InspectorHostRes InspectorBackendMojoImpl::InspectorBackendMojoImpl( inspector::InspectorHost* host) - : host_(host) { + : host_(host), binding_(this) { + inspector_service_provider_.AddService(this); } InspectorBackendMojoImpl::~InspectorBackendMojoImpl() { @@ -96,11 +106,13 @@ InspectorBackendMojoImpl::~InspectorBackendMojoImpl() { void InspectorBackendMojoImpl::Connect() { mojo::Shell* shell = host_->GetShell(); - mojo::ServiceProviderPtr inspector_service_provider; + mojo::InterfaceRequest 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", - GetProxy(&inspector_service_provider)); - mojo::ConnectToService(inspector_service_provider.get(), &frontend_); - frontend_.set_client(this); + service_provider_request.Pass()); + mojo::ConnectToService(&inspector_service_provider_, &frontend_); // Theoretically we should load our state from the inspector cookie. inspector_state_ = @@ -149,6 +161,12 @@ void InspectorBackendMojoImpl::OnMessage(const mojo::String& message) { dispatcher_->dispatch(wtf_message); } +void InspectorBackendMojoImpl::Create( + mojo::ApplicationConnection* connection, + mojo::InterfaceRequest request) { + binding_.Bind(request.Pass()); +} + } // namespace blink namespace inspector { diff --git a/framework/inspector/inspector.sky b/framework/inspector/inspector.sky index 62c53fb3bf2..4d5107de19a 100644 --- a/framework/inspector/inspector.sky +++ b/framework/inspector/inspector.sky @@ -1,6 +1,7 @@ + @@ -11,7 +12,6 @@ diff --git a/services/inspector/inspector.mojom b/services/inspector/inspector.mojom index 4d65b439716..2208720cb8d 100644 --- a/services/inspector/inspector.mojom +++ b/services/inspector/inspector.mojom @@ -8,12 +8,10 @@ interface InspectorServer { Listen(int32 port) => (); }; -[Client=InspectorBackend] interface InspectorFrontend { SendMessage(string message); }; -[Client=InspectorFrontend] interface InspectorBackend { OnConnect(); OnDisconnect(); diff --git a/services/inspector/server.cc b/services/inspector/server.cc index 3cb300cd548..c5cd23e58bb 100644 --- a/services/inspector/server.cc +++ b/services/inspector/server.cc @@ -4,6 +4,7 @@ #include "mojo/application/application_runner_chromium.h" #include "mojo/common/weak_binding_set.h" +#include "mojo/common/weak_interface_ptr_set.h" #include "mojo/public/c/system/main.h" #include "mojo/public/cpp/application/application_delegate.h" #include "mojo/public/cpp/application/application_impl.h" @@ -38,6 +39,12 @@ class Server : public mojo::ApplicationDelegate, mojo::ApplicationConnection* connection) override { connection->AddService(this); connection->AddService(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()); return true; } @@ -72,6 +79,7 @@ class Server : public mojo::ApplicationDelegate, int connection_id_; scoped_ptr web_server_; + mojo::WeakInterfacePtrSet backends_; mojo::WeakBindingSet frontend_bindings_; mojo::Binding server_binding_; @@ -98,27 +106,26 @@ void Server::OnWebSocketRequest( } web_server_->AcceptWebSocket(connection_id, info); connection_id_ = connection_id; - frontend_bindings_.ForAllBindings( - [](InspectorFrontend::Client* client) { client->OnConnect(); }); + backends_.ForAllPtrs([](InspectorBackend* backend) { backend->OnConnect(); }); } void Server::OnWebSocketMessage( int connection_id, const std::string& data) { DCHECK_EQ(connection_id, connection_id_); - frontend_bindings_.ForAllBindings( - [data](InspectorFrontend::Client* client) { client->OnMessage(data); }); + backends_.ForAllPtrs( + [data](InspectorBackend* backend) { backend->OnMessage(data); }); } void Server::OnClose(int connection_id) { if (connection_id != connection_id_) return; connection_id_ = kNotConnected; - frontend_bindings_.ForAllBindings( - [](InspectorFrontend::Client* client) { client->OnDisconnect(); }); + backends_.ForAllPtrs( + [](InspectorBackend* backend) { backend->OnDisconnect(); }); } void Server::Listen(int32_t port, const mojo::Closure& callback) { - frontend_bindings_.CloseAllBindings(); // Assume caller represents a new app. + backends_.CloseAll(); // Assume caller represents a new app. // TODO(eseidel): Early-out here if we're already bound to the right port. web_server_.reset();