New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 774376 link

Starred by 1 user

Issue metadata

Status: Duplicate
Merged: issue 766916
Owner:
Closed: Oct 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 3
Type: Bug

Blocked on:
issue 773532

Blocking:
issue v8:6076
issue v8:2137
issue v8:6031



Sign in to add a comment

Intl.DateTimeFormat / Date.prototype.toLocaleString() : timeZone is not initialized to the OS timezone

Project Member Reported by js...@chromium.org, Oct 13 2017

Issue description

Spun off from  bug 771868  . Related to but different from: 612010 

*** How to reproduce 
A. 
1. Start  Chrome 
2. Change the OS timezone (to Australia/Sydney)
3. Check the timezone and the output of Date.prototype.{toString, toLocaleString} in an existing tab and a new tab. 
    http://i18nl10n.com/chrome/tzchange.html 


B. 
1. Quit Chrome
2. Start Chrome
3. Check at http://i18nl10n.com/chrome/tzchange.html

***  Actual: 

In A3, the existing tab works fine, but new tabs do not. timeZone is undefined in Intl.DateTimeFormat.resolvedOptions(). 

In B3,  timeZone is still undefined. 

And if OS timezone is Australia/Sydney (currently DST is in effect and UTC+11), the time zone offset is 1 hour off (UTC+10) when timeZone is undefined. 

*** Expected

In A3, new tabs have (new Intl.DateTimeFormta()).resolvedOptions() have timeZone field defined to a value corresponding to the OS tz (e.g. Australia/Sydney). And tzoffset is correct (UTC+11 for Sydney). 

The same should be the case of B3. 

*** The cause

When a render process starts *before* the sandbox kicks in, InitializeICU does NOT initialize the ICU timezone (it's initialized only on Linux [1] ).  Later when Intl.DateTimeFormat() is used, it's too late to read the OS timezone (because of the sandbox). 

When Date.prototype.toString() and other methods in Date.prototype.* (non-Intl) were switched to use ICU instead of the OS API [2], they're also broken for the above reason ( bug 771868  ).


I'll make a CL (1 liner). And, perhaps, I can make a (browser or blink) test to check if resolvedOptions.timeZone is defined. That test should work independent of the actual OS timezone. 


[1] Chrome OS is fine (ICU timezone is sync'd with the OS tz elsewhere). Windows is likely to be affected as well. Android should be fine, too because it does not rely on ICU for the OS timezone detection (it uses a JNI call). 

[2] The OS API works inside the sandbox because there's a sandbox hole for localtime*().
 

Comment 1 by js...@chromium.org, Oct 13 2017

Windows should be fine because icu::TimeZone::createDefault() is called at the beginning of a renderer process before the sandbox kicks in. (I'll test it on Windows). 

https://cs.chromium.org/chromium/src/content/renderer/renderer_main_platform_delegate_win.cc?rcl=a917c6ffa9a74d05b904d272f4c5cf4f39dfb30c&l=54

Linux (other than CrOS) has this:

https://cs.chromium.org/chromium/src/base/i18n/icu_util.cc?rcl=a917c6ffa9a74d05b904d272f4c5cf4f39dfb30c&l=317

Hmm... maybe, we'd better move this to renderer_main_delegate_<platform> for Linux and add that to the corresponding place in macOS as is done for Windows. 
( I remember thinking about that in the past, but don't remember why I didn't do that way). 

An alternative is to call icu::TimeZone::createDefault() on all platforms except for CrOS and Android in InitializeICU (icu_util) when sandbox is used. 

Comment 2 by js...@chromium.org, Oct 13 2017

Non-ICU way also has an issue on macOS (kinda the opposite issue). 
See  bug 773532  . 

Comment 3 by js...@chromium.org, Oct 15 2017

hmmm.  With  64.0.3240.0 (Official Build) canary (64-bit) that should include the revert of  enabling icu-timezone-data in v8, this issue is gone.  Perhaps thanks to a revised sandbox policy for timezone related files ( see  bug 773532  ). 

So, with that sandbox hole for timezone related files, I don't need to change
InitializeICU() or  RendererMainPlatformDelegate::PlatformInitialize()  on macOS to call icu::TimeZone::createDefault().  Perhaps, that's why I decided to call  icu::TimeZone::createDefault() only on Linux in the first place in InitializeICU(). 

#if defined(OS_LINUX) && !defined(OS_CHROMEOS)
  if (result)
    std::unique_ptr<icu::TimeZone> zone(icu::TimeZone::createDefault());
#endif

Multiple related CLs got landed over a short span of time ( 2 days or so). I'll try a local build with them added one by one to find out which one of them did fix this issue. 

Once I know, I'll close this bug. 

Comment 4 by js...@chromium.org, Oct 15 2017

> Once I know, I'll close this bug. 

Perhaps, I'll make a CL to add a comment to explain why we don't need to call icu::TimeZone::createDefault() on macOS. That way, future code archaeology would be easier.

Comment 5 by js...@chromium.org, Oct 16 2017

Blockedon: 773532
Mergedinto: 766916
Status: Duplicate (was: Assigned)
I think this is also due to  bug 773532  and  bug 754053   on macOS 10.13 and  bug 773532  on macOS 10.12 or earlier. 

When reading off the Olson timezone ID from the target path of /etc/localtime (due to a sandbox or  bug 754053 ), ICU falls back to another method. That method fails for Ausrailia/Sydney when DST is in effect. 

So, the root cause of this bug is the same as  bug 766916 . Duping it to that bug. 

Sign in to add a comment