devtools: Change how commands are handled by the delegate.

The DevToolsManagerDelegate is now able to send the protocol response
itself when it needs to (through the DevToolsAgentHost). So have the
delegate do that, instead of sending the response through the
DevToolsSession.

This change makes it possible, in follow up CLs, to use UberDispatcher
to handle the commands and send the response back using FrontendChannel.

BUG=758061
TBR=eseckler@ for //headless, for trivial update to devtools API change.

Change-Id: Ib0b521a24c4f4d26f8cd0fe3a6995141acd630ef
Reviewed-on: https://chromium-review.googlesource.com/677954
Commit-Queue: Sadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#503654}

chrome/browser/devtools/chrome_devtools_manager_delegate.cc

@@ -6,6 +6,7 @@
 
 #include <utility>
 
+#include "base/json/json_writer.h"
 #include "base/memory/ptr_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "build/build_config.h"
@@ -118,6 +119,12 @@ void ToggleAdBlocking(bool enabled, content::DevToolsAgentHost* agent_host) {
   }
 }
 
+std::string ToString(std::unique_ptr<base::DictionaryValue> value) {
+  std::string json;
+  base::JSONWriter::Write(*value, &json);
+  return json;
+}
+
 }  // namespace
 
 // static
@@ -347,34 +354,52 @@ void ChromeDevToolsManagerDelegate::Inspect(
   DevToolsWindow::OpenDevToolsWindow(agent_host, nullptr);
 }
 
-base::DictionaryValue* ChromeDevToolsManagerDelegate::HandleCommand(
+bool ChromeDevToolsManagerDelegate::HandleCommand(
     DevToolsAgentHost* agent_host,
     int session_id,
     base::DictionaryValue* command_dict) {
   int id = 0;
   std::string method;
   base::DictionaryValue* params = nullptr;
   if (!DevToolsProtocol::ParseCommand(command_dict, &id, &method, &params))
-    return nullptr;
+    return false;
 
   // Do not actually handle the enable/disable commands, just keep track of the
   // enable state.
-  if (method == chrome::devtools::Page::enable::kName)
+  if (method == chrome::devtools::Page::enable::kName) {
     TogglePageEnable(true /* enable */, agent_host);
-  if (method == chrome::devtools::Page::disable::kName)
+    return false;
+  }
+
+  if (method == chrome::devtools::Page::disable::kName) {
     TogglePageEnable(false /* enable */, agent_host);
+    return false;
+  }
 
-  auto* result = HandleBrowserCommand(id, method, params).release();
-  if (result)
-    return result;
+  auto result = HandleBrowserCommand(id, method, params);
+  if (result) {
+    agent_host->SendProtocolMessageToClient(session_id,
+                                            ToString(std::move(result)));
+    return true;
+  }
 
-  if (method == chrome::devtools::Page::setAdBlockingEnabled::kName)
-    return SetAdBlockingEnabled(agent_host, id, params).release();
+  if (method == chrome::devtools::Page::setAdBlockingEnabled::kName) {
+    result = SetAdBlockingEnabled(agent_host, id, params);
+    DCHECK(result);
+    agent_host->SendProtocolMessageToClient(session_id,
+                                            ToString(std::move(result)));
+    return true;
+  }
 
-  if (method == chrome::devtools::Target::setRemoteLocations::kName)
-    return SetRemoteLocations(agent_host, id, params).release();
+  if (method == chrome::devtools::Target::setRemoteLocations::kName) {
+    result = SetRemoteLocations(agent_host, id, params);
+    DCHECK(result);
+    agent_host->SendProtocolMessageToClient(session_id,
+                                            ToString(std::move(result)));
+    return true;
+  }
 
-  return nullptr;
+  return false;
 }
 
 std::string ChromeDevToolsManagerDelegate::GetTargetType(

chrome/browser/devtools/chrome_devtools_manager_delegate.h

@@ -34,10 +34,9 @@ class ChromeDevToolsManagerDelegate :
 
   // content::DevToolsManagerDelegate implementation.
   void Inspect(content::DevToolsAgentHost* agent_host) override;
-  base::DictionaryValue* HandleCommand(
-      content::DevToolsAgentHost* agent_host,
-      int session_id,
-      base::DictionaryValue* command_dict) override;
+  bool HandleCommand(content::DevToolsAgentHost* agent_host,
+                     int session_id,
+                     base::DictionaryValue* command_dict) override;
   std::string GetTargetType(content::WebContents* web_contents) override;
   std::string GetTargetTitle(content::WebContents* web_contents) override;
   scoped_refptr<content::DevToolsAgentHost> CreateNewTarget(

content/browser/devtools/devtools_session.cc

@@ -80,12 +80,10 @@ protocol::Response::Status DevToolsSession::Dispatch(
   if (value && value->IsType(base::Value::Type::DICTIONARY) && delegate) {
     base::DictionaryValue* dict_value =
         static_cast<base::DictionaryValue*>(value.get());
-    std::unique_ptr<base::DictionaryValue> response(
-        delegate->HandleCommand(agent_host_, session_id_, dict_value));
-    if (response) {
-      SendResponse(std::move(response));
+
+    if (delegate->HandleCommand(agent_host_, session_id_, dict_value))
       return protocol::Response::kSuccess;
-    }
+
     if (delegate->HandleAsyncCommand(agent_host_, session_id_, dict_value,
                                      base::Bind(&DevToolsSession::SendResponse,
                                                 weak_factory_.GetWeakPtr()))) {

content/public/browser/devtools_manager_delegate.cc

@@ -39,11 +39,10 @@ void DevToolsManagerDelegate::SessionDestroyed(
     content::DevToolsAgentHost* agent_host,
     int session_id) {}
 
-base::DictionaryValue* DevToolsManagerDelegate::HandleCommand(
-    DevToolsAgentHost* agent_host,
-    int session_id,
-    base::DictionaryValue* command) {
-  return nullptr;
+bool DevToolsManagerDelegate::HandleCommand(DevToolsAgentHost* agent_host,
+                                            int session_id,
+                                            base::DictionaryValue* command) {
+  return false;
 }
 
 bool DevToolsManagerDelegate::HandleAsyncCommand(

content/public/browser/devtools_manager_delegate.h

@@ -49,10 +49,10 @@ class CONTENT_EXPORT DevToolsManagerDelegate {
   virtual void SessionDestroyed(content::DevToolsAgentHost* agent_host,
                                 int session_id);
 
-  // Result ownership is passed to the caller.
-  virtual base::DictionaryValue* HandleCommand(DevToolsAgentHost* agent_host,
-                                               int session_id,
-                                               base::DictionaryValue* command);
+  // Returns true if the command has been handled, false otherwise.
+  virtual bool HandleCommand(DevToolsAgentHost* agent_host,
+                             int session_id,
+                             base::DictionaryValue* command);
 
   using CommandCallback =
       base::Callback<void(std::unique_ptr<base::DictionaryValue> response)>;

headless/lib/browser/headless_devtools_manager_delegate.cc

@@ -7,6 +7,7 @@
 #include <string>
 #include <utility>
 
+#include "base/json/json_writer.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/devtools_agent_host.h"
 #include "content/public/browser/devtools_frontend_host.h"
@@ -100,6 +101,12 @@ void PDFCreated(
 }
 #endif
 
+std::string ToString(std::unique_ptr<base::DictionaryValue> value) {
+  std::string json;
+  base::JSONWriter::Write(*value, &json);
+  return json;
+}
+
 }  // namespace
 
 #if BUILDFLAG(ENABLE_BASIC_PRINTING)
@@ -199,33 +206,37 @@ HeadlessDevToolsManagerDelegate::HeadlessDevToolsManagerDelegate(
 
 HeadlessDevToolsManagerDelegate::~HeadlessDevToolsManagerDelegate() {}
 
-base::DictionaryValue* HeadlessDevToolsManagerDelegate::HandleCommand(
+bool HeadlessDevToolsManagerDelegate::HandleCommand(
     content::DevToolsAgentHost* agent_host,
     int session_id,
     base::DictionaryValue* command) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
   if (!browser_)
-    return nullptr;
+    return false;
 
   int id;
   std::string method;
   if (!command->GetInteger("id", &id) || !command->GetString("method", &method))
-    return nullptr;
+    return false;
 
   auto find_it = command_map_.find(method);
   if (find_it == command_map_.end())
-    return nullptr;
+    return false;
 
   // Handle Browser domain commands only from Browser DevToolsAgentHost.
   if (method.find("Browser.") == 0 &&
       agent_host->GetType() != content::DevToolsAgentHost::kTypeBrowser)
-    return nullptr;
+    return false;
 
   const base::DictionaryValue* params = nullptr;
   command->GetDictionary("params", &params);
   auto cmd_result = find_it->second.Run(id, params);
-  return cmd_result.release();
+  if (!cmd_result)
+    return false;
+  agent_host->SendProtocolMessageToClient(session_id,
+                                          ToString(std::move(cmd_result)));
+  return true;
 }
 
 bool HeadlessDevToolsManagerDelegate::HandleAsyncCommand(

headless/lib/browser/headless_devtools_manager_delegate.h

@@ -39,9 +39,9 @@ class HeadlessDevToolsManagerDelegate
   ~HeadlessDevToolsManagerDelegate() override;
 
   // DevToolsManagerDelegate implementation:
-  base::DictionaryValue* HandleCommand(content::DevToolsAgentHost* agent_host,
-                                       int session_id,
-                                       base::DictionaryValue* command) override;
+  bool HandleCommand(content::DevToolsAgentHost* agent_host,
+                     int session_id,
+                     base::DictionaryValue* command) override;
   bool HandleAsyncCommand(content::DevToolsAgentHost* agent_host,
                           int session_id,
                           base::DictionaryValue* command,