[chromedriver] Implement command: switchToWindow.

[chromedriver] Implement command: switchToWindow.
NOTRY=true

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

[chrisgao (Use stgao instead), 2013-02-11 22:56:37]

https://chromiumcodereview.appspot.com/12224106/diff/1/chrome/test/chromedriv...
File chrome/test/chromedriver/session_commands.cc (right):

https://chromiumcodereview.appspot.com/12224106/diff/1/chrome/test/chromedriv...
chrome/test/chromedriver/session_commands.cc:110: if (window_name == name) {
Should we check if two windows share the same name?

https://chromiumcodereview.appspot.com/12224106/diff/1/chrome/test/chromedriv...
chrome/test/chromedriver/session_commands.cc:121: session->mouse_position =
WebPoint(0, 0);
Add chrome/test/chromedriver/basic_types.h

[craigdh, 2013-02-11 23:40:12]

lgtm, though we may want to think about raising an error whenever there could be
a name collision.

https://chromiumcodereview.appspot.com/12224106/diff/1/chrome/test/chromedriv...
File chrome/test/chromedriver/session_commands.cc (right):

https://chromiumcodereview.appspot.com/12224106/diff/1/chrome/test/chromedriv...
chrome/test/chromedriver/session_commands.cc:110: if (window_name == name) {
On 2013/02/11 22:56:37, Shuotao Gao wrote:
> Should we check if two windows share the same name?

Seems like that would be a common issue. Also what do the window handles look
like? If they're just integers I can see opaque collisions there as well.

[chrisgao (Use stgao instead), 2013-02-12 01:26:01]

Hi Frank and Ken,

Would you please take a look at this CL when you have a cycle?

Best wishes,
Shuotao Gao

https://chromiumcodereview.appspot.com/12224106/diff/1/chrome/test/chromedriv...
File chrome/test/chromedriver/session_commands.cc (right):

https://chromiumcodereview.appspot.com/12224106/diff/1/chrome/test/chromedriv...
chrome/test/chromedriver/session_commands.cc:110: if (window_name == name) {
On 2013/02/11 23:40:12, craigdh wrote:
> On 2013/02/11 22:56:37, Shuotao Gao wrote:
> > Should we check if two windows share the same name?
> 
> Seems like that would be a common issue. Also what do the window handles look
> like? If they're just integers I can see opaque collisions there as well.

Window handles look like: "1", "2", etc. for Linux; "12_1", "12_2", etc for Mac
and Windows.

Now also check collision between window name and window handle.

https://chromiumcodereview.appspot.com/12224106/diff/1/chrome/test/chromedriv...
chrome/test/chromedriver/session_commands.cc:121: session->mouse_position =
WebPoint(0, 0);
On 2013/02/11 22:56:37, Shuotao Gao wrote:
> Add chrome/test/chromedriver/basic_types.h

Done.

[kkania, 2013-02-12 01:33:03]

lgtm

https://chromiumcodereview.appspot.com/12224106/diff/6002/chrome/test/chromed...
File chrome/test/chromedriver/run_py_tests.py (right):

https://chromiumcodereview.appspot.com/12224106/diff/6002/chrome/test/chromed...
chrome/test/chromedriver/run_py_tests.py:70: def testSwitchToWindow(self):
add case for switching by window name too?

https://chromiumcodereview.appspot.com/12224106/diff/6002/chrome/test/chromed...
File chrome/test/chromedriver/session_commands.cc (right):

https://chromiumcodereview.appspot.com/12224106/diff/6002/chrome/test/chromed...
chrome/test/chromedriver/session_commands.cc:112: return Status(kUnknownError,
"window name or handle collision:" + name);
i'm not sure I'd bother doing this. It's not in the spec, nor in the old cd or
other drivers, and it will probably break some tests.

https://chromiumcodereview.appspot.com/12224106/diff/6002/chrome/test/chromed...
File chrome/test/chromedriver/session_commands.h (right):

https://chromiumcodereview.appspot.com/12224106/diff/6002/chrome/test/chromed...
chrome/test/chromedriver/session_commands.h:57: // Change focus to another
window. The window to change focus to may be
i think focus is a bit misleading. It just changes the target.

[kkania, 2013-02-12 01:34:56]

On 2013/02/12 01:33:03, kkania wrote:
> lgtm
> 
>
https://chromiumcodereview.appspot.com/12224106/diff/6002/chrome/test/chromed...
> File chrome/test/chromedriver/run_py_tests.py (right):
> 
>
https://chromiumcodereview.appspot.com/12224106/diff/6002/chrome/test/chromed...
> chrome/test/chromedriver/run_py_tests.py:70: def testSwitchToWindow(self):
> add case for switching by window name too?
> 
>
https://chromiumcodereview.appspot.com/12224106/diff/6002/chrome/test/chromed...
> File chrome/test/chromedriver/session_commands.cc (right):
> 
>
https://chromiumcodereview.appspot.com/12224106/diff/6002/chrome/test/chromed...
> chrome/test/chromedriver/session_commands.cc:112: return Status(kUnknownError,
> "window name or handle collision:" + name);
> i'm not sure I'd bother doing this. It's not in the spec, nor in the old cd or
> other drivers, and it will probably break some tests.
> 
>
https://chromiumcodereview.appspot.com/12224106/diff/6002/chrome/test/chromed...
> File chrome/test/chromedriver/session_commands.h (right):
> 
>
https://chromiumcodereview.appspot.com/12224106/diff/6002/chrome/test/chromed...
> chrome/test/chromedriver/session_commands.h:57: // Change focus to another
> window. The window to change focus to may be
> i think focus is a bit misleading. It just changes the target.

It'd probably be a good idea to add some string to the beginning of the handle
name so that it doesn't conflict with the handles. How about prepend "CDwindow-"
or something?

[chrisgao (Use stgao instead), 2013-02-12 20:38:16]

https://chromiumcodereview.appspot.com/12224106/diff/6002/chrome/test/chromed...
File chrome/test/chromedriver/run_py_tests.py (right):

https://chromiumcodereview.appspot.com/12224106/diff/6002/chrome/test/chromed...
chrome/test/chromedriver/run_py_tests.py:70: def testSwitchToWindow(self):
On 2013/02/12 01:33:04, kkania wrote:
> add case for switching by window name too?

Done.

https://chromiumcodereview.appspot.com/12224106/diff/6002/chrome/test/chromed...
File chrome/test/chromedriver/session_commands.cc (right):

https://chromiumcodereview.appspot.com/12224106/diff/6002/chrome/test/chromed...
chrome/test/chromedriver/session_commands.cc:112: return Status(kUnknownError,
"window name or handle collision:" + name);
On 2013/02/12 01:33:04, kkania wrote:
> i'm not sure I'd bother doing this. It's not in the spec, nor in the old cd or
> other drivers, and it will probably break some tests.

Ok. Do the same way as the old cd.

https://chromiumcodereview.appspot.com/12224106/diff/6002/chrome/test/chromed...
File chrome/test/chromedriver/session_commands.h (right):

https://chromiumcodereview.appspot.com/12224106/diff/6002/chrome/test/chromed...
chrome/test/chromedriver/session_commands.h:57: // Change focus to another
window. The window to change focus to may be
On 2013/02/12 01:33:04, kkania wrote:
> i think focus is a bit misleading. It just changes the target.

Done.

[commit-bot: I haz the power, 2013-02-12 22:04:05]

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

[commit-bot: I haz the power, 2013-02-12 22:07:49]

Change committed as 182035