Bug 52848 – Keep track of the latest update timestamp in the backing store

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Anders Carlsson]

Keep track of the latest update timestamp in the backing store

[Anders Carlsson]

Created attachment 79659 [details]
Patch

[Darin Adler]

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.

[Anders Carlsson]

(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.

[Anders Carlsson]

Committed r76305: <http://trac.webkit.org/changeset/76305>