[chromedriver] When launching a Chrome App, automatically switch to the new window.

[chromedriver] When launching a Chrome App, automatically switch to the new window.

BUG=chromedriver:1130
Committed: https://crrev.com/72ffbe61199a03312123efe7e9829251cfe5c407
Cr-Commit-Position: refs/heads/master@{#336444}

[samuong, 2015-06-23 14:42:16]

samuong@chromium.org changed reviewers:
+ stgao@chromium.org

[samuong, 2015-06-23 14:42:16]

See the issue tracker for context. Note that I wanted to keep app-specific logic
out of chrome_impl.cc, so I added a Callback that gets called when we find a new
window while parsing the window list from DevTools. If you think there's a
simpler way, let me know.

[stgao, 2015-06-26 17:55:48]

lgtm with some nits.

https://codereview.chromium.org/1196663005/diff/1/chrome/test/chromedriver/ch...
File chrome/test/chromedriver/chrome/chrome_desktop_impl.h (right):

https://codereview.chromium.org/1196663005/diff/1/chrome/test/chromedriver/ch...
chrome/test/chromedriver/chrome/chrome_desktop_impl.h:60: Status
WaitForNewAppWindow(const std::string& extension_id,
should it be app_id instead of extension_id?

https://codereview.chromium.org/1196663005/diff/1/chrome/test/chromedriver/ch...
chrome/test/chromedriver/chrome/chrome_desktop_impl.h:61: std::string*
web_view_id,
IMHO, the output parameters usually go at the end of the parameter list. Same
for other occurrences below.

https://codereview.chromium.org/1196663005/diff/1/chrome/test/chromedriver/se...
File chrome/test/chromedriver/session_commands.cc (right):

https://codereview.chromium.org/1196663005/diff/1/chrome/test/chromedriver/se...
chrome/test/chromedriver/session_commands.cc:305: base::TimeDelta timeout =
base::TimeDelta::FromSeconds(60);
How do we come to this timeout setting? Should it be an optional input from the
end user?

[samuong, 2015-06-26 18:09:26]

https://codereview.chromium.org/1196663005/diff/1/chrome/test/chromedriver/ch...
File chrome/test/chromedriver/chrome/chrome_desktop_impl.h (right):

https://codereview.chromium.org/1196663005/diff/1/chrome/test/chromedriver/ch...
chrome/test/chromedriver/chrome/chrome_desktop_impl.h:60: Status
WaitForNewAppWindow(const std::string& extension_id,
On 2015/06/26 17:55:47, Shuotao wrote:
> should it be app_id instead of extension_id?

Done. This is essentially the same thing, since both apps and extensions are
both built on top of the extensions system. But I've changed it to |app_id|
since this function only works in with apps.

https://codereview.chromium.org/1196663005/diff/1/chrome/test/chromedriver/ch...
chrome/test/chromedriver/chrome/chrome_desktop_impl.h:61: std::string*
web_view_id,
On 2015/06/26 17:55:47, Shuotao wrote:
> IMHO, the output parameters usually go at the end of the parameter list. Same
> for other occurrences below.

Done.

https://codereview.chromium.org/1196663005/diff/1/chrome/test/chromedriver/se...
File chrome/test/chromedriver/session_commands.cc (right):

https://codereview.chromium.org/1196663005/diff/1/chrome/test/chromedriver/se...
chrome/test/chromedriver/session_commands.cc:305: base::TimeDelta timeout =
base::TimeDelta::FromSeconds(60);
On 2015/06/26 17:55:47, Shuotao wrote:
> How do we come to this timeout setting? Should it be an optional input from
the
> end user?

It's just an attempt at a reasonable default, but we should let end users
control this if they want to override the default. We've got an issue for this
(https://code.google.com/p/chromedriver/issues/detail?id=1105) so I'll add a
switch or capability to control all of these timeouts in a separate CL.

[samuong, 2015-06-26 18:09:34]

The CQ bit was checked by samuong@chromium.org

[commit-bot: I haz the power, 2015-06-26 18:10:24]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1196663005/1

[samuong, 2015-06-26 18:12:38]

The CQ bit was unchecked by samuong@chromium.org

[samuong, 2015-06-26 18:23:14]

The CQ bit was checked by samuong@chromium.org

[samuong, 2015-06-26 18:23:14]

The patchset sent to the CQ was uploaded after l-g-t-m from stgao@chromium.org
Link to the patchset: https://codereview.chromium.org/1196663005/#ps40001
(title: "add copyright message to test app background.js")

[commit-bot: I haz the power, 2015-06-26 18:23:48]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1196663005/40001

[commit-bot: I haz the power, 2015-06-26 19:39:22]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-06-26 19:40:20]

Patchset 3 (id:??) landed as
https://crrev.com/72ffbe61199a03312123efe7e9829251cfe5c407
Cr-Commit-Position: refs/heads/master@{#336444}