[webkit-reviews] review granted: [Bug 218681] [JSC] Add TimeZone range cache over ICU TimeZone API : [Attachment 413540] Patch

[bugzilla-daemon at webkit.org]

Ross Kirsling <ross.kirsling at sony.com> has granted Yusuke Suzuki
<ysuzuki at apple.com>'s request for review:
Bug 218681: [JSC] Add TimeZone range cache over ICU TimeZone API
https://bugs.webkit.org/show_bug.cgi?id=218681

Attachment 413540: Patch

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




--- Comment #5 from Ross Kirsling <ross.kirsling at sony.com> ---
Comment on attachment 413540
  --> https://bugs.webkit.org/attachment.cgi?id=413540
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).