Cookie commands for the webdriver protocol

chrome/test/webdriver/commands/cookie_commands.cc

Index: chrome/test/webdriver/commands/cookie_commands.cc
diff --git a/chrome/test/webdriver/commands/cookie_commands.cc b/chrome/test/webdriver/commands/cookie_commands.cc
new file mode 100644
index 0000000000000000000000000000000000000000..6a9635a31250f89c3d834e202b643256b743079f
--- /dev/null
+++ b/chrome/test/webdriver/commands/cookie_commands.cc
@@ -0,0 +1,172 @@
+// Copyright (c) 2010 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.
+
+#include "base/string_util.h"
+
+#include "chrome/test/webdriver/commands/cookie_commands.h"

[kkania, 2011/02/08 18:35:22]

wrong include order. the format is:
#include "chrome/test/webdriver/commands/cookie_commands.h"

#include <string>
#include <vector>

#include "base/string_util.h"
#include "chrome/test/webdriver/cookie.h"

[Joe, 2011/02/09 06:32:10]

Done.

+#include "chrome/test/webdriver/cookie.h"
+
+#include <string>
+#include <vector>

[kkania, 2011/02/08 18:35:22]

you need to add to your includes:
#include "base/scoped_ptr.h"
#include "base/values.h"
#include "chrome/test/webdriver/commands/response.h"
#include "chrome/test/webdriver/session.h"

[Joe, 2011/02/09 06:32:10]

Done.

+
+namespace webdriver {
+
+bool CookieCommand::Init(Response* const response) {
+  if (WebDriverCommand::Init(response)) {
+    if (session_->GetURL(&current_url_)) {

[kkania, 2011/02/08 18:35:22]

I think this is broken. The GetURL function takes a std::string*, not a GURL.
The easiest fix would be to change it to GetGURL and use your new function. Or
you could use std::string instead of GURL

[Joe, 2011/02/09 06:32:10]

It's not broken it uses void GetGURL(GURL* gurl, bool* success); in
automation.cc

On 2011/02/08 18:35:22, kkania wrote:

+      return true;
+    }
+    SET_WEBDRIVER_ERROR(response, "Failed to query current page URL",
+                        kInternalServerError);
+    return false;
+  }
+
+  SET_WEBDRIVER_ERROR(response, "Failed to init Cookie Command",

[kkania, 2011/02/08 18:35:22]

You don't need this. Use the error set from WebDriverCommand::Init, which is
more helpful for the user.

[Joe, 2011/02/09 06:32:10]

Done.

+                      kInternalServerError);
+  return false;
+}
+
+void CookieCommand::ExecuteGet(Response* const response) {
+  // TODO(JMikhail): Add GetJSONCookies to automation proxy since
+  // GetCookies does not return the necessary information
+  std::string cookies;
+  std::vector<std::string> tokens;
+
+  if (!session_->GetCookies(current_url_, &cookies)) {
+    response->set_status(kUnknownError);
+    response->set_value(new StringValue("Failed to fetch cookies"));
+    return;
+  }
+
+  Tokenize(cookies, std::string(";"), &tokens);

[kkania, 2011/02/08 18:35:22]

std::string is unnecessary here, just do ";"

[Joe, 2011/02/09 06:32:10]

Done.

+  ListValue* l = new ListValue();

[kkania, 2011/02/08 18:35:22]

usually the only time people accept a one character variable is for looping.
change 'l' to 'cookie_list'

[Joe, 2011/02/09 06:32:10]

Done.

+  for (std::vector<std::string>::iterator i = tokens.begin();
+       i != tokens.end(); ++i) {
+    Cookie cookie(*i);
+    if (cookie.IsValid()) {
+      l->Append(cookie.ToJSON());
+    } else {
+      LOG(ERROR) << "Failed to parse cookie: " << *i;
+    }
+  }
+
+  response->set_status(kSuccess);
+  response->set_value(l);
+}
+
+void CookieCommand::ExecutePost(Response* const response) {
+  std::string cookie_string;
+  DictionaryValue* cookie_dict;
+  scoped_ptr<Cookie> cookie;
+
+  // Check to see if the cookie was passed in as a string.
+  if (GetStringASCIIParameter("cookie", &cookie_string)) {

[kkania, 2011/02/08 18:35:22]

get rid of this and follow the spec, which says that the input cookie will be in
JSON format.

[Joe, 2011/02/09 06:32:10]

It does i just support one more format since it needs to be comverted to a
string for chrome
On 2011/02/08 18:35:22, kkania wrote:

+    cookie.reset(new Cookie(cookie_string));
+  } else {
+    // Check to see if the cookie was passed in as a JSON onject.
+    if (GetDictionaryParameter("cookie", &cookie_dict)) {
+      cookie.reset(new Cookie(*cookie_dict));
+    } else {
+      // No valid cookie sent.
+      response->set_value(new StringValue("Cookie key not found"));

[kkania, 2011/02/08 18:35:22]

Use SET_WEBDRIVER_ERROR

[Joe, 2011/02/09 06:32:10]

Done.

+      response->set_status(kBadRequest);
+      return;
+    }
+  }
+
+  // Make sure the cookie is formated preoperly.
+  if (!cookie->IsValid()) {
+    response->set_value(new StringValue("Invalid cookie"));

[kkania, 2011/02/08 18:35:22]

use SET_WEBDRIVER_ERROR

[Joe, 2011/02/09 06:32:10]

Done.

+    response->set_status(kBadRequest);
+    return;
+  }
+
+  if (!session_->SetCookie(current_url_, cookie->ToString())) {
+    response->set_status(kUnableToSetCookie);

[kkania, 2011/02/08 18:35:22]

use SET_WEBDRIVER_ERROR

[Joe, 2011/02/09 06:32:10]

Done.

+    response->set_value(new StringValue("Failed to set cookie"));
+    return;
+  }
+
+  response->set_status(kSuccess);
+  response->set_value(new StringValue(cookie->ToString()));

[kkania, 2011/02/08 18:35:22]

the spec does not list a return value

[Joe, 2011/02/09 06:32:10]

I set the value for my own debugging, it's ignored in the drivers
On 2011/02/08 18:35:22, kkania wrote:

+}
+
+void CookieCommand::ExecuteDelete(Response* const response) {
+  std::string cookies;
+  std::vector<std::string> tokens;
+
+  if (!session_->GetCookies(current_url_, &cookies)) {
+    response->set_status(kUnknownError);

[kkania, 2011/02/08 18:35:22]

SET_WEBDRIVER_ERR

[Joe, 2011/02/09 06:32:10]

Done.

+    response->set_value(new StringValue("Failed to fetch cookies"));
+    return;
+  }
+
+  Tokenize(cookies, std::string(":"), &tokens);

[kkania, 2011/02/08 18:35:22]

std::string is unnecessary

[Joe, 2011/02/09 06:32:10]

Done.

+  for (std::vector<std::string>::iterator i = tokens.begin();
+       i != tokens.end(); ++i) {
+    Cookie cookie(*i);
+    if (cookie.IsValid()) {
+      if (!session_->DeleteCookie(current_url_, cookie.get_name())) {
+        LOG(INFO) << "Could not delete cookie: " << cookie.get_name() << "\n"

[kkania, 2011/02/08 18:35:22]

use VLOG(1) instead of LOG(INFO)

[Joe, 2011/02/09 06:32:10]

Done.

+                  << "Contents of cookie: " << cookie.ToString();
+        SET_WEBDRIVER_ERROR(response, "Could not delete all cookies",
+                            kInternalServerError);
+        return;
+      }
+    } else {
+      LOG(ERROR) << "Failed to parse cookie: " << *i;
+    }
+  }
+
+  response->set_status(kSuccess);
+  response->set_value(new StringValue("Cookies delete"));
+}
+
+bool NamedCookieCommand::Init(Response* const response) {
+  if (WebDriverCommand::Init(response)) {
+    if (!session_->GetURL(&current_url_)) {
+      SET_WEBDRIVER_ERROR(response, "Failed to query current page URL",
+                          kInternalServerError);
+      return false;
+    }
+
+    // There should be at least 5 segments to match
+    // /session/:sessionId/cookie/:name
+    cookie_name_ = GetPathVariable(4);
+    SET_WEBDRIVER_ERROR(response, "No cookie name specified",
+                        kBadRequest);
+    return false;
+  }
+
+  SET_WEBDRIVER_ERROR(response, "Failed to init Named Cookie Command",
+                      kInternalServerError);
+  return false;
+}
+
+void NamedCookieCommand::ExecuteGet(Response* const response) {

[kkania, 2011/02/08 18:35:22]

GET isn't listed in the spec for named cookie. Why is it here? If it's not for
webdriver, can you please remove it.

[Joe, 2011/02/09 06:32:10]

I added it for completeness and would like to also change the spec for
webdriver.  It affects nothing while it's here.
On 2011/02/08 18:35:22, kkania wrote:

+  std::string cookie;
+
+  if (!session_->GetCookieByName(current_url_, cookie_name_, &cookie)) {
+    response->set_status(kUnknownError);
+    response->set_value(new StringValue("Failed to fetch cookie"));
+    return;
+  }
+
+  response->set_status(kSuccess);
+  response->set_value(new StringValue(cookie));
+}
+
+void NamedCookieCommand::ExecuteDelete(Response* const response) {
+  if (!session_->DeleteCookie(current_url_, cookie_name_)) {
+    response->set_status(kUnknownError);

[kkania, 2011/02/08 18:35:22]

use SET_WEBDRIVER_ERROR

[Joe, 2011/02/09 06:32:10]

Done.

+    response->set_value(new StringValue("Failed to delete cookie"));
+    return;
+  }
+
+  response->set_status(kSuccess);
+  response->set_value(new StringValue("Cookie deleted"));
+}
+
+}  // namespace webdriver
+