Headless: enable multiclient access to headless targets.

Change-Id: I7008b98fbf392801b796517a9a442c34815b13c4
Reviewed-on: https://chromium-review.googlesource.com/1040988Reviewed-by: Alex Clarke <alexclarke@chromium.org>
Reviewed-by: Eric Seckler <eseckler@chromium.org>
Commit-Queue: Pavel Feldman <pfeldman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#555886}

content/browser/devtools/devtools_agent_host_impl.cc

@@ -208,16 +208,6 @@ bool DevToolsAgentHostImpl::AttachRestrictedClient(
   return InnerAttachClient(client, true /* restricted */);
 }
 
-void DevToolsAgentHostImpl::ForceAttachClient(DevToolsAgentHostClient* client) {
-  if (SessionByClient(client))
-    return;
-  scoped_refptr<DevToolsAgentHostImpl> protect(this);
-  if (!sessions_.empty())
-    ForceDetachAllSessions();
-  DCHECK(sessions_.empty());
-  InnerAttachClient(client, false /* restricted */);
-}
-
 bool DevToolsAgentHostImpl::DetachClient(DevToolsAgentHostClient* client) {
   if (!SessionByClient(client))
     return false;

content/browser/devtools/devtools_agent_host_impl.h

@@ -38,7 +38,6 @@ class CONTENT_EXPORT DevToolsAgentHostImpl : public DevToolsAgentHost {
   // DevToolsAgentHost implementation.
   void AttachClient(DevToolsAgentHostClient* client) override;
   bool AttachRestrictedClient(DevToolsAgentHostClient* client) override;
-  void ForceAttachClient(DevToolsAgentHostClient* client) override;
   bool DetachClient(DevToolsAgentHostClient* client) override;
   bool DispatchProtocolMessage(DevToolsAgentHostClient* client,
                                const std::string& message) override;

content/browser/devtools/forwarding_agent_host.cc

@@ -55,12 +55,6 @@ void ForwardingAgentHost::AttachClient(DevToolsAgentHostClient* client) {
   session_proxies_[client].reset(new SessionProxy(this, client));
 }
 
-void ForwardingAgentHost::ForceAttachClient(DevToolsAgentHostClient* client) {
-  while (!session_proxies_.empty())
-    session_proxies_.begin()->second->ConnectionClosed();
-  AttachClient(client);
-}
-
 bool ForwardingAgentHost::DetachClient(DevToolsAgentHostClient* client) {
   auto it = session_proxies_.find(client);
   if (it == session_proxies_.end())

content/browser/devtools/forwarding_agent_host.h

@@ -28,7 +28,6 @@ class ForwardingAgentHost : public DevToolsAgentHostImpl {
 
   // DevToolsAgentHost implementation
   void AttachClient(DevToolsAgentHostClient* client) override;
-  void ForceAttachClient(DevToolsAgentHostClient* client) override;
   bool DetachClient(DevToolsAgentHostClient* client) override;
   bool DispatchProtocolMessage(DevToolsAgentHostClient* client,
                                const std::string& message) override;

content/public/browser/devtools_agent_host.h

@@ -121,10 +121,6 @@ class CONTENT_EXPORT DevToolsAgentHost
   // Returns |true| on success.
   virtual bool AttachRestrictedClient(DevToolsAgentHostClient* client) = 0;
 
-  // Attaches |client| to this agent host to start debugging. Disconnects
-  // any existing clients.
-  virtual void ForceAttachClient(DevToolsAgentHostClient* client) = 0;
-
   // Already attached client detaches from this agent host to stop debugging it.
   // Returns true iff detach succeeded.
   virtual bool DetachClient(DevToolsAgentHostClient* client) = 0;

headless/lib/browser/headless_browser_impl.cc

@@ -260,16 +260,9 @@ HeadlessDevToolsTarget* HeadlessBrowserImpl::GetDevToolsTarget() {
   return agent_host_ ? this : nullptr;
 }
 
-bool HeadlessBrowserImpl::AttachClient(HeadlessDevToolsClient* client) {
+void HeadlessBrowserImpl::AttachClient(HeadlessDevToolsClient* client) {
   DCHECK(agent_host_);
-  return HeadlessDevToolsClientImpl::From(client)->AttachToHost(
-      agent_host_.get());
-}
-
-void HeadlessBrowserImpl::ForceAttachClient(HeadlessDevToolsClient* client) {
-  DCHECK(agent_host_);
-  HeadlessDevToolsClientImpl::From(client)->ForceAttachToHost(
-      agent_host_.get());
+  HeadlessDevToolsClientImpl::From(client)->AttachToHost(agent_host_.get());
 }
 
 void HeadlessBrowserImpl::DetachClient(HeadlessDevToolsClient* client) {

headless/lib/browser/headless_browser_impl.h

@@ -63,8 +63,7 @@ class HEADLESS_EXPORT HeadlessBrowserImpl : public HeadlessBrowser,
   HeadlessDevToolsTarget* GetDevToolsTarget() override;
 
   // HeadlessDevToolsTarget implementation:
-  bool AttachClient(HeadlessDevToolsClient* client) override;
-  void ForceAttachClient(HeadlessDevToolsClient* client) override;
+  void AttachClient(HeadlessDevToolsClient* client) override;
   void DetachClient(HeadlessDevToolsClient* client) override;
   bool IsAttached() override;
 

headless/lib/browser/headless_devtools_client_impl.cc

@@ -74,21 +74,11 @@ HeadlessDevToolsClientImpl::HeadlessDevToolsClientImpl()
 
 HeadlessDevToolsClientImpl::~HeadlessDevToolsClientImpl() = default;
 
-bool HeadlessDevToolsClientImpl::AttachToHost(
+void HeadlessDevToolsClientImpl::AttachToHost(
     content::DevToolsAgentHost* agent_host) {
   DCHECK(!agent_host_);
-  if (agent_host->IsAttached())
-    return false;
   agent_host->AttachClient(this);
   agent_host_ = agent_host;
-  return true;
-}
-
-void HeadlessDevToolsClientImpl::ForceAttachToHost(
-    content::DevToolsAgentHost* agent_host) {
-  DCHECK(!agent_host_);
-  agent_host_ = agent_host;
-  agent_host_->ForceAttachClient(this);
 }
 
 void HeadlessDevToolsClientImpl::DetachFromHost(

headless/lib/browser/headless_web_contents_impl.cc

@@ -514,15 +514,8 @@ HeadlessDevToolsTarget* HeadlessWebContentsImpl::GetDevToolsTarget() {
   return web_contents()->GetMainFrame()->IsRenderFrameLive() ? this : nullptr;
 }
 
-bool HeadlessWebContentsImpl::AttachClient(HeadlessDevToolsClient* client) {
-  return HeadlessDevToolsClientImpl::From(client)->AttachToHost(
-      agent_host_.get());
-}
-
-void HeadlessWebContentsImpl::ForceAttachClient(
-    HeadlessDevToolsClient* client) {
-  HeadlessDevToolsClientImpl::From(client)->ForceAttachToHost(
-      agent_host_.get());
+void HeadlessWebContentsImpl::AttachClient(HeadlessDevToolsClient* client) {
+  HeadlessDevToolsClientImpl::From(client)->AttachToHost(agent_host_.get());
 }
 
 void HeadlessWebContentsImpl::DetachClient(HeadlessDevToolsClient* client) {

headless/lib/browser/headless_web_contents_impl.h

@@ -71,8 +71,7 @@ class HEADLESS_EXPORT HeadlessWebContentsImpl
   std::string GetMainFrameDevToolsId() const override;
 
   // HeadlessDevToolsTarget implementation:
-  bool AttachClient(HeadlessDevToolsClient* client) override;
-  void ForceAttachClient(HeadlessDevToolsClient* client) override;
+  void AttachClient(HeadlessDevToolsClient* client) override;
   void DetachClient(HeadlessDevToolsClient* client) override;
   bool IsAttached() override;
 

headless/lib/headless_devtools_client_browsertest.cc

@@ -945,13 +945,11 @@ class HeadlessDevToolsClientAttachTest
     HeadlessDevToolsTarget* devtools_target =
         web_contents_->GetDevToolsTarget();
 
-    // Try attaching: there's already a client attached.
-    EXPECT_FALSE(devtools_target->AttachClient(other_devtools_client_.get()));
     EXPECT_TRUE(devtools_target->IsAttached());
     // Detach the existing client, attach the other client.
     devtools_target->DetachClient(devtools_client_.get());
     EXPECT_FALSE(devtools_target->IsAttached());
-    EXPECT_TRUE(devtools_target->AttachClient(other_devtools_client_.get()));
+    devtools_target->AttachClient(other_devtools_client_.get());
     EXPECT_TRUE(devtools_target->IsAttached());
 
     // Now, let's make sure this devtools client works.
@@ -968,9 +966,10 @@ class HeadlessDevToolsClientAttachTest
     HeadlessDevToolsTarget* devtools_target =
         web_contents_->GetDevToolsTarget();
 
-    // Try attach, then force-attach the original client.
-    EXPECT_FALSE(devtools_target->AttachClient(devtools_client_.get()));
-    devtools_target->ForceAttachClient(devtools_client_.get());
+    EXPECT_TRUE(devtools_target->IsAttached());
+    devtools_target->DetachClient(other_devtools_client_.get());
+    EXPECT_FALSE(devtools_target->IsAttached());
+    devtools_target->AttachClient(devtools_client_.get());
     EXPECT_TRUE(devtools_target->IsAttached());
 
     devtools_client_->GetRuntime()->Evaluate(

headless/public/headless_devtools_target.h

@@ -20,16 +20,9 @@ class HEADLESS_EXPORT HeadlessDevToolsTarget {
   // Attach or detach a client to this target. A client must be attached in
   // order to send commands or receive notifications from the target.
   //
-  // A single client may be attached to at most one target at a time. If
-  // the target already has a client attached, AttachClient will return false.
-  // ForceAttachClient will detach any existing connection before attaching
-  // |client|.
-  // Note that currently also only one client may be attached to a single target
-  // at a time.
-  //
+  // A single client may be attached to at most one target at a time.
   // |client| must outlive this target.
-  virtual bool AttachClient(HeadlessDevToolsClient* client) = 0;
-  virtual void ForceAttachClient(HeadlessDevToolsClient* client) = 0;
+  virtual void AttachClient(HeadlessDevToolsClient* client) = 0;
   virtual void DetachClient(HeadlessDevToolsClient* client) = 0;
 
   // Returns true if a devtools client is attached.

headless/public/internal/headless_devtools_client_impl.h

@@ -124,8 +124,7 @@ class HEADLESS_EXPORT HeadlessDevToolsClientImpl
       const char* method,
       base::RepeatingCallback<void(const base::Value&)> callback) override;
 
-  bool AttachToHost(content::DevToolsAgentHost* agent_host);
-  void ForceAttachToHost(content::DevToolsAgentHost* agent_host);
+  void AttachToHost(content::DevToolsAgentHost* agent_host);
   void DetachFromHost(content::DevToolsAgentHost* agent_host);
 
   void SetTaskRunnerForTests(

headless/public/util/compositor_controller_unittest.cc

@@ -53,7 +53,6 @@ class CompositorControllerTest : public ::testing::Test {
     client_.SetTaskRunnerForTests(task_runner_);
     mock_host_ = base::MakeRefCounted<MockDevToolsAgentHost>();
 
-    EXPECT_CALL(*mock_host_, IsAttached()).WillOnce(Return(false));
     EXPECT_CALL(*mock_host_, AttachClient(&client_));
     client_.AttachToHost(mock_host_.get());
     virtual_time_controller_ =

headless/public/util/testing/mock_devtools_agent_host.h

@@ -19,8 +19,6 @@ class MockDevToolsAgentHost : public content::DevToolsAgentHost {
   MOCK_METHOD1(AttachClient, void(content::DevToolsAgentHostClient* client));
   MOCK_METHOD1(AttachRestrictedClient,
                bool(content::DevToolsAgentHostClient* client));
-  MOCK_METHOD1(ForceAttachClient,
-               void(content::DevToolsAgentHostClient* client));
   MOCK_METHOD1(DetachClient, bool(content::DevToolsAgentHostClient* client));
   MOCK_METHOD2(DispatchProtocolMessage,
                bool(content::DevToolsAgentHostClient* client,

headless/public/util/virtual_time_controller_test.cc

@@ -30,7 +30,6 @@ class VirtualTimeControllerTest : public ::testing::Test {
     client_.SetTaskRunnerForTests(task_runner_);
     mock_host_ = base::MakeRefCounted<MockDevToolsAgentHost>();
 
-    EXPECT_CALL(*mock_host_, IsAttached()).WillOnce(Return(false));
     EXPECT_CALL(*mock_host_, AttachClient(&client_));
     client_.AttachToHost(mock_host_.get());
     controller_ = std::make_unique<VirtualTimeController>(&client_, 0);