Bug 52848 - Keep track of the latest update timestamp in the backing store
Summary: Keep track of the latest update timestamp in the backing store
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Anders Carlsson
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-01-20 15:07 PST by Anders Carlsson
Modified: 2011-01-20 16:13 PST (History)
1 user (show)

See Also:

Attachments
Patch (4.52 KB, patch)
2011-01-20 15:43 PST, Anders Carlsson 		darin: review+
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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>