Make it possible to have multiple InspectorBackends

This will make it possible to connect a c++
inspector backend in addition to the JS one.

We lose the feature of running multiple inspector
servers in this patch, but we weren't using that feature
and could easily add it back if we plan to use it.

This patch has a *ton* of boilerplate code due
to crbug.com/431911, hopefully that will be fixed
soon and we can delete all the Impl nonsense.

R=abarth@chromium.org

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

framework/inspector/inspector.sky

@@ -93,5 +93,4 @@ window.frontendConnection = new connection.Connection(
     inspector.InspectorFrontend.proxyClass);
 
 window.frontend = frontendConnection.remote;
-frontend.listen(9898);
 </script>

services/inspector/BUILD.gn

@@ -5,8 +5,6 @@
 import("//mojo/public/tools/bindings/mojom.gni")
 
 group("inspector") {
-  testonly = true
-
   deps = [
     ":sky_inspector_server",
   ]
@@ -14,8 +12,6 @@ group("inspector") {
 
 shared_library("sky_inspector_server") {
   sources = [
-    "inspector_frontend_impl.cc",
-    "inspector_frontend_impl.h",
     "server.cc",
   ]
 

services/inspector/inspector.mojom

@@ -4,9 +4,12 @@
 
 module sky;
 
+interface InspectorServer {
+  Listen(int32 port) => ();
+};
+
 [Client=InspectorBackend]
 interface InspectorFrontend {
-  Listen(int32 port);
   SendMessage(string message);
 };
 

services/inspector/inspector_frontend_impl.cc

-// 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.
-
-#include "sky/services/inspector/inspector_frontend_impl.h"
-
-#include "base/lazy_instance.h"
-#include "net/server/http_server.h"
-#include "net/socket/tcp_server_socket.h"
-
-namespace sky {
-namespace inspector {
-namespace {
-const int kNotConnected = -1;
-static base::LazyInstance<std::map<int, InspectorFrontendImpl*>> g_servers =
-    LAZY_INSTANCE_INITIALIZER;
-}
-
-InspectorFrontendImpl::InspectorFrontendImpl()
-  : connection_id_(kNotConnected) {
-}
-
-InspectorFrontendImpl::~InspectorFrontendImpl() {
-  StopListening();
-}
-
-void InspectorFrontendImpl::OnConnect(int connection_id) {
-}
-
-void InspectorFrontendImpl::OnHttpRequest(
-    int connection_id, const net::HttpServerRequestInfo& info) {
-  web_server_->Send500(connection_id, "websockets protocol only");
-}
-
-void InspectorFrontendImpl::OnWebSocketRequest(
-    int connection_id, const net::HttpServerRequestInfo& info) {
-  if (connection_id_ != kNotConnected) {
-    web_server_->Close(connection_id);
-    return;
-  }
-  web_server_->AcceptWebSocket(connection_id, info);
-  connection_id_ = connection_id;
-  client()->OnConnect();
-}
-
-void InspectorFrontendImpl::OnWebSocketMessage(
-    int connection_id, const std::string& data) {
-  DCHECK_EQ(connection_id, connection_id_);
-  client()->OnMessage(data);
-}
-
-void InspectorFrontendImpl::OnClose(int connection_id) {
-  if (connection_id != connection_id_)
-    return;
-  connection_id_ = kNotConnected;
-  client()->OnDisconnect();
-}
-
-void InspectorFrontendImpl::Listen(int32_t port) {
-  Register(port);
-  scoped_ptr<net::ServerSocket> server_socket(
-      new net::TCPServerSocket(NULL, net::NetLog::Source()));
-  server_socket->ListenWithAddressAndPort("0.0.0.0", port, 1);
-  web_server_.reset(new net::HttpServer(server_socket.Pass(), this));
-}
-
-void InspectorFrontendImpl::StopListening() {
-  if (!web_server_)
-    return;
-  web_server_.reset();
-  Unregister();
-}
-
-void InspectorFrontendImpl::Register(int port) {
-  auto& servers = g_servers.Get();
-  auto iter = servers.find(port);
-  if (iter != servers.end())
-    iter->second->StopListening();
-  DCHECK(servers.find(port) == servers.end());
-  servers[port] = this;
-  port_ = port;
-}
-
-void InspectorFrontendImpl::Unregister() {
-  DCHECK(g_servers.Get().find(port_)->second == this);
-  g_servers.Get().erase(port_);
-  port_ = kNotConnected;
-}
-
-void InspectorFrontendImpl::SendMessage(const mojo::String& message) {
-  if (connection_id_ == kNotConnected)
-    return;
-  web_server_->SendOverWebSocket(connection_id_, message);
-}
-
-}  // namespace inspector
-}  // namespace sky

services/inspector/inspector_frontend_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 SKY_SERVICES_INSPECTOR_INSPECTOR_FRONTEND_IMPL_H_
-#define SKY_SERVICES_INSPECTOR_INSPECTOR_FRONTEND_IMPL_H_
-
-#include "mojo/public/cpp/application/interface_factory_impl.h"
-#include "mojo/public/cpp/bindings/interface_impl.h"
-#include "net/server/http_server.h"
-#include "net/socket/tcp_server_socket.h"
-#include "sky/services/inspector/inspector.mojom.h"
-
-namespace sky {
-namespace inspector {
-
-class InspectorFrontendImpl : public mojo::InterfaceImpl<InspectorFrontend>,
-                             public net::HttpServer::Delegate {
- public:
-  InspectorFrontendImpl();
-  virtual ~InspectorFrontendImpl();
-
- private:
-  // From net::HttpServer::Delegate
-  virtual void OnConnect(int connection_id) override;
-  virtual void OnHttpRequest(
-      int connection_id, const net::HttpServerRequestInfo& info) override;
-  virtual void OnWebSocketRequest(
-      int connection_id, const net::HttpServerRequestInfo& info) override;
-  virtual void OnWebSocketMessage(
-      int connection_id, const std::string& data) override;
-  virtual void OnClose(int connection_id) override;
-
-  // From InspectorFrontend
-  virtual void Listen(int32_t port) override;
-  virtual void SendMessage(const mojo::String&) override;
-
-  void StopListening();
-
-  void Register(int port);
-  void Unregister();
-
-  int port_;
-  int connection_id_;
-  scoped_ptr<net::HttpServer> web_server_;
-
-  MOJO_DISALLOW_COPY_AND_ASSIGN(InspectorFrontendImpl);
-};
-
-typedef mojo::InterfaceFactoryImpl<
-    InspectorFrontendImpl> InspectorFrontendFactory;
-
-}  // namespace tester
-}  // namespace sky
-
-#endif  // SKY_SERVICES_INSPECTOR_INSPECTOR_FRONTEND_IMPL_H_

services/inspector/server.cc

@@ -6,32 +6,244 @@
 #include "mojo/public/c/system/main.h"
 #include "mojo/public/cpp/application/application_delegate.h"
 #include "mojo/public/cpp/application/application_impl.h"
-#include "sky/services/inspector/inspector_frontend_impl.h"
+#include "mojo/public/cpp/application/interface_factory_impl.h"
+#include "mojo/public/cpp/bindings/interface_impl.h"
+#include "net/server/http_server.h"
+#include "net/socket/tcp_server_socket.h"
+#include "sky/services/inspector/inspector.mojom.h"
+
 
 namespace sky {
 namespace inspector {
 
-class Server : public mojo::ApplicationDelegate {
+// TODO(eseidel): None of this Impl nonsense is necessary: crbug.com/431963
+class InspectorServerImpl : public mojo::InterfaceImpl<InspectorServer> {
+public:
+  class Delegate {
   public:
-  Server() {}
-  virtual ~Server() {}
+    virtual void Register(InspectorServerImpl* impl) = 0;
+    virtual void Unregister(InspectorServerImpl* impl) = 0;
+    virtual void Listen(int32_t port) = 0;
+  };
+  InspectorServerImpl(Delegate* delegate) : delegate_(delegate) {
+    delegate_->Register(this);
+  }
+  virtual ~InspectorServerImpl() {
+    delegate_->Unregister(this);
+  }
 
- private:
-  // Overridden from mojo::ApplicationDelegate:
-  virtual void Initialize(mojo::ApplicationImpl* app) override {
+  // InspectorServer:
+  void Listen(int32_t port, const mojo::Callback<void()>& callback) override {
+    delegate_->Listen(port);
+    callback.Run();
+  }
+
+  void OnShutdown() {
+    delete this;
+  }
+
+private:
+  // InterfaceImpl:
+  void OnConnectionError() override {
+    delete this; // crbug.com/431911
   }
 
-  virtual bool ConfigureIncomingConnection(
+  Delegate* delegate_;
+};
+
+class InspectorFrontendImpl : public mojo::InterfaceImpl<InspectorFrontend> {
+ public:
+  class Delegate {
+  public:
+    virtual void Register(InspectorFrontendImpl*) = 0;
+    virtual void Unregister(InspectorFrontendImpl*) = 0;
+    virtual void SendMessage(const mojo::String&) = 0;
+  };
+
+  InspectorFrontendImpl(Delegate* delegate);
+  virtual ~InspectorFrontendImpl();
+
+  void OnShutdown();
+
+ private:
+  // InspectorFrontend:
+  void SendMessage(const mojo::String& message) override;
+
+  // InterfaceImpl:
+  void OnConnectionError() override;
+
+  Delegate* delegate_;
+
+  MOJO_DISALLOW_COPY_AND_ASSIGN(InspectorFrontendImpl);
+};
+
+InspectorFrontendImpl::InspectorFrontendImpl(Delegate* delegate)
+    : delegate_(delegate) {
+  delegate_->Register(this);
+}
+
+InspectorFrontendImpl::~InspectorFrontendImpl() {
+  delegate_->Unregister(this);
+}
+
+void InspectorFrontendImpl::OnShutdown() {
+  client()->OnDisconnect();
+  delete this;
+}
+
+void InspectorFrontendImpl::OnConnectionError() {
+  delete this; // crbug.com/431911
+}
+
+void InspectorFrontendImpl::SendMessage(const mojo::String& message) {
+  delegate_->SendMessage(message);
+}
+
+
+namespace {
+const int kNotConnected = -1;
+}
+
+class Server : public mojo::ApplicationDelegate,
+    public InspectorFrontendImpl::Delegate,
+    public InspectorServerImpl::Delegate,
+    public mojo::InterfaceFactory<InspectorFrontend>,
+    public mojo::InterfaceFactory<InspectorServer>,
+    public net::HttpServer::Delegate {
+ public:
+  Server() : connection_id_(kNotConnected) {}
+  virtual ~Server();
+
+ private:
+  // mojo::ApplicationDelegate:
+  void Initialize(mojo::ApplicationImpl* app) override {
+  }
+  bool ConfigureIncomingConnection(
       mojo::ApplicationConnection* connection) override {
-    connection->AddService(&frontend_factory_);
+    connection->AddService<InspectorFrontend>(this);
+    connection->AddService<InspectorServer>(this);
     return true;
   }
 
-  InspectorFrontendFactory frontend_factory_;
+  // InterfaceFactory<InspectorFrontend>:
+  void Create(mojo::ApplicationConnection* connection,
+        mojo::InterfaceRequest<InspectorFrontend> request) override {
+    // Weak instead of strong, per crbug.com/431911
+    WeakBindToRequest(new InspectorFrontendImpl(this), &request);
+  }
+
+  // InterfaceFactory<InspectorServer>:
+  void Create(mojo::ApplicationConnection* connection,
+        mojo::InterfaceRequest<InspectorServer> request) override {
+    // Weak instead of strong, per crbug.com/431911
+    WeakBindToRequest(new InspectorServerImpl(this), &request);
+  }
+
+  // InspectorServerImpl::Delegate:
+  void Register(InspectorServerImpl*) override;
+  void Unregister(InspectorServerImpl*) override;
+  void Listen(int32_t port) override;
+
+  // InspectorFrontendImpl::Delegate:
+  void Register(InspectorFrontendImpl*) override;
+  void Unregister(InspectorFrontendImpl*) override;
+  void SendMessage(const mojo::String& message) override;
+
+  // net::HttpServer::Delegate:
+  void OnConnect(int connection_id) override;
+  void OnHttpRequest(
+      int connection_id, const net::HttpServerRequestInfo& info) override;
+  void OnWebSocketRequest(
+      int connection_id, const net::HttpServerRequestInfo& info) override;
+  void OnWebSocketMessage(
+      int connection_id, const std::string& data) override;
+  void OnClose(int connection_id) override;
+
+  void CloseAllAgentConnections();
+
+  int connection_id_;
+  scoped_ptr<net::HttpServer> web_server_;
+  ObserverList<InspectorFrontendImpl> agents_;
+  ObserverList<InspectorServerImpl> clients_;
 
   DISALLOW_COPY_AND_ASSIGN(Server);
 };
 
+Server::~Server()
+{
+  FOR_EACH_OBSERVER(InspectorServerImpl, clients_, OnShutdown());
+  CloseAllAgentConnections();
+}
+
+void Server::CloseAllAgentConnections() {
+  FOR_EACH_OBSERVER(InspectorFrontendImpl, agents_, OnShutdown());
+}
+
+void Server::OnConnect(int connection_id) {
+}
+
+void Server::OnHttpRequest(
+    int connection_id, const net::HttpServerRequestInfo& info) {
+  web_server_->Send500(connection_id, "websockets protocol only");
+}
+
+void Server::OnWebSocketRequest(
+    int connection_id, const net::HttpServerRequestInfo& info) {
+  if (connection_id_ != kNotConnected) {
+    web_server_->Close(connection_id);
+    return;
+  }
+  web_server_->AcceptWebSocket(connection_id, info);
+  connection_id_ = connection_id;
+  FOR_EACH_OBSERVER(InspectorFrontendImpl, agents_, client()->OnConnect());
+}
+
+void Server::OnWebSocketMessage(
+    int connection_id, const std::string& data) {
+  DCHECK_EQ(connection_id, connection_id_);
+  FOR_EACH_OBSERVER(InspectorFrontendImpl, agents_, client()->OnMessage(data));
+}
+
+void Server::OnClose(int connection_id) {
+  if (connection_id != connection_id_)
+    return;
+  connection_id_ = kNotConnected;
+  FOR_EACH_OBSERVER(InspectorFrontendImpl, agents_, client()->OnDisconnect());
+}
+
+void Server::Register(InspectorServerImpl* client) {
+  clients_.AddObserver(client);
+}
+
+void Server::Unregister(InspectorServerImpl* client) {
+  clients_.RemoveObserver(client);
+}
+
+void Server::Register(InspectorFrontendImpl* agent) {
+  agents_.AddObserver(agent);
+}
+
+void Server::Unregister(InspectorFrontendImpl* agent) {
+  agents_.RemoveObserver(agent);
+}
+
+void Server::Listen(int32_t port) {
+  CloseAllAgentConnections(); // Assume caller represents a new app.
+
+  // TODO(eseidel): Early-out here if we're already bound to the right port.
+  web_server_.reset();
+  scoped_ptr<net::ServerSocket> server_socket(
+      new net::TCPServerSocket(NULL, net::NetLog::Source()));
+  server_socket->ListenWithAddressAndPort("0.0.0.0", port, 1);
+  web_server_.reset(new net::HttpServer(server_socket.Pass(), this));
+}
+
+void Server::SendMessage(const mojo::String& message) {
+  if (connection_id_ == kNotConnected)
+    return;
+  web_server_->SendOverWebSocket(connection_id_, message);
+}
+
 }  // namespace inspector
 }  // namespace sky
 

viewer/BUILD.gn

@@ -42,16 +42,18 @@ shared_library("viewer") {
   include_dirs = [ ".." ]
 
   deps = [
+    ":bindings",
+    ":sky_modules",
     "//base:i18n",
     "//cc",
     "//cc/surfaces",
     "//gin",
     "//mojo/application",
-    "//mojo/edk/js",
     "//mojo/cc",
     "//mojo/common",
     "//mojo/converters/geometry",
     "//mojo/converters/surfaces",
+    "//mojo/edk/js",
     "//mojo/public/c/system:for_shared_library",
     "//mojo/public/cpp/bindings",
     "//mojo/public/cpp/system",
@@ -67,12 +69,11 @@ shared_library("viewer") {
     "//net",
     "//skia",
     "//sky/engine/public:blink",
+    "//sky/services/inspector:bindings",
     "//sky/viewer/cc",
     "//third_party/icu",
     "//ui/native_theme",
     "//url",
-    ":bindings",
-    ":sky_modules",
   ]
 }
 

viewer/services/inspector_impl.cc

@@ -4,10 +4,12 @@
 
 #include "sky/viewer/services/inspector_impl.h"
 
+#include "base/bind.h"
 #include "sky/engine/public/web/WebDocument.h"
 #include "sky/engine/public/web/WebElement.h"
 #include "sky/engine/public/web/WebFrame.h"
 #include "sky/engine/public/web/WebView.h"
+#include "sky/services/inspector/inspector.mojom.h"
 #include "sky/viewer/document_view.h"
 
 namespace sky {
@@ -19,9 +21,21 @@ InspectorServiceImpl::InspectorServiceImpl(DocumentView* view)
 InspectorServiceImpl::~InspectorServiceImpl() {
 }
 
+void Ignored() {}
+
 void InspectorServiceImpl::Inject() {
   if (!view_)
     return;
+
+  mojo::ServiceProviderPtr inpector_service_provider;
+  view_->shell()->ConnectToApplication("mojo:sky_inspector_server",
+                                       GetProxy(&inpector_service_provider));
+  InspectorServerPtr inspector;
+  ConnectToService(inpector_service_provider.get(), &inspector);
+  inspector->Listen(9898, base::Bind(&Ignored));
+  // Listen drops existing agents/backends, wait before registering new ones.
+  inspector.WaitForIncomingMethodCall();
+
   view_->web_view()->injectModule("/sky/framework/inspector/inspector.sky");
 }