Reland "Implement a new spec for timezone offset calculation"

This is a reland of dbdede0
after a webkit layout test (geolocation-api/timestamp.html) was
fixed by
https://chromium-review.googlesource.com/c/chromium/src/+/994343 .

Original change's description:
> 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}

Bug: v8:3547, chromium:417640, v8:5714
Change-Id: I47536c111143f75e3cfeecf5d9761c43a98a10f5
Cq-Include-Trybots: luci.v8.try:v8_linux_noi18n_rel_ng;master.tryserver.blink:linux_trusty_blink_rel
Reviewed-on: https://chromium-review.googlesource.com/995971
Commit-Queue: Jungshik Shin <jshin@chromium.org>
Reviewed-by: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#52372}

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);
 }