Several bug fixes for the ChromeDriver:

- Should be able to focus on a frame using its frame element, not just
  an ID, name, or index.
- Element equality checks now work properly
- Session-level command handlers should be registered with mongoose after
  the element handlers to avoid clashes from greedy wildcard matching.
- Remove spam from test logs by returning 204 on favicon requests instead of
  returning 404.

This change also updates src/third_party/webdriver/python to r11767.

BUG=none
TEST=none

Review URL: http://codereview.chromium.org/6723004
Patch from Jason Leyba <jleyba@chromium.org>.

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

11 files changed, 135 insertions, 26 deletions

diff --git a/DEPS b/DEPS
index d792fc8..9ed54b0 100644
--- a/DEPS
+++ b/DEPS
@@ -181,9 +181,9 @@ deps = {
   # python egg is installed. We run tests directly from src, so import
   # the code into the structure expected by the tests.
   "src/third_party/webdriver/python/selenium":
-    "http://selenium.googlecode.com/svn/trunk/py/selenium@11696",
+    "http://selenium.googlecode.com/svn/trunk/py/selenium@11776",
   "src/third_party/webdriver/python/selenium/test":
-    "http://selenium.googlecode.com/svn/trunk/py/test@11696",
+    "http://selenium.googlecode.com/svn/trunk/py/test@11776",
 
   "src/third_party/libvpx":
     "/trunk/deps/third_party/libvpx@" +
diff --git a/chrome/test/webdriver/WEBDRIVER_TESTS b/chrome/test/webdriver/WEBDRIVER_TESTS
index b41912d..0b977c0 100644
--- a/chrome/test/webdriver/WEBDRIVER_TESTS
+++ b/chrome/test/webdriver/WEBDRIVER_TESTS
@@ -27,7 +27,6 @@
     'cookie_tests',
     # Automation proxy only returns cookie name=value pairs.
     '-cookie_tests.CookieTest.testAddCookie',
-    '-cookie_tests.CookieTest.testGetGoogleCookie',
     'correct_event_firing_tests',
     'driver_element_finding_test',
     'element_attribute_tests',
diff --git a/chrome/test/webdriver/chromedriver_tests.py b/chrome/test/webdriver/chromedriver_tests.py
index 02168e2..29f2304 100755
--- a/chrome/test/webdriver/chromedriver_tests.py
+++ b/chrome/test/webdriver/chromedriver_tests.py
@@ -26,6 +26,7 @@ sys.path += [chromedriver_paths.PYTHON_BINDINGS]
 
 import simplejson as json
 
+from selenium.webdriver.remote.command import Command
 from selenium.webdriver.remote.webdriver import WebDriver
 
 
@@ -108,6 +109,14 @@ class BasicTest(unittest.TestCase):
     except urllib2.HTTPError, expected:
       self.assertEquals(404, expected.code)
 
+  def testShouldReturn204ForFaviconRequests(self):
+    request_url = self._launcher.GetURL() + '/favicon.ico'
+    response = SendRequest(request_url, method='GET')
+    try:
+      self.assertEquals(204, response.code)
+    finally:
+      response.close()
+
   def testCanStartChromeDriverOnSpecificPort(self):
     launcher = ChromeDriverLauncher(port=9520)
     self.assertEquals(9520, launcher.GetPort())
@@ -290,6 +299,31 @@ class UrlBaseTest(unittest.TestCase):
     self.assertEquals(data['sessionId'], url_parts[4])
 
 
+# TODO(jleyba): Port this to WebDriver's own python test suite.
+class ElementEqualityTest(unittest.TestCase):
+  """Tests that the server properly checks element equality."""
+
+  def setUp(self):
+    self._launcher = ChromeDriverLauncher(root_path=os.path.dirname(__file__))
+    self._driver = WebDriver(self._launcher.GetURL(), {})
+
+  def tearDown(self):
+    self._driver.quit()
+    self._launcher.Kill()
+
+  def testElementEquality(self):
+    self._driver.get(self._launcher.GetURL() + '/test_page.html')
+    body1 = self._driver.find_element_by_tag_name('body')
+    body2 = self._driver.execute_script('return document.body')
+
+    # TODO(jleyba): WebDriver's python bindings should expose a proper API
+    # for this.
+    result = body1.execute(Command.ELEMENT_EQUALS, {
+      'other': body2.id
+    })
+    self.assertTrue(result['value'])
+
+
 if __name__ == '__main__':
   unittest.main(module='chromedriver_tests',
                 testRunner=GTestTextTestRunner(verbosity=1))
diff --git a/chrome/test/webdriver/commands/find_element_commands.cc b/chrome/test/webdriver/commands/find_element_commands.cc
index e4d50de..a40be0b 100644
--- a/chrome/test/webdriver/commands/find_element_commands.cc
+++ b/chrome/test/webdriver/commands/find_element_commands.cc
@@ -37,6 +37,8 @@ void FindElementCommand::ExecutePost(Response* const response) {
   // TODO(jmikhail): The findElement(s) atom should handle this conversion.
   if (locator == "class name") {
     locator = LocatorType::kClassName;
+  } else if (locator == "css selector") {
+    locator = LocatorType::kCss;
   } else if (locator == "link text") {
     locator = LocatorType::kLinkText;
   } else if (locator == "partial link text") {
diff --git a/chrome/test/webdriver/commands/target_locator_commands.cc b/chrome/test/webdriver/commands/target_locator_commands.cc
index 270de02..26a4a43 100644
--- a/chrome/test/webdriver/commands/target_locator_commands.cc
+++ b/chrome/test/webdriver/commands/target_locator_commands.cc
@@ -9,6 +9,7 @@
 #include "chrome/test/webdriver/commands/response.h"
 #include "chrome/test/webdriver/error_codes.h"
 #include "chrome/test/webdriver/session.h"
+#include "chrome/test/webdriver/web_element_id.h"
 
 namespace webdriver {
 
@@ -108,6 +109,7 @@ bool SwitchFrameCommand::DoesPost() {
 void SwitchFrameCommand::ExecutePost(Response* const response) {
   std::string id;
   int index = 0;
+  WebElementId element;
   if (GetStringParameter("id", &id)) {
     ErrorCode code = session_->SwitchToFrameWithNameOrId(id);
     if (code != kSuccess) {
@@ -120,6 +122,12 @@ void SwitchFrameCommand::ExecutePost(Response* const response) {
       SET_WEBDRIVER_ERROR(response, "Could not switch to frame", code);
       return;
     }
+  } else if (GetWebElementParameter("id", &element)) {
+    ErrorCode code = session_->SwitchToFrameWithElement(element);
+    if (code != kSuccess) {
+      SET_WEBDRIVER_ERROR(response, "Could not switch to frame", code);
+      return;
+    }
   } else if (IsNullParameter("id")) {
     session_->SwitchToTopFrame();
   } else {
@@ -130,6 +138,20 @@ void SwitchFrameCommand::ExecutePost(Response* const response) {
   response->SetStatus(kSuccess);
 }
 
+bool SwitchFrameCommand::GetWebElementParameter(const std::string& key,
+                                                WebElementId* out) const {
+  DictionaryValue* value;
+  if (!GetDictionaryParameter(key, &value))
+    return false;
+
+  WebElementId id(value);
+  if (!id.is_valid())
+    return false;
+
+  *out = id;
+  return true;
+}
+
 ActiveElementCommand::ActiveElementCommand(
     const std::vector<std::string>& path_segments,
     DictionaryValue* parameters)
diff --git a/chrome/test/webdriver/commands/target_locator_commands.h b/chrome/test/webdriver/commands/target_locator_commands.h
index 9294ab8..6ab3a6c 100644
--- a/chrome/test/webdriver/commands/target_locator_commands.h
+++ b/chrome/test/webdriver/commands/target_locator_commands.h
@@ -15,6 +15,7 @@ class DictionaryValue;
 namespace webdriver {
 
 class Response;
+class WebElementId;
 
 // Gets the current window handle.
 // REST URL: /session/:sessionId/window_handle
@@ -76,6 +77,8 @@ class SwitchFrameCommand : public WebDriverCommand {
   virtual void ExecutePost(Response* const response);
 
  private:
+  bool GetWebElementParameter(const std::string& key, WebElementId* out) const;
+
   DISALLOW_COPY_AND_ASSIGN(SwitchFrameCommand);
 };
 
diff --git a/chrome/test/webdriver/commands/webelement_commands.cc b/chrome/test/webdriver/commands/webelement_commands.cc
index 1f4e4dd..91fc690 100644
--- a/chrome/test/webdriver/commands/webelement_commands.cc
+++ b/chrome/test/webdriver/commands/webelement_commands.cc
@@ -215,7 +215,9 @@ void ElementEqualsCommand::ExecuteGet(Response* const response) {
 
   scoped_ptr<ListValue> args(new ListValue);
   args->Append(element.ToValue());
-  args->Append(Value::CreateStringValue(path_segments_.at(6)));
+
+  WebElementId other_element(path_segments_.at(6));
+  args->Append(other_element.ToValue());
 
   Value* result = NULL;
   ErrorCode status = session_->ExecuteScript(script, args.get(), &result);
diff --git a/chrome/test/webdriver/dispatch.cc b/chrome/test/webdriver/dispatch.cc
index 4750604..6e0a531 100644
--- a/chrome/test/webdriver/dispatch.cc
+++ b/chrome/test/webdriver/dispatch.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
 
@@ -59,6 +59,15 @@ void Shutdown(struct mg_connection* connection,
   shutdown_event->Signal();
 }
 
+void SendNoContentResponse(struct mg_connection* connection,
+                           const struct mg_request_info* request_info,
+                           void* user_data) {
+  std::string response = "HTTP/1.1 204 No Content\r\n"
+                         "Content-Length:0\r\n"
+                         "\r\n";
+  mg_write(connection, response.data(), response.length());
+}
+
 void SendNotImplementedError(struct mg_connection* connection,
                              const struct mg_request_info* request_info,
                              void* user_data) {
@@ -245,7 +254,11 @@ void DispatchHelper(Command* command_ptr,
 }  // namespace internal
 
 Dispatcher::Dispatcher(struct mg_context* context, const std::string& root)
-    : context_(context), root_(root) {}
+    : context_(context), root_(root) {
+  // Overwrite mongoose's default handler for /favicon.ico to always return a
+  // 204 response so we don't spam the logs with 404s.
+  mg_set_uri_callback(context_, "/favicon.ico", &SendNoContentResponse, NULL);
+}
 
 Dispatcher::~Dispatcher() {}
 
diff --git a/chrome/test/webdriver/server.cc b/chrome/test/webdriver/server.cc
index fede212..b94d348 100644
--- a/chrome/test/webdriver/server.cc
+++ b/chrome/test/webdriver/server.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
 
@@ -77,24 +77,7 @@ void InitCallbacks(struct mg_context* ctx, Dispatcher* dispatcher,
                    base::WaitableEvent* shutdown_event) {
   dispatcher->AddShutdown("/shutdown", shutdown_event);
 
-  dispatcher->Add<CreateSession>(       "/session");
-  dispatcher->Add<BackCommand>(         "/session/*/back");
-  dispatcher->Add<ExecuteCommand>(      "/session/*/execute");
-  dispatcher->Add<ForwardCommand>(      "/session/*/forward");
-  dispatcher->Add<SwitchFrameCommand>(  "/session/*/frame");
-  dispatcher->Add<RefreshCommand>(      "/session/*/refresh");
-  dispatcher->Add<SourceCommand>(       "/session/*/source");
-  dispatcher->Add<SpeedCommand>(        "/session/*/speed");
-  dispatcher->Add<TitleCommand>(        "/session/*/title");
-  dispatcher->Add<URLCommand>(          "/session/*/url");
-  dispatcher->Add<WindowCommand>(       "/session/*/window");
-  dispatcher->Add<WindowHandleCommand>( "/session/*/window_handle");
-  dispatcher->Add<WindowHandlesCommand>("/session/*/window_handles");
-  dispatcher->Add<ImplicitWaitCommand>( "/session/*/timeouts/implicit_wait");
-
-  // Cookie functions.
-  dispatcher->Add<CookieCommand>(     "/session/*/cookie");
-  dispatcher->Add<NamedCookieCommand>("/session/*/cookie/*");
+  dispatcher->Add<CreateSession>("/session");
 
   // WebElement commands
   dispatcher->Add<FindOneElementCommand>(  "/session/*/element");
@@ -124,6 +107,30 @@ void InitCallbacks(struct mg_context* ctx, Dispatcher* dispatcher,
   dispatcher->Add<DragCommand>( "/session/*/element/*/drag");
   dispatcher->Add<HoverCommand>("/session/*/element/*/hover");
 
+  // All session based commands should be listed after the element based
+  // commands to avoid potential mapping conflicts from an overzealous
+  // wildcard match. For example, /session/*/title maps to the handler to
+  // fetch the page title. If mapped first, this would overwrite the handler
+  // for /session/*/element/*/attribute/title, which should fetch the title
+  // attribute of the element.
+  dispatcher->Add<BackCommand>(         "/session/*/back");
+  dispatcher->Add<ExecuteCommand>(      "/session/*/execute");
+  dispatcher->Add<ForwardCommand>(      "/session/*/forward");
+  dispatcher->Add<SwitchFrameCommand>(  "/session/*/frame");
+  dispatcher->Add<RefreshCommand>(      "/session/*/refresh");
+  dispatcher->Add<SourceCommand>(       "/session/*/source");
+  dispatcher->Add<SpeedCommand>(        "/session/*/speed");
+  dispatcher->Add<TitleCommand>(        "/session/*/title");
+  dispatcher->Add<URLCommand>(          "/session/*/url");
+  dispatcher->Add<WindowCommand>(       "/session/*/window");
+  dispatcher->Add<WindowHandleCommand>( "/session/*/window_handle");
+  dispatcher->Add<WindowHandlesCommand>("/session/*/window_handles");
+  dispatcher->Add<ImplicitWaitCommand>( "/session/*/timeouts/implicit_wait");
+
+  // Cookie functions.
+  dispatcher->Add<CookieCommand>(     "/session/*/cookie");
+  dispatcher->Add<NamedCookieCommand>("/session/*/cookie/*");
+
   // Commands that have not been implemented yet. We list these out explicitly
   // so that tests that attempt to use them fail with a meaningful error.
   dispatcher->SetNotImplemented("/session/*/execute_async");
diff --git a/chrome/test/webdriver/session.cc b/chrome/test/webdriver/session.cc
index 39bcd86..9d9ed9e 100644
--- a/chrome/test/webdriver/session.cc
+++ b/chrome/test/webdriver/session.cc
@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chrome/test/webdriver/session_manager.h"
+#include "chrome/test/webdriver/session.h"
 
 #include <sstream>
 #include <vector>
@@ -476,6 +476,29 @@ ErrorCode Session::SwitchToFrameWithIndex(int index) {
   return SwitchToFrameWithJavaScriptLocatedFrame(script, &args);
 }
 
+ErrorCode Session::SwitchToFrameWithElement(const WebElementId& element) {
+  // TODO(jleyba): Extract this, and the other frame switch methods to an atom.
+  std::string script =
+      "var element = arguments[0];"
+      "console.info('Attempting to switch to ' + element);"
+      "if (element.nodeType != 1 || !/^i?frame$/i.test(element.tagName)) {"
+      "  console.info('Element is not a frame: ' + element + "
+      "' {nodeType:' + element.nodeType + ',tagName:' + element.tagName + '}');"
+      "  return null;"
+      "}"
+      "for (var i = 0; i < window.frames.length; i++) {"
+      "  if (element.contentWindow == window.frames[i]) {"
+      "    return '(//iframe|//frame)[' + (i + 1) + ']';"
+      "  }"
+      "}"
+      "console.info('Frame is not connected to this DOM tree');"
+      "return null;";
+
+  ListValue args;
+  args.Append(element.ToValue());
+  return SwitchToFrameWithJavaScriptLocatedFrame(script, &args);
+}
+
 void Session::SwitchToTopFrame() {
   current_target_.frame_path = FramePath();
 }
diff --git a/chrome/test/webdriver/session.h b/chrome/test/webdriver/session.h
index 58994f8..d738fe4 100644
--- a/chrome/test/webdriver/session.h
+++ b/chrome/test/webdriver/session.h
@@ -118,6 +118,10 @@ class Session {
   // Switches the frame used by default. |index| is the zero-based frame index.
   ErrorCode SwitchToFrameWithIndex(int index);
 
+  // Switches to the frame identified by the given |element|. The element must
+  // be either an IFRAME or FRAME element.
+  ErrorCode SwitchToFrameWithElement(const WebElementId& element);
+
   // Switches the frame used by default to the topmost frame.
   void SwitchToTopFrame();