[chromedriver] Improve timeout behavior. Prompted by user who executes script on busy page.

[chromedriver] Improve timeout behavior. Prompted by user who executes script on busy page.
-change session timeout to base::TimeDelta
-change command timeout to 10m
-refactor and simplify wait for events
-also, fix some assertions on windows with unzipping
BUG=none
R=frankf@chromium.org

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

[frankf, 2013-08-28 23:15:49]

https://codereview.chromium.org/23542005/diff/3001/chrome/test/chromedriver/c...
File chrome/test/chromedriver/chrome/devtools_client_impl.cc (right):

https://codereview.chromium.org/23542005/diff/3001/chrome/test/chromedriver/c...
chrome/test/chromedriver/chrome/devtools_client_impl.cc:261: switch
(socket_->ReceiveNextMessage(&message, timeout)) {
IIRC, the reason we didn't pass the timeout through was because it's hard to
reason about it since the function is recursive. Can the Ensure* calls above
take arbitrary long time?

[kkania, 2013-08-29 14:07:45]

https://codereview.chromium.org/23542005/diff/3001/chrome/test/chromedriver/c...
File chrome/test/chromedriver/chrome/devtools_client_impl.cc (right):

https://codereview.chromium.org/23542005/diff/3001/chrome/test/chromedriver/c...
chrome/test/chromedriver/chrome/devtools_client_impl.cc:261: switch
(socket_->ReceiveNextMessage(&message, timeout)) {
On 2013/08/28 23:15:50, frankf wrote:
> IIRC, the reason we didn't pass the timeout through was because it's hard to
> reason about it since the function is recursive. Can the Ensure* calls above
> take arbitrary long time? 

Yes. This CL doesn't change the contract that the HandleEventsUntil may take
much longer than the given timeout, if it's processing nested commands/events.

This CL does change:
-simplifies and improves HandleEvents* funcs by using the custom wait instead of
the hardcoded 1m
-lengthens the timeout for commands to 10m. It's really rare to see a command
timeout, but the 10m helps users who haven't bothered to setup hard timeouts and
separate processes for their tests (like our python tests). We could make this
configurable, but I'd prefer not to expose this complexity to the user.

[frankf, 2013-08-29 17:49:29]

https://codereview.chromium.org/23542005/diff/3001/chrome/test/chromedriver/c...
File chrome/test/chromedriver/chrome/devtools_client_impl.cc (right):

https://codereview.chromium.org/23542005/diff/3001/chrome/test/chromedriver/c...
chrome/test/chromedriver/chrome/devtools_client_impl.cc:242: const
base::TimeDelta& timeout) {
My problem is with the timeout param here. What does it conceptually mean?
What's the point of having a non-constant timeout to ReceiveNextMessage below if
you don't know how long it takes to get to line 261.

[frankf, 2013-08-29 22:04:41]

lgtm

[commit-bot: I haz the power, 2013-08-29 22:08:30]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kkania@chromium.org/23542005/3001

[kkania, 2013-08-30 02:13:29]

Committed patchset #1 manually as r220476 (presubmit successful).