Prepare to enable icu_use_data_file_flat on CrOS

Prepare to enable icu_use_data_file_flag on CrOS

1. Enabling icu_use_data_file_flag led to a crash because GetStringUTF8 (that relies on ICU to determine whether the current UI language is RTL) is called before InitializeICU is called. Instead of using GetStringUTF8, get the string directly from the resource bundle because there's no need to adjust the string for the RTL locale in this particular case. 

2. update chromite to a revision that copies icudtl.dat when icu_use_data_file_flag is on. 

The switch (icu_use_data_file) will be flipped in a follow-up CL (either in build/common.gypi or chromeos-chrome ebuild file or both as necessary). 

BUG=72633
TEST=cros_{amd,x86,amd64} pass all the tests. No actual change with this CL.


Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=248325

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=248954

[jungshik at Google, 2014-01-28 00:20:20]

Tony: can you take a look at l10n_utils.cc?
Mark: common.gypi

Mike: Do I have to do anything on CrOS (especially packaging-wise) to get it to
use the ICU data in a separate file? 


The ICU data used to be statically linked to the Chrome binary and InitializeICU
used to be no-op. Now it has to mmap icudtl.dat)

I wonder if icudtl.dat (the ICU data separated from the chrome binary) has to be
listed somewhere to be installed/updated (e.g. Linux needs some
installer-related changes : https://codereview.chromium.org/102413007/ ). 

Nikita : Are you aware of any use of ICU API in OOBE (or eleswhere on Chrome OS)
before InitializeICU (in base/i18n/icu_util.cc)?

[vapier, 2014-01-28 00:25:54]

what file should it be installing/using in the fs ?

[tony, 2014-01-28 00:32:46]

https://codereview.chromium.org/135963010/diff/140001/ui/base/resource/resour...
File ui/base/resource/resource_bundle.cc (right):

https://codereview.chromium.org/135963010/diff/140001/ui/base/resource/resour...
ui/base/resource/resource_bundle.cc:72:
l10n_util::GetStringUTF8Raw(IDS_UI_FONT_FAMILY_CROS));
Can we call:
ResourceBundle& rb = ResourceBundle::GetSharedInstance();
rb.GetLocalizedString(IDS_UI_FONT_FAMILY_CROS);

instead of adding to the l10n_util API?

[jungshik at Google, 2014-01-28 00:36:03]

On 2014/01/28 00:32:46, tony wrote:
>
https://codereview.chromium.org/135963010/diff/140001/ui/base/resource/resour...
> File ui/base/resource/resource_bundle.cc (right):
> 
>
https://codereview.chromium.org/135963010/diff/140001/ui/base/resource/resour...
> ui/base/resource/resource_bundle.cc:72:
> l10n_util::GetStringUTF8Raw(IDS_UI_FONT_FAMILY_CROS));
> Can we call:
> ResourceBundle& rb = ResourceBundle::GetSharedInstance();
> rb.GetLocalizedString(IDS_UI_FONT_FAMILY_CROS);
> 
> instead of adding to the l10n_util API?

Yeah. I forgot that. I'll use it. Thanks.

[jungshik at Google, 2014-01-28 00:37:50]

On 2014/01/28 00:25:54, vapier wrote:
> what file should it be installing/using in the fs ?

/opt/google/chrome/icudtl.dat will be added (~ 10MB) and the size of chrome in
the same directory will be reduced by as much.

[vapier, 2014-01-28 01:06:02]

you'll probably want to update chromite/lib/chrome_util.py in the ChromiumOS
checkout to list this new file.  then update the DEPS file to point to the new
chromite rev.

[Mark Mentovai, 2014-01-28 17:19:46]

LGTM

https://codereview.chromium.org/135963010/diff/180001/build/common.gypi
File build/common.gypi (right):

https://codereview.chromium.org/135963010/diff/180001/build/common.gypi#newco...
build/common.gypi:1310: # TODO(jungshik): Turn this on on Android.
Use spaces, not tabs.

[jungshik at Google, 2014-01-28 21:26:19]

On 2014/01/28 01:06:02, vapier wrote:
> you'll probably want to update chromite/lib/chrome_util.py in the ChromiumOS
> checkout to list this new file.  then update the DEPS file to point to the new
> chromite rev.

Thanks. The CL is up for review at
https://chromium-review.googlesource.com/#/c/184155/

[jungshik at Google, 2014-01-28 21:36:49]

On 2014/01/28 17:19:46, Mark Mentovai wrote:
> LGTM
> 
> https://codereview.chromium.org/135963010/diff/180001/build/common.gypi
> File build/common.gypi (right):
> 
>
https://codereview.chromium.org/135963010/diff/180001/build/common.gypi#newco...
> build/common.gypi:1310: # TODO(jungshik): Turn this on on Android.
> Use spaces, not tabs.

Thanks. Tab removed. 

I'll update this CL once more with DEPS change to roll chromite once the
chromite CL is committed.

[Nikita (slow), 2014-01-29 15:06:29]

On 2014/01/28 00:20:20, Jungshik Shin wrote:
> Nikita : Are you aware of any use of ICU API in OOBE (or eleswhere on Chrome
OS)
> before InitializeICU (in base/i18n/icu_util.cc)?

I'm not aware of it.

[jungshik at Google, 2014-01-29 17:51:07]

On 2014/01/29 15:06:29, Nikita Kostylev wrote:
> On 2014/01/28 00:20:20, Jungshik Shin wrote:
> > Nikita : Are you aware of any use of ICU API in OOBE (or eleswhere on Chrome
> OS)
> > before InitializeICU (in base/i18n/icu_util.cc)?
> 
> I'm not aware of it.

Thank you for the answer, Nikita. 

Tony, can you take another look? Now the resource bundle is directly looked up
instead of relying GetString*. Thanks

[tony, 2014-01-29 23:28:05]

LGTM!

[jungshik at Google, 2014-01-31 10:11:31]

Thanks, Tony !

@Mike, can you approve chromite roll to the revision with
https://chromium-review.googlesource.com/#/c/184155/ ?  Thanks.

[vapier, 2014-01-31 15:40:53]

lgtm

[jungshik at Google, 2014-01-31 19:31:25]

The CQ bit was checked by jshin@chromium.org

[commit-bot: I haz the power, 2014-01-31 19:32:40]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jshin@chromium.org/135963010/220001

[commit-bot: I haz the power, 2014-01-31 20:58:31]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-01-31 20:58:31]

Retried try job too often on win_rel for step(s) unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2014-01-31 20:58:35]

CQ bit was unchecked on CL. Ignoring.

[commit-bot: I haz the power, 2014-01-31 20:58:35]

CQ bit was unchecked on CL. Ignoring.

[jungshik at Google, 2014-01-31 22:20:20]

The CQ bit was checked by jshin@chromium.org

[commit-bot: I haz the power, 2014-01-31 22:20:46]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jshin@chromium.org/135963010/220001

[commit-bot: I haz the power, 2014-02-01 01:44:46]

Change committed as 248325

[commit-bot: I haz the power, 2014-02-01 01:44:53]

CQ bit was unchecked on CL. Ignoring.

[commit-bot: I haz the power, 2014-02-01 01:44:55]

CQ bit was unchecked on CL. Ignoring.

[Cait (Slow), 2014-02-02 04:10:50]

A revert of this CL has been created in
https://codereview.chromium.org/151163003/ by caitkp@chromium.org.

The reason for reverting is: This is breaking the Chrome OS bots on the
waterfall:

http://build.chromium.org/p/chromium.chromiumos/buildstatus?builder=ChromiumO...
http://build.chromium.org/p/chromium.chromiumos/buildstatus?builder=ChromiumO...


From discussion on chromium-dev:

Looking at /var/log/messages for one of the failing tests, Chrome is exiting
with SIGABRT over and over, and ui.LATEST has the following over and over:
[0201/085858:FATAL:content_main_runner.cc(735)] Check failed:
base::i18n::InitializeICU(). .

[jungshik at Google, 2014-02-04 23:34:54]

On 2014/02/02 04:10:50, Cait Phillips wrote:
> A revert of this CL has been created in
cros_{x86,arm,amd64} trybots failed in build_package step and didn't get to run
vmtest. The CQ still went ahead and committed the CL. It looks like
chromeos-chrome ebuild does not copy icudtl.dat even though 'deploy_chrome'
described in 'Simple Chrome' does (which I used for the local testing). 

I may have to add icu_use_data_file_flag to chromeos-chrome ebuild directly. In
the meantime, I'm removing the swtich flip from this CL and gonna land the prep
part (resource_bundle.cc and chromite roll). 

> https://codereview.chromium.org/151163003/ by
https://mail.google.com/mail/?view=cm&fs=1&tf=1&to=caitkp@chromium.org.
> 
> The reason for reverting is: This is breaking the Chrome OS bots on the
> waterfall:
> 
>
http://build.chromium.org/p/chromium.chromiumos/buildstatus?builder=ChromiumO...
>
http://build.chromium.org/p/chromium.chromiumos/buildstatus?builder=ChromiumO...
> 
> 
> From discussion on chromium-dev:
> 
> Looking at /var/log/messages for one of the failing tests, Chrome is exiting
> with SIGABRT over and over, and ui.LATEST has the following over and over:
> [0201/085858:FATAL:content_main_runner.cc(735)] Check failed:
> base::i18n::InitializeICU(). .

[jungshik at Google, 2014-02-04 23:35:43]

The CQ bit was checked by jshin@chromium.org

[commit-bot: I haz the power, 2014-02-04 23:41:16]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jshin@chromium.org/135963010/710001

[commit-bot: I haz the power, 2014-02-05 01:37:49]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jshin@chromium.org/135963010/710001

[commit-bot: I haz the power, 2014-02-05 06:41:10]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-02-05 06:41:11]

Retried try job too often on win_rel for step(s) telemetry_perf_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[jungshik at Google, 2014-02-05 08:45:25]

The CQ bit was checked by jshin@chromium.org

[commit-bot: I haz the power, 2014-02-05 08:52:24]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jshin@chromium.org/135963010/710001

[commit-bot: I haz the power, 2014-02-05 10:22:13]

Change committed as 248954