Bug 52848

Summary: Keep track of the latest update timestamp in the backing store
Product: WebKit Reporter: Anders Carlsson <andersca>
Component: New BugsAssignee: Anders Carlsson <andersca>
Status: RESOLVED FIXED    
Severity: Normal CC: darin
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: OS X 10.5   
Attachments:
Description Flags
Patch darin: review+

Description Anders Carlsson 2011-01-20 15:07:07 PST
Keep track of the latest update timestamp in the backing store
Comment 1 Anders Carlsson 2011-01-20 15:43:40 PST
Created attachment 79659 [details]
Patch
Comment 2 Darin Adler 2011-01-20 16:05:24 PST
Comment on attachment 79659 [details]
Patch

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

> Source/WebKit2/Shared/UpdateInfo.h:44
> +    UpdateInfo() : timestamp(0) { }

Is 0 the best thing for a not-set value? Maybe NaN or -infinity? Or maybe 0 is OK.

> Source/WebKit2/UIProcess/BackingStore.cpp:47
> +    , m_latestUpdateTimestamp(0)

Is 0 the best way to represent this? I guess a date in the distant past is OK, but for that we could also use -infinity. Or maybe 0 is OK.

> Source/WebKit2/UIProcess/BackingStore.cpp:69
> +    platformIncorporateUpdate(bitmap.get(), updateInfo);

I kinda hate using platform as a function name prefix. Especially since this is not the same function as incorporateUpdate -- it’s only a part of the work. I would prefer this have the same name the function would have if there was only one platform.
Comment 3 Anders Carlsson 2011-01-20 16:12:24 PST
(In reply to comment #2)
> (From update of attachment 79659 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=79659&action=review
> 
> > Source/WebKit2/Shared/UpdateInfo.h:44
> > +    UpdateInfo() : timestamp(0) { }
> 
> Is 0 the best thing for a not-set value? Maybe NaN or -infinity? Or maybe 0 is OK.
> 

This should always be initialized directly afterwards by the caller so it really doesn't matter.

> > Source/WebKit2/UIProcess/BackingStore.cpp:47
> > +    , m_latestUpdateTimestamp(0)
> 
> Is 0 the best way to represent this? I guess a date in the distant past is OK, but for that we could also use -infinity. Or maybe 0 is OK.

I think 0 is OK. 

> 
> > Source/WebKit2/UIProcess/BackingStore.cpp:69
> > +    platformIncorporateUpdate(bitmap.get(), updateInfo);
> 
> I kinda hate using platform as a function name prefix. Especially since this is not the same function as incorporateUpdate -- it’s only a part of the work. I would prefer this have the same name the function would have if there was only one platform.

I changed this to be just another overload of incorporateUpdate.
Comment 4 Anders Carlsson 2011-01-20 16:13:37 PST
Committed r76305: <http://trac.webkit.org/changeset/76305>