Revert "Implement a new spec for timezone offset calculation"

This reverts commit dbdede0.

Reason for revert: Fails webkit_tests, blocks roll: https://build.chromium.org/p/client.v8.fyi/builders/V8-Blink%20Linux%2064

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}

TBR=adamk@chromium.org,littledan@chromium.org,mlippautz@chromium.org,jshin@chromium.org

Change-Id: I6b3bf4427c761b106280d565a3912cd8e25cf87e
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: v8:3547, chromium:417640, v8:5714
Cq-Include-Trybots: luci.v8.try:v8_linux_noi18n_rel_ng
Reviewed-on: https://chromium-review.googlesource.com/994192
Reviewed-by: Clemens Hammacher <clemensh@chromium.org>
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#52338}

src/base/platform/platform-aix.cc

@@ -39,21 +39,21 @@ namespace base {
 class AIXTimezoneCache : public PosixTimezoneCache {
   const char* LocalTimezone(double time) override;
 
-  double LocalTimeOffset(double time_ms, bool is_utc) override;
+  double LocalTimeOffset() override;
 
   ~AIXTimezoneCache() override {}
 };
 
-const char* AIXTimezoneCache::LocalTimezone(double time_ms) {
-  if (std::isnan(time_ms)) return "";
-  time_t tv = static_cast<time_t>(floor(time_ms / msPerSecond));
+const char* AIXTimezoneCache::LocalTimezone(double time) {
+  if (std::isnan(time)) return "";
+  time_t tv = static_cast<time_t>(floor(time / 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 time_ms, bool is_utc) {
+double AIXTimezoneCache::LocalTimeOffset() {
   // 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(double time_ms, bool is_utc) override;
+  double LocalTimeOffset() 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 LocalTimeOffset(double time_ms, bool is_utc) {
+double CygwinTimezoneCache::LocalTimeOffset() {
   // 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,9 +18,7 @@ const char* PosixDefaultTimezoneCache::LocalTimezone(double time) {
   return t->tm_zone;
 }
 
-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.
+double PosixDefaultTimezoneCache::LocalTimeOffset() {
   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(double time_ms, bool is_utc) override;
+  double LocalTimeOffset() override;
 
   ~PosixDefaultTimezoneCache() override {}
 };

src/base/platform/platform-solaris.cc

@@ -37,7 +37,8 @@ namespace base {
 class SolarisTimezoneCache : public PosixTimezoneCache {
   const char* LocalTimezone(double time) override;
 
-  double LocalTimeOffset(double time, bool is_utc) override;
+  double LocalTimeOffset() override;
+
   ~SolarisTimezoneCache() override {}
 };
 
@@ -50,7 +51,7 @@ const char* SolarisTimezoneCache::LocalTimezone(double time) {
   return tzname[0];  // The location of the timezone string on Solaris.
 }
 
-double SolarisTimezoneCache::LocalTimeOffset(double time, bool is_utc) {
+double SolarisTimezoneCache::LocalTimeOffset() {
   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(double time, bool is_utc) override;
+  double LocalTimeOffset() override;
 
   double DaylightSavingsOffset(double time) override;
 
@@ -466,9 +466,7 @@ 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 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.
+double WindowsTimezoneCache::LocalTimeOffset() {
   // 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,8 +20,10 @@ 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(double time_ms, bool is_utc) = 0;
+  virtual double LocalTimeOffset() = 0;
 
   // Called when the local timezone changes
   virtual void Clear() = 0;

src/date.cc

@@ -52,14 +52,8 @@ 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;
@@ -212,70 +206,6 @@ 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,9 +75,13 @@ class DateCache {
     return year % 4 == 0 && (year % 100 != 0 || year % 400 == 0);
   }
 
-  // ECMA 262 - ES#sec-local-time-zone-adjustment
-  int LocalOffsetInMs(int64_t time, bool is_utc) {
-    return GetLocalOffsetFromOS(time, is_utc);
+
+  // ECMA 262 - 15.9.1.7.
+  int LocalOffsetInMs() {
+    if (local_offset_ms_ == kInvalidLocalOffsetInMs)  {
+      local_offset_ms_ = GetLocalOffsetFromOS();
+    }
+    return local_offset_ms_;
   }
 
 
@@ -99,16 +103,53 @@ class DateCache {
     return static_cast<int>((time_ms - local_ms) / kMsPerMin);
   }
 
-  // ECMA 262 - ES#sec-localtime-t
-  // LocalTime(t) = t + LocalTZA(t, true)
+  // ECMA 262 - 15.9.1.9
+  // LocalTime(t) = t + LocalTZA + DaylightSavingTA(t)
   int64_t ToLocal(int64_t time_ms) {
-    return time_ms + LocalOffsetInMs(time_ms, true);
+    return time_ms + LocalOffsetInMs() + DaylightSavingsOffsetInMs(time_ms);
   }
 
-  // ECMA 262 - ES#sec-utc-t
-  // UTC(t) = t - LocalTZA(t, false)
+  // ECMA 262 - 15.9.1.9
+  // UTC(t) = t - LocalTZA - DaylightSavingTA(t - LocalTZA)
   int64_t ToUTC(int64_t time_ms) {
-    return time_ms - LocalOffsetInMs(time_ms, false);
+    // 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);
   }
 
 
@@ -167,7 +208,11 @@ class DateCache {
     return static_cast<int>(tz_cache_->DaylightSavingsOffset(time_ms));
   }
 
-  virtual int GetLocalOffsetFromOS(int64_t time_ms, bool is_utc);
+  virtual int GetLocalOffsetFromOS() {
+    double offset = tz_cache_->LocalTimeOffset();
+    DCHECK_LT(offset, kInvalidLocalOffsetInMs);
+    return static_cast<int>(offset);
+  }
 
  private:
   // The implementation relies on the fact that no time zones have

src/intl.cc

@@ -14,7 +14,6 @@
 #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"
@@ -374,41 +373,23 @@ icu::TimeZone* ICUTimezoneCache::GetTimeZone() {
   return timezone_;
 }
 
-bool ICUTimezoneCache::GetOffsets(double time_ms, bool is_utc,
-                                  int32_t* raw_offset, int32_t* dst_offset) {
+bool ICUTimezoneCache::GetOffsets(double time_ms, int32_t* raw_offset,
+                                  int32_t* dst_offset) {
   UErrorCode status = U_ZERO_ERROR;
-  // 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);
-  }
-
+  GetTimeZone()->getOffset(time_ms, false, *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, true, &raw_offset, &dst_offset)) return 0;
+  if (!GetOffsets(time_ms, &raw_offset, &dst_offset)) return 0;
   return dst_offset;
 }
 
-double ICUTimezoneCache::LocalTimeOffset(double time_ms, bool is_utc) {
+double ICUTimezoneCache::LocalTimeOffset() {
   int32_t raw_offset, dst_offset;
-  if (!GetOffsets(time_ms, is_utc, &raw_offset, &dst_offset)) return 0;
-  return raw_offset + dst_offset;
+  if (!GetOffsets(icu::Calendar::getNow(), &raw_offset, &dst_offset)) return 0;
+  return raw_offset;
 }
 
 void ICUTimezoneCache::Clear() {

src/intl.h

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

test/cctest/test-date.cc

@@ -53,8 +53,9 @@ class DateCacheMock: public DateCache {
     return rule == nullptr ? 0 : rule->offset_sec * 1000;
   }
 
-  virtual int GetLocalOffsetFromOS(int64_t time_sec, bool is_utc) {
-    return local_offset_ + GetDaylightSavingsOffsetFromOS(time_sec);
+
+  virtual int GetLocalOffsetFromOS() {
+    return local_offset_;
   }
 
  private:
@@ -112,7 +113,8 @@ 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(time, true);
+  int64_t expected = time + date_cache->GetLocalOffsetFromOS() +
+                     date_cache->GetDaylightSavingsOffsetFromOS(time / 1000);
   CHECK_EQ(actual, expected);
 }