[Webkit-unassigned] [Bug 119785] Replace currentTime() with monotonicallyIncreasingTime() in WebCore

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=119785





--- Comment #15 from Arunprasad Rajkumar <arurajku at cisco.com>  2013-08-16 11:07:43 PST ---
(From update of attachment 208798)
View in context: https://bugs.webkit.org/attachment.cgi?id=208798&action=review

Hi Alexey, Corrected and uploaded a patch.

>> Source/WTF/ChangeLog:3
>> +        <https://webkit.org/b/119785> [WebCore] Replace currentTime() with monotonicallyIncreasingTime() in all possible places - (Part 1)
> 
> A note about why this change is being made would be helpful.

Eric, added a note by using hints provided by Alexey

>> Source/WebCore/ChangeLog:3
>> +        <https://webkit.org/b/119785> [WebCore] Replace currentTime() with monotonicallyIncreasingTime() in all possible places - (Part 1)
> 
> We use bracketed prefixes to denote bugs that are of limited interest, so that people who are not interested in a specific platoform could ignore these with a quick glance. Please don’t use prefixes on bugs that touch general cross-platform code.

Thanks for changing the title.(I will consider this point while raising bugs)

>> Source/WebCore/ChangeLog:7
>> +        No new tests needed, behaviour is not changed.
> 
> This is not accurate, the intention of this patch is to change behavior. There is no reasonable way to test the changes in regression tests, and this is what you should say here. Or just omit the explanation.

Removed

>> Source/WTF/wtf/CurrentTime.h:58
>> +// Monotonic time in milliseconds.
> 
> This comment is not helpful, please just remove it.

Done

>> Source/WebCore/html/HTMLMediaElement.cpp:1563
>> +    m_previousProgressTime = WTF::monotonicallyIncreasingTime();
> 
> Please remove the WTF:: prefix. We sould never use it, except perhaps in some platform code to disambiguate with names from OS headers.

Done

>> Source/WebCore/html/HTMLMediaElement.h:677
>> +    mutable double m_minimumClockTimeToCacheMediaTime; // Value is based on WTF::monotonicallyIncreasingTime.
> 
> I don’t think that these comments are helpful.

Removed

>> Source/WebCore/html/MediaController.cpp:676
>> +    double now = WTF::monotonicallyIncreasingTime();
> 
> Please remove the WTF:: prefix.

Done

>> Source/WebCore/inspector/InspectorCSSAgent.cpp:189
>> +    double matchTimeMs = WTF::monotonicallyIncreasingTimeMS() - m_currentMatchData.startTime;
> 
> Ditto. Also, please change Ms to MS while touching this code.

Done

>> Source/WebCore/platform/ClockGeneric.cpp:82
>> +    return WTF::monotonicallyIncreasingTime();
> 
> Please remove the WTF:: prefix.

Done

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.