[date] Add ICU backend for timezone info behind a flag

src/i18n.cc

Index: src/i18n.cc
diff --git a/src/i18n.cc b/src/i18n.cc
index d2245ef34a9a319a53b4cf4b4ea05ec095fef2d5..213c7feb7e4af74b38ed378289d4c39cccb6ad94 100644
--- a/src/i18n.cc
+++ b/src/i18n.cc
@@ -899,5 +899,86 @@ void V8BreakIterator::DeleteBreakIterator(
   GlobalHandles::Destroy(reinterpret_cast<Object**>(data.GetParameter()));
 }
 
+ICUTimezoneCache::ICUTimezoneCache() : timezone_(nullptr) { Clear(); }
+
+ICUTimezoneCache::~ICUTimezoneCache() { Clear(); }
+
+const char* ICUTimezoneCache::LocalTimezone(double time_ms) {
+  bool is_dst = DaylightSavingsOffset(time_ms) != 0;
+  char* name = is_dst ? dst_timezone_name_ : timezone_name_;
+  if (name[0] == '\0') {
+    icu::UnicodeString result;
+    GetTimeZone()->getDisplayName(is_dst, icu::TimeZone::SHORT, result);

[jungshik at Google, 2017/03/08 07:15:20]

By using SHORT, you're matching v8 on Linux, but v8 on Windows would be better
matched by 'LONG'. 

Given this platform disparity, I'm really tempted to use ISO 8601 for
Date.toString(). 

[jungshik at Google, 2017/03/08 07:27:43]

Dan, you talked about root locale in your comment, but here you're not using
ROOT locale perhaps because SHORT + ROOT locale does not give you what you want.


UnicodeString & 	getDisplayName (UBool daylight, EDisplayType style, const
Locale &locale, UnicodeString &result) const. 

Again, I think "LONG" is better (matching v8's behavior on Windows) or just
using ISO 8601 (breaking from the current behavior) can be considered. 

Another thing about v8 on Windows. Currently, localized timezone name is used
inside parens for Date.prototype.toString(). We don't have to preserve this, do
we? 

[Dan Ehrenberg, 2017/03/11 09:10:50]

OK, I switched to LONG, but I am not sure whether this is web-compatible.

+
+    // With the SHORT format and the default locale, the result should be of
+    // the form PST or CET, short and within ASCII range.
+    int32_t length = result.length();
+    CHECK(length < kMaxTimezoneChars);
+    for (int32_t i = 0; i < length; i++) {
+      UChar ch = result[i];
+      CHECK(ch == static_cast<UChar>(static_cast<char>(ch)));
+      name[i] = static_cast<char>(ch);
+    }
+    name[length] = '\0';
+  }
+  return const_cast<const char*>(name);
+}
+
+icu::TimeZone* ICUTimezoneCache::GetTimeZone() {
+  if (timezone_ == nullptr) {
+    timezone_ = icu::TimeZone::createDefault();
+  }
+  return timezone_;
+}
+
+bool ICUTimezoneCache::GetOffsets(double time_ms, int32_t* raw_offset,
+                                  int32_t* dst_offset) {
+  UErrorCode status = U_ZERO_ERROR;
+  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, &raw_offset, &dst_offset)) return 0;
+  return dst_offset;
+}
+
+double ICUTimezoneCache::LocalTimeOffset() {
+  int32_t raw_offset, dst_offset;
+  if (!GetOffsets(icu::Calendar::getNow(), &raw_offset, &dst_offset)) return 0;
+  return raw_offset;
+}
+
+void ICUTimezoneCache::Clear() {
+  delete timezone_;
+  timezone_ = nullptr;
+  timezone_name_[0] = '\0';
+  dst_timezone_name_[0] = '\0';
+}
+
+ICUOSTimezoneCache::ICUOSTimezoneCache() : os_tz_cache_(nullptr) {}
+
+ICUOSTimezoneCache::~ICUOSTimezoneCache() { delete os_tz_cache_; }
+
+const char* ICUOSTimezoneCache::LocalTimezone(double time_ms) {
+  const char* result = ICUTimezoneCache::LocalTimezone(time_ms);
+  // ICU often returns timezones which are not as descriptive as they
+  // could be, e.g., GMT+1 for Europe/Madrid when in en_US. When a timezone
+  // starts with GMT, call out to the OS to get the 'real' name.

[jungshik at Google, 2017/03/08 06:44:41]

IMHO,  if-clause had better be dropped for the reason I gave in the previous
comment. 

Also, note that checking for GMT does not work in all locales. For instance, fr
uses UTC instead of GMT. 

[jungshik at Google, 2017/03/08 07:15:20]

"GMT+1030 (LHST)" :   Who'd know what LHST stands for (among en-US locale
users)?  

On Windows, v8 has a more descriptive long name inside parens while on Linux, it
has abbreviations 'cryptic' to most users. 

Because the current bahavior does not have a platform parity anyway, I was
planning to use a long descriptive name obtained from ICU everywhere inside
parens.  

That is, instead of falling back to the OS names, you can get a descriptive
timezone name from ICU. 
See
http://icu-project.org/apiref/icu4c/classicu_1_1TimeZone.html#a7330eac5e0d8ab...
 . 

EDisplayType can be set to "LONG" to get 'Eastern Daylight Saving time' (or
depending on locale, 'North American Eastern Daylight Saving Time' in that
language).  In most locales and frequently used timezones, this should not
fallback to UTC offset formats. 

Wait..   for Date.toString(), do we want to show a localized format? 

If it's ok to break from the current behavior (which varies a lot from platform
to platform and from implementation to implementation), I'm tempted to use ISO
8601 for Date.toString()

+  if (strncmp(result, "GMT", 3) == 0) {
+    if (os_tz_cache_ == nullptr) {
+      os_tz_cache_ = base::OS::CreateTimezoneCache();
+    }
+    return os_tz_cache_->LocalTimezone(time_ms);
+  }
+  return result;
+}
+
+void ICUOSTimezoneCache::Clear() {
+  ICUTimezoneCache::Clear();
+  if (os_tz_cache_) os_tz_cache_->Clear();
+}
+
 }  // namespace internal
 }  // namespace v8