[GTK] Avoid std::chrono::microseconds overflows in setSharedTimerFireInterval

[Zan Dobersek]

[GTK] Avoid std::chrono::microseconds overflows in setSharedTimerFireInterval

[Zan Dobersek]

Created attachment 252960 [details]
Patch

[Carlos Garcia Campos]

Comment on attachment 252960 [details]
Patch

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

> Source/WebCore/platform/gtk/SharedTimerGtk.cpp:53
> +    // Passed-in value is in seconds, which might overflow when casting to std::chrono::microseconds.
> +    auto interval = std::chrono::duration<double>(intervalValue);
> +    auto delay = std::chrono::microseconds::max();
> +    if (interval < delay)
> +        delay = std::chrono::duration_cast<std::chrono::microseconds>(interval);

Could we do something like this in GMainLoopSource instead? To fix any other cases where we are passing a double casted to microseconds.

[Zan Dobersek]

Comment on attachment 252960 [details]
Patch

Needs a bit more thought.

[Darin Adler]

Comment on attachment 252960 [details]
Patch

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

> Source/WebCore/platform/gtk/SharedTimerGtk.cpp:49
> +    // Passed-in value is in seconds, which might overflow when casting to std::chrono::microseconds.

This seems like an issue that might happen cross-platform as we move from the traditional “doubles in seconds” to std::chrono across the board. I wonder what the good idiom to avoid this is.

[Zan Dobersek]

A version of this patch landed in r192058.
https://trac.webkit.org/changeset/192058

[Zan Dobersek]

(In reply to comment #4)
> Comment on attachment 252960 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=252960&action=review
> 
> > Source/WebCore/platform/gtk/SharedTimerGtk.cpp:49
> > +    // Passed-in value is in seconds, which might overflow when casting to std::chrono::microseconds.
> 
> This seems like an issue that might happen cross-platform as we move from
> the traditional “doubles in seconds” to std::chrono across the board. I
> wonder what the good idiom to avoid this is.

If possible, we could try specializing the std::chrono::duration<> template for the WTF::Checked class.

[Darin Adler]

For most uses we might need clamping casts that turn out-of-range values into min/max.