[ChromeDriver] W3C compliant SetTimeouts command

Update the SetTimeouts command to accept both pre-W3C and W3C format
parameters.

Bug: chromedriver:2057
Change-Id: Ib380b62fbd279812c62e04e6dc4ce214dc042343
Reviewed-on: https://chromium-review.googlesource.com/1166212Reviewed-by: Jonathon Kereliuk <kereliuk@chromium.org>
Reviewed-by: Caleb Rouleau <crouleau@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581736}

chrome/test/chromedriver/client/chromedriver.py

@@ -356,9 +356,8 @@ class ChromeDriver(object):
     return self.ExecuteCommand(
         Command.FIND_ELEMENTS, {'using': strategy, 'value': target})
 
-  def SetTimeout(self, type, timeout):
-    return self.ExecuteCommand(
-        Command.SET_TIMEOUT, {'type' : type, 'ms': timeout})
+  def SetTimeouts(self, params):
+    return self.ExecuteCommand(Command.SET_TIMEOUTS, params)
 
   def GetCurrentUrl(self):
     return self.ExecuteCommand(Command.GET_CURRENT_URL)

chrome/test/chromedriver/client/command_executor.py

@@ -101,7 +101,7 @@ class Command(object):
       _Method.POST, '/session/:sessionId/timeouts/implicit_wait')
   SET_SCRIPT_TIMEOUT = (
       _Method.POST, '/session/:sessionId/timeouts/async_script')
-  SET_TIMEOUT = (_Method.POST, '/session/:sessionId/timeouts')
+  SET_TIMEOUTS = (_Method.POST, '/session/:sessionId/timeouts')
   GET_TIMEOUTS = (_Method.GET, '/session/:sessionId/timeouts')
   EXECUTE_SQL = (_Method.POST, '/session/:sessionId/execute_sql')
   GET_LOCATION = (_Method.GET, '/session/:sessionId/location')

chrome/test/chromedriver/server/http_handler.cc

@@ -100,7 +100,7 @@ HttpHandler::HttpHandler(
           WrapToCommand("GetTimeouts", base::Bind(&ExecuteGetTimeouts))),
       CommandMapping(
           kPost, "session/:sessionId/timeouts",
-          WrapToCommand("SetTimeout", base::Bind(&ExecuteSetTimeout))),
+          WrapToCommand("SetTimeouts", base::Bind(&ExecuteSetTimeouts))),
       CommandMapping(kPost, "session/:sessionId/url",
                      WrapToCommand("Navigate", base::Bind(&ExecuteGet))),
       CommandMapping(

chrome/test/chromedriver/session_commands.cc

@@ -542,9 +542,11 @@ Status ExecuteSwitchToWindow(Session* session,
   return Status(kOk);
 }
 
-Status ExecuteSetTimeout(Session* session,
-                         const base::DictionaryValue& params,
-                         std::unique_ptr<base::Value>* value) {
+// Handles legacy format SetTimeout command.
+// TODO(johnchen@chromium.org): Remove when we stop supporting legacy protocol.
+Status ExecuteSetTimeoutLegacy(Session* session,
+                               const base::DictionaryValue& params,
+                               std::unique_ptr<base::Value>* value) {
   double ms_double;
   if (!params.GetDouble("ms", &ms_double))
     return Status(kUnknownError, "'ms' must be a double");
@@ -554,8 +556,6 @@ Status ExecuteSetTimeout(Session* session,
 
   base::TimeDelta timeout =
       base::TimeDelta::FromMilliseconds(static_cast<int>(ms_double));
-  // TODO(frankf): implicit and script timeout should be cleared
-  // if negative timeout is specified.
   if (type == "implicit") {
     session->implicit_wait = timeout;
   } else if (type == "script") {
@@ -570,6 +570,41 @@ Status ExecuteSetTimeout(Session* session,
   return Status(kOk);
 }
 
+Status ExecuteSetTimeoutsW3C(Session* session,
+                             const base::DictionaryValue& params,
+                             std::unique_ptr<base::Value>* value) {
+  for (const auto& setting : params.DictItems()) {
+    int timeout_ms;
+    if (!setting.second.GetAsInteger(&timeout_ms) || timeout_ms < 0)
+      return Status(kInvalidArgument, "value must be a non-negative integer");
+    base::TimeDelta timeout = base::TimeDelta::FromMilliseconds(timeout_ms);
+    const std::string& type = setting.first;
+    if (type == "script") {
+      session->script_timeout = timeout;
+    } else if (type == "pageLoad") {
+      session->page_load_timeout = timeout;
+    } else if (type == "implicit") {
+      session->implicit_wait = timeout;
+    } else {
+      return Status(kInvalidArgument, "unknown type of timeout: " + type);
+    }
+  }
+  return Status(kOk);
+}
+
+Status ExecuteSetTimeouts(Session* session,
+                          const base::DictionaryValue& params,
+                          std::unique_ptr<base::Value>* value) {
+  // TODO(johnchen@chromium.org): Remove legacy version support when we stop
+  // supporting non-W3C protocol. At that time, we can delete the legacy
+  // function and merge the W3C function into this function.
+  if (params.HasKey("ms")) {
+    return ExecuteSetTimeoutLegacy(session, params, value);
+  } else {
+    return ExecuteSetTimeoutsW3C(session, params, value);
+  }
+}
+
 Status ExecuteGetTimeouts(Session* session,
                           const base::DictionaryValue& params,
                           std::unique_ptr<base::Value>* value) {

chrome/test/chromedriver/session_commands.h

@@ -80,9 +80,9 @@ Status ExecuteSwitchToWindow(Session* session,
 
 // Configure the amount of time that a particular type of operation can execute
 // for before they are aborted and a timeout error is returned to the client.
-Status ExecuteSetTimeout(Session* session,
-                         const base::DictionaryValue& params,
-                         std::unique_ptr<base::Value>* value);
+Status ExecuteSetTimeouts(Session* session,
+                          const base::DictionaryValue& params,
+                          std::unique_ptr<base::Value>* value);
 
 // Get the implicit, script and page load timeouts in milliseconds.
 Status ExecuteGetTimeouts(Session* session,

chrome/test/chromedriver/session_commands_unittest.cc

@@ -41,6 +41,41 @@ TEST(SessionCommandsTest, ExecuteGetTimeouts) {
   ASSERT_EQ(implicit, 0);
 }
 
+TEST(SessionCommandsTest, ExecuteSetTimeouts) {
+  Session session("id");
+  base::DictionaryValue params;
+  std::unique_ptr<base::Value> value;
+
+  // W3C spec doesn't forbid passing in an empty object, so we should get kOk.
+  Status status = ExecuteSetTimeouts(&session, params, &value);
+  ASSERT_EQ(kOk, status.code());
+
+  params.SetInteger("pageLoad", 5000);
+  status = ExecuteSetTimeouts(&session, params, &value);
+  ASSERT_EQ(kOk, status.code());
+
+  params.SetInteger("script", 5000);
+  params.SetInteger("implicit", 5000);
+  status = ExecuteSetTimeouts(&session, params, &value);
+  ASSERT_EQ(kOk, status.code());
+
+  params.SetInteger("implicit", -5000);
+  status = ExecuteSetTimeouts(&session, params, &value);
+  ASSERT_EQ(kInvalidArgument, status.code());
+
+  params.Clear();
+  params.SetInteger("unknown", 5000);
+  status = ExecuteSetTimeouts(&session, params, &value);
+  ASSERT_EQ(kInvalidArgument, status.code());
+
+  // Old pre-W3C format.
+  params.Clear();
+  params.SetDouble("ms", 5000.0);
+  params.SetString("type", "page load");
+  status = ExecuteSetTimeouts(&session, params, &value);
+  ASSERT_EQ(kOk, status.code());
+}
+
 TEST(SessionCommandsTest, MergeCapabilities) {
   base::DictionaryValue primary;
   primary.SetString("strawberry", "velociraptor");

chrome/test/chromedriver/test/run_py_tests.py

@@ -485,7 +485,7 @@ class ChromeDriverTest(ChromeDriverBaseTestWithWebServer):
                       self._driver.ExecuteScript, '{{{')
 
   def testExecuteAsyncScript(self):
-    self._driver.SetTimeout('script', 3000)
+    self._driver.SetTimeouts({'script': 3000})
     self.assertRaises(
         chromedriver.ScriptTimeout,
         self._driver.ExecuteAsyncScript,
@@ -1699,7 +1699,7 @@ class ChromeDriverSiteIsolation(ChromeDriverBaseTestWithWebServer):
       # This test is unreliable on Windows, as FindElement can be called too
       # soon, before the child frame is fully loaded. This causes element not
       # found error. Add an implicit wait works around this issue.
-      self._driver.SetTimeout('implicit', 2000)
+      self._driver.SetTimeouts({'implicit': 2000})
     self._driver.Load(self.GetHttpUrlForFile(
         '/chromedriver/cross_domain_iframe.html'))
     a_outer = self._driver.FindElement('tag name', 'a')
@@ -1741,7 +1741,7 @@ class ChromeDriverPageLoadTimeoutTest(ChromeDriverBaseTestWithWebServer):
     # about 0.1 second on Linux and Windows, but takes half a second or longer
     # on Mac. So we use longer timeout on Mac, 0.5 second on others.
     timeout = 3000 if util.GetPlatformName() == 'mac' else 500
-    self._driver.SetTimeout('page load', timeout)
+    self._driver.SetTimeouts({'pageLoad': timeout})
 
   def tearDown(self):
     super(ChromeDriverPageLoadTimeoutTest, self).tearDown()