DevTools: remove DevToolsProtocol::Event (since Notification should be used). Drive-by fixes.

Review URL: https://chromiumcodereview.appspot.com/14228002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@193937 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 162 insertions, 150 deletions

diff --git a/content/browser/devtools/devtools_protocol.cc b/content/browser/devtools/devtools_protocol.cc
index 28d6fe8..1363e6c 100644
--- a/content/browser/devtools/devtools_protocol.cc
+++ b/content/browser/devtools/devtools_protocol.cc
@@ -6,6 +6,7 @@
 
 #include "base/json/json_reader.h"
 #include "base/json/json_writer.h"
+#include "base/stringprintf.h"
 
 namespace content {
 
@@ -20,25 +21,65 @@ const char kErrorCodeParam[] = "code";
 const char kErrorMessageParam[] = "message";
 const int kNoId = -1;
 
+// JSON RPC 2.0 spec: http://www.jsonrpc.org/specification#error_object
+enum Error {
+  kErrorParseError = -32700,
+  kErrorInvalidRequest = -32600,
+  kErrorNoSuchMethod = -32601,
+  kErrorInvalidParams = -32602,
+  kErrorInternalError = -32603
+};
+
 }  // namespace
 
 using base::DictionaryValue;
 using base::Value;
 
+DevToolsProtocol::Message::~Message() {
+}
+
+DevToolsProtocol::Message::Message(const std::string& method,
+                                   DictionaryValue* params)
+    : method_(method),
+      params_(params) {
+  size_t pos = method.find(".");
+  if (pos != std::string::npos && pos > 0)
+    domain_ = method.substr(0, pos);
+}
+
 DevToolsProtocol::Command::~Command() {
 }
 
+std::string DevToolsProtocol::Command::Serialize() {
+  DictionaryValue command;
+  command.SetInteger(kIdParam, id_);
+  command.SetString(kMethodParam, method_);
+  if (params_.get())
+    command.Set(kParamsParam, params_->DeepCopy());
+
+  std::string json_command;
+  base::JSONWriter::Write(&command, &json_command);
+  return json_command;
+}
+
 scoped_ptr<DevToolsProtocol::Response>
-DevToolsProtocol::Command::SuccessResponse(base::DictionaryValue* result) {
+DevToolsProtocol::Command::SuccessResponse(DictionaryValue* result) {
   return scoped_ptr<DevToolsProtocol::Response>(
       new DevToolsProtocol::Response(id_, result));
 }
 
 scoped_ptr<DevToolsProtocol::Response>
-DevToolsProtocol::Command::ErrorResponse(int error_code,
-                                         const std::string& error_message) {
+DevToolsProtocol::Command::InternalErrorResponse(const std::string& message) {
+  return scoped_ptr<DevToolsProtocol::Response>(
+      new DevToolsProtocol::Response(id_, kErrorInternalError, message));
+}
+
+scoped_ptr<DevToolsProtocol::Response>
+DevToolsProtocol::Command::InvalidParamResponse(const std::string& param) {
+  std::string message =
+      base::StringPrintf("Missing or invalid '%s' parameter", param.c_str());
   return scoped_ptr<DevToolsProtocol::Response>(
-      new DevToolsProtocol::Response(id_, error_code, error_message));
+      new DevToolsProtocol::Response(id_, kErrorInvalidParams, message));
 }
 
 scoped_ptr<DevToolsProtocol::Response>
@@ -48,13 +89,10 @@ DevToolsProtocol::Command::NoSuchMethodErrorResponse() {
 }
 
 DevToolsProtocol::Command::Command(int id,
-                                   const std::string& domain,
                                    const std::string& method,
                                    DictionaryValue* params)
-    : id_(id),
-      domain_(domain),
-      method_(method),
-      params_(params) {
+    : Message(method, params),
+      id_(id) {
 }
 
 std::string DevToolsProtocol::Response::Serialize() {
@@ -98,17 +136,14 @@ DevToolsProtocol::Response::~Response() {
 
 DevToolsProtocol::Notification::Notification(const std::string& method,
                                              DictionaryValue* params)
-    : method_(method),
-      params_(params) {
+    : Message(method, params) {
 }
 
 DevToolsProtocol::Notification::~Notification() {
 }
 
 std::string DevToolsProtocol::Notification::Serialize() {
-  DictionaryValue response;
-
-  base::DictionaryValue notification;
+  DictionaryValue notification;
   notification.SetString(kMethodParam, method_);
   if (params_.get())
     notification.Set(kParamsParam, params_->DeepCopy());
@@ -118,27 +153,6 @@ std::string DevToolsProtocol::Notification::Serialize() {
   return json_notification;
 }
 
-DevToolsProtocol::Event::~Event() {
-}
-
-std::string DevToolsProtocol::Event::Serialize() {
-  DictionaryValue dictionary;
-
-  dictionary.SetString(kMethodParam, method_);
-  if (params_)
-    dictionary.Set(kParamsParam, params_->DeepCopy());
-
-  std::string result;
-  base::JSONWriter::Write(&dictionary, &result);
-  return result;
-}
-
-DevToolsProtocol::Event::Event(const std::string& method,
-                               DictionaryValue* params)
-    : method_(method),
-      params_(params) {
-}
-
 DevToolsProtocol::Handler::~Handler() {
 }
 
@@ -166,62 +180,87 @@ void DevToolsProtocol::Handler::RegisterCommandHandler(
 
 void DevToolsProtocol::Handler::SendNotification(
     const std::string& method,
-    base::DictionaryValue* params) {
+    DictionaryValue* params) {
   DevToolsProtocol::Notification notification(method, params);
   if (!notifier_.is_null())
     notifier_.Run(notification.Serialize());
 }
 
+static bool ParseMethod(DictionaryValue* command,
+                        std::string* method) {
+  if (!command->GetString(kMethodParam, method))
+    return false;
+  size_t pos = method->find(".");
+  if (pos == std::string::npos || pos == 0)
+    return false;
+  return true;
+}
+
 // static
 DevToolsProtocol::Command* DevToolsProtocol::ParseCommand(
     const std::string& json,
     std::string* error_response) {
-  int parse_error_code;
-  std::string error_message;
-  scoped_ptr<base::Value> command(
-      base::JSONReader::ReadAndReturnError(
-          json, 0, &parse_error_code, &error_message));
-
-  if (!command || !command->IsType(base::Value::TYPE_DICTIONARY)) {
-    Response response(0, kErrorParseError, error_message);
-    *error_response = response.Serialize();
+  scoped_ptr<DictionaryValue> command_dict(ParseMessage(json, error_response));
+  if (!command_dict)
     return NULL;
-  }
-
-  base::DictionaryValue* command_dict = NULL;
-  command->GetAsDictionary(&command_dict);
 
   int id;
   std::string method;
-  bool ok = true;
-  ok &= command_dict->GetInteger(kIdParam, &id);
-  ok &= id >= 0;
-  ok &= command_dict->GetString(kMethodParam, &method);
+  bool ok = command_dict->GetInteger(kIdParam, &id) && id >= 0;
+  ok = ok && ParseMethod(command_dict.get(), &method);
   if (!ok) {
-    Response response(kNoId, kErrorInvalidRequest, "Invalid request");
+    Response response(kNoId, kErrorInvalidRequest, "No such method");
     *error_response = response.Serialize();
     return NULL;
   }
 
-  size_t pos = method.find(".");
-  if (pos == std::string::npos || pos == 0) {
-    Response response(id, kErrorNoSuchMethod, "No such method");
-    *error_response = response.Serialize();
+  DictionaryValue* params = NULL;
+  command_dict->GetDictionary(kParamsParam, &params);
+  return new Command(id, method, params ? params->DeepCopy() : NULL);
+}
+
+// static
+DevToolsProtocol::Notification*
+DevToolsProtocol::ParseNotification(const std::string& json) {
+  scoped_ptr<DictionaryValue> dict(ParseMessage(json, NULL));
+  if (!dict)
     return NULL;
-  }
 
-  std::string domain = method.substr(0, pos);
+  std::string method;
+  bool ok = ParseMethod(dict.get(), &method);
+  if (!ok)
+    return NULL;
 
-  base::DictionaryValue* params = NULL;
-  command_dict->GetDictionary(kParamsParam, &params);
-  return new Command(id, domain, method, params ? params->DeepCopy() : NULL);
+  DictionaryValue* params = NULL;
+  dict->GetDictionary(kParamsParam, &params);
+  return new Notification(method, params ? params->DeepCopy() : NULL);
 }
 
 //static
-DevToolsProtocol::Event* DevToolsProtocol::CreateEvent(
+DevToolsProtocol::Notification* DevToolsProtocol::CreateNotification(
     const std::string& method,
-    base::DictionaryValue* params) {
-  return new Event(method, params);
+    DictionaryValue* params) {
+  return new Notification(method, params);
+}
+
+// static
+DictionaryValue* DevToolsProtocol::ParseMessage(
+    const std::string& json,
+    std::string* error_response) {
+  int parse_error_code;
+  std::string error_message;
+  scoped_ptr<Value> message(
+      base::JSONReader::ReadAndReturnError(
+          json, 0, &parse_error_code, &error_message));
+
+  if (!message || !message->IsType(Value::TYPE_DICTIONARY)) {
+    Response response(0, kErrorParseError, error_message);
+    if (error_response)
+      *error_response = response.Serialize();
+    return NULL;
+  }
+
+  return static_cast<DictionaryValue*>(message.release());
 }
 
 }  // namespace content
diff --git a/content/browser/devtools/devtools_protocol.h b/content/browser/devtools/devtools_protocol.h
index 9755476..a2ef7db 100644
--- a/content/browser/devtools/devtools_protocol.h
+++ b/content/browser/devtools/devtools_protocol.h
@@ -21,45 +21,55 @@ class DevToolsProtocol {
  public:
   typedef base::Callback<void(const std::string& message)> Notifier;
 
-  // JSON RPC 2.0 spec: http://www.jsonrpc.org/specification#error_object
-  enum Error {
-    kErrorParseError = -32700,
-    kErrorInvalidRequest = -32600,
-    kErrorNoSuchMethod = -32601,
-    kErrorInvalidParams = -32602,
-    kErrorInternalError = -32603
-  };
-
   class Response;
 
-  class Command {
+  class Message {
    public:
-    ~Command();
+    virtual ~Message();
 
-    int id() { return id_; }
     std::string domain() { return domain_; }
     std::string method() { return method_; }
     base::DictionaryValue* params() { return params_.get(); }
+    virtual std::string Serialize() = 0;
+
+   protected:
+    Message(const std::string& method,
+            base::DictionaryValue* params);
+
+    std::string domain_;
+    std::string method_;
+    scoped_ptr<base::DictionaryValue> params_;
+
+   private:
+    DISALLOW_COPY_AND_ASSIGN(Message);
+  };
+
+  class Command : public Message {
+   public:
+    virtual  ~Command();
+
+    int id() { return id_; }
+
+    virtual std::string Serialize() OVERRIDE;
 
     // Creates success response. Takes ownership of |result|.
     scoped_ptr<Response> SuccessResponse(base::DictionaryValue* result);
 
     // Creates error response. Caller takes ownership of the return value.
-    scoped_ptr<Response> ErrorResponse(int error_code,
-                                       const std::string& error_message);
+    scoped_ptr<Response> InternalErrorResponse(const std::string& message);
+
+    // Creates error response. Caller takes ownership of the return value.
+    scoped_ptr<Response> InvalidParamResponse(const std::string& param);
 
     // Creates error response. Caller takes ownership of the return value.
     scoped_ptr<Response> NoSuchMethodErrorResponse();
 
    private:
     friend class DevToolsProtocol;
-    Command(int id, const std::string& domain, const std::string& method,
+    Command(int id, const std::string& method,
             base::DictionaryValue* params);
 
     int id_;
-    std::string domain_;
-    std::string method_;
-    scoped_ptr<base::DictionaryValue> params_;
 
     DISALLOW_COPY_AND_ASSIGN(Command);
   };
@@ -85,37 +95,20 @@ class DevToolsProtocol {
     DISALLOW_COPY_AND_ASSIGN(Response);
   };
 
-  class Notification {
+  class Notification : public Message {
    public:
-    // Takes ownership of |params|.
-    Notification(const std::string& method, base::DictionaryValue* params);
-    ~Notification();
+    virtual ~Notification();
 
-    std::string Serialize();
-
-   private:
-    std::string method_;
-    scoped_ptr<base::DictionaryValue> params_;
-
-    DISALLOW_COPY_AND_ASSIGN(Notification);
-  };
-
-  class Event {
-   public:
-    ~Event();
-
-    std::string Serialize();
+    virtual std::string Serialize() OVERRIDE;
 
    private:
     friend class DevToolsProtocol;
 
-    // Takes ownership over |params|.
-    Event(const std::string& method, base::DictionaryValue* params);
-
-    std::string method_;
-    scoped_ptr<base::DictionaryValue> params_;
+    // Takes ownership of |params|.
+    Notification(const std::string& method,
+                 base::DictionaryValue* params);
 
-    DISALLOW_COPY_AND_ASSIGN(Event);
+    DISALLOW_COPY_AND_ASSIGN(Notification);
   };
 
   class Handler {
@@ -152,10 +145,15 @@ class DevToolsProtocol {
   static Command* ParseCommand(const std::string& json,
                                std::string* error_response);
 
-  static Event* CreateEvent(const std::string& method,
-                            base::DictionaryValue* params);
+  static Notification* ParseNotification(const std::string& json);
+
+  static Notification* CreateNotification(const std::string& method,
+                                          base::DictionaryValue* params);
 
  private:
+  static DictionaryValue* ParseMessage(const std::string& json,
+                                       std::string* error_response);
+
   DevToolsProtocol() {}
   ~DevToolsProtocol() {}
 };
diff --git a/content/browser/devtools/devtools_tracing_handler.cc b/content/browser/devtools/devtools_tracing_handler.cc
index 5caa2e3..5605c27 100644
--- a/content/browser/devtools/devtools_tracing_handler.cc
+++ b/content/browser/devtools/devtools_tracing_handler.cc
@@ -6,7 +6,6 @@
 
 #include "base/bind.h"
 #include "base/callback.h"
-#include "base/json/json_writer.h"
 #include "base/location.h"
 #include "base/strings/string_split.h"
 #include "base/values.h"
@@ -104,7 +103,6 @@ DevToolsTracingHandler::OnStart(DevToolsProtocol::Command* command) {
   return command->SuccessResponse(NULL);
 }
 
-
 scoped_ptr<DevToolsProtocol::Response>
 DevToolsTracingHandler::OnEnd(DevToolsProtocol::Command* command) {
   TraceController::GetInstance()->EndTracingAsync(this);
diff --git a/content/browser/devtools/devtools_tracing_handler.h b/content/browser/devtools/devtools_tracing_handler.h
index 86ddf00..64f1af7 100644
--- a/content/browser/devtools/devtools_tracing_handler.h
+++ b/content/browser/devtools/devtools_tracing_handler.h
@@ -11,8 +11,6 @@
 
 namespace content {
 
-class DevToolsWebSocketSender;
-
 // This class bridges DevTools remote debugging server with the trace
 // infrastructure.
 class DevToolsTracingHandler
diff --git a/content/browser/devtools/render_view_devtools_agent_host.cc b/content/browser/devtools/render_view_devtools_agent_host.cc
index dd29281..07e4d89 100644
--- a/content/browser/devtools/render_view_devtools_agent_host.cc
+++ b/content/browser/devtools/render_view_devtools_agent_host.cc
@@ -306,11 +306,11 @@ void RenderViewDevToolsAgentHost::RenderViewHostDestroyed(
 }
 
 void RenderViewDevToolsAgentHost::RenderViewCrashed() {
-  scoped_ptr<DevToolsProtocol::Event> event(
-      DevToolsProtocol::CreateEvent(
+  scoped_ptr<DevToolsProtocol::Notification> notification(
+      DevToolsProtocol::CreateNotification(
           devtools::Inspector::targetCrashed::kName, NULL));
   DevToolsManagerImpl::GetInstance()->
-      DispatchOnInspectorFrontend(this, event->Serialize());
+      DispatchOnInspectorFrontend(this, notification->Serialize());
 }
 
 bool RenderViewDevToolsAgentHost::OnRvhMessageReceived(
diff --git a/content/browser/devtools/renderer_overrides_handler.cc b/content/browser/devtools/renderer_overrides_handler.cc
index f7db2e8..7b736e3 100644
--- a/content/browser/devtools/renderer_overrides_handler.cc
+++ b/content/browser/devtools/renderer_overrides_handler.cc
@@ -11,7 +11,6 @@
 #include "base/bind_helpers.h"
 #include "base/files/file_path.h"
 #include "base/string16.h"
-#include "base/stringprintf.h"
 #include "base/values.h"
 #include "content/browser/child_process_security_policy_impl.h"
 #include "content/browser/devtools/devtools_protocol_constants.h"
@@ -64,22 +63,16 @@ RendererOverridesHandler::GrantPermissionsForSetFileInputFiles(
   base::ListValue* file_list = NULL;
   const char* param =
       devtools::DOM::setFileInputFiles::kParamFiles;
-  if (!params || !params->GetList(param, &file_list)) {
-    return command->ErrorResponse(
-        DevToolsProtocol::kErrorInvalidParams,
-        base::StringPrintf("Missing or invalid '%s' parameter", param));
-  }
+  if (!params || !params->GetList(param, &file_list))
+    return command->InvalidParamResponse(param);
   RenderViewHost* host = agent_->GetRenderViewHost();
   if (!host)
     return scoped_ptr<DevToolsProtocol::Response>();
 
   for (size_t i = 0; i < file_list->GetSize(); ++i) {
     base::FilePath::StringType file;
-    if (!file_list->GetString(i, &file)) {
-      return command->ErrorResponse(
-          DevToolsProtocol::kErrorInvalidParams,
-          base::StringPrintf("'%s' must be a list of strings", param));
-    }
+    if (!file_list->GetString(i, &file))
+      return command->InvalidParamResponse(param);
     ChildProcessSecurityPolicyImpl::GetInstance()->GrantReadFile(
         host->GetProcess()->GetID(), base::FilePath(file));
   }
@@ -93,11 +86,8 @@ RendererOverridesHandler::PageHandleJavaScriptDialog(
   const char* paramAccept =
       devtools::Page::handleJavaScriptDialog::kParamAccept;
   bool accept;
-  if (!params || !params->GetBoolean(paramAccept, &accept)) {
-    return command->ErrorResponse(
-        DevToolsProtocol::kErrorInvalidParams,
-        base::StringPrintf("Missing or invalid '%s' parameter", paramAccept));
-  }
+  if (!params || !params->GetBoolean(paramAccept, &accept))
+    return command->InvalidParamResponse(paramAccept);
   string16 prompt_override;
   string16* prompt_override_ptr = &prompt_override;
   if (!params || !params->GetString(
@@ -118,9 +108,7 @@ RendererOverridesHandler::PageHandleJavaScriptDialog(
       }
     }
   }
-  return command->ErrorResponse(
-      DevToolsProtocol::kErrorInternalError,
-      "No JavaScript dialog to handle");
+  return command->InternalErrorResponse("No JavaScript dialog to handle");
 }
 
 scoped_ptr<DevToolsProtocol::Response>
@@ -129,17 +117,11 @@ RendererOverridesHandler::PageNavigate(
   base::DictionaryValue* params = command->params();
   std::string url;
   const char* param = devtools::Page::navigate::kParamUrl;
-  if (!params || !params->GetString(param, &url)) {
-    return command->ErrorResponse(
-        DevToolsProtocol::kErrorInvalidParams,
-        base::StringPrintf("Missing or invalid '%s' parameter",
-                           param));
-  }
+  if (!params || !params->GetString(param, &url))
+    return command->InvalidParamResponse(param);
   GURL gurl(url);
   if (!gurl.is_valid()) {
-    return command->ErrorResponse(
-        DevToolsProtocol::kErrorInternalError,
-        "Cannot navigate to invalid URL");
+    return command->InternalErrorResponse("Cannot navigate to invalid URL");
   }
   RenderViewHost* host = agent_->GetRenderViewHost();
   if (host) {
@@ -150,9 +132,7 @@ RendererOverridesHandler::PageNavigate(
       return command->SuccessResponse(new base::DictionaryValue());
     }
   }
-  return command->ErrorResponse(
-      DevToolsProtocol::kErrorInternalError,
-      "No WebContents to navigate");
+  return command->InternalErrorResponse("No WebContents to navigate");
 }
 
 scoped_ptr<DevToolsProtocol::Response>
@@ -160,8 +140,7 @@ RendererOverridesHandler::PageCaptureScreenshot(
     DevToolsProtocol::Command* command) {
   std::string base_64_data;
   if (!CaptureScreenshot(&base_64_data))
-    return command->ErrorResponse(DevToolsProtocol::kErrorInternalError,
-                                  "Unable to capture a screenshot");
+    return command->InternalErrorResponse("Unable to capture a screenshot");
 
   base::DictionaryValue* response = new base::DictionaryValue();
   response->SetString(