Bug 80514

Summary: [chromium] Times in the cc should be expressed in seconds.
Product: WebKit Reporter: vollick
Component: WebKit Misc.Assignee: vollick
Status: RESOLVED FIXED    
Severity: Normal CC: cc-bugs, jamesr, nduca, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 79537    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch none

Description vollick 2012-03-07 07:57:36 PST
Milliseconds are used a lot -- seconds should be used instead.
Comment 1 vollick 2012-03-07 10:49:01 PST
Created attachment 130647 [details]
Patch
Comment 2 James Robinson 2012-03-07 11:14:29 PST
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?
Comment 3 vollick 2012-03-07 12:06:40 PST
Created attachment 130666 [details]
Patch
Comment 4 vollick 2012-03-07 12:09:31 PST
(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.
Comment 5 vollick 2012-03-09 10:36:15 PST
Created attachment 131060 [details]
Patch
Comment 6 James Robinson 2012-03-09 12:53:06 PST
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
Comment 7 vollick 2012-03-09 12:58:10 PST
(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.
Comment 8 vollick 2012-03-15 07:50:28 PDT
(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?
Comment 9 vollick 2012-03-17 12:22:55 PDT
Created attachment 132467 [details]
Patch

Freshened patch
Comment 10 James Robinson 2012-03-19 17:39:25 PDT
Comment on attachment 132467 [details]
Patch

Still looks good.  Dunno if this will apply still, but worth a shot.
Comment 11 WebKit Review Bot 2012-03-19 19:16:05 PDT
Comment on attachment 132467 [details]
Patch

Clearing flags on attachment: 132467

Committed r111308: <http://trac.webkit.org/changeset/111308>
Comment 12 WebKit Review Bot 2012-03-19 19:16:09 PDT
All reviewed patches have been landed.  Closing bug.