Use WeakBindingSet to manage inspector connections

sky/services/inspector/server.cc

Index: sky/services/inspector/server.cc
diff --git a/sky/services/inspector/server.cc b/sky/services/inspector/server.cc
index 0963dd18b7e6bd614a4c3e2b03c281d97fb967c9..8ca458d4dbd0841e93911ca8977917011e7cf597 100644
--- a/sky/services/inspector/server.cc
+++ b/sky/services/inspector/server.cc
@@ -3,11 +3,11 @@
 // found in the LICENSE file.
 
 #include "mojo/application/application_runner_chromium.h"
+#include "mojo/common/weak_binding_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"
-#include "mojo/public/cpp/application/interface_factory_impl.h"
-#include "mojo/public/cpp/bindings/interface_impl.h"
+#include "mojo/public/cpp/bindings/binding.h"
 #include "net/server/http_server.h"
 #include "net/socket/tcp_server_socket.h"
 #include "sky/services/inspector/inspector.mojom.h"
@@ -16,102 +16,18 @@
 namespace sky {
 namespace inspector {
 
-// TODO(eseidel): None of this Impl nonsense is necessary: crbug.com/431963
-class InspectorServerImpl : public mojo::InterfaceImpl<InspectorServer> {
-public:
-  class Delegate {
-  public:
-    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);
-  }
-
-  // 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
-  }
-
-  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 InspectorFrontend,
+               public InspectorServer,
+               public mojo::InterfaceFactory<InspectorFrontend>,
+               public mojo::InterfaceFactory<InspectorServer>,
+               public net::HttpServer::Delegate {
  public:
-  Server() : connection_id_(kNotConnected) {}
+  Server() : connection_id_(kNotConnected), server_binding_(this) {}
   virtual ~Server();
 
  private:
@@ -128,25 +44,19 @@ class Server : public mojo::ApplicationDelegate,
   // 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);
+    frontend_bindings_.AddBinding(this, request.Pass());
   }
 
   // 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);
+    server_binding_.Bind(request.PassMessagePipe());
   }
 
-  // InspectorServerImpl::Delegate:
-  void Register(InspectorServerImpl*) override;
-  void Unregister(InspectorServerImpl*) override;
-  void Listen(int32_t port) override;
+  // InspectorServer:
+  void Listen(int32_t port, const mojo::Closure& callback) override;
 
-  // InspectorFrontendImpl::Delegate:
-  void Register(InspectorFrontendImpl*) override;
-  void Unregister(InspectorFrontendImpl*) override;
+  // InspectorFrontend:
   void SendMessage(const mojo::String& message) override;
 
   // net::HttpServer::Delegate:
@@ -163,20 +73,20 @@ class Server : public mojo::ApplicationDelegate,
 
   int connection_id_;
   scoped_ptr<net::HttpServer> web_server_;
-  ObserverList<InspectorFrontendImpl> agents_;
-  ObserverList<InspectorServerImpl> clients_;
+
+  mojo::WeakBindingSet<InspectorFrontend> frontend_bindings_;
+  mojo::Binding<InspectorServer> server_binding_;
 
   DISALLOW_COPY_AND_ASSIGN(Server);
 };
 
 Server::~Server()
 {
-  FOR_EACH_OBSERVER(InspectorServerImpl, clients_, OnShutdown());
   CloseAllAgentConnections();
 }
 
 void Server::CloseAllAgentConnections() {
-  FOR_EACH_OBSERVER(InspectorFrontendImpl, agents_, OnShutdown());
+  frontend_bindings_.CloseAllBindings();

[eseidel, 2014/11/14 00:30:16]

I would just inline this now.

 }
 
 void Server::OnConnect(int connection_id) {
@@ -195,39 +105,26 @@ void Server::OnWebSocketRequest(
   }
   web_server_->AcceptWebSocket(connection_id, info);
   connection_id_ = connection_id;
-  FOR_EACH_OBSERVER(InspectorFrontendImpl, agents_, client()->OnConnect());
+  frontend_bindings_.ForAllBindings(
+      [](InspectorFrontend::Client* client) { 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));
+  frontend_bindings_.ForAllBindings(
+      [data](InspectorFrontend::Client* client) { 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);
+  frontend_bindings_.ForAllBindings(
+      [](InspectorFrontend::Client* client) { client->OnDisconnect(); });
 }
 
-void Server::Listen(int32_t port) {
+void Server::Listen(int32_t port, const mojo::Closure& callback) {
   CloseAllAgentConnections(); // Assume caller represents a new app.
 
   // TODO(eseidel): Early-out here if we're already bound to the right port.
@@ -236,6 +133,7 @@ void Server::Listen(int32_t port) {
       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));
+  callback.Run();
 }
 
 void Server::SendMessage(const mojo::String& message) {