[webkit-reviews] review denied: [Bug 204114] WebDriver: fix handling of session timeouts for values higher than MAX_INT : [Attachment 383354] Patch

[bugzilla-daemon at webkit.org]

Brian Burg <bburg at apple.com> has denied Carlos Garcia Campos
<cgarcia at igalia.com>'s request for review:
Bug 204114: WebDriver: fix handling of session timeouts for values higher than
MAX_INT
https://bugs.webkit.org/show_bug.cgi?id=204114

Attachment 383354: Patch

https://bugs.webkit.org/attachment.cgi?id=383354&action=review




--- Comment #5 from Brian Burg <bburg at apple.com> ---
Comment on attachment 383354
  --> https://bugs.webkit.org/attachment.cgi?id=383354
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=383354&action=review

r- because its needs to be rebased.

Code changes look otherwise fine.

Only concern: I understand moving from int to Seconds, but not from Seconds to
double. Is there any practical benefit to using raw double over Seconds to
represent seconds? It seems like the entire point of Seconds is to replace
usage of doubles.

> Source/WebDriver/Session.h:54
> +    double pageLoadTimeout() const { return m_pageLoadTimeout; }

Why?

> Source/WebKit/WebProcess/Automation/WebAutomationSessionProxy.cpp:269
> +	   JSValueMakeNumber(context, callbackTimeout.valueOr(-1))

Okay.

> Source/WebKit/WebProcess/Automation/WebAutomationSessionProxy.js:67
> +	       if (!resultReported && callbackTimeout >= 0)

Okay.