[chromedriver] Fix race where we might not wait for page load after navigating.

[chromedriver] Fix race where we might not wait for page load after navigating.

Change the NavigationTracker to assume the page is loading after Page.navigate.
BUG=chromedriver:347
R=chrisgao@chromium.org, craigdh@chromium.org

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

[craigdh, 2013-05-23 23:37:33]

https://codereview.chromium.org/15772009/diff/7/chrome/test/chromedriver/chro...
File chrome/test/chromedriver/chrome/devtools_client_impl.cc (right):

https://codereview.chromium.org/15772009/diff/7/chrome/test/chromedriver/chro...
chrome/test/chromedriver/chrome/devtools_client_impl.cc:176: Status
DevToolsClientImpl::WaitForRendererRoundTrip() {
Can we have a comment here? Is this is an arbitrary simple command?

https://codereview.chromium.org/15772009/diff/7/chrome/test/chromedriver/chro...
File chrome/test/chromedriver/chrome/web_view_impl.cc (right):

https://codereview.chromium.org/15772009/diff/7/chrome/test/chromedriver/chro...
chrome/test/chromedriver/chrome/web_view_impl.cc:113: // load start event is
sent from the renderer.
Could you expand the comment a bit more? Is it that we could receive the
response from the Page.navigate command before we received a load started event?

[chrisgao (Use stgao instead), 2013-05-24 17:35:31]

lgtm when Craig's comments are addressed.

https://codereview.chromium.org/15772009/diff/7/chrome/test/chromedriver/chro...
File chrome/test/chromedriver/chrome/devtools_client.h (right):

https://codereview.chromium.org/15772009/diff/7/chrome/test/chromedriver/chro...
chrome/test/chromedriver/chrome/devtools_client.h:52: // asynchronous commands
may return out of order.
we might give an example of asynchronous command here.

[kkania, 2013-05-28 15:56:58]

changed technique. turns out in some cases (incl process switches) navigations
can be delayed until the prev page unloads; it is possible to do a round trip to
the renderer here before the page starts to navigate

instead, now we just set the navigation tracker state to loading when we receive
a successful Page.navigate response.

https://codereview.chromium.org/15772009/diff/7/chrome/test/chromedriver/chro...
File chrome/test/chromedriver/chrome/devtools_client.h (right):

https://codereview.chromium.org/15772009/diff/7/chrome/test/chromedriver/chro...
chrome/test/chromedriver/chrome/devtools_client.h:52: // asynchronous commands
may return out of order.
On 2013/05/24 17:35:32, chrisgao wrote:
> we might give an example of asynchronous command here.

Done.

https://codereview.chromium.org/15772009/diff/7/chrome/test/chromedriver/chro...
File chrome/test/chromedriver/chrome/devtools_client_impl.cc (right):

https://codereview.chromium.org/15772009/diff/7/chrome/test/chromedriver/chro...
chrome/test/chromedriver/chrome/devtools_client_impl.cc:176: Status
DevToolsClientImpl::WaitForRendererRoundTrip() {
On 2013/05/23 23:37:33, craigdh wrote:
> Can we have a comment here? Is this is an arbitrary simple command?

Done.

https://codereview.chromium.org/15772009/diff/7/chrome/test/chromedriver/chro...
File chrome/test/chromedriver/chrome/web_view_impl.cc (right):

https://codereview.chromium.org/15772009/diff/7/chrome/test/chromedriver/chro...
chrome/test/chromedriver/chrome/web_view_impl.cc:113: // load start event is
sent from the renderer.
On 2013/05/23 23:37:33, craigdh wrote:
> Could you expand the comment a bit more? Is it that we could receive the
> response from the Page.navigate command before we received a load started
event?

Done.

[craigdh, 2013-05-28 17:46:28]

lgtm.

[kkania, 2013-05-28 21:00:00]

Committed patchset #4 manually as r202651 (presubmit successful).