Bug 80514 – [chromium] Times in the cc should be expressed in seconds.

• Component: WebKit Misc.
• Status: RESOLVED FIXED

Milliseconds are used a lot -- seconds should be used instead.

Created attachment 130647 [details]
Patch

[James Robinson]

Comment on attachment 130647 [details]
Patch

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

> Source/WebCore/platform/graphics/chromium/cc/CCDelayBasedTimeSource.cpp:36
> +PassRefPtr<CCDelayBasedTimeSource> CCDelayBasedTimeSource::create(double interval, double dummy, CCThread* thread)

what's "dummy" for?

Created attachment 130666 [details]
Patch

(In reply to comment #2)
> (From update of attachment 130647 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=130647&action=review
> 
> > Source/WebCore/platform/graphics/chromium/cc/CCDelayBasedTimeSource.cpp:36
> > +PassRefPtr<CCDelayBasedTimeSource> CCDelayBasedTimeSource::create(double interval, double dummy, CCThread* thread)
> 
> what's "dummy" for?

Sorry -- I was in the process of removing those (should be gone in the latest patch). I was just breaking signatures.

Created attachment 131060 [details]
Patch

[James Robinson]

Comment on attachment 131060 [details]
Patch

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

Cool

> Source/WebCore/platform/graphics/chromium/cc/CCDelayBasedTimeSource.cpp:96
> +double CCDelayBasedTimeSource::monotonicallyIncreasingTime() const

we can probably just delete this function completely at this point

(In reply to comment #6)
> (From update of attachment 131060 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=131060&action=review
> 
> Cool
> 
> > Source/WebCore/platform/graphics/chromium/cc/CCDelayBasedTimeSource.cpp:96
> > +double CCDelayBasedTimeSource::monotonicallyIncreasingTime() const
> 
> we can probably just delete this function completely at this point

I thought the same thing. The only reason I kept it around was because it is overridden in the test code to fake different monotonic times.

(In reply to comment #7)
> (In reply to comment #6)
> > (From update of attachment 131060 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=131060&action=review
> > 
> > Cool
> > 
> > > Source/WebCore/platform/graphics/chromium/cc/CCDelayBasedTimeSource.cpp:96
> > > +double CCDelayBasedTimeSource::monotonicallyIncreasingTime() const
> > 
> > we can probably just delete this function completely at this point
> 
> I thought the same thing. The only reason I kept it around was because it is overridden in the test code to fake different monotonic times.

James, are you comfortable keeping this method around for testing purposes?

Created attachment 132467 [details]
Patch

Freshened patch

[James Robinson]

Comment on attachment 132467 [details]
Patch

Still looks good.  Dunno if this will apply still, but worth a shot.

[WebKit Review Bot]

Comment on attachment 132467 [details]
Patch

Clearing flags on attachment: 132467

Committed r111308: <http://trac.webkit.org/changeset/111308>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.