[chromedriver] Add page loading strategy to capabilities

[chromedriver] Add page loading strategy to capabilities

Allows users to explicitly control how chromedriver waits for pending
navigations.
Current strategy: normal (Waits for entire page to load)
Added strategy: none (No wait time)

BUG=
R=samuong@chromium.org

Committed: https://crrev.com/48d04d73048bbb1f8901dad6df688f3f035559ed
Cr-Commit-Position: refs/heads/master@{#410483}

[evajiang, 2016-07-06 18:20:58]

evajiang@google.com changed reviewers:
+ samuong@chromium.org

[evajiang, 2016-07-06 18:20:58]

evajiang@google.com changed required reviewers:
+ samuong@chromium.org

[evajiang, 2016-07-06 18:23:31]

Description was changed from

==========
[chromedriver] Add page loading strategy to capabilities

Allows users to explicitly control how chromedriver waits for pending
navigations.
Current strategy: normal (Waits for entire page to load)
Added strategy: none (No wait time)

BUG=
==========

to

==========
[chromedriver] Add page loading strategy to capabilities

Allows users to explicitly control how chromedriver waits for pending
navigations.
Current strategy: normal (Waits for entire page to load)
Added strategy: none (No wait time)

BUG=
R=samuong@chromium.org
==========

[evajiang, 2016-07-06 18:23:58]

evajiang@google.com changed reviewers:
+ prasadv@chromium.org

[samuong, 2016-07-25 22:44:33]

https://codereview.chromium.org/2125123002/diff/20001/chrome/test/chromedrive...
File chrome/test/chromedriver/capabilities.cc (right):

https://codereview.chromium.org/2125123002/diff/20001/chrome/test/chromedrive...
chrome/test/chromedriver/capabilities.cc:590: page_loading("normal"),
normal, eager and none should be defined constants somewhere

https://codereview.chromium.org/2125123002/diff/20001/chrome/test/chromedrive...
chrome/test/chromedriver/capabilities.cc:608: parser_map["pageLoadingStrategy"]
= base::Bind(&ParseString, &page_loading);
this should be called "pageLoadStrategy" (see
https://w3c.github.io/webdriver/webdriver-spec.html#capabilities)

https://codereview.chromium.org/2125123002/diff/20001/chrome/test/chromedrive...
File chrome/test/chromedriver/capabilities.h (right):

https://codereview.chromium.org/2125123002/diff/20001/chrome/test/chromedrive...
chrome/test/chromedriver/capabilities.h:145: std::string page_loading;
let's call this "page_load_strategy" to be consistent with the capability name

https://codereview.chromium.org/2125123002/diff/20001/chrome/test/chromedrive...
File chrome/test/chromedriver/chrome/web_view_impl.cc (right):

https://codereview.chromium.org/2125123002/diff/20001/chrome/test/chromedrive...
chrome/test/chromedriver/chrome/web_view_impl.cc:142: debugger_(new
DebuggerTracker(client.get())){
nit: space before the {

https://codereview.chromium.org/2125123002/diff/20001/chrome/test/chromedrive...
chrome/test/chromedriver/chrome/web_view_impl.cc:148:
navigation_tracker_.reset(new NavigationTrackerNone());
create a "factory" method in PageLoadStrategy instead

class PageLoadStrategy {
  // ...
  static PageLoadStrategy* Create(std::string strategy);
  // ...
};

you can then call it from the initializer list like

navigation_tracker_(PageLoadStrategy::Create(strategy)),

https://codereview.chromium.org/2125123002/diff/20001/chrome/test/chromedrive...
chrome/test/chromedriver/chrome/web_view_impl.cc:149:
client_.reset(client.release());
this should be moved back into the initalizer list as it was before

https://codereview.chromium.org/2125123002/diff/20001/chrome/test/chromedrive...
File chrome/test/chromedriver/session_commands.cc (left):

https://codereview.chromium.org/2125123002/diff/20001/chrome/test/chromedrive...
chrome/test/chromedriver/session_commands.cc:190: 
nit: don't delete the blank line

[samuong, 2016-07-27 18:08:04]

https://codereview.chromium.org/2125123002/diff/40001/chrome/test/chromedrive...
File chrome/test/chromedriver/chrome/chrome_impl.cc (right):

https://codereview.chromium.org/2125123002/diff/40001/chrome/test/chromedrive...
chrome/test/chromedriver/chrome/chrome_impl.cc:133: std::string
ChromeImpl::GetPageLoadStrategy() const {
nit: name this page_load_strategy()

https://codereview.chromium.org/2125123002/diff/40001/chrome/test/chromedrive...
chrome/test/chromedriver/chrome/chrome_impl.cc:141: return Status(kUnknownError,
"load strategy unsupported");
We should check this when parsing the capabilities. You can create a new
function in capabilities.cc like ParsePageLoadStrategy, which does this. Note
that this is an optional capability, and should default to kNormal.

Also, you can just name this function set_page_load_strategy().

https://codereview.chromium.org/2125123002/diff/40001/chrome/test/chromedrive...
File chrome/test/chromedriver/chrome/devtools_http_client.h (right):

https://codereview.chromium.org/2125123002/diff/40001/chrome/test/chromedrive...
chrome/test/chromedriver/chrome/devtools_http_client.h:79: std::string
strategy);
call this page_load_strategy, just 'strategy' is too generic

https://codereview.chromium.org/2125123002/diff/40001/chrome/test/chromedrive...
File chrome/test/chromedriver/chrome/navigation_tracker_none.h (right):

https://codereview.chromium.org/2125123002/diff/40001/chrome/test/chromedrive...
chrome/test/chromedriver/chrome/navigation_tracker_none.h:13: class
NavigationTrackerNone : public PageLoadStrategy {
I wish we had a better name for this. What do you think of
NonBlockingNavigationTracker?

https://codereview.chromium.org/2125123002/diff/40001/chrome/test/chromedrive...
chrome/test/chromedriver/chrome/navigation_tracker_none.h:23: void
set_timed_out(bool timed_out) override;
nit: add a blank line before private:

https://codereview.chromium.org/2125123002/diff/40001/chrome/test/chromedrive...
File chrome/test/chromedriver/chrome/page_load_strategy.cc (right):

https://codereview.chromium.org/2125123002/diff/40001/chrome/test/chromedrive...
chrome/test/chromedriver/chrome/page_load_strategy.cc:17: else
this should explicitly check for strategy == kNormal

https://codereview.chromium.org/2125123002/diff/40001/chrome/test/chromedrive...
chrome/test/chromedriver/chrome/page_load_strategy.cc:18: return new
NavigationTracker(client, browser_info, dialog_manager);
and then the else branch should have: "NOTREACHED()"
also add a comment saying that the value of |strategy| is validated when parsing
capabilities (in ParsePageLoadStrategy)

https://codereview.chromium.org/2125123002/diff/40001/chrome/test/chromedrive...
File chrome/test/chromedriver/chrome/page_load_strategy.h (right):

https://codereview.chromium.org/2125123002/diff/40001/chrome/test/chromedrive...
chrome/test/chromedriver/chrome/page_load_strategy.h:13: const std::string
kEager = "eager";
rather than creating a new namespace, make these static members of the
PageLoadStrategy class (e.g. load_strategy::kNormal ->
PageLoadStrategy::kNormal)

https://codereview.chromium.org/2125123002/diff/40001/chrome/test/chromedrive...
chrome/test/chromedriver/chrome/page_load_strategy.h:32: static
PageLoadStrategy* Create(std::string strategy,
nit: newline after the '(' so that all the params line up

https://codereview.chromium.org/2125123002/diff/40001/chrome/test/chromedrive...
File chrome/test/chromedriver/chrome/web_view_impl.cc (right):

https://codereview.chromium.org/2125123002/diff/40001/chrome/test/chromedrive...
chrome/test/chromedriver/chrome/web_view_impl.cc:129: std::string strategy)
strategy -> page_load_strategy

https://codereview.chromium.org/2125123002/diff/40001/chrome/test/chromedrive...
File chrome/test/chromedriver/chrome/web_view_impl.h (right):

https://codereview.chromium.org/2125123002/diff/40001/chrome/test/chromedrive...
chrome/test/chromedriver/chrome/web_view_impl.h:48: std::string strategy);
strategy -> page_load_strategy

[evajiang, 2016-07-28 17:15:04]

https://codereview.chromium.org/2125123002/diff/20001/chrome/test/chromedrive...
File chrome/test/chromedriver/capabilities.cc (right):

https://codereview.chromium.org/2125123002/diff/20001/chrome/test/chromedrive...
chrome/test/chromedriver/capabilities.cc:590: page_loading("normal"),
On 2016/07/25 at 22:44:33, samuong wrote:
> normal, eager and none should be defined constants somewhere

Done.

https://codereview.chromium.org/2125123002/diff/20001/chrome/test/chromedrive...
chrome/test/chromedriver/capabilities.cc:608: parser_map["pageLoadingStrategy"]
= base::Bind(&ParseString, &page_loading);
On 2016/07/25 at 22:44:33, samuong wrote:
> this should be called "pageLoadStrategy" (see
https://w3c.github.io/webdriver/webdriver-spec.html#capabilities)

Done.

https://codereview.chromium.org/2125123002/diff/20001/chrome/test/chromedrive...
File chrome/test/chromedriver/capabilities.h (right):

https://codereview.chromium.org/2125123002/diff/20001/chrome/test/chromedrive...
chrome/test/chromedriver/capabilities.h:145: std::string page_loading;
On 2016/07/25 at 22:44:33, samuong wrote:
> let's call this "page_load_strategy" to be consistent with the capability name

Done.

https://codereview.chromium.org/2125123002/diff/20001/chrome/test/chromedrive...
File chrome/test/chromedriver/chrome/web_view_impl.cc (right):

https://codereview.chromium.org/2125123002/diff/20001/chrome/test/chromedrive...
chrome/test/chromedriver/chrome/web_view_impl.cc:142: debugger_(new
DebuggerTracker(client.get())){
On 2016/07/25 at 22:44:33, samuong wrote:
> nit: space before the {

Done.

https://codereview.chromium.org/2125123002/diff/20001/chrome/test/chromedrive...
chrome/test/chromedriver/chrome/web_view_impl.cc:148:
navigation_tracker_.reset(new NavigationTrackerNone());
On 2016/07/25 at 22:44:33, samuong wrote:
> create a "factory" method in PageLoadStrategy instead
> 
> class PageLoadStrategy {
>   // ...
>   static PageLoadStrategy* Create(std::string strategy);
>   // ...
> };
> 
> you can then call it from the initializer list like
> 
> navigation_tracker_(PageLoadStrategy::Create(strategy)),

Done.

https://codereview.chromium.org/2125123002/diff/20001/chrome/test/chromedrive...
chrome/test/chromedriver/chrome/web_view_impl.cc:149:
client_.reset(client.release());
On 2016/07/25 at 22:44:33, samuong wrote:
> this should be moved back into the initalizer list as it was before

Done.

https://codereview.chromium.org/2125123002/diff/20001/chrome/test/chromedrive...
File chrome/test/chromedriver/session_commands.cc (left):

https://codereview.chromium.org/2125123002/diff/20001/chrome/test/chromedrive...
chrome/test/chromedriver/session_commands.cc:190: 
On 2016/07/25 at 22:44:33, samuong wrote:
> nit: don't delete the blank line

Done.

https://codereview.chromium.org/2125123002/diff/40001/chrome/test/chromedrive...
File chrome/test/chromedriver/chrome/chrome_impl.cc (right):

https://codereview.chromium.org/2125123002/diff/40001/chrome/test/chromedrive...
chrome/test/chromedriver/chrome/chrome_impl.cc:133: std::string
ChromeImpl::GetPageLoadStrategy() const {
On 2016/07/27 at 18:08:04, samuong wrote:
> nit: name this page_load_strategy()

Done.

https://codereview.chromium.org/2125123002/diff/40001/chrome/test/chromedrive...
chrome/test/chromedriver/chrome/chrome_impl.cc:141: return Status(kUnknownError,
"load strategy unsupported");
On 2016/07/27 at 18:08:04, samuong wrote:
> We should check this when parsing the capabilities. You can create a new
function in capabilities.cc like ParsePageLoadStrategy, which does this. Note
that this is an optional capability, and should default to kNormal.
> 
> Also, you can just name this function set_page_load_strategy().

Done.

https://codereview.chromium.org/2125123002/diff/40001/chrome/test/chromedrive...
File chrome/test/chromedriver/chrome/devtools_http_client.h (right):

https://codereview.chromium.org/2125123002/diff/40001/chrome/test/chromedrive...
chrome/test/chromedriver/chrome/devtools_http_client.h:79: std::string
strategy);
On 2016/07/27 at 18:08:04, samuong wrote:
> call this page_load_strategy, just 'strategy' is too generic

Done.

https://codereview.chromium.org/2125123002/diff/40001/chrome/test/chromedrive...
File chrome/test/chromedriver/chrome/navigation_tracker_none.h (right):

https://codereview.chromium.org/2125123002/diff/40001/chrome/test/chromedrive...
chrome/test/chromedriver/chrome/navigation_tracker_none.h:13: class
NavigationTrackerNone : public PageLoadStrategy {
On 2016/07/27 at 18:08:04, samuong wrote:
> I wish we had a better name for this. What do you think of
NonBlockingNavigationTracker?

Sounds good!
Done.

https://codereview.chromium.org/2125123002/diff/40001/chrome/test/chromedrive...
chrome/test/chromedriver/chrome/navigation_tracker_none.h:23: void
set_timed_out(bool timed_out) override;
On 2016/07/27 at 18:08:04, samuong wrote:
> nit: add a blank line before private:

Done.

https://codereview.chromium.org/2125123002/diff/40001/chrome/test/chromedrive...
File chrome/test/chromedriver/chrome/page_load_strategy.cc (right):

https://codereview.chromium.org/2125123002/diff/40001/chrome/test/chromedrive...
chrome/test/chromedriver/chrome/page_load_strategy.cc:17: else
On 2016/07/27 at 18:08:04, samuong wrote:
> this should explicitly check for strategy == kNormal

Done.

https://codereview.chromium.org/2125123002/diff/40001/chrome/test/chromedrive...
chrome/test/chromedriver/chrome/page_load_strategy.cc:18: return new
NavigationTracker(client, browser_info, dialog_manager);
On 2016/07/27 at 18:08:04, samuong wrote:
> and then the else branch should have: "NOTREACHED()"
> also add a comment saying that the value of |strategy| is validated when
parsing capabilities (in ParsePageLoadStrategy)

Done.

https://codereview.chromium.org/2125123002/diff/40001/chrome/test/chromedrive...
File chrome/test/chromedriver/chrome/page_load_strategy.h (right):

https://codereview.chromium.org/2125123002/diff/40001/chrome/test/chromedrive...
chrome/test/chromedriver/chrome/page_load_strategy.h:13: const std::string
kEager = "eager";
On 2016/07/27 at 18:08:04, samuong wrote:
> rather than creating a new namespace, make these static members of the
PageLoadStrategy class (e.g. load_strategy::kNormal ->
PageLoadStrategy::kNormal)

Done.

https://codereview.chromium.org/2125123002/diff/40001/chrome/test/chromedrive...
chrome/test/chromedriver/chrome/page_load_strategy.h:32: static
PageLoadStrategy* Create(std::string strategy,
On 2016/07/27 at 18:08:04, samuong wrote:
> nit: newline after the '(' so that all the params line up

Done.

https://codereview.chromium.org/2125123002/diff/40001/chrome/test/chromedrive...
File chrome/test/chromedriver/chrome/web_view_impl.cc (right):

https://codereview.chromium.org/2125123002/diff/40001/chrome/test/chromedrive...
chrome/test/chromedriver/chrome/web_view_impl.cc:129: std::string strategy)
On 2016/07/27 at 18:08:04, samuong wrote:
> strategy -> page_load_strategy

Done.

https://codereview.chromium.org/2125123002/diff/40001/chrome/test/chromedrive...
File chrome/test/chromedriver/chrome/web_view_impl.h (right):

https://codereview.chromium.org/2125123002/diff/40001/chrome/test/chromedrive...
chrome/test/chromedriver/chrome/web_view_impl.h:48: std::string strategy);
On 2016/07/27 at 18:08:04, samuong wrote:
> strategy -> page_load_strategy

Done.

[samuong, 2016-07-29 23:33:47]

It's mostly nitpicking now...

https://codereview.chromium.org/2125123002/diff/60001/chrome/test/chromedrive...
File chrome/test/chromedriver/chrome/non_blocking_navigation_tracker.h (right):

https://codereview.chromium.org/2125123002/diff/60001/chrome/test/chromedrive...
chrome/test/chromedriver/chrome/non_blocking_navigation_tracker.h:17:
NonBlockingNavigationTracker() : timed_out_(false) {}
nit: put the constructor first (before the destructor)

https://codereview.chromium.org/2125123002/diff/60001/chrome/test/chromedrive...
File chrome/test/chromedriver/chrome/page_load_strategy.cc (right):

https://codereview.chromium.org/2125123002/diff/60001/chrome/test/chromedrive...
chrome/test/chromedriver/chrome/page_load_strategy.cc:26: "value of 'strategy'
is validated when capability is parsed";
just say: "invalid strategy '" << strategy << "'"

https://codereview.chromium.org/2125123002/diff/60001/chrome/test/chromedrive...
File chrome/test/chromedriver/chrome/page_load_strategy.h (right):

https://codereview.chromium.org/2125123002/diff/60001/chrome/test/chromedrive...
chrome/test/chromedriver/chrome/page_load_strategy.h:17: public:
nit: one space before "public:"

https://codereview.chromium.org/2125123002/diff/60001/chrome/test/chromedrive...
chrome/test/chromedriver/chrome/page_load_strategy.h:17: public:
nit: one space before "public:"

https://codereview.chromium.org/2125123002/diff/60001/chrome/test/chromedrive...
chrome/test/chromedriver/chrome/page_load_strategy.h:44: #endif //
CHROME_TEST_CHROMEDRIVER_CHROME_PAGE_LOAD_STRATEGY_H_
nit: two spaces before "//"

https://codereview.chromium.org/2125123002/diff/60001/chrome/test/chromedrive...
chrome/test/chromedriver/chrome/page_load_strategy.h:44: #endif //
CHROME_TEST_CHROMEDRIVER_CHROME_PAGE_LOAD_STRATEGY_H_
nit: two spaces before "//"

https://codereview.chromium.org/2125123002/diff/60001/chrome/test/chromedrive...
File chrome/test/chromedriver/chrome/web_view_impl.cc (right):

https://codereview.chromium.org/2125123002/diff/60001/chrome/test/chromedrive...
chrome/test/chromedriver/chrome/web_view_impl.cc:137:
browser_info,dialog_manager_.get())),
nit: space after "browser_info,"

https://codereview.chromium.org/2125123002/diff/60001/chrome/test/chromedrive...
File chrome/test/chromedriver/test/run_py_tests.py (right):

https://codereview.chromium.org/2125123002/diff/60001/chrome/test/chromedrive...
chrome/test/chromedriver/test/run_py_tests.py:554: def
testPageLoadCapabilityDefault(self):
instead of a comment, just name the function so that it's obvious:

def testPageLoadStrategyIsNormalByDefault(self):

[evajiang, 2016-08-01 16:35:47]

https://codereview.chromium.org/2125123002/diff/60001/chrome/test/chromedrive...
File chrome/test/chromedriver/chrome/non_blocking_navigation_tracker.h (right):

https://codereview.chromium.org/2125123002/diff/60001/chrome/test/chromedrive...
chrome/test/chromedriver/chrome/non_blocking_navigation_tracker.h:17:
NonBlockingNavigationTracker() : timed_out_(false) {}
On 2016/07/29 at 23:33:45, samuong wrote:
> nit: put the constructor first (before the destructor)

Done.

https://codereview.chromium.org/2125123002/diff/60001/chrome/test/chromedrive...
File chrome/test/chromedriver/chrome/page_load_strategy.cc (right):

https://codereview.chromium.org/2125123002/diff/60001/chrome/test/chromedrive...
chrome/test/chromedriver/chrome/page_load_strategy.cc:26: "value of 'strategy'
is validated when capability is parsed";
On 2016/07/29 at 23:33:45, samuong wrote:
> just say: "invalid strategy '" << strategy << "'"

Done.

https://codereview.chromium.org/2125123002/diff/60001/chrome/test/chromedrive...
File chrome/test/chromedriver/chrome/page_load_strategy.h (right):

https://codereview.chromium.org/2125123002/diff/60001/chrome/test/chromedrive...
chrome/test/chromedriver/chrome/page_load_strategy.h:17: public:
On 2016/07/29 at 23:33:46, samuong wrote:
> nit: one space before "public:"

Done.

https://codereview.chromium.org/2125123002/diff/60001/chrome/test/chromedrive...
chrome/test/chromedriver/chrome/page_load_strategy.h:44: #endif //
CHROME_TEST_CHROMEDRIVER_CHROME_PAGE_LOAD_STRATEGY_H_
On 2016/07/29 at 23:33:45, samuong wrote:
> nit: two spaces before "//"

Done.

https://codereview.chromium.org/2125123002/diff/60001/chrome/test/chromedrive...
File chrome/test/chromedriver/chrome/web_view_impl.cc (right):

https://codereview.chromium.org/2125123002/diff/60001/chrome/test/chromedrive...
chrome/test/chromedriver/chrome/web_view_impl.cc:137:
browser_info,dialog_manager_.get())),
On 2016/07/29 at 23:33:47, samuong wrote:
> nit: space after "browser_info,"

Done.

https://codereview.chromium.org/2125123002/diff/60001/chrome/test/chromedrive...
File chrome/test/chromedriver/test/run_py_tests.py (right):

https://codereview.chromium.org/2125123002/diff/60001/chrome/test/chromedrive...
chrome/test/chromedriver/test/run_py_tests.py:554: def
testPageLoadCapabilityDefault(self):
On 2016/07/29 at 23:33:47, samuong wrote:
> instead of a comment, just name the function so that it's obvious:
> 
> def testPageLoadStrategyIsNormalByDefault(self):

Done.

[samuong, 2016-08-08 19:11:38]

just a few more changes, then we're ready to land

https://codereview.chromium.org/2125123002/diff/80001/chrome/test/chromedrive...
File chrome/test/chromedriver/chrome/non_blocking_navigation_tracker.cc (right):

https://codereview.chromium.org/2125123002/diff/80001/chrome/test/chromedrive...
chrome/test/chromedriver/chrome/non_blocking_navigation_tracker.cc:18:
timed_out_ = timed_out;
this never gets used, so there's no need to set it (just make it an empty
function and remove the member variable from the class)

https://codereview.chromium.org/2125123002/diff/80001/chrome/test/chromedrive...
File chrome/test/chromedriver/test/run_py_tests.py (right):

https://codereview.chromium.org/2125123002/diff/80001/chrome/test/chromedrive...
chrome/test/chromedriver/test/run_py_tests.py:554: def
testPageLoadCapabilityIsNormalByDefault(self):
nit: rename this to testPageLoadStrategyIsNormalByDefault

https://codereview.chromium.org/2125123002/diff/80001/chrome/test/chromedrive...
chrome/test/chromedriver/test/run_py_tests.py:1717: def
testPageLoadCapabilityNone(self):
nit: rename this to testDoesntWaitWhenPageLoadStrategyIsNone

https://codereview.chromium.org/2125123002/diff/80001/chrome/test/chromedrive...
chrome/test/chromedriver/test/run_py_tests.py:1742: def
testPageLoadStrategyUnsupported(self):
nit: rename this to testUnsupportedPageLoadStrategyRaisesException

[evajiang, 2016-08-08 21:58:20]

https://codereview.chromium.org/2125123002/diff/80001/chrome/test/chromedrive...
File chrome/test/chromedriver/chrome/non_blocking_navigation_tracker.cc (right):

https://codereview.chromium.org/2125123002/diff/80001/chrome/test/chromedrive...
chrome/test/chromedriver/chrome/non_blocking_navigation_tracker.cc:18:
timed_out_ = timed_out;
On 2016/08/08 at 19:11:38, samuong wrote:
> this never gets used, so there's no need to set it (just make it an empty
function and remove the member variable from the class)

Done.

https://codereview.chromium.org/2125123002/diff/80001/chrome/test/chromedrive...
File chrome/test/chromedriver/test/run_py_tests.py (right):

https://codereview.chromium.org/2125123002/diff/80001/chrome/test/chromedrive...
chrome/test/chromedriver/test/run_py_tests.py:554: def
testPageLoadCapabilityIsNormalByDefault(self):
On 2016/08/08 at 19:11:38, samuong wrote:
> nit: rename this to testPageLoadStrategyIsNormalByDefault

Done.

https://codereview.chromium.org/2125123002/diff/80001/chrome/test/chromedrive...
chrome/test/chromedriver/test/run_py_tests.py:1717: def
testPageLoadCapabilityNone(self):
On 2016/08/08 at 19:11:38, samuong wrote:
> nit: rename this to testDoesntWaitWhenPageLoadStrategyIsNone

Done.

https://codereview.chromium.org/2125123002/diff/80001/chrome/test/chromedrive...
chrome/test/chromedriver/test/run_py_tests.py:1742: def
testPageLoadStrategyUnsupported(self):
On 2016/08/08 at 19:11:38, samuong wrote:
> nit: rename this to testUnsupportedPageLoadStrategyRaisesException

Done.

[samuong, 2016-08-08 22:01:44]

lgtm

[evajiang, 2016-08-08 22:02:14]

The CQ bit was checked by evajiang@google.com

[commit-bot: I haz the power, 2016-08-08 22:03:06]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-08 22:44:05]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-08-08 22:46:01]

Description was changed from

==========
[chromedriver] Add page loading strategy to capabilities

Allows users to explicitly control how chromedriver waits for pending
navigations.
Current strategy: normal (Waits for entire page to load)
Added strategy: none (No wait time)

BUG=
R=samuong@chromium.org
==========

to

==========
[chromedriver] Add page loading strategy to capabilities

Allows users to explicitly control how chromedriver waits for pending
navigations.
Current strategy: normal (Waits for entire page to load)
Added strategy: none (No wait time)

BUG=
R=samuong@chromium.org

Committed: https://crrev.com/48d04d73048bbb1f8901dad6df688f3f035559ed
Cr-Commit-Position: refs/heads/master@{#410483}
==========

[commit-bot: I haz the power, 2016-08-08 22:46:02]

Patchset 6 (id:??) landed as
https://crrev.com/48d04d73048bbb1f8901dad6df688f3f035559ed
Cr-Commit-Position: refs/heads/master@{#410483}