RESOLVED FIXED 62175
[GTK] Implement monotonicallyIncreasingClock()
https://bugs.webkit.org/show_bug.cgi?id=62175
Summary [GTK] Implement monotonicallyIncreasingClock()
James Simonsen
Reported 2011-06-06 18:59:12 PDT
In order to support some of the new HTML5 specs, and to fix other bugs, we need to implement a monotonically increasing clock on all platforms. See https://bugs.webkit.org/show_bug.cgi?id=37743 for more details.
Attachments
Patch (1.23 KB, patch)
2011-07-21 16:08 PDT, Devdatta Deshpande
mrobinson: review-
A HTML file used to unit test the change (502 bytes, text/html)
2011-07-21 16:12 PDT, Devdatta Deshpande
no flags
Modified patch after review comments (1.39 KB, patch)
2011-07-28 12:37 PDT, Devdatta Deshpande
no flags
Patch (1.63 KB, patch)
2011-07-28 15:06 PDT, Devdatta Deshpande
no flags
Patch (1.65 KB, patch)
2011-07-28 15:23 PDT, Devdatta Deshpande
webkit-ews: commit-queue-
Implementation of monotonically increasing clock on GTK (1.39 KB, patch)
2011-08-01 15:35 PDT, Devdatta Deshpande
no flags
Patch (1.39 KB, patch)
2011-08-10 05:28 PDT, Devdatta Deshpande
no flags
Gavin Barraclough
Comment 1 2011-06-23 14:01:18 PDT
Doesn't look like this bug wants to be in the JSC component. Maybe 'Platform' would be better? - or possibly WTF, if that is where this will be implemented?, since I'm unsure I'll conservatively file to misc. for now.
Devdatta Deshpande
Comment 2 2011-07-21 16:08:47 PDT
Devdatta Deshpande
Comment 3 2011-07-21 16:12:47 PDT
Created attachment 101660 [details] A HTML file used to unit test the change Without the patch, the timer stops working if the system time is changed to a time in past. Using monotonically increasing timer instead of system time helps in fixing this issue.
Martin Robinson
Comment 4 2011-07-28 10:17:11 PDT
Comment on attachment 101659 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=101659&action=review This looks sane to me. > Source/JavaScriptCore/ChangeLog:9 > + Reviewed by NOBODY (OOPS!). > + > + * wtf/CurrentTime.cpp: > + (WTF::monotonicallyIncreasingTime): Please fill in this ChangeLog, explaining how the existing implementation of monotonicallyIncreasingTime can fail. > Source/JavaScriptCore/wtf/CurrentTime.cpp:313 > +#elif PLATFORM(GTK) I think it would be better to use #if ENABLE(GLIB_SUPPORT) here.
Devdatta Deshpande
Comment 5 2011-07-28 12:37:10 PDT
Created attachment 102287 [details] Modified patch after review comments Incorporated the review comments by Martin.
Early Warning System Bot
Comment 6 2011-07-28 14:43:05 PDT
Comment on attachment 102287 [details] Modified patch after review comments Attachment 102287 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/9221040
Devdatta Deshpande
Comment 7 2011-07-28 15:06:52 PDT
Gyuyoung Kim
Comment 8 2011-07-28 15:09:38 PDT
Early Warning System Bot
Comment 9 2011-07-28 15:12:24 PDT
Devdatta Deshpande
Comment 10 2011-07-28 15:23:40 PDT
WebKit Review Bot
Comment 11 2011-07-28 16:05:30 PDT
Attachment 102307 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source..." exit_code: 1 Source/JavaScriptCore/wtf/CurrentTime.cpp:68: "glib.h" already included at Source/JavaScriptCore/wtf/CurrentTime.cpp:60 [build/include] [4] Total errors found: 1 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.
Early Warning System Bot
Comment 12 2011-07-28 18:15:20 PDT
Devdatta Deshpande
Comment 13 2011-08-01 15:35:19 PDT
Created attachment 102574 [details] Implementation of monotonically increasing clock on GTK
Devdatta Deshpande
Comment 14 2011-08-01 15:38:20 PDT
(In reply to comment #4) > (From update of attachment 101659 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=101659&action=review > > This looks sane to me. > > > Source/JavaScriptCore/ChangeLog:9 > > + Reviewed by NOBODY (OOPS!). > > + > > + * wtf/CurrentTime.cpp: > > + (WTF::monotonicallyIncreasingTime): > > Please fill in this ChangeLog, explaining how the existing implementation of monotonicallyIncreasingTime can fail. Please find the explanation in the attached patch. > > > Source/JavaScriptCore/wtf/CurrentTime.cpp:313 > > +#elif PLATFORM(GTK) > > I think it would be better to use #if ENABLE(GLIB_SUPPORT) here. g_get_monotonic_time is supported in glib since version 2.28. I am getting compilation errors on QT when I use this API. So, implementing this under PLATFORM(GTK) instead of ENABLE(GLIB_SUPPORT).
Martin Robinson
Comment 15 2011-08-08 01:15:11 PDT
Comment on attachment 102574 [details] Implementation of monotonically increasing clock on GTK View in context: https://bugs.webkit.org/attachment.cgi?id=102574&action=review Loooks good to me. I'll land this one. > Source/JavaScriptCore/wtf/CurrentTime.cpp:334 > + gint64 nowMicroSecs = g_get_monotonic_time(); > + return static_cast<double>(nowMicroSecs / 1000000.0); This should probably just be return static_cast<double>(g_get_monotonic_time() / 1000000.0);
Devdatta Deshpande
Comment 16 2011-08-10 05:28:18 PDT
WebKit Review Bot
Comment 17 2011-08-11 09:49:12 PDT
Comment on attachment 103468 [details] Patch Clearing flags on attachment: 103468 Committed r92859: <http://trac.webkit.org/changeset/92859>
WebKit Review Bot
Comment 18 2011-08-11 09:49:18 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.