[webkit-reviews] review denied: [Bug 46283] Allow CSS animations to run at more than 40FPS : [Attachment 69599] Patch

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has denied Noam Rosenthal
<noam.rosenthal at nokia.com>'s request for review:
Bug 46283: Allow CSS animations to run at more than 40FPS
https://bugs.webkit.org/show_bug.cgi?id=46283

Attachment 69599: Patch
https://bugs.webkit.org/attachment.cgi?id=69599&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=69599&action=review

> WebCore/page/Settings.h:204
> +	   static void setMinDOMTimerInterval(double); // Interval specified in
seconds.	
> +	   static void setAnimationTimerDelay(double); // Timer delay specified
in seconds.

Comments are strictly worse than code.	Please include the units in the code,
and then you don't need the "seconds" comment.

Example:
void setAnimationTimerDelay(double seconds)
would be better, but:

void setAnimationTimerDelaySeconds(double); woudl be even better!

> WebKit/qt/Api/qgraphicswebview.cpp:250
> +    Settings::setAnimationTimerDelay(0.0167);

This constant would be much clearer if documented, and ideally computed from
values more human readable.

Also, this method would be much clearer if it mentioned the units in question.