Adding a LaunchApp command to Chromedriver

chrome/test/chromedriver/window_commands.cc

 // Copyright (c) 2013 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.
 
 #include "chrome/test/chromedriver/window_commands.h"
 
 #include <list>
 #include <string>
 
 #include "base/callback.h"
@@ skipping 229 matching lines @@
     Session* session,
     WebView* web_view,
     const base::DictionaryValue& params,
     scoped_ptr<base::Value>* value) {
   std::string url;
   if (!params.GetString("url", &url))
     return Status(kUnknownError, "'url' must be a string");
   return web_view->Load(url);
 }
 
+Status ExecuteLaunchApp(

[chrisgao (Use stgao instead), 2014/01/07 23:36:36]

Is the app always opened in the current target window (a tab in chrome) ?
If not, do we want to switch to the window of the app by default?

[bustamante, 2014/01/13 21:19:29]

Depending on the app type there's different behaviors, v1 packaged/hosted apps
will open in a new window.  Packaged apps will have the id in the url, while
hosted (bookmark) app will open like a normal window created by the user.

v2 apps will just received an "OnLaunched" event and decide what to do from
there.  That usually will involve creating a new window, but they aren't
required too.

I added code that will switch the active window to the last one opened, which
works for each app type.  There's a couple caveats that if a new window is
simultaneously created independent of chromedriver it may get focus, and if the
app doesn't create a window when launching this will return an unrelated window.
 But those seem like rare use cases.

What do you think?

[chrisgao (Use stgao instead), 2014/01/15 21:39:56]

Sorry, I might mislead you on this.

After looking into the code, I found session->chrome->GetWebViewIds is not
reliable to return the list of opened windows in order, especially when two or
more windows are opened between two calls of session->chrome->GetWebViewIds.
This method could only do a best guess. And the underlying devtools command
returns the list of windows ordered by "last activity time" which is defined by
the time when the window is shown or selected. If the user tries to debug the
test and interact with the opened windows, then session->chrome->GetWebViewIds
won't live up to its API document. I will update the comment of this method in
chrome.h.

And after thinking it more thoroughly, I feel that it might be better to leave
the switchTo app window to the users.

First, we could not reliably tell which window is the last one opened (we assume
it is the app window) while the user could. The user could do a diff of two
calls to command driver.window_handles (rely on session->chrome->GetWebViewIds)
if there is only one window opened. If two or more windows are opened, the user
could still make it by querying the url or title of the window, or checking some
text in the page of that window.
I think this also applies to app window.

Second, it might be better to deal with an app window the same way as normal web
page window for consistency. For normal web page window (it could be opened due
to a click or javascript), we leave the switchTo to the user. For app, when it's
launched, although I believe almost 100% the user is going to test and interact
with it, the user might still do the switchTo in the same way as normal web page
window (they get used to that).

Third, some apps will be opened in the current window/tab while others (and most
of them) are in a new one. If we try our best to do the switchTo, we might end
in the wrong window. If the user handles that corner case in their code, this
might lead to two code paths for different types or implementation of apps.

What do you think?

[bustamante, 2014/01/15 23:51:38]

Yeah that makes sense, if users won't mind switching to the window themselves I
agree that's the best to make sure they get desired behavior.  Thanks for
catching that taking the last window from GetWebViewIds wouldn't be reliable.

Done.

+    Session* session,
+    WebView* web_view,
+    const base::DictionaryValue& params,
+    scoped_ptr<base::Value>* value) {
+  std::string id;
+  if (!params.GetString("id", &id))
+    return Status(kUnknownError, "'id' must be a string");
+
+  AutomationExtension* extension = NULL;
+  Status status =
+      session->chrome->GetAsDesktop()->GetAutomationExtension(&extension);

[chrisgao (Use stgao instead), 2014/01/07 23:36:36]

We may need to test whether we are on desktop or android like
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/test/chrome....
If not, return an error saying LaunchApp unsupported?

[bustamante, 2014/01/13 21:19:29]

That's a great point, Done.

+
+  return extension->LaunchApp(id);

[chrisgao (Use stgao instead), 2014/01/07 23:36:36]

We may also need to check the returned status from GetAutomationExtension above,
as there might be some error already and |extension| could be NULL in that case.

[bustamante, 2014/01/13 21:19:29]

Done.

+}
+
 Status ExecuteExecuteScript(
     Session* session,
     WebView* web_view,
     const base::DictionaryValue& params,
     scoped_ptr<base::Value>* value) {
   std::string script;
   if (!params.GetString("script", &script))
     return Status(kUnknownError, "'script' must be a string");
   if (script == ":takeHeapSnapshot") {
     return web_view->TakeHeapSnapshot(value);
@@ skipping 604 matching lines @@
   return status;
 }
 
 Status ExecuteTakeHeapSnapshot(
     Session* session,
     WebView* web_view,
     const base::DictionaryValue& params,
     scoped_ptr<base::Value>* value) {
   return web_view->TakeHeapSnapshot(value);
 }