Introduce DevToolsExternalAgentProxy interface

content/public/browser/devtools_remote_agent_connector.h

+// Copyright (c) 2013 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 CONTENT_PUBLIC_BROWSER_DEVTOOLS_REMOTE_AGENT_CONNECTOR_H_
+#define CONTENT_PUBLIC_BROWSER_DEVTOOLS_REMOTE_AGENT_CONNECTOR_H_
+
+#include "content/common/content_export.h"
+
+namespace content {
+
+class DevToolsAgentHost;
+
+// Describes interface for sending messages to a remote DevTools agent.
+class CONTENT_EXPORT DevToolsRemoteAgentConnector {

[pfeldman, 2013/03/21 06:24:32]

"Connector" does not give me enough information, I would not have it in
content/public. ExternalDevToolsAgentHost would sound bever even though it is
not inherited from DevToolsAgentHost.

[Vladislav Kaznacheev, 2013/03/21 06:55:53]

It does sound better but still not good enough. How about
DevToolsAgentHostProxy?
On 2013/03/21 06:24:32, pfeldman wrote:

+ public:
+

[pfeldman, 2013/03/21 06:24:32]

Extra blank line.

[Vladislav Kaznacheev, 2013/03/21 06:55:53]

Done.

+  class Delegate {
+   public:
+     // Informs the remote agent that DevToolsAgentHost attached.
+     virtual void Attach() {};
+
+     // Informs the remote agent that DevToolsAgentHost detached.
+     virtual void Detach() {};
+
+     // Sends a message to a remote agent.
+     virtual void Send(const std::string& message) {};

[pfeldman, 2013/03/21 06:24:32]

SendMessageToBackend

[Vladislav Kaznacheev, 2013/03/21 06:55:53]

Done.

+
+   protected:
+    virtual ~Delegate() {}
+  };
+
+  // Creates DevToolsAgentHost that communicates with a remote agent via
+  // the provided |delegate|.
+  // The |delegate| object is not owned by the connector.

[pfeldman, 2013/03/21 06:24:32]

It is not clear it creates DevToolsAgentHost from its signature. You only say
"takes ownership" when it does. So you should say "caller takes ownership".

[Vladislav Kaznacheev, 2013/03/21 06:55:53]

"creates DevToolsAgentHost" was a typo. Changed the second sentence to "caller
keeps the |delegate| ownership"
On 2013/03/21 06:24:32, pfeldman wrote:

+  static DevToolsRemoteAgentConnector* CreateFor(Delegate* delegate);
+
+  // Returns the agent host associated with this connector.
+  virtual DevToolsAgentHost* GetAgentHost() = 0;

[pfeldman, 2013/03/21 06:24:32]

What does associated with the connector mean?

[Vladislav Kaznacheev, 2013/03/21 06:55:53]

Changed to "Returns the local DevToolsAgentHost for the remote agent."

On 2013/03/21 06:24:32, pfeldman wrote:

+
+  // To be called when the connection is closed.
+  virtual void ConnectionClosed() = 0;

[pfeldman, 2013/03/21 06:24:32]

Adding convenience DispatchMessageFromFrontend would be useful to the client. We
would not even need a getter above them (at least for now), exactly as you
originally suggested.

[Vladislav Kaznacheev, 2013/03/21 06:55:53]

Cannot remove the getter as we need to pass DevToolsAgentHost to
DevToolsManager::RegisterDevToolsClientHostFor attaching the client.

Why do we need DispatchMessageFromFrontend? DevToolsAgentHost does not have
that, only its *Impl does. We only ever send anything through
DevToolsClientHost.


On 2013/03/21 06:24:32, pfeldman wrote:

+
+  virtual ~DevToolsRemoteAgentConnector() {}
+};
+
+}  // namespace content
+
+#endif  // CONTENT_PUBLIC_BROWSER_DEVTOOLS_REMOTE_AGENT_CONNECTOR_H_