Fix SysStrings unit test failure on Chromecast

Fix SysStrings unit test failure on Chromecast

Chromecast has a utf8 locale, and all calls to setlocale with non-NULL
locale name fail.  Some of these issues were addressed previously in
https://codereview.chromium.org/790903003/
but we still had failures remaining in SysStrings unit tests, in
particular ScopedLocale was failing to set the locale.

BUG=internal b/19627216

Committed: https://crrev.com/2a7adaba24106ab82ae09d36bc7cf1668a14fccd
Cr-Commit-Position: refs/heads/master@{#319814}

[halliwell, 2015-03-07 01:14:02]

halliwell@chromium.org changed reviewers:
+ lcwu@chromium.org, thestig@chromium.org

[Lei Zhang, 2015-03-07 01:30:19]

https://codereview.chromium.org/987943002/diff/1/base/test/scoped_locale.cc
File base/test/scoped_locale.cc (right):

https://codereview.chromium.org/987943002/diff/1/base/test/scoped_locale.cc#n...
base/test/scoped_locale.cc:16: EXPECT_TRUE(locale.find("utf-8") !=
std::string::npos);
I don't think this is right. For instance, on Mac, this code will run whe
chrome/common/time_format_browsertest.cc calls is. The locale would not be set. 
The test still passes, but it fails to test the code path in ICU it is suppose
to test.

[halliwell, 2015-03-07 02:31:14]

On 2015/03/07 01:30:19, Lei Zhang wrote:
> https://codereview.chromium.org/987943002/diff/1/base/test/scoped_locale.cc
> File base/test/scoped_locale.cc (right):
> 
>
https://codereview.chromium.org/987943002/diff/1/base/test/scoped_locale.cc#n...
> base/test/scoped_locale.cc:16: EXPECT_TRUE(locale.find("utf-8") !=
> std::string::npos);
> I don't think this is right. For instance, on Mac, this code will run whe
> chrome/common/time_format_browsertest.cc calls is. The locale would not be
set. 
> The test still passes, but it fails to test the code path in ICU it is suppose
> to test.

Mmmm, good point.  I missed that usage.

In patchset 2 I've localised the change to the tests that fail, these tests are
only for Linux so it's a much less wide-reaching change.

[halliwell, 2015-03-07 02:42:49]

On 2015/03/07 02:31:14, halliwell wrote:
> On 2015/03/07 01:30:19, Lei Zhang wrote:
> > https://codereview.chromium.org/987943002/diff/1/base/test/scoped_locale.cc
> > File base/test/scoped_locale.cc (right):
> > 
> >
>
https://codereview.chromium.org/987943002/diff/1/base/test/scoped_locale.cc#n...
> > base/test/scoped_locale.cc:16: EXPECT_TRUE(locale.find("utf-8") !=
> > std::string::npos);
> > I don't think this is right. For instance, on Mac, this code will run whe
> > chrome/common/time_format_browsertest.cc calls is. The locale would not be
> set. 
> > The test still passes, but it fails to test the code path in ICU it is
suppose
> > to test.
> 
> Mmmm, good point.  I missed that usage.
> 
> In patchset 2 I've localised the change to the tests that fail, these tests
are
> only for Linux so it's a much less wide-reaching change.

I will also need to make some gyp/GN changes to add the define for the unit test
target.  But let me know if this approach is ok.

[Lei Zhang, 2015-03-07 03:03:24]

lgtm

[halliwell, 2015-03-09 15:21:35]

On 2015/03/07 03:03:24, Lei Zhang wrote:
> lgtm

@thestig - please ptal at gyp+gn changes.

[Lei Zhang, 2015-03-09 22:33:35]

https://codereview.chromium.org/987943002/diff/40001/base/base.gypi
File base/base.gypi (right):

https://codereview.chromium.org/987943002/diff/40001/base/base.gypi#newcode1054
base/base.gypi:1054: # Enable more direct string conversions on platforms with
native utf8
Can we revert the change to this file, and instead add the define to
base_unittests in base/base.gyp? So the gyp and gn changes are a closer match.

[halliwell, 2015-03-09 22:50:21]

https://codereview.chromium.org/987943002/diff/40001/base/base.gypi
File base/base.gypi (right):

https://codereview.chromium.org/987943002/diff/40001/base/base.gypi#newcode1054
base/base.gypi:1054: # Enable more direct string conversions on platforms with
native utf8
On 2015/03/09 22:33:35, Lei Zhang wrote:
> Can we revert the change to this file, and instead add the define to
> base_unittests in base/base.gyp? So the gyp and gn changes are a closer match.

Done.

[Lei Zhang, 2015-03-09 23:01:00]

lgtm++

[halliwell, 2015-03-09 23:01:35]

The CQ bit was checked by halliwell@chromium.org

[commit-bot: I haz the power, 2015-03-09 23:03:11]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/987943002/60001

[commit-bot: I haz the power, 2015-03-10 01:51:34]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-03-10 01:52:24]

Patchset 4 (id:??) landed as
https://crrev.com/2a7adaba24106ab82ae09d36bc7cf1668a14fccd
Cr-Commit-Position: refs/heads/master@{#319814}