Bug 127263 - [EFL][WK2] Use nanoseconds in TimerWorkItem class consistently
Summary: [EFL][WK2] Use nanoseconds in TimerWorkItem class consistently
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit EFL (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jinwoo Song
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-01-19 22:16 PST by Jinwoo Song
Modified: 2014-01-21 17:58 PST (History)
10 users (show)

See Also:


Attachments
Patch (7.25 KB, patch)
2014-01-19 22:23 PST, Jinwoo Song
no flags Details | Formatted Diff | Diff
Patch (5.74 KB, patch)
2014-01-19 23:17 PST, Jinwoo Song
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jinwoo Song 2014-01-19 22:16:41 PST
After r162276 and r162300, TimerWorkItem is getting nanoseconds as parameter.
So it would be better to use nanoseconds in TimerWorkItem for consistency.
Comment 1 Jinwoo Song 2014-01-19 22:23:43 PST
Created attachment 221614 [details]
Patch
Comment 2 WebKit Commit Bot 2014-01-19 22:24:56 PST
Attachment 221614 [details] did not pass style-queue:


ERROR: Source/WebKit2/Platform/efl/DispatchQueueWorkItemEfl.h:57:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/Platform/efl/DispatchQueueWorkItemEfl.h:66:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 2 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Gyuyoung Kim 2014-01-19 22:30:14 PST
Comment on attachment 221614 [details]
Patch

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

Looks make sense except for style nits and a new function in CurrentTime.

> Source/WTF/wtf/CurrentTime.h:64
> +inline double monotonicallyIncreasingTimeNS()

Should we add this function ? Can't we use this functionality by using monotonicallyIncreasingTimeMS() ?

It looks this new function need to be reviewed by other reviewer.
Comment 4 Jinwoo Song 2014-01-19 23:16:05 PST
Comment on attachment 221614 [details]
Patch

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

Regarding the style error, I got a guide from andersca to make a space after 'void'. It seems that the style-checker need to be updated for this rule.

>> Source/WTF/wtf/CurrentTime.h:64
>> +inline double monotonicallyIncreasingTimeNS()
> 
> Should we add this function ? Can't we use this functionality by using monotonicallyIncreasingTimeMS() ?
> 
> It looks this new function need to be reviewed by other reviewer.

It would be better not to add new function as it is only used in EFL port. I'll use monotonicallyIncreasingTime().
Comment 5 Jinwoo Song 2014-01-19 23:17:22 PST
Created attachment 221619 [details]
Patch
Comment 6 WebKit Commit Bot 2014-01-19 23:18:28 PST
Attachment 221619 [details] did not pass style-queue:


ERROR: Source/WebKit2/Platform/efl/DispatchQueueWorkItemEfl.h:57:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/Platform/efl/DispatchQueueWorkItemEfl.h:66:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 2 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 7 Gyuyoung Kim 2014-01-20 00:47:07 PST
CC'ing Byungwoo
Comment 8 Gyuyoung Kim 2014-01-20 00:54:53 PST
Comment on attachment 221619 [details]
Patch

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

Byungwoo, any comment ?

> Source/WebKit2/Platform/efl/DispatchQueueEfl.cpp:245
> +    double timeOutSeconds = (m_timerWorkItems[0]->expirationTimeNanoSeconds() - monotonicallyIncreasingTime() * nanoSecondsPerSecond) / nanoSecondsPerSecond;

It looks "* nanoSecondsPerSecond) / nanoSecondsPerSecond;" is useless.
Comment 9 Jinwoo Song 2014-01-20 20:58:15 PST
Comment on attachment 221619 [details]
Patch

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

>> Source/WebKit2/Platform/efl/DispatchQueueEfl.cpp:245
>> +    double timeOutSeconds = (m_timerWorkItems[0]->expirationTimeNanoSeconds() - monotonicallyIncreasingTime() * nanoSecondsPerSecond) / nanoSecondsPerSecond;
> 
> It looks "* nanoSecondsPerSecond) / nanoSecondsPerSecond;" is useless.

monotonicallyIncreasingTime() is seconds but 'm_timerWorkItems[0]->expirationTimeNanoSeconds()' is nanoseconds.
Comment 10 Gyuyoung Kim 2014-01-21 00:43:32 PST
Comment on attachment 221619 [details]
Patch

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

>>> Source/WebKit2/Platform/efl/DispatchQueueEfl.cpp:245
>>> +    double timeOutSeconds = (m_timerWorkItems[0]->expirationTimeNanoSeconds() - monotonicallyIncreasingTime() * nanoSecondsPerSecond) / nanoSecondsPerSecond;
>> 
>> It looks "* nanoSecondsPerSecond) / nanoSecondsPerSecond;" is useless.
> 
> monotonicallyIncreasingTime() is seconds but 'm_timerWorkItems[0]->expirationTimeNanoSeconds()' is nanoseconds.

I'm sorry. I mistook this code. Yes, looks correct.
Comment 11 WebKit Commit Bot 2014-01-21 17:58:33 PST
Comment on attachment 221619 [details]
Patch

Clearing flags on attachment: 221619

Committed r162490: <http://trac.webkit.org/changeset/162490>
Comment 12 WebKit Commit Bot 2014-01-21 17:58:37 PST
All reviewed patches have been landed.  Closing bug.