Bug 218681 – [JSC] Add TimeZone range cache over ICU TimeZone API

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

[Yusuke Suzuki]

[JSC] Add TimeZone range cache over ICU TimeZone API

[Yusuke Suzuki]

Created attachment 413519 [details]
Patch

[Yusuke Suzuki]

Created attachment 413520 [details]
Patch

[Yusuke Suzuki]

Created attachment 413538 [details]
Patch

[Yusuke Suzuki]

Created attachment 413540 [details]
Patch

[Ross Kirsling]

Comment on attachment 413540 [details]
Patch

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

r=me, looks good based on the code that existed prior to r269320.

> Source/JavaScriptCore/runtime/JSDateMath.cpp:104
> +    auto& timeZoneCache = *bitwise_cast<icu::TimeZone*>(this->timeZoneCache());

Is the `this->` necessary?

> Source/JavaScriptCore/runtime/JSDateMath.cpp:151
> +        if (millisecondsFromEpoch <= newEnd) {

nit: You could flip this to be an early out for less nesting if you wanted (ditto below).

> Source/JavaScriptCore/runtime/JSDateMath.cpp:166
> +                // the point in time where the DST offset change occurred. Updated

nit: Updated -> Update (ditto below).

[Yusuke Suzuki]

Comment on attachment 413540 [details]
Patch

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

Thanks!

>> Source/JavaScriptCore/runtime/JSDateMath.cpp:104
>> +    auto& timeZoneCache = *bitwise_cast<icu::TimeZone*>(this->timeZoneCache());
> 
> Is the `this->` necessary?

Yes, because `timeZoneCache` name is also used by the variable :)

>> Source/JavaScriptCore/runtime/JSDateMath.cpp:151
>> +        if (millisecondsFromEpoch <= newEnd) {
> 
> nit: You could flip this to be an early out for less nesting if you wanted (ditto below).

Nice. Done

>> Source/JavaScriptCore/runtime/JSDateMath.cpp:166
>> +                // the point in time where the DST offset change occurred. Updated
> 
> nit: Updated -> Update (ditto below).

Nice, fixed.

[Yusuke Suzuki]

Committed r269576: <https://trac.webkit.org/changeset/269576>

[Radar WebKit Bug Importer]

<rdar://problem/71176359>

[WebKit Commit Bot]

Re-opened since this is blocked by bug 219915

[Yusuke Suzuki]

The reverted patch is once integrated since we workaround ICU missing API issue differently.