Adding Date::ResetCache() API so that the cache values in the Date object...

Adding Date::ResetCache() API so that the cache values in the Date object
can be reset to allow DST / timezone changes to be re-cached and reflected
in the Date object.

Committed: http://code.google.com/p/v8/source/detail?r=6067

[Søren Thygesen Gjesse, 2010-12-17 10:54:55]

LGTM with comments addressed.

http://codereview.chromium.org/5978001/diff/1/include/v8.h
File include/v8.h (right):

http://codereview.chromium.org/5978001/diff/1/include/v8.h#newcode1359
include/v8.h:1359: /**
Please start the comment with something like this "Notification that the
embedder has changed the time zone or daylight savings." and the follow with "V8
keeps a cache ...".

Also note that this API should not be called more that needed as otherwise the
performance of date operation will suffer.

http://codereview.chromium.org/5978001/diff/1/include/v8.h#newcode1364
include/v8.h:1364: V8EXPORT static void ResetCache();
I think the name ResetCache here is a bit to low-level. How about
TimeZoneChangeNotification?

[marklam, 2010-12-17 11:12:29]

http://codereview.chromium.org/5978001/diff/1/include/v8.h
File include/v8.h (right):

http://codereview.chromium.org/5978001/diff/1/include/v8.h#newcode1359
include/v8.h:1359: /**
On 2010/12/17 10:54:55, Søren Gjesse wrote:
> Please start the comment with something like this "Notification that the
> embedder has changed the time zone or daylight savings." and the follow with
"V8
> keeps a cache ...".
> 
> Also note that this API should not be called more that needed as otherwise the
> performance of date operation will suffer.

Will do.  I do have one concern with not knowing what the best name for the
notification would be.

http://codereview.chromium.org/5978001/diff/1/include/v8.h#newcode1364
include/v8.h:1364: V8EXPORT static void ResetCache();
On 2010/12/17 10:54:55, Søren Gjesse wrote:
> I think the name ResetCache here is a bit to low-level. How about
> TimeZoneChangeNotification?

The implementation is really a reset of all values that is cached in the Date
object.  Would calling it TimeZoneChangeNotification be misleading?  For one,
this notification also covers changes in Daylight Savings time, local time
offset, etc.  Would all those fall under TimeZoneChange?  I'm fine with this
name if you think that the name covers all the types of changes that the Date
object needs to be notified of.

Alternatively, how about calling DateTimeChangeNotification?  That would
describe what this function is trying to do, won't it?

[marklam, 2010-12-17 11:45:57]

Søren, I've update the changes.  Please take another look to see if the comment
in v8.h is adequate.  Thanks.

[William Hesse, 2010-12-17 12:16:59]

This change will run up against the problem on Windows and Mac that the tzinfo
structure cannot be updated correctly by tzset in a sandboxed environment. 
There are many bugs on this issue, but the one with the most complete comment is
http://code.google.com/p/chromium/issues/detail?id=43293.

If this is added to the API, comments about the limitations of the call should
be included at its declaration.

[William Hesse, 2010-12-17 12:26:45]

Actually, since the problem is not really in V8, it should not be commented in
the API.  Embedding V8 in any context in which tzset and tzinfo do not work
properly will, of course, not work properly, but that should be clear to the
embedder, since they will see the same problem if they try these calls from
outside V8.

On 2010/12/17 12:16:59, William Hesse wrote:
> This change will run up against the problem on Windows and Mac that the tzinfo
> structure cannot be updated correctly by tzset in a sandboxed environment. 
> There are many bugs on this issue, but the one with the most complete comment
is
> http://code.google.com/p/chromium/issues/detail?id=43293.
> 
> If this is added to the API, comments about the limitations of the call should
> be included at its declaration.

[Søren Thygesen Gjesse, 2010-12-17 12:46:28]

LGTM

Committed: http://code.google.com/p/v8/source/detail?r=6042.

Thank you for the pathc.

[Søren Thygesen Gjesse, 2010-12-17 12:47:26]

It was actually committed in http://code.google.com/p/v8/source/detail?r=6067.

Closing issue.