[chromedriver] Implement commands: findChildElement, findChildElements.

[chromedriver] Implement commands: findChildElement, findChildElements.


Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=177267

[chrisgao (Use stgao instead), 2013-01-15 20:53:59]

Please take a look at the new unit testcases.

https://codereview.chromium.org/11884058/diff/1/chrome/test/chromedriver/comm...
File chrome/test/chromedriver/commands.cc (right):

https://codereview.chromium.org/11884058/diff/1/chrome/test/chromedriver/comm...
chrome/test/chromedriver/commands.cc:36: if (has_root_element &&
!params.GetString("id", &root_element))
In the webdriver Json
protocol(http://code.google.com/p/selenium/wiki/JsonWireProtocol#/session/:sessionId/element/:id/element),
the root element id is in the url. Here I assume that it is a paramter.

https://codereview.chromium.org/11884058/diff/1/chrome/test/chromedriver/comm...
File chrome/test/chromedriver/commands_unittest.cc (right):

https://codereview.chromium.org/11884058/diff/1/chrome/test/chromedriver/comm...
chrome/test/chromedriver/commands_unittest.cc:338: session.implicit_wait = 100;
I think we need these new testcases, but maybe we need to set the waiting
timeout for only one testcase. How do you think about it?

[kkania, 2013-01-15 22:24:36]

https://codereview.chromium.org/11884058/diff/1/chrome/test/chromedriver/chro...
File chrome/test/chromedriver/chromedriver.py (right):

https://codereview.chromium.org/11884058/diff/1/chrome/test/chromedriver/chro...
chrome/test/chromedriver/chromedriver.py:110: def FindChildElement(self,
root_element, strategy, target):
we should create a Element class that has these methods. see
http://code.google.com/searchframe#OAMlx_jo-ck/src/third_party/webdriver/pyli...

https://codereview.chromium.org/11884058/diff/1/chrome/test/chromedriver/comm...
File chrome/test/chromedriver/commands.cc (right):

https://codereview.chromium.org/11884058/diff/1/chrome/test/chromedriver/comm...
chrome/test/chromedriver/commands.cc:36: if (has_root_element &&
!params.GetString("id", &root_element))
On 2013/01/15 20:53:59, Shuotao Gao wrote:
> In the webdriver Json
>
protocol(http://code.google.com/p/selenium/wiki/JsonWireProtocol#/session/:sessionId/element/:id/element),
> the root element id is in the url. Here I assume that it is a paramter.

Does this actually work with the Java bindings?

https://codereview.chromium.org/11884058/diff/1/chrome/test/chromedriver/comm...
File chrome/test/chromedriver/commands_unittest.cc (right):

https://codereview.chromium.org/11884058/diff/1/chrome/test/chromedriver/comm...
chrome/test/chromedriver/commands_unittest.cc:207: FindElementChrome(bool
element_exists, bool only_one, int query_count)
this is a bit ugly. just wondering if there's a cleaner way to do this

https://codereview.chromium.org/11884058/diff/1/chrome/test/chromedriver/comm...
chrome/test/chromedriver/commands_unittest.cc:227: const base::Value*
GetResult() { return result_.get(); }
i don't understand the purpose of this

https://codereview.chromium.org/11884058/diff/1/chrome/test/chromedriver/comm...
chrome/test/chromedriver/commands_unittest.cc:287: scoped_ptr<Chrome>(new
FindElementChrome(false, true, 1)));
4 spaces; fix others too

https://codereview.chromium.org/11884058/diff/1/chrome/test/chromedriver/comm...
chrome/test/chromedriver/commands_unittest.cc:338: session.implicit_wait = 100;
On 2013/01/15 20:53:59, Shuotao Gao wrote:
> I think we need these new testcases, but maybe we need to set the waiting
> timeout for only one testcase. How do you think about it?

I think it'd be better to test it in both.

https://codereview.chromium.org/11884058/diff/1/chrome/test/chromedriver/comm...
chrome/test/chromedriver/commands_unittest.cc:348: ASSERT_EQ("frame_id3",
chrome->GetFrame());
i think a lot of this code is duplicated, right? can you share

https://codereview.chromium.org/11884058/diff/1/chrome/test/chromedriver/comm...
chrome/test/chromedriver/commands_unittest.cc:407: session.implicit_wait = 0;
isn't this set to 0 by default; fix the other = 0 cases too

[chrisgao (Use stgao instead), 2013-01-16 19:06:24]

https://codereview.chromium.org/11884058/diff/1/chrome/test/chromedriver/chro...
File chrome/test/chromedriver/chromedriver.py (right):

https://codereview.chromium.org/11884058/diff/1/chrome/test/chromedriver/chro...
chrome/test/chromedriver/chromedriver.py:110: def FindChildElement(self,
root_element, strategy, target):
On 2013/01/15 22:24:36, kkania wrote:
> we should create a Element class that has these methods. see
>
http://code.google.com/searchframe#OAMlx_jo-ck/src/third_party/webdriver/pyli...

Done.

https://codereview.chromium.org/11884058/diff/1/chrome/test/chromedriver/comm...
File chrome/test/chromedriver/commands_unittest.cc (right):

https://codereview.chromium.org/11884058/diff/1/chrome/test/chromedriver/comm...
chrome/test/chromedriver/commands_unittest.cc:207: FindElementChrome(bool
element_exists, bool only_one, int query_count)
On 2013/01/15 22:24:36, kkania wrote:
> this is a bit ugly. just wondering if there's a cleaner way to do this

Done.

https://codereview.chromium.org/11884058/diff/1/chrome/test/chromedriver/comm...
chrome/test/chromedriver/commands_unittest.cc:227: const base::Value*
GetResult() { return result_.get(); }
On 2013/01/15 22:24:36, kkania wrote:
> i don't understand the purpose of this

Used to verified that the returned result is expected.

https://codereview.chromium.org/11884058/diff/1/chrome/test/chromedriver/comm...
chrome/test/chromedriver/commands_unittest.cc:287: scoped_ptr<Chrome>(new
FindElementChrome(false, true, 1)));
On 2013/01/15 22:24:36, kkania wrote:
> 4 spaces; fix others too

Done.

https://codereview.chromium.org/11884058/diff/1/chrome/test/chromedriver/comm...
chrome/test/chromedriver/commands_unittest.cc:348: ASSERT_EQ("frame_id3",
chrome->GetFrame());
On 2013/01/15 22:24:36, kkania wrote:
> i think a lot of this code is duplicated, right? can you share

Done.

https://codereview.chromium.org/11884058/diff/1/chrome/test/chromedriver/comm...
chrome/test/chromedriver/commands_unittest.cc:407: session.implicit_wait = 0;
On 2013/01/15 22:24:36, kkania wrote:
> isn't this set to 0 by default; fix the other = 0 cases too

Done.

[kkania, 2013-01-16 19:45:10]

lgtm

https://codereview.chromium.org/11884058/diff/13002/chrome/test/chromedriver/...
File chrome/test/chromedriver/commands_unittest.cc (right):

https://codereview.chromium.org/11884058/diff/13002/chrome/test/chromedriver/...
chrome/test/chromedriver/commands_unittest.cc:413: session.implicit_wait = 10;
set all the implicit waits to 1000. if for some reason a thread doesn't wake up
in time, we don't want this to flake

https://codereview.chromium.org/11884058/diff/13002/chrome/test/chromedriver/...
File chrome/test/chromedriver/webelement.py (right):

https://codereview.chromium.org/11884058/diff/13002/chrome/test/chromedriver/...
chrome/test/chromedriver/webelement.py:1: # Copyright (c) 2012 The Chromium
Authors. All rights reserved.
2013

https://codereview.chromium.org/11884058/diff/13002/chrome/test/chromedriver/...
chrome/test/chromedriver/webelement.py:6: """Reperents an HTML element."""
Represents

[chrisgao (Use stgao instead), 2013-01-16 20:08:20]

https://codereview.chromium.org/11884058/diff/13002/chrome/test/chromedriver/...
File chrome/test/chromedriver/commands_unittest.cc (right):

https://codereview.chromium.org/11884058/diff/13002/chrome/test/chromedriver/...
chrome/test/chromedriver/commands_unittest.cc:413: session.implicit_wait = 10;
On 2013/01/16 19:45:10, kkania wrote:
> set all the implicit waits to 1000. if for some reason a thread doesn't wake
up
> in time, we don't want this to flake
Done. But 1000 is too long for the testcase timeout, so still set it to 2.

https://codereview.chromium.org/11884058/diff/13002/chrome/test/chromedriver/...
File chrome/test/chromedriver/webelement.py (right):

https://codereview.chromium.org/11884058/diff/13002/chrome/test/chromedriver/...
chrome/test/chromedriver/webelement.py:1: # Copyright (c) 2012 The Chromium
Authors. All rights reserved.
On 2013/01/16 19:45:10, kkania wrote:
> 2013

Done.

https://codereview.chromium.org/11884058/diff/13002/chrome/test/chromedriver/...
chrome/test/chromedriver/webelement.py:6: """Reperents an HTML element."""
On 2013/01/16 19:45:10, kkania wrote:
> Represents

Done.

[commit-bot: I haz the power, 2013-01-16 20:24:38]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/chrisgao@chromium.org/11884058/13003

[commit-bot: I haz the power, 2013-01-16 23:37:15]

Change committed as 177267