[Webkit-unassigned] [Bug 74769] [chromium] Add CCTimer class for the compositor

[bugzilla-daemon at webkit.org]

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





--- Comment #7 from James Robinson <jamesr at chromium.org>  2011-12-21 15:01:02 PST ---
(From update of attachment 119963)
View in context: https://bugs.webkit.org/attachment.cgi?id=119963&action=review

> Source/WebCore/platform/graphics/chromium/cc/CCTimer.h:35
> +#include <cfloat>

what is cfloat for? does it need to be included in this header?

> Source/WebCore/platform/graphics/chromium/cc/CCTimer.h:48
> +    CCTimer(CCThread* thread, CCTimerClient* client) : m_client(client), m_thread(thread), m_task(0) { }

please expand the initializer list to one statement per line

to keep this header smaller and more readable could you move the implementations of the c'tor/d'tor into the .cpp? I don't expect this to be performance-critical code

> Source/WebCore/platform/graphics/chromium/cc/CCTimer.h:51
> +    void startOneShot(double interval);

is this seconds, milliseconds, microseconds, something else? we try to tag parameter names that represent periods of time with a suffix to indicate the unit

CCThread and CCDelayBasedTimeSource both have time values of milliseconds with a "Ms" suffix, can we stick with that?

can you document in the header what this function does if there's already a timer pending (looks like it cancels the previous task, which i think is great behavior but it should be documented)

> Source/WebCore/platform/graphics/chromium/cc/CCTimer.h:52
> +    void stop() { deschedule(); }

why are stop() and deschedule() different functions if they are just aliases of each other? Can you fold them into one function?

> Source/WebCore/platform/graphics/chromium/cc/CCTimer.h:59
> +        Task(CCTimer* timer) : CCThread::Task(0), m_timer(timer) { }

explicit on this constructor, please

can you expand the member initializations out to separate lines? we very rarely combine multiple statements to a single line in webkit

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