Bug 127263 – [EFL][WK2] Use nanoseconds in TimerWorkItem class consistently

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit EFL

[Jinwoo Song]

After r162276 and r162300, TimerWorkItem is getting nanoseconds as parameter.
So it would be better to use nanoseconds in TimerWorkItem for consistency.

[Jinwoo Song]

Created attachment 221614 [details]
Patch

[WebKit Commit Bot]

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.

[Gyuyoung Kim]

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.

[Jinwoo Song]

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().

[Jinwoo Song]

Created attachment 221619 [details]
Patch

[WebKit Commit Bot]

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.

[Gyuyoung Kim]

CC'ing Byungwoo

[Gyuyoung Kim]

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.

[Jinwoo Song]

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.

[Gyuyoung Kim]

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.

[WebKit Commit Bot]

Comment on attachment 221619 [details]
Patch

Clearing flags on attachment: 221619

Committed r162490: <http://trac.webkit.org/changeset/162490>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.