Implement a new spec for timezone offset calculation

tc39/ecma262#778 was recently merged
to Ecma 262.

It changes the way to convert between "local time" and UTC in such
a way that it'd work for all timezones whether or not there has
been any change in the timezone offset of the standard time. For
instance, Europe/Moscow and some parts of US state of Indiana have
changed the standard (non-DST) timezone offset a few times. The
previous spec assumes that the the standard timezone offset is
constant, but the new spec take into account the offset change
history.

In addition, it specifies a new way to calculate the timezone
offset during a timezone transition (either in and
out of DST or timezone offset shift).

During a negative transition (e.g.  fall backward / getting
out of DST), repeated times are to be interpreted as if the
offset before the transition is in effect.

During a positive transition (e.g. spring forward / getting
into DST), skipped times are to be treated similarly. That
is, they are to be interpreted as if the offset before the
transition is in effect.

With icu-timezone-data, v8 is compliant to the new spec for the
past and the future as well as now whether or not the standard
timezone offset of a given timezone has changed over time
(e.g. Europe/Moscow, Pacific/Apia). With icu-timezone-data,
Australia/Lord_Howe (30 minute DST change) also works per spec.

Without icu-timezone-data, it works only for timezones of which
the standard timezone offset is the same as the current offset
(e.g. most North American timezones other than parts of Indiana)
and of which the DST shift is an hour. For instance, it doesn't work
for Europe/Moscow in 2010 when the standard timezone offset was
+4h because the current (2018) standard timezone offset is +3h. Neither
does it for Lord Howe in Australia with the DST shift of 0.5 hr.

This CL used to require one of the two ICU CLs below, but not
any more.

  https://chromium-review.googlesource.com/c/chromium/deps/icu/+/572652
  https://chromium-review.googlesource.com/851265  (a proposed CL to the
  upstream ICU).

Bug: v8:3547,chromium:417640,v8:5714
Cq-Include-Trybots: luci.v8.try:v8_linux_noi18n_rel_ng
Change-Id: Ib162295da5bee31b2390bd0918157014aebd3e33
Reviewed-on: https://chromium-review.googlesource.com/572148
Commit-Queue: Jungshik Shin <jshin@chromium.org>
Reviewed-by: Daniel Ehrenberg <littledan@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#52332}

src/base/platform/platform-aix.cc

@@ -39,21 +39,21 @@ namespace base {
 class AIXTimezoneCache : public PosixTimezoneCache {
   const char* LocalTimezone(double time) override;
 
-  double LocalTimeOffset() override;
+  double LocalTimeOffset(double time_ms, bool is_utc) override;
 
   ~AIXTimezoneCache() override {}
 };
 
-const char* AIXTimezoneCache::LocalTimezone(double time) {
-  if (std::isnan(time)) return "";
-  time_t tv = static_cast<time_t>(floor(time / msPerSecond));
+const char* AIXTimezoneCache::LocalTimezone(double time_ms) {
+  if (std::isnan(time_ms)) return "";
+  time_t tv = static_cast<time_t>(floor(time_ms / msPerSecond));
   struct tm tm;
   struct tm* t = localtime_r(&tv, &tm);
   if (nullptr == t) return "";
   return tzname[0];  // The location of the timezone string on AIX.
 }
 
-double AIXTimezoneCache::LocalTimeOffset() {
+double AIXTimezoneCache::LocalTimeOffset(double time_ms, bool is_utc) {
   // On AIX, struct tm does not contain a tm_gmtoff field.
   time_t utc = time(nullptr);
   DCHECK_NE(utc, -1);

src/base/platform/platform-cygwin.cc

@@ -66,7 +66,7 @@ uint8_t* RandomizedVirtualAlloc(size_t size, DWORD flags, DWORD protect,
 class CygwinTimezoneCache : public PosixTimezoneCache {
   const char* LocalTimezone(double time) override;
 
-  double LocalTimeOffset() override;
+  double LocalTimeOffset(double time_ms, bool is_utc) override;
 
   ~CygwinTimezoneCache() override {}
 };
@@ -80,7 +80,7 @@ const char* CygwinTimezoneCache::LocalTimezone(double time) {
   return tzname[0];  // The location of the timezone string on Cygwin.
 }
 
-double CygwinTimezoneCache::LocalTimeOffset() {
+double LocalTimeOffset(double time_ms, bool is_utc) {
   // On Cygwin, struct tm does not contain a tm_gmtoff field.
   time_t utc = time(nullptr);
   DCHECK_NE(utc, -1);

src/base/platform/platform-posix-time.cc

@@ -18,7 +18,9 @@ const char* PosixDefaultTimezoneCache::LocalTimezone(double time) {
   return t->tm_zone;
 }
 
-double PosixDefaultTimezoneCache::LocalTimeOffset() {
+double PosixDefaultTimezoneCache::LocalTimeOffset(double time_ms, bool is_utc) {
+  // Preserve the old behavior for non-ICU implementation by ignoring both
+  // time_ms and is_utc.
   time_t tv = time(nullptr);
   struct tm tm;
   struct tm* t = localtime_r(&tv, &tm);

src/base/platform/platform-posix-time.h

@@ -13,7 +13,7 @@ namespace base {
 class PosixDefaultTimezoneCache : public PosixTimezoneCache {
  public:
   const char* LocalTimezone(double time_ms) override;
-  double LocalTimeOffset() override;
+  double LocalTimeOffset(double time_ms, bool is_utc) override;
 
   ~PosixDefaultTimezoneCache() override {}
 };

src/base/platform/platform-solaris.cc

@@ -37,8 +37,7 @@ namespace base {
 class SolarisTimezoneCache : public PosixTimezoneCache {
   const char* LocalTimezone(double time) override;
 
-  double LocalTimeOffset() override;
-
+  double LocalTimeOffset(double time, bool is_utc) override;
   ~SolarisTimezoneCache() override {}
 };
 
@@ -51,7 +50,7 @@ const char* SolarisTimezoneCache::LocalTimezone(double time) {
   return tzname[0];  // The location of the timezone string on Solaris.
 }
 
-double SolarisTimezoneCache::LocalTimeOffset() {
+double SolarisTimezoneCache::LocalTimeOffset(double time, bool is_utc) {
   tzset();
   return -static_cast<double>(timezone * msPerSecond);
 }

src/base/platform/platform-win32.cc

@@ -115,7 +115,7 @@ class WindowsTimezoneCache : public TimezoneCache {
 
   const char* LocalTimezone(double time) override;
 
-  double LocalTimeOffset() override;
+  double LocalTimeOffset(double time, bool is_utc) override;
 
   double DaylightSavingsOffset(double time) override;
 
@@ -466,7 +466,9 @@ const char* WindowsTimezoneCache::LocalTimezone(double time) {
 
 // Returns the local time offset in milliseconds east of UTC without
 // taking daylight savings time into account.
-double WindowsTimezoneCache::LocalTimeOffset() {
+double WindowsTimezoneCache::LocalTimeOffset(double time_ms, bool is_utc) {
+  // Ignore is_utc and time_ms for now. That way, the behavior wouldn't
+  // change with icu_timezone_data disabled.
   // Use current time, rounded to the millisecond.
   Win32Time t(OS::TimeCurrentMillis());
   // Time::LocalOffset inlcudes any daylight savings offset, so subtract it.

src/base/timezone-cache.h

@@ -20,10 +20,8 @@ class TimezoneCache {
   // ES #sec-local-time-zone-adjustment
   // Local Time Zone Adjustment
   //
-  // TODO(littledan): Make more accurate with another parameter along the
-  // lines of this spec change:
   // https://github.com/tc39/ecma262/pull/778
-  virtual double LocalTimeOffset() = 0;
+  virtual double LocalTimeOffset(double time_ms, bool is_utc) = 0;
 
   // Called when the local timezone changes
   virtual void Clear() = 0;

src/date.cc

@@ -52,8 +52,14 @@ void DateCache::ResetDateCache() {
   dst_usage_counter_ = 0;
   before_ = &dst_[0];
   after_ = &dst_[1];
-  local_offset_ms_ = kInvalidLocalOffsetInMs;
   ymd_valid_ = false;
+#ifdef V8_INTL_SUPPORT
+  if (!FLAG_icu_timezone_data) {
+#endif
+    local_offset_ms_ = kInvalidLocalOffsetInMs;
+#ifdef V8_INTL_SUPPORT
+  }
+#endif
   tz_cache_->Clear();
   tz_name_ = nullptr;
   dst_tz_name_ = nullptr;
@@ -206,6 +212,70 @@ void DateCache::BreakDownTime(int64_t time_ms, int* year, int* month, int* day,
   *ms = time_in_day_ms % 1000;
 }
 
+// Implements LocalTimeZonedjustment(t, isUTC)
+// ECMA 262 - ES#sec-local-time-zone-adjustment
+int DateCache::GetLocalOffsetFromOS(int64_t time_ms, bool is_utc) {
+  double offset;
+#ifdef V8_INTL_SUPPORT
+  if (FLAG_icu_timezone_data) {
+    offset = tz_cache_->LocalTimeOffset(static_cast<double>(time_ms), is_utc);
+  } else {
+#endif
+    // When ICU timezone data is not used, we need to compute the timezone
+    // offset for a given local time.
+    //
+    // The following shows that using DST for (t - LocalTZA - hour) produces
+    // correct conversion where LocalTZA is the timezone offset in winter (no
+    // DST) and the timezone offset is assumed to have no historical change.
+    // Note that it does not work for the past and the future if LocalTZA (no
+    // DST) is different from the current LocalTZA (no DST). For instance,
+    // this will break for Europe/Moscow in 2012 ~ 2013 because LocalTZA was
+    // 4h instead of the current 3h (as of 2018).
+    //
+    // Consider transition to DST at local time L1.
+    // Let L0 = L1 - hour, L2 = L1 + hour,
+    //     U1 = UTC time that corresponds to L1,
+    //     U0 = U1 - hour.
+    // Transitioning to DST moves local clock one hour forward L1 => L2, so
+    // U0 = UTC time that corresponds to L0 = L0 - LocalTZA,
+    // U1 = UTC time that corresponds to L1 = L1 - LocalTZA,
+    // U1 = UTC time that corresponds to L2 = L2 - LocalTZA - hour.
+    // Note that DST(U0 - hour) = 0, DST(U0) = 0, DST(U1) = 1.
+    // U0 = L0 - LocalTZA - DST(L0 - LocalTZA - hour),
+    // U1 = L1 - LocalTZA - DST(L1 - LocalTZA - hour),
+    // U1 = L2 - LocalTZA - DST(L2 - LocalTZA - hour).
+    //
+    // Consider transition from DST at local time L1.
+    // Let L0 = L1 - hour,
+    //     U1 = UTC time that corresponds to L1,
+    //     U0 = U1 - hour, U2 = U1 + hour.
+    // Transitioning from DST moves local clock one hour back L1 => L0, so
+    // U0 = UTC time that corresponds to L0 (before transition)
+    //    = L0 - LocalTZA - hour.
+    // U1 = UTC time that corresponds to L0 (after transition)
+    //    = L0 - LocalTZA = L1 - LocalTZA - hour
+    // U2 = UTC time that corresponds to L1 = L1 - LocalTZA.
+    // Note that DST(U0) = 1, DST(U1) = 0, DST(U2) = 0.
+    // U0 = L0 - LocalTZA - DST(L0 - LocalTZA - hour) = L0 - LocalTZA - DST(U0).
+    // U2 = L1 - LocalTZA - DST(L1 - LocalTZA - hour) = L1 - LocalTZA - DST(U1).
+    // It is impossible to get U1 from local time.
+    if (local_offset_ms_ == kInvalidLocalOffsetInMs) {
+      // This gets the constant LocalTZA (arguments are ignored).
+      local_offset_ms_ =
+          tz_cache_->LocalTimeOffset(static_cast<double>(time_ms), is_utc);
+    }
+    offset = local_offset_ms_;
+    if (!is_utc) {
+      const int kMsPerHour = 3600 * 1000;
+      time_ms -= (offset + kMsPerHour);
+    }
+    offset += DaylightSavingsOffsetInMs(time_ms);
+#ifdef V8_INTL_SUPPORT
+  }
+#endif
+  DCHECK_LT(offset, kInvalidLocalOffsetInMs);
+  return static_cast<int>(offset);
+}
 
 void DateCache::ExtendTheAfterSegment(int time_sec, int offset_ms) {
   if (after_->offset_ms == offset_ms &&

src/date.h

@@ -75,13 +75,9 @@ class DateCache {
     return year % 4 == 0 && (year % 100 != 0 || year % 400 == 0);
   }
 
-
-  // ECMA 262 - 15.9.1.7.
-  int LocalOffsetInMs() {
-    if (local_offset_ms_ == kInvalidLocalOffsetInMs)  {
-      local_offset_ms_ = GetLocalOffsetFromOS();
-    }
-    return local_offset_ms_;
+  // ECMA 262 - ES#sec-local-time-zone-adjustment
+  int LocalOffsetInMs(int64_t time, bool is_utc) {
+    return GetLocalOffsetFromOS(time, is_utc);
   }
 
 
@@ -103,53 +99,16 @@ class DateCache {
     return static_cast<int>((time_ms - local_ms) / kMsPerMin);
   }
 
-  // ECMA 262 - 15.9.1.9
-  // LocalTime(t) = t + LocalTZA + DaylightSavingTA(t)
+  // ECMA 262 - ES#sec-localtime-t
+  // LocalTime(t) = t + LocalTZA(t, true)
   int64_t ToLocal(int64_t time_ms) {
-    return time_ms + LocalOffsetInMs() + DaylightSavingsOffsetInMs(time_ms);
+    return time_ms + LocalOffsetInMs(time_ms, true);
   }
 
-  // ECMA 262 - 15.9.1.9
-  // UTC(t) = t - LocalTZA - DaylightSavingTA(t - LocalTZA)
+  // ECMA 262 - ES#sec-utc-t
+  // UTC(t) = t - LocalTZA(t, false)
   int64_t ToUTC(int64_t time_ms) {
-    // We need to compute UTC time that corresponds to the given local time.
-    // Literally following spec here leads to incorrect time computation at
-    // the points were we transition to and from DST.
-    //
-    // The following shows that using DST for (t - LocalTZA - hour) produces
-    // correct conversion.
-    //
-    // Consider transition to DST at local time L1.
-    // Let L0 = L1 - hour, L2 = L1 + hour,
-    //     U1 = UTC time that corresponds to L1,
-    //     U0 = U1 - hour.
-    // Transitioning to DST moves local clock one hour forward L1 => L2, so
-    // U0 = UTC time that corresponds to L0 = L0 - LocalTZA,
-    // U1 = UTC time that corresponds to L1 = L1 - LocalTZA,
-    // U1 = UTC time that corresponds to L2 = L2 - LocalTZA - hour.
-    // Note that DST(U0 - hour) = 0, DST(U0) = 0, DST(U1) = 1.
-    // U0 = L0 - LocalTZA - DST(L0 - LocalTZA - hour),
-    // U1 = L1 - LocalTZA - DST(L1 - LocalTZA - hour),
-    // U1 = L2 - LocalTZA - DST(L2 - LocalTZA - hour).
-    //
-    // Consider transition from DST at local time L1.
-    // Let L0 = L1 - hour,
-    //     U1 = UTC time that corresponds to L1,
-    //     U0 = U1 - hour, U2 = U1 + hour.
-    // Transitioning from DST moves local clock one hour back L1 => L0, so
-    // U0 = UTC time that corresponds to L0 (before transition)
-    //    = L0 - LocalTZA - hour.
-    // U1 = UTC time that corresponds to L0 (after transition)
-    //    = L0 - LocalTZA = L1 - LocalTZA - hour
-    // U2 = UTC time that corresponds to L1 = L1 - LocalTZA.
-    // Note that DST(U0) = 1, DST(U1) = 0, DST(U2) = 0.
-    // U0 = L0 - LocalTZA - DST(L0 - LocalTZA - hour) = L0 - LocalTZA - DST(U0).
-    // U2 = L1 - LocalTZA - DST(L1 - LocalTZA - hour) = L1 - LocalTZA - DST(U1).
-    // It is impossible to get U1 from local time.
-
-    const int kMsPerHour = 3600 * 1000;
-    time_ms -= LocalOffsetInMs();
-    return time_ms - DaylightSavingsOffsetInMs(time_ms - kMsPerHour);
+    return time_ms - LocalOffsetInMs(time_ms, false);
   }
 
 
@@ -208,11 +167,7 @@ class DateCache {
     return static_cast<int>(tz_cache_->DaylightSavingsOffset(time_ms));
   }
 
-  virtual int GetLocalOffsetFromOS() {
-    double offset = tz_cache_->LocalTimeOffset();
-    DCHECK_LT(offset, kInvalidLocalOffsetInMs);
-    return static_cast<int>(offset);
-  }
+  virtual int GetLocalOffsetFromOS(int64_t time_ms, bool is_utc);
 
  private:
   // The implementation relies on the fact that no time zones have

src/intl.cc

@@ -14,6 +14,7 @@
 #include "src/isolate.h"
 #include "src/objects-inl.h"
 #include "src/string-case.h"
+#include "unicode/basictz.h"
 #include "unicode/calendar.h"
 #include "unicode/gregocal.h"
 #include "unicode/timezone.h"
@@ -373,23 +374,41 @@ icu::TimeZone* ICUTimezoneCache::GetTimeZone() {
   return timezone_;
 }
 
-bool ICUTimezoneCache::GetOffsets(double time_ms, int32_t* raw_offset,
-                                  int32_t* dst_offset) {
+bool ICUTimezoneCache::GetOffsets(double time_ms, bool is_utc,
+                                  int32_t* raw_offset, int32_t* dst_offset) {
   UErrorCode status = U_ZERO_ERROR;
-  GetTimeZone()->getOffset(time_ms, false, *raw_offset, *dst_offset, status);
+  // TODO(jshin): ICU TimeZone class handles skipped time differently from
+  // Ecma 262 (https://github.com/tc39/ecma262/pull/778) and icu::TimeZone
+  // class does not expose the necessary API. Fixing
+  // http://bugs.icu-project.org/trac/ticket/13268 would make it easy to
+  // implement the proposed spec change. A proposed fix for ICU is
+  //    https://chromium-review.googlesource.com/851265 .
+  // In the meantime, use an internal (still public) API of icu::BasicTimeZone.
+  // Once it's accepted by the upstream, get rid of cast. Note that casting
+  // TimeZone to BasicTimeZone is safe because we know that icu::TimeZone used
+  // here is a BasicTimeZone.
+  if (is_utc) {
+    GetTimeZone()->getOffset(time_ms, false, *raw_offset, *dst_offset, status);
+  } else {
+    static_cast<const icu::BasicTimeZone*>(GetTimeZone())
+        ->getOffsetFromLocal(time_ms, icu::BasicTimeZone::kFormer,
+                             icu::BasicTimeZone::kFormer, *raw_offset,
+                             *dst_offset, status);
+  }
+
   return U_SUCCESS(status);
 }
 
 double ICUTimezoneCache::DaylightSavingsOffset(double time_ms) {
   int32_t raw_offset, dst_offset;
-  if (!GetOffsets(time_ms, &raw_offset, &dst_offset)) return 0;
+  if (!GetOffsets(time_ms, true, &raw_offset, &dst_offset)) return 0;
   return dst_offset;
 }
 
-double ICUTimezoneCache::LocalTimeOffset() {
+double ICUTimezoneCache::LocalTimeOffset(double time_ms, bool is_utc) {
   int32_t raw_offset, dst_offset;
-  if (!GetOffsets(icu::Calendar::getNow(), &raw_offset, &dst_offset)) return 0;
-  return raw_offset;
+  if (!GetOffsets(time_ms, is_utc, &raw_offset, &dst_offset)) return 0;
+  return raw_offset + dst_offset;
 }
 
 void ICUTimezoneCache::Clear() {

src/intl.h

@@ -48,14 +48,15 @@ class ICUTimezoneCache : public base::TimezoneCache {
 
   double DaylightSavingsOffset(double time_ms) override;
 
-  double LocalTimeOffset() override;
+  double LocalTimeOffset(double time_ms, bool is_utc) override;
 
   void Clear() override;
 
  private:
   icu::TimeZone* GetTimeZone();
 
-  bool GetOffsets(double time_ms, int32_t* raw_offset, int32_t* dst_offset);
+  bool GetOffsets(double time_ms, bool is_utc, int32_t* raw_offset,
+                  int32_t* dst_offset);
 
   icu::TimeZone* timezone_;
 

test/cctest/test-date.cc

@@ -53,9 +53,8 @@ class DateCacheMock: public DateCache {
     return rule == nullptr ? 0 : rule->offset_sec * 1000;
   }
 
-
-  virtual int GetLocalOffsetFromOS() {
-    return local_offset_;
+  virtual int GetLocalOffsetFromOS(int64_t time_sec, bool is_utc) {
+    return local_offset_ + GetDaylightSavingsOffsetFromOS(time_sec);
   }
 
  private:
@@ -113,8 +112,7 @@ static void CheckDST(int64_t time) {
   Isolate* isolate = CcTest::i_isolate();
   DateCache* date_cache = isolate->date_cache();
   int64_t actual = date_cache->ToLocal(time);
-  int64_t expected = time + date_cache->GetLocalOffsetFromOS() +
-                     date_cache->GetDaylightSavingsOffsetFromOS(time / 1000);
+  int64_t expected = time + date_cache->GetLocalOffsetFromOS(time, true);
   CHECK_EQ(actual, expected);
 }