[chromedriver]Implement command: title.

[chromedriver]Implement command: title.


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

[kkania, 2013-01-04 04:35:33]

lgtm after fix

https://codereview.chromium.org/11746025/diff/2001/chrome/test/chromedriver/c...
File chrome/test/chromedriver/command_executor_impl.cc (right):

https://codereview.chromium.org/11746025/diff/2001/chrome/test/chromedriver/c...
chrome/test/chromedriver/command_executor_impl.cc:48: command_map_.Set("title",
base::Bind(execute_session_command,
i think it should be getTitle; see
http://code.google.com/p/selenium/source/browse/trunk/java/client/src/org/ope...

https://codereview.chromium.org/11746025/diff/2001/chrome/test/chromedriver/c...
File chrome/test/chromedriver/commands.cc (right):

https://codereview.chromium.org/11746025/diff/2001/chrome/test/chromedriver/c...
chrome/test/chromedriver/commands.cc:115: "function(){ return document.title;
}", args, value);
see how the old chromedriver does it webdriver::Session::GetTitle; you might
want to follow the same approach; i think the idea is to return the title as the
user would see it

https://codereview.chromium.org/11746025/diff/2001/chrome/test/chromedriver/t...
File chrome/test/chromedriver/test.py (right):

https://codereview.chromium.org/11746025/diff/2001/chrome/test/chromedriver/t...
chrome/test/chromedriver/test.py:50:
driver.Load('http://www.google.com/webhp?hl=en')
don't use real pages in tests; use a test page

https://codereview.chromium.org/11746025/diff/2001/chrome/test/chromedriver/t...
chrome/test/chromedriver/test.py:51: import time
don't sleep in tests; that leads to slower tests or flakiness

[chrisgao (Use stgao instead), 2013-01-05 01:19:09]

As command Get to load a page returns before the page is completely loaded,
testcase (like testGetTitle) based on loading a page almost always fails.

https://codereview.chromium.org/11746025/diff/2001/chrome/test/chromedriver/c...
File chrome/test/chromedriver/command_executor_impl.cc (right):

https://codereview.chromium.org/11746025/diff/2001/chrome/test/chromedriver/c...
chrome/test/chromedriver/command_executor_impl.cc:48: command_map_.Set("title",
base::Bind(execute_session_command,
On 2013/01/04 04:35:34, kkania wrote:
> i think it should be getTitle; see
>
http://code.google.com/p/selenium/source/browse/trunk/java/client/src/org/ope...

Done.

https://codereview.chromium.org/11746025/diff/2001/chrome/test/chromedriver/c...
File chrome/test/chromedriver/commands.cc (right):

https://codereview.chromium.org/11746025/diff/2001/chrome/test/chromedriver/c...
chrome/test/chromedriver/commands.cc:115: "function(){ return document.title;
}", args, value);
On 2013/01/04 04:35:34, kkania wrote:
> see how the old chromedriver does it webdriver::Session::GetTitle; you might
> want to follow the same approach; i think the idea is to return the title as
the
> user would see it
Followed the approach of the old chromedriver.

But when attribute <title> is NOT specified. For different urls, the user will
see different titles:
1. file://some/path/file_name.html
  "file_name.html" shows in tab title.
2. https://some.hostname
  "https://some.hostname" shows up.
3. http://some.hostname
  Only "some.hostname" shows up.

Do you think we should deal with different types of protocols in the url?

https://codereview.chromium.org/11746025/diff/2001/chrome/test/chromedriver/t...
File chrome/test/chromedriver/test.py (right):

https://codereview.chromium.org/11746025/diff/2001/chrome/test/chromedriver/t...
chrome/test/chromedriver/test.py:50:
driver.Load('http://www.google.com/webhp?hl=en')
On 2013/01/04 04:35:34, kkania wrote:
> don't use real pages in tests; use a test page

Done.

https://codereview.chromium.org/11746025/diff/2001/chrome/test/chromedriver/t...
chrome/test/chromedriver/test.py:51: import time
On 2013/01/04 04:35:34, kkania wrote:
> don't sleep in tests; that leads to slower tests or flakiness

Done.

https://codereview.chromium.org/11746025/diff/20001/chrome/test/chromedriver/...
File chrome/test/chromedriver/test.py (right):

https://codereview.chromium.org/11746025/diff/20001/chrome/test/chromedriver/...
chrome/test/chromedriver/test.py:26: cls._http_server =
webserver.WebServer(chrome_paths.GetTestData())
HTTP server is shared by all testcases.

https://codereview.chromium.org/11746025/diff/20001/chrome/test/chromedriver/...
chrome/test/chromedriver/test.py:36: def setUp(self):
Add setUp and tearDown to ensure that chrome window or session is closed even
when testcase fails.
Otherwise, failed testcase affects other testcases, because the
remote-debugging-port is hard-codes as 33081 in
chrome/test/chromedriver/chrome_launcher_impl.cc

https://codereview.chromium.org/11746025/diff/20001/chrome/test/chromedriver/...
chrome/test/chromedriver/test.py:66: self.assertEqual('page test',
self._driver.GetTitle())
As Load returns before the page is fully loaded, this testcase almost always
fails.

[kkania, 2013-01-08 21:26:29]

https://codereview.chromium.org/11746025/diff/20001/chrome/test/chromedriver/...
File chrome/test/chromedriver/test.py (right):

https://codereview.chromium.org/11746025/diff/20001/chrome/test/chromedriver/...
chrome/test/chromedriver/test.py:66: self.assertEqual('page test',
self._driver.GetTitle())
On 2013/01/05 01:19:10, Shuotao Gao wrote:
> As Load returns before the page is fully loaded, this testcase almost always
> fails.

We can't use load then until it waits for the navigation. For now, we can
driver.ExecuteScript('document.title = "title") and then check the result.

https://codereview.chromium.org/11746025/diff/20001/chrome/test/chromedriver/...
File chrome/test/chromedriver/webserver.py (right):

https://codereview.chromium.org/11746025/diff/20001/chrome/test/chromedriver/...
chrome/test/chromedriver/webserver.py:30: """Starts the Web server on its own
thread on an ephemeral port.
web

https://codereview.chromium.org/11746025/diff/20001/chrome/test/chromedriver/...
chrome/test/chromedriver/webserver.py:31: It is an HTTP server if parameter
server_cert_and_key_path is not provied.
provided

https://codereview.chromium.org/11746025/diff/20001/chrome/test/chromedriver/...
chrome/test/chromedriver/webserver.py:32: Otherwises, it is an HTTPS server.
Otherwise

https://codereview.chromium.org/11746025/diff/20001/chrome/test/chromedriver/...
chrome/test/chromedriver/webserver.py:39: server_cert_and_key_path: path to a
PEM file containing the cert and key.
explain what happens if it is none

[chrisgao (Use stgao instead), 2013-01-08 22:23:34]

https://chromiumcodereview.appspot.com/11746025/diff/20001/chrome/test/chrome...
File chrome/test/chromedriver/test.py (right):

https://chromiumcodereview.appspot.com/11746025/diff/20001/chrome/test/chrome...
chrome/test/chromedriver/test.py:66: self.assertEqual('page test',
self._driver.GetTitle())
On 2013/01/08 21:26:29, kkania wrote:
> On 2013/01/05 01:19:10, Shuotao Gao wrote:
> > As Load returns before the page is fully loaded, this testcase almost always
> > fails.
> 
> We can't use load then until it waits for the navigation. For now, we can
> driver.ExecuteScript('document.title = "title") and then check the result.

Done.

https://chromiumcodereview.appspot.com/11746025/diff/20001/chrome/test/chrome...
File chrome/test/chromedriver/webserver.py (right):

https://chromiumcodereview.appspot.com/11746025/diff/20001/chrome/test/chrome...
chrome/test/chromedriver/webserver.py:30: """Starts the Web server on its own
thread on an ephemeral port.
On 2013/01/08 21:26:29, kkania wrote:
> web

Done.

https://chromiumcodereview.appspot.com/11746025/diff/20001/chrome/test/chrome...
chrome/test/chromedriver/webserver.py:31: It is an HTTP server if parameter
server_cert_and_key_path is not provied.
On 2013/01/08 21:26:29, kkania wrote:
> provided

Done.

https://chromiumcodereview.appspot.com/11746025/diff/20001/chrome/test/chrome...
chrome/test/chromedriver/webserver.py:32: Otherwises, it is an HTTPS server.
On 2013/01/08 21:26:29, kkania wrote:
> Otherwise

Done.

[commit-bot: I haz the power, 2013-01-08 22:24:57]

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

[commit-bot: I haz the power, 2013-01-08 22:25:00]

Failed to apply patch for chrome/test/chromedriver/command_executor_impl.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file chrome/test/chromedriver/command_executor_impl.cc
  Hunk #1 FAILED at 45.
  1 out of 1 hunk FAILED -- saving rejects to file
chrome/test/chromedriver/command_executor_impl.cc.rej

Patch:       chrome/test/chromedriver/command_executor_impl.cc
Index: chrome/test/chromedriver/command_executor_impl.cc
diff --git a/chrome/test/chromedriver/command_executor_impl.cc
b/chrome/test/chromedriver/command_executor_impl.cc
index
ee8a1bd5d6e7ed68a9462db3639ab4aa5a20a401..7a2d57a97f96c20960171cc361069d668a551cc1
100644
--- a/chrome/test/chromedriver/command_executor_impl.cc
+++ b/chrome/test/chromedriver/command_executor_impl.cc
@@ -45,6 +45,8 @@ void CommandExecutorImpl::Init() {
       base::Bind(&ExecuteGet)));
   command_map_.Set("executeScript", base::Bind(execute_session_command,
       base::Bind(&ExecuteExecuteScript)));
+  command_map_.Set("getTitle", base::Bind(execute_session_command,
+      base::Bind(&ExecuteGetTitle)));
   Command quit_command = base::Bind(execute_session_command,
       base::Bind(&ExecuteQuit, &session_map_));
   command_map_.Set("quit", quit_command);

[commit-bot: I haz the power, 2013-01-09 00:04:30]

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

[commit-bot: I haz the power, 2013-01-09 03:50:05]

Change committed as 175699