Turn on icu_case_mapping by default

Turn on icu_case_mapping by default

Update string-capitalize expected result because now it
passes all the tests in the file.
Mark fast/js/string-capitalization as failing with no_i18n.

Relanding after revert because the failure was taken care of
by Adam's CL at https://codereview.chromium.org/2597543002 .

3rd langing after a crash is taken care of in
https://codereview.chromium.org/2621393002

In addition to the previous version of this CL (PS #4) that landed and
reverted, drop String.prototype.to(Locale){Upper,Lower}Case from the
whitelist of built-in functions for side-effect-free-debugging.

BUG=v8:4477, v8:4476
TEST=test262/{built-ins,intl402}/Strings/*, webkit/fast/js/*,
     mjsunit/string-case, intl/general/case*

Cr-Original-Original-Commit-Position: refs/heads/master@{#41834}
Committed: https://chromium.googlesource.com/v8/v8/+/7c79e23c34ea971947eedc6e42d8a882617c0e47
Review-Url: https://codereview.chromium.org/2588963002
Cr-Original-Commit-Position: refs/heads/master@{#41883}
Committed: https://chromium.googlesource.com/v8/v8/+/a42c8c67dece5328896b13d37c5c846a2a0f5b0b
Review-Url: https://codereview.chromium.org/2588963002
Cr-Commit-Position: refs/heads/master@{#42524}
Committed: https://chromium.googlesource.com/v8/v8/+/c70ec473baf72dc38e1def98d097b7fbf2ec544a

[jungshik at Google, 2016-12-19 22:26:01]

jshin@chromium.org changed reviewers:
+ littledan@chromium.org

[jungshik at Google, 2016-12-19 22:26:01]

Dan, can you take a look? Thanks

(this is in conflict with another CL to turn on DateTimeFormat.formatToParts()).
I'll resolve the conflict after whichever of the two goes in first.

[Dan Ehrenberg, 2016-12-19 22:29:02]

littledan@chromium.org changed reviewers:
+ yangguo@chromium.org

[Dan Ehrenberg, 2016-12-19 22:29:02]

lgtm

Let's also get Yang's signoff here, though.

[jungshik at Google, 2016-12-19 22:50:57]

The CQ bit was checked by jshin@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-19 22:51:08]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-19 23:11:45]

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

[commit-bot: I haz the power, 2016-12-19 23:11:46]

Dry run: Try jobs failed on following builders:
  v8_linux64_gyp_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_gyp_rel_ng/build...)
  v8_linux64_gyp_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_gyp_rel_ng_trigg...)

[jungshik at Google, 2016-12-20 01:31:58]

The CQ bit was checked by jshin@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-20 01:32:00]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-20 01:59:28]

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

[commit-bot: I haz the power, 2016-12-20 01:59:29]

Dry run: This issue passed the CQ dry run.

[Yang, 2016-12-20 05:29:25]

On 2016/12/19 22:29:02, Dan Ehrenberg wrote:
> lgtm
> 
> Let's also get Yang's signoff here, though.

lgtm.

[jungshik at Google, 2016-12-20 08:54:34]

The CQ bit was checked by jshin@chromium.org

[jungshik at Google, 2016-12-20 08:54:34]

The patchset sent to the CQ was uploaded after l-g-t-m from
littledan@chromium.org
Link to the patchset: https://codereview.chromium.org/2588963002/#ps40001
(title: "string-capitalize expected updated: now it passes")

[commit-bot: I haz the power, 2016-12-20 08:54:38]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[jungshik at Google, 2016-12-20 08:54:55]

The CQ bit was unchecked by jshin@chromium.org

[jungshik at Google, 2016-12-20 08:59:11]

The CQ bit was checked by jshin@chromium.org

[commit-bot: I haz the power, 2016-12-20 08:59:22]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-20 09:00:51]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1482224351405350,
"parent_rev": "0a5bffe24d18bbad321c56e9125325b6b7589d50", "commit_rev":
"7c79e23c34ea971947eedc6e42d8a882617c0e47"}

[commit-bot: I haz the power, 2016-12-20 09:00:56]

Description was changed from

==========
Turn on icu_case_mapping by default

BUG=v8:4477, v8:4476
TEST=test262/{built-ins,intl402}/Strings/*, webkit/fast/js/*,
     mjsunit/string-case, intl/general/case*
==========

to

==========
Turn on icu_case_mapping by default

BUG=v8:4477, v8:4476
TEST=test262/{built-ins,intl402}/Strings/*, webkit/fast/js/*,
     mjsunit/string-case, intl/general/case*

Review-Url: https://codereview.chromium.org/2588963002
Cr-Commit-Position: refs/heads/master@{#41834}
Committed:
https://chromium.googlesource.com/v8/v8/+/7c79e23c34ea971947eedc6e42d8a882617...
==========

[commit-bot: I haz the power, 2016-12-20 09:00:57]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/v8/v8/+/7c79e23c34ea971947eedc6e42d8a882617...

[Michael Achenbach, 2016-12-20 09:28:20]

A revert of this CL (patchset #3 id:40001) has been created in
https://codereview.chromium.org/2593543002/ by machenbach@chromium.org.

The reason for reverting is: Causes gc stress failures:
https://build.chromium.org/p/client.v8/builders/V8%20Linux64%20GC%20Stress%20....

[Michael Achenbach, 2016-12-20 09:49:38]

Also fails:
https://build.chromium.org/p/client.v8/builders/V8%20Linux%20-%20noi18n%20-%2...

[jungshik at Google, 2016-12-20 18:24:49]

On 2016/12/20 09:49:38, Michael Achenbach (OOO) wrote:
> Also fails:
>
https://build.chromium.org/p/client.v8/builders/V8%20Linux%20-%20noi18n%20-%2...

string-capitalization failure is odd. Because with this ON, it should all pass
and that's what updated expected.txt included in the CL has (all PASS's instead
of two FAIL's at the beginning).

[jungshik at Google, 2016-12-20 18:31:17]

On 2016/12/20 09:28:20, Michael Achenbach (OOO) wrote:
> A revert of this CL (patchset #3 id:40001) has been created in
> https://codereview.chromium.org/2593543002/ by
https://mail.google.com/mail/?view=cm&fs=1&tf=1&to=machenbach@chromium.org.
> 
> The reason for reverting is: Causes gc stress failures:
>
https://build.chromium.org/p/client.v8/builders/V8%20Linux64%20GC%20Stress%20....

Dan and Yang, do you have any suggestion as to where to look/what to do about
this?  It's a bit unexpected that there's a higher gc stress with this change
than before. 

Michael, is there a way to reproduce this issue locally? If I use exactly the
build option (gn ars) and the same command to invoke test, should I able to
reproduce the issue? Well, let me try it.

[jungshik at Google, 2016-12-20 21:32:53]

On 2016/12/20 18:24:49, jungshik at google wrote:
> On 2016/12/20 09:49:38, Michael Achenbach (OOO) wrote:
> > Also fails:
> >
>
https://build.chromium.org/p/client.v8/builders/V8%20Linux%20-%20noi18n%20-%2...
> 
> string-capitalization failure is odd. Because with this ON, it should all pass
> and that's what updated expected.txt included in the CL has (all PASS's
instead
> of two FAIL's at the beginning).

Oh. That was no-i18n bot. I have to update test/webkit/webkit.status to mark
that test failing with no-i18n.

[jungshik at Google, 2016-12-21 00:58:23]

On 2016/12/20 18:31:17, jungshik at google wrote:
> On 2016/12/20 09:28:20, Michael Achenbach (OOO) wrote:
> > A revert of this CL (patchset #3 id:40001) has been created in
> > https://codereview.chromium.org/2593543002/ by
> https://mail.google.com/mail/?view=cm&fs=1&tf=1&to=machenbach@chromium.org.
> > 
> > The reason for reverting is: Causes gc stress failures:
> >
>
https://build.chromium.org/p/client.v8/builders/V8%20Linux64%20GC%20Stress%20....
> 
> Dan and Yang, do you have any suggestion as to where to look/what to do about
> this?  It's a bit unexpected that there's a higher gc stress with this change
> than before. 
> 
> Michael, is there a way to reproduce this issue locally? If I use exactly the
> build option (gn ars) and the same command to invoke test, should I able to
> reproduce the issue? Well, let me try it.

I did the following, but couldn't reproduce the failure: 

1. build a debug build using gn args as used by a bot
2. Run 
 out/gcdeb/d8 --test --random-seed=2140216864 --wasm_guard_pages
--invoke-weak-callbacks --nohard-abort --nodead-code-elimination
--nofold-constants --enable-slow-asserts --verify-heap --allow-natives-syntax
--noalways-opt test/mjsunit/regress/regress-trap-allocation-memento.js
--gc-interval=500 --stress-compaction --concurrent-recompilation-queue-length=64
--concurrent-recompilation-delay=500 --concurrent-recompilation

---------

Before the buildbot log goes away, I'm copying it here:

Test: mjsunit/regress/regress-trap-allocation-memento
Flags: --wasm_guard_pages --invoke-weak-callbacks
Command: /b/swarm_slave/w/irJ0sGJM/out/Debug/d8 --test --random-seed=2140216864
--wasm_guard_pages --invoke-weak-callbacks --nohard-abort
--nodead-code-elimination --nofold-constants --enable-slow-asserts --verify-heap
--allow-natives-syntax --noalways-opt
/b/swarm_slave/w/irJ0sGJM/test/mjsunit/regress/regress-trap-allocation-memento.js
--gc-interval=500 --stress-compaction --concurrent-recompilation-queue-length=64
--concurrent-recompilation-delay=500 --concurrent-recompilation

Build environment:
 gn_args: is_component_build = true is_debug = true target_cpu = "x64" use_goma
= true v8_embed_script = "test/mjsunit/mjsunit.js" v8_enable_backtrace = true
v8_enable_slow_dchecks = true v8_test_isolation_mode = "prepare"

Run #1
Exit code: 1
Result: FAIL
Expected outcomes: PASS
Duration: 00:00:103

Stdout:
<embedded>:225: Failure: expected <"fast double elements"> found <"fast smi only
elements">

Stack: Error
    at new MjsUnitAssertionError (<embedded>:31:16)
    at fail (<embedded>:225:11)
    at assertEquals (<embedded>:296:7)
    at assertKind
(/b/swarm_slave/w/irJ0sGJM/test/mjsunit/regress/regress-trap-allocation-memento.js:22:3)
    at run_test
(/b/swarm_slave/w/irJ0sGJM/test/mjsunit/regress/regress-trap-allocation-memento.js:38:5)
    at
/b/swarm_slave/w/irJ0sGJM/test/mjsunit/regress/regress-trap-allocation-memento.js:43:3
    at
/b/swarm_slave/w/irJ0sGJM/test/mjsunit/regress/regress-trap-allocation-memento.js:50:3
    throw new MjsUnitAssertionError(message);
    ^
Error
    at new MjsUnitAssertionError (<embedded>:31:16)
    at fail (<embedded>:225:11)
    at assertEquals (<embedded>:296:7)
    at assertKind
(/b/swarm_slave/w/irJ0sGJM/test/mjsunit/regress/regress-trap-allocation-memento.js:22:3)
    at run_test
(/b/swarm_slave/w/irJ0sGJM/test/mjsunit/regress/regress-trap-allocation-memento.js:38:5)
    at
/b/swarm_slave/w/irJ0sGJM/test/mjsunit/regress/regress-trap-allocation-memento.js:43:3
    at
/b/swarm_slave/w/irJ0sGJM/test/mjsunit/regress/regress-trap-allocation-memento.js:50:3

[jungshik at Google, 2016-12-21 06:13:24]

On 2016/12/21 00:58:23, jungshik at google wrote:
> On 2016/12/20 18:31:17, jungshik at google wrote:
> > On 2016/12/20 09:28:20, Michael Achenbach (OOO) wrote:
> > > A revert of this CL (patchset #3 id:40001) has been created in
> > > https://codereview.chromium.org/2593543002/ by
> > https://mail.google.com/mail/?view=cm&fs=1&tf=1&to=machenbach@chromium.org.
> > > 
> > > The reason for reverting is: Causes gc stress failures:
> > >
> >
>
https://build.chromium.org/p/client.v8/builders/V8%20Linux64%20GC%20Stress%20....

Adam's CL ( https://codereview.chromium.org/2597543002/ ) made this a non-issue.
I'll land it again with rebase.

[jungshik at Google, 2016-12-21 06:17:17]

Description was changed from

==========
Turn on icu_case_mapping by default

BUG=v8:4477, v8:4476
TEST=test262/{built-ins,intl402}/Strings/*, webkit/fast/js/*,
     mjsunit/string-case, intl/general/case*

Review-Url: https://codereview.chromium.org/2588963002
Cr-Commit-Position: refs/heads/master@{#41834}
Committed:
https://chromium.googlesource.com/v8/v8/+/7c79e23c34ea971947eedc6e42d8a882617...
==========

to

==========
Turn on icu_case_mapping by default

Update string-capitalize expected result because now it
passes all the tests in the file.
Mark fast/js/string-capitalization as failing with no_i18n.

Relanding after revert because the failure was taken care of
by https://codereview.chromium.org/2597543002 .


BUG=v8:4477, v8:4476
TEST=test262/{built-ins,intl402}/Strings/*, webkit/fast/js/*,
     mjsunit/string-case, intl/general/case*

Cr-Commit-Position: refs/heads/master@{#41834}
Committed:
https://chromium.googlesource.com/v8/v8/+/7c79e23c34ea971947eedc6e42d8a882617...
==========

[jungshik at Google, 2016-12-21 06:19:15]

Description was changed from

==========
Turn on icu_case_mapping by default

Update string-capitalize expected result because now it
passes all the tests in the file.
Mark fast/js/string-capitalization as failing with no_i18n.

Relanding after revert because the failure was taken care of
by https://codereview.chromium.org/2597543002 .


BUG=v8:4477, v8:4476
TEST=test262/{built-ins,intl402}/Strings/*, webkit/fast/js/*,
     mjsunit/string-case, intl/general/case*

Cr-Commit-Position: refs/heads/master@{#41834}
Committed:
https://chromium.googlesource.com/v8/v8/+/7c79e23c34ea971947eedc6e42d8a882617...
==========

to

==========
Turn on icu_case_mapping by default

Update string-capitalize expected result because now it
passes all the tests in the file.
Mark fast/js/string-capitalization as failing with no_i18n.

Relanding after revert because the failure was taken care of
by https://codereview.chromium.org/2597543002 .


BUG=v8:4477, v8:4476
TEST=test262/{built-ins,intl402}/Strings/*, webkit/fast/js/*,
     mjsunit/string-case, intl/general/case*

Cr-Commit-Position: refs/heads/master@{#41834}
Committed:
https://chromium.googlesource.com/v8/v8/+/7c79e23c34ea971947eedc6e42d8a882617...
==========

[jungshik at Google, 2016-12-21 06:19:43]

Description was changed from

==========
Turn on icu_case_mapping by default

Update string-capitalize expected result because now it
passes all the tests in the file.
Mark fast/js/string-capitalization as failing with no_i18n.

Relanding after revert because the failure was taken care of
by https://codereview.chromium.org/2597543002 .


BUG=v8:4477, v8:4476
TEST=test262/{built-ins,intl402}/Strings/*, webkit/fast/js/*,
     mjsunit/string-case, intl/general/case*

Cr-Commit-Position: refs/heads/master@{#41834}
Committed:
https://chromium.googlesource.com/v8/v8/+/7c79e23c34ea971947eedc6e42d8a882617...
==========

to

==========
Turn on icu_case_mapping by default

Update string-capitalize expected result because now it
passes all the tests in the file.
Mark fast/js/string-capitalization as failing with no_i18n.

Relanding after revert because the failure was taken care of
by Adam's CL at https://codereview.chromium.org/2597543002 .


BUG=v8:4477, v8:4476
TEST=test262/{built-ins,intl402}/Strings/*, webkit/fast/js/*,
     mjsunit/string-case, intl/general/case*

Cr-Commit-Position: refs/heads/master@{#41834}
Committed:
https://chromium.googlesource.com/v8/v8/+/7c79e23c34ea971947eedc6e42d8a882617...
==========

[jungshik at Google, 2016-12-21 06:20:26]

The CQ bit was checked by jshin@chromium.org

[jungshik at Google, 2016-12-21 06:20:27]

The patchset sent to the CQ was uploaded after l-g-t-m from
littledan@chromium.org, yangguo@chromium.org
Link to the patchset: https://codereview.chromium.org/2588963002/#ps60001
(title: "rebase + string-capitalization marked as failing with no_i18n")

[commit-bot: I haz the power, 2016-12-21 06:20:30]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-21 06:48:48]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1482301226743580,
"parent_rev": "6f66961d41624086cff5ac0219627c396a425f69", "commit_rev":
"a42c8c67dece5328896b13d37c5c846a2a0f5b0b"}

[commit-bot: I haz the power, 2016-12-21 06:48:54]

Description was changed from

==========
Turn on icu_case_mapping by default

Update string-capitalize expected result because now it
passes all the tests in the file.
Mark fast/js/string-capitalization as failing with no_i18n.

Relanding after revert because the failure was taken care of
by Adam's CL at https://codereview.chromium.org/2597543002 .


BUG=v8:4477, v8:4476
TEST=test262/{built-ins,intl402}/Strings/*, webkit/fast/js/*,
     mjsunit/string-case, intl/general/case*

Cr-Commit-Position: refs/heads/master@{#41834}
Committed:
https://chromium.googlesource.com/v8/v8/+/7c79e23c34ea971947eedc6e42d8a882617...
==========

to

==========
Turn on icu_case_mapping by default

Update string-capitalize expected result because now it
passes all the tests in the file.
Mark fast/js/string-capitalization as failing with no_i18n.

Relanding after revert because the failure was taken care of
by Adam's CL at https://codereview.chromium.org/2597543002 .


BUG=v8:4477, v8:4476
TEST=test262/{built-ins,intl402}/Strings/*, webkit/fast/js/*,
     mjsunit/string-case, intl/general/case*

Cr-Original-Commit-Position: refs/heads/master@{#41834}
Committed:
https://chromium.googlesource.com/v8/v8/+/7c79e23c34ea971947eedc6e42d8a882617...
Review-Url: https://codereview.chromium.org/2588963002
Cr-Commit-Position: refs/heads/master@{#41883}
Committed:
https://chromium.googlesource.com/v8/v8/+/a42c8c67dece5328896b13d37c5c846a2a0...
==========

[commit-bot: I haz the power, 2016-12-21 06:48:55]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/v8/v8/+/a42c8c67dece5328896b13d37c5c846a2a0...

[adamk, 2016-12-22 17:59:26]

A revert of this CL (patchset #4 id:60001) has been created in
https://codereview.chromium.org/2601553002/ by adamk@chromium.org.

The reason for reverting is: Causes crashes on Canary: crbug.com/676643.

[jungshik at Google, 2017-01-10 01:22:45]

Dan and Yang, could you take a look at my comment below? Thanks

https://codereview.chromium.org/2588963002/diff/80001/src/js/i18n.js
File src/js/i18n.js (right):

https://codereview.chromium.org/2588963002/diff/80001/src/js/i18n.js#newcode2033
src/js/i18n.js:2033: if (langIndex === -1) {
I tightened the comparison here just in case although it's not likely to help
with the crash. Nonetheless, it should be a good hygiene, I guess.

https://codereview.chromium.org/2588963002/diff/80001/src/js/i18n.js#newcode2038
src/js/i18n.js:2038: CUSTOM_CASE_LANGUAGES[langIndex]);
In runtime-i18n.cc, CUSTOM_CASE_LANGUAGES[langIndex] is assumed to be
SeqOneByteString with the following code: 


RUNTIME_FUNCTION(Runtime_StringLocaleConvertCase) {
  HandleScope scope(isolate);
  DCHECK_EQ(args.length(), 3);
  CONVERT_ARG_HANDLE_CHECKED(String, s, 0);
  CONVERT_BOOLEAN_ARG_CHECKED(is_upper, 1);
  CONVERT_ARG_HANDLE_CHECKED(SeqOneByteString, lang, 2);  <=== crash

  // All the languages requiring special handling ("az", "el", "lt", "tr")
  // have a 2-letter language code.
  DCHECK(lang->length() == 2);
  uint8_t lang_str[3];
  memcpy(lang_str, lang->GetChars(), 2);
  lang_str[2] = 0;
  s = String::Flatten(s);


And, I loaded one of minidump into WinDBG and it appears that that's where it
crashed. Is my assumption wrong about SeqOneByteString when it's one of
CUSTOM_CASE_LANGUAGES above?

[jungshik at Google, 2017-01-18 01:21:40]

Description was changed from

==========
Turn on icu_case_mapping by default

Update string-capitalize expected result because now it
passes all the tests in the file.
Mark fast/js/string-capitalization as failing with no_i18n.

Relanding after revert because the failure was taken care of
by Adam's CL at https://codereview.chromium.org/2597543002 .


BUG=v8:4477, v8:4476
TEST=test262/{built-ins,intl402}/Strings/*, webkit/fast/js/*,
     mjsunit/string-case, intl/general/case*

Cr-Original-Commit-Position: refs/heads/master@{#41834}
Committed:
https://chromium.googlesource.com/v8/v8/+/7c79e23c34ea971947eedc6e42d8a882617...
Review-Url: https://codereview.chromium.org/2588963002
Cr-Commit-Position: refs/heads/master@{#41883}
Committed:
https://chromium.googlesource.com/v8/v8/+/a42c8c67dece5328896b13d37c5c846a2a0...
==========

to

==========
Turn on icu_case_mapping by default

Update string-capitalize expected result because now it
passes all the tests in the file.
Mark fast/js/string-capitalization as failing with no_i18n.

Relanding after revert because the failure was taken care of
by Adam's CL at https://codereview.chromium.org/2597543002 .

3rd langing after a crash is taken care of in 
https://codereview.chromium.org/2621393002


BUG=v8:4477, v8:4476
TEST=test262/{built-ins,intl402}/Strings/*, webkit/fast/js/*,
     mjsunit/string-case, intl/general/case*

Cr-Original-Commit-Position: refs/heads/master@{#41834}
Committed:
https://chromium.googlesource.com/v8/v8/+/7c79e23c34ea971947eedc6e42d8a882617...
Review-Url: https://codereview.chromium.org/2588963002
Cr-Commit-Position: refs/heads/master@{#41883}
Committed:
https://chromium.googlesource.com/v8/v8/+/a42c8c67dece5328896b13d37c5c846a2a0...
==========

[jungshik at Google, 2017-01-18 01:22:34]

The CQ bit was checked by jshin@chromium.org

[jungshik at Google, 2017-01-18 01:22:35]

The patchset sent to the CQ was uploaded after l-g-t-m from
littledan@chromium.org, yangguo@chromium.org
Link to the patchset: https://codereview.chromium.org/2588963002/#ps100001
(title: "go back to PS 4")

[commit-bot: I haz the power, 2017-01-18 01:22:39]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-18 01:43:26]

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

[commit-bot: I haz the power, 2017-01-18 01:43:27]

Try jobs failed on following builders:
  v8_linux64_avx2_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng/buil...)
  v8_linux64_avx2_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng_trig...)

[Dan Ehrenberg, 2017-01-18 19:10:23]

On 2017/01/18 01:43:27, commit-bot: I haz the power wrote:
> Try jobs failed on following builders:
>   v8_linux64_avx2_rel_ng on master.tryserver.v8 (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng/buil...)
>   v8_linux64_avx2_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng_trig...)

Looks like these failures are simply due to debug-evaluate finding that
functions which access Unicode functionality. Similarly,
'String.prototype.normalize' is blacklisted as side-effecting (even though it
shouldn't be). The failing tests passes locally for me if I just add these four
lines to test/debugger/debug/debug-evaluate-no-side-effect-builtins.js at line
54:

        if (f == "toLowerCase") continue;
        if (f == "toUpperCase") continue;
        if (f == "toLocaleLowerCase") continue;
        if (f == "toLocaleUpperCase") continue;

Yang, is it OK to ban case conversion from side-effect-free debug-evaluate? Or
should we look into better tracking to understand that these actually don't have
side effects?

[jungshik at Google, 2017-01-18 19:57:58]

Description was changed from

==========
Turn on icu_case_mapping by default

Update string-capitalize expected result because now it
passes all the tests in the file.
Mark fast/js/string-capitalization as failing with no_i18n.

Relanding after revert because the failure was taken care of
by Adam's CL at https://codereview.chromium.org/2597543002 .

3rd langing after a crash is taken care of in 
https://codereview.chromium.org/2621393002


BUG=v8:4477, v8:4476
TEST=test262/{built-ins,intl402}/Strings/*, webkit/fast/js/*,
     mjsunit/string-case, intl/general/case*

Cr-Original-Commit-Position: refs/heads/master@{#41834}
Committed:
https://chromium.googlesource.com/v8/v8/+/7c79e23c34ea971947eedc6e42d8a882617...
Review-Url: https://codereview.chromium.org/2588963002
Cr-Commit-Position: refs/heads/master@{#41883}
Committed:
https://chromium.googlesource.com/v8/v8/+/a42c8c67dece5328896b13d37c5c846a2a0...
==========

to

==========
Turn on icu_case_mapping by default

Update string-capitalize expected result because now it
passes all the tests in the file.
Mark fast/js/string-capitalization as failing with no_i18n.

Relanding after revert because the failure was taken care of
by Adam's CL at https://codereview.chromium.org/2597543002 .

3rd langing after a crash is taken care of in
https://codereview.chromium.org/2621393002

In addition to the previous CLs that landed and reverted,
drop String.prototype.to(Locale){Upper,Lower}Case from the
whitelist of built-in functions for side-effect-free-debugging.

BUG=v8:4477, v8:4476
TEST=test262/{built-ins,intl402}/Strings/*, webkit/fast/js/*,
     mjsunit/string-case, intl/general/case*

Cr-Original-Commit-Position: refs/heads/master@{#41834}
Committed:
https://chromium.googlesource.com/v8/v8/+/7c79e23c34ea971947eedc6e42d8a882617...
Review-Url: https://codereview.chromium.org/2588963002
Cr-Commit-Position: refs/heads/master@{#41883}
Committed:
https://chromium.googlesource.com/v8/v8/+/a42c8c67dece5328896b13d37c5c846a2a0...
==========

[jungshik at Google, 2017-01-18 19:58:47]

On 2017/01/18 01:43:27, commit-bot: I haz the power wrote:
> Try jobs failed on following builders:
>   v8_linux64_avx2_rel_ng on master.tryserver.v8 (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng/buil...)
>   v8_linux64_avx2_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng_trig...)

https://codereview.chromium.org/2634523002 whitelisted some built-ins as
debug-side-effect free , but apparently my implementation of ICU-based case
conversion does not clear the bar. (both my old implementation and new
implementation had the same issue). 

Yang and Dan,  how should I proceed?  The best would be to change
icu-case-conversion code to be debug-side-effect-free.   A less-than-optimal
alternative would be to drop String.prototype.{lower,upper}Case from the
whitelist. 


I can locally reproduce the bot failure with the following build and command:
 
gn_args: dcheck_always_on = true is_component_build = false is_debug = false
symbol_level = 1 target_cpu = "x64" use_goma = true v8_has_valgrind = true
v8_test_isolation_mode = "prepare"

out/relside/d8  --test --random-seed=-360368526 --enable-inspector
--allow-natives-syntax --nohard-abort --nodead-code-elimination
--nofold-constants --ignition --side-effect-free-debug-evaluate
test/mjsunit/mjsunit.js test/debugger/test-api.js
test/debugger/debug/debug-evaluate-no-side-effect-builtins.js

[jungshik at Google, 2017-01-18 20:00:26]

Dan and Yang, can you take a look at PS #7? 

I dropped to(Locale){Lower,Upper}Case from the side-effect-free debugging
whitelist. 

If it's not a good idea, any advice on what's required to get them
s-e-f-debugging would be appreciated.

[Yang, 2017-01-18 20:01:00]

On 2017/01/18 19:58:47, jungshik at Google (ooo) wrote:
> On 2017/01/18 01:43:27, commit-bot: I haz the power wrote:
> > Try jobs failed on following builders:
> >   v8_linux64_avx2_rel_ng on master.tryserver.v8 (JOB_FAILED,
> >
>
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng/buil...)
> >   v8_linux64_avx2_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED,
> >
>
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng_trig...)
> 
> https://codereview.chromium.org/2634523002 whitelisted some built-ins as
> debug-side-effect free , but apparently my implementation of ICU-based case
> conversion does not clear the bar. (both my old implementation and new
> implementation had the same issue). 
> 
> Yang and Dan,  how should I proceed?  The best would be to change
> icu-case-conversion code to be debug-side-effect-free.   A less-than-optimal
> alternative would be to drop String.prototype.{lower,upper}Case from the
> whitelist. 
> 
> 
> I can locally reproduce the bot failure with the following build and command:
>  
> gn_args: dcheck_always_on = true is_component_build = false is_debug = false
> symbol_level = 1 target_cpu = "x64" use_goma = true v8_has_valgrind = true
> v8_test_isolation_mode = "prepare"
> 
> out/relside/d8  --test --random-seed=-360368526 --enable-inspector
> --allow-natives-syntax --nohard-abort --nodead-code-elimination
> --nofold-constants --ignition --side-effect-free-debug-evaluate
> test/mjsunit/mjsunit.js test/debugger/test-api.js
> test/debugger/debug/debug-evaluate-no-side-effect-builtins.js

changing the test case to skip these functions sgtm!

[Yang, 2017-01-18 20:02:22]

On 2017/01/18 20:01:00, Yang wrote:
> On 2017/01/18 19:58:47, jungshik at Google (ooo) wrote:
> > On 2017/01/18 01:43:27, commit-bot: I haz the power wrote:
> > > Try jobs failed on following builders:
> > >   v8_linux64_avx2_rel_ng on master.tryserver.v8 (JOB_FAILED,
> > >
> >
>
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng/buil...)
> > >   v8_linux64_avx2_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED,
> > >
> >
>
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng_trig...)
> > 
> > https://codereview.chromium.org/2634523002 whitelisted some built-ins as
> > debug-side-effect free , but apparently my implementation of ICU-based case
> > conversion does not clear the bar. (both my old implementation and new
> > implementation had the same issue). 
> > 
> > Yang and Dan,  how should I proceed?  The best would be to change
> > icu-case-conversion code to be debug-side-effect-free.   A less-than-optimal
> > alternative would be to drop String.prototype.{lower,upper}Case from the
> > whitelist. 
> > 
> > 
> > I can locally reproduce the bot failure with the following build and
command:
> >  
> > gn_args: dcheck_always_on = true is_component_build = false is_debug = false
> > symbol_level = 1 target_cpu = "x64" use_goma = true v8_has_valgrind = true
> > v8_test_isolation_mode = "prepare"
> > 
> > out/relside/d8  --test --random-seed=-360368526 --enable-inspector
> > --allow-natives-syntax --nohard-abort --nodead-code-elimination
> > --nofold-constants --ignition --side-effect-free-debug-evaluate
> > test/mjsunit/mjsunit.js test/debugger/test-api.js
> > test/debugger/debug/debug-evaluate-no-side-effect-builtins.js
> 
> changing the test case to skip these functions sgtm!

A bit more background to this: side-effect-free debug-evaluate is a new feature
that is still undergoing development. As such, we do have a lot of false
negatives. I recently cleared all String builtins except for the locale-specific
ones, since these alter the state of i18n objects.

[jungshik at Google, 2017-01-18 20:05:13]

On 2017/01/18 19:10:23, Dan Ehrenberg wrote:
> On 2017/01/18 01:43:27, commit-bot: I haz the power wrote:
> > Try jobs failed on following builders:
> >   v8_linux64_avx2_rel_ng on master.tryserver.v8 (JOB_FAILED,
> >
>
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng/buil...)
> >   v8_linux64_avx2_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED,
> >
>
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng_trig...)
> 
> Looks like these failures are simply due to debug-evaluate finding that
> functions which access Unicode functionality. Similarly,
> 'String.prototype.normalize' is blacklisted as side-effecting (even though it
> shouldn't be). The failing tests passes locally for me if I just add these
four
> lines to test/debugger/debug/debug-evaluate-no-side-effect-builtins.js at line
> 54:
> 
>         if (f == "toLowerCase") continue;
>         if (f == "toUpperCase") continue;
>         if (f == "toLocaleLowerCase") continue;
>         if (f == "toLocaleUpperCase") continue;
> 
> Yang, is it OK to ban case conversion from side-effect-free debug-evaluate? Or
> should we look into better tracking to understand that these actually don't
have
> side effects?

Oh. My comment and Dan's comment crossed in transit. :-)  Yeah, I have the same
question and I'd appreciate Yang's advice.

[jungshik at Google, 2017-01-18 20:07:20]

On 2017/01/18 20:05:13, jungshik at Google (ooo) wrote:
> On 2017/01/18 19:10:23, Dan Ehrenberg wrote:
> > On 2017/01/18 01:43:27, commit-bot: I haz the power wrote:
> > > Try jobs failed on following builders:
> > >   v8_linux64_avx2_rel_ng on master.tryserver.v8 (JOB_FAILED,
> > >
> >
>
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng/buil...)
> > >   v8_linux64_avx2_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED,
> > >
> >
>
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng_trig...)
> > 
> > Looks like these failures are simply due to debug-evaluate finding that
> > functions which access Unicode functionality. Similarly,
> > 'String.prototype.normalize' is blacklisted as side-effecting (even though
it
> > shouldn't be). The failing tests passes locally for me if I just add these
> four
> > lines to test/debugger/debug/debug-evaluate-no-side-effect-builtins.js at
line
> > 54:
> > 
> >         if (f == "toLowerCase") continue;
> >         if (f == "toUpperCase") continue;
> >         if (f == "toLocaleLowerCase") continue;
> >         if (f == "toLocaleUpperCase") continue;
> > 
> > Yang, is it OK to ban case conversion from side-effect-free debug-evaluate?
Or
> > should we look into better tracking to understand that these actually don't
> have
> > side effects?
> 
> Oh. My comment and Dan's comment crossed in transit. :-)  Yeah, I have the
same
> question and I'd appreciate Yang's advice.

Wow, thank you for the quick reply, Yang. 

Do you like 'fail' statements to be added or just skipping? (PS #7 vs PS #8) 
Thanks again

[Dan Ehrenberg, 2017-01-18 20:09:24]

On 2017/01/18 20:02:22, Yang wrote:
> On 2017/01/18 20:01:00, Yang wrote:
> > On 2017/01/18 19:58:47, jungshik at Google (ooo) wrote:
> > > On 2017/01/18 01:43:27, commit-bot: I haz the power wrote:
> > > > Try jobs failed on following builders:
> > > >   v8_linux64_avx2_rel_ng on master.tryserver.v8 (JOB_FAILED,
> > > >
> > >
> >
>
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng/buil...)
> > > >   v8_linux64_avx2_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED,
> > > >
> > >
> >
>
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng_trig...)
> > > 
> > > https://codereview.chromium.org/2634523002 whitelisted some built-ins as
> > > debug-side-effect free , but apparently my implementation of ICU-based
case
> > > conversion does not clear the bar. (both my old implementation and new
> > > implementation had the same issue). 
> > > 
> > > Yang and Dan,  how should I proceed?  The best would be to change
> > > icu-case-conversion code to be debug-side-effect-free.   A
less-than-optimal
> > > alternative would be to drop String.prototype.{lower,upper}Case from the
> > > whitelist. 
> > > 
> > > 
> > > I can locally reproduce the bot failure with the following build and
> command:
> > >  
> > > gn_args: dcheck_always_on = true is_component_build = false is_debug =
false
> > > symbol_level = 1 target_cpu = "x64" use_goma = true v8_has_valgrind = true
> > > v8_test_isolation_mode = "prepare"
> > > 
> > > out/relside/d8  --test --random-seed=-360368526 --enable-inspector
> > > --allow-natives-syntax --nohard-abort --nodead-code-elimination
> > > --nofold-constants --ignition --side-effect-free-debug-evaluate
> > > test/mjsunit/mjsunit.js test/debugger/test-api.js
> > > test/debugger/debug/debug-evaluate-no-side-effect-builtins.js
> > 
> > changing the test case to skip these functions sgtm!
> 
> A bit more background to this: side-effect-free debug-evaluate is a new
feature
> that is still undergoing development. As such, we do have a lot of false
> negatives. I recently cleared all String builtins except for the
locale-specific
> ones, since these alter the state of i18n objects.

It's true that the i18n functions side-effect some i18n objects, but I believe
this is an unobservable implementation detail--it's just populating and using
caches. Would this be compatible with the model of debug-evaluate? I'm
completely fine with not whitelisting these functions for now, while it's in
development, though.

[jungshik at Google, 2017-01-18 20:12:31]

The CQ bit was checked by jshin@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-18 20:12:41]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Yang, 2017-01-18 20:37:31]

On 2017/01/18 20:09:24, Dan Ehrenberg wrote:
> On 2017/01/18 20:02:22, Yang wrote:
> > On 2017/01/18 20:01:00, Yang wrote:
> > > On 2017/01/18 19:58:47, jungshik at Google (ooo) wrote:
> > > > On 2017/01/18 01:43:27, commit-bot: I haz the power wrote:
> > > > > Try jobs failed on following builders:
> > > > >   v8_linux64_avx2_rel_ng on master.tryserver.v8 (JOB_FAILED,
> > > > >
> > > >
> > >
> >
>
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng/buil...)
> > > > >   v8_linux64_avx2_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED,
> > > > >
> > > >
> > >
> >
>
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng_trig...)
> > > > 
> > > > https://codereview.chromium.org/2634523002 whitelisted some built-ins as
> > > > debug-side-effect free , but apparently my implementation of ICU-based
> case
> > > > conversion does not clear the bar. (both my old implementation and new
> > > > implementation had the same issue). 
> > > > 
> > > > Yang and Dan,  how should I proceed?  The best would be to change
> > > > icu-case-conversion code to be debug-side-effect-free.   A
> less-than-optimal
> > > > alternative would be to drop String.prototype.{lower,upper}Case from the
> > > > whitelist. 
> > > > 
> > > > 
> > > > I can locally reproduce the bot failure with the following build and
> > command:
> > > >  
> > > > gn_args: dcheck_always_on = true is_component_build = false is_debug =
> false
> > > > symbol_level = 1 target_cpu = "x64" use_goma = true v8_has_valgrind =
true
> > > > v8_test_isolation_mode = "prepare"
> > > > 
> > > > out/relside/d8  --test --random-seed=-360368526 --enable-inspector
> > > > --allow-natives-syntax --nohard-abort --nodead-code-elimination
> > > > --nofold-constants --ignition --side-effect-free-debug-evaluate
> > > > test/mjsunit/mjsunit.js test/debugger/test-api.js
> > > > test/debugger/debug/debug-evaluate-no-side-effect-builtins.js
> > > 
> > > changing the test case to skip these functions sgtm!
> > 
> > A bit more background to this: side-effect-free debug-evaluate is a new
> feature
> > that is still undergoing development. As such, we do have a lot of false
> > negatives. I recently cleared all String builtins except for the
> locale-specific
> > ones, since these alter the state of i18n objects.
> 
> It's true that the i18n functions side-effect some i18n objects, but I believe
> this is an unobservable implementation detail--it's just populating and using
> caches. Would this be compatible with the model of debug-evaluate? I'm
> completely fine with not whitelisting these functions for now, while it's in
> development, though.

I know it's implementation detail, but the side-effect check is not implemented
to recognize that, at least at the moment. Skipping these particular functions
in the test case as shown in #47 would be great.

[commit-bot: I haz the power, 2017-01-18 20:39:50]

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

[commit-bot: I haz the power, 2017-01-18 20:39:51]

Dry run: Try jobs failed on following builders:
  v8_linux64_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_rel_ng/builds/19473)
  v8_linux64_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_rel_ng_triggered...)

[jungshik at Google, 2017-01-18 20:46:36]

Dan, I don't see test/test262/local-tests/test/intl402/6.2.2_b.js in my tree
(after git pull and gclient sync). Am I missing something? 

Test: test262/intl402/6.2.2_b
Flags: --use-strict
Command: /b/s/w/iru18BgP/out/Release/d8 --test --random-seed=831298184
--use-strict --nohard-abort --nodead-code-elimination --nofold-constants
/b/s/w/iru18BgP/test/test262/data/harness/sta.js
/b/s/w/iru18BgP/test/test262/data/harness/assert.js
/b/s/w/iru18BgP/test/test262/harness-adapt.js
/b/s/w/iru18BgP/test/test262/data/harness/testIntl.js
/b/s/w/iru18BgP/test/test262/local-tests/test/intl402/6.2.2_b.js
--invoke-weak-callbacks --omit-quit

Build environment:
 gn_args: is_asan = true is_component_build = false is_debug = false is_lsan =
true symbol_level = 1 target_cpu = "x64" use_goma = true v8_test_isolation_mode
= "prepare"

Run #1
Exit code: 1
Result: FAIL
Expected outcomes: PASS
Duration: 00:00:111

Stdout:
/b/s/w/iru18BgP/test/test262/data/harness/testIntl.js:30: Test262Error: Invalid
language tag de-tester-Tester was not rejected. (Testing with Collator.)
            throw e;
            ^

[jungshik at Google, 2017-01-18 21:05:12]

On 2017/01/18 20:46:36, jungshik at Google wrote:
> Dan, I don't see test/test262/local-tests/test/intl402/6.2.2_b.js in my tree
> (after git pull and gclient sync). Am I missing something? 

I found 
test/test262/data/test/intl402/6.2.2_b.js. instead. However, it does not have
'de-tester-Tester'. 
Anyway, we're failing that test. Regex in i18n.js does not check for duplicated
variant subtags ('tester' and 'Tester'), but 
it has nothing to do with this CL. That is, it should fail even without this CL.



> 
> Test: test262/intl402/6.2.2_b
> Flags: --use-strict
> Command: /b/s/w/iru18BgP/out/Release/d8 --test --random-seed=831298184
> --use-strict --nohard-abort --nodead-code-elimination --nofold-constants
> /b/s/w/iru18BgP/test/test262/data/harness/sta.js
> /b/s/w/iru18BgP/test/test262/data/harness/assert.js
> /b/s/w/iru18BgP/test/test262/harness-adapt.js
> /b/s/w/iru18BgP/test/test262/data/harness/testIntl.js
> /b/s/w/iru18BgP/test/test262/local-tests/test/intl402/6.2.2_b.js
> --invoke-weak-callbacks --omit-quit
> 

> Run #1
> Exit code: 1
> Result: FAIL
> Expected outcomes: PASS
> Duration: 00:00:111
> 
> Stdout:
> /b/s/w/iru18BgP/test/test262/data/harness/testIntl.js:30: Test262Error:
Invalid
> language tag de-tester-Tester was not rejected. (Testing with Collator.)
>             throw e;
>             ^

[Dan Ehrenberg, 2017-01-18 23:23:20]

On 2017/01/18 21:05:12, jungshik at Google wrote:
> On 2017/01/18 20:46:36, jungshik at Google wrote:
> > Dan, I don't see test/test262/local-tests/test/intl402/6.2.2_b.js in my tree
> > (after git pull and gclient sync). Am I missing something? 
> 
> I found 
> test/test262/data/test/intl402/6.2.2_b.js. instead. However, it does not have
> 'de-tester-Tester'. 
> Anyway, we're failing that test. Regex in i18n.js does not check for
duplicated
> variant subtags ('tester' and 'Tester'), but 
> it has nothing to do with this CL. That is, it should fail even without this
CL.
> 
> 
> 
> > 
> > Test: test262/intl402/6.2.2_b
> > Flags: --use-strict
> > Command: /b/s/w/iru18BgP/out/Release/d8 --test --random-seed=831298184
> > --use-strict --nohard-abort --nodead-code-elimination --nofold-constants
> > /b/s/w/iru18BgP/test/test262/data/harness/sta.js
> > /b/s/w/iru18BgP/test/test262/data/harness/assert.js
> > /b/s/w/iru18BgP/test/test262/harness-adapt.js
> > /b/s/w/iru18BgP/test/test262/data/harness/testIntl.js
> > /b/s/w/iru18BgP/test/test262/local-tests/test/intl402/6.2.2_b.js
> > --invoke-weak-callbacks --omit-quit
> > 
> 
> > Run #1
> > Exit code: 1
> > Result: FAIL
> > Expected outcomes: PASS
> > Duration: 00:00:111
> > 
> > Stdout:
> > /b/s/w/iru18BgP/test/test262/data/harness/testIntl.js:30: Test262Error:
> Invalid
> > language tag de-tester-Tester was not rejected. (Testing with Collator.)
> >             throw e;
> >             ^

That test is out for review in
https://codereview.chromium.org/2639333003/ . Seems like there must be some sort
of infrastructure issue that I didn't handle in
https://docs.google.com/document/d/16bj7AIDgZLv4WOsUEzQ5NzcEN9_xo095e88Pz8FC5...
properly. Maybe the bots don't clear out newly added files in that directory
between runs. +machenbach do you have any thoughts?

[adamk, 2017-01-19 00:48:53]

On 2017/01/18 23:23:20, Dan Ehrenberg wrote:
> On 2017/01/18 21:05:12, jungshik at Google wrote:
> > On 2017/01/18 20:46:36, jungshik at Google wrote:
> > > Dan, I don't see test/test262/local-tests/test/intl402/6.2.2_b.js in my
tree
> > > (after git pull and gclient sync). Am I missing something? 
> > 
> > I found 
> > test/test262/data/test/intl402/6.2.2_b.js. instead. However, it does not
have
> > 'de-tester-Tester'. 
> > Anyway, we're failing that test. Regex in i18n.js does not check for
> duplicated
> > variant subtags ('tester' and 'Tester'), but 
> > it has nothing to do with this CL. That is, it should fail even without this
> CL.
> > 
> > 
> > 
> > > 
> > > Test: test262/intl402/6.2.2_b
> > > Flags: --use-strict
> > > Command: /b/s/w/iru18BgP/out/Release/d8 --test --random-seed=831298184
> > > --use-strict --nohard-abort --nodead-code-elimination --nofold-constants
> > > /b/s/w/iru18BgP/test/test262/data/harness/sta.js
> > > /b/s/w/iru18BgP/test/test262/data/harness/assert.js
> > > /b/s/w/iru18BgP/test/test262/harness-adapt.js
> > > /b/s/w/iru18BgP/test/test262/data/harness/testIntl.js
> > > /b/s/w/iru18BgP/test/test262/local-tests/test/intl402/6.2.2_b.js
> > > --invoke-weak-callbacks --omit-quit
> > > 
> > 
> > > Run #1
> > > Exit code: 1
> > > Result: FAIL
> > > Expected outcomes: PASS
> > > Duration: 00:00:111
> > > 
> > > Stdout:
> > > /b/s/w/iru18BgP/test/test262/data/harness/testIntl.js:30: Test262Error:
> > Invalid
> > > language tag de-tester-Tester was not rejected. (Testing with Collator.)
> > >             throw e;
> > >             ^
> 
> That test is out for review in
> https://codereview.chromium.org/2639333003/ . Seems like there must be some
sort
> of infrastructure issue that I didn't handle in
>
https://docs.google.com/document/d/16bj7AIDgZLv4WOsUEzQ5NzcEN9_xo095e88Pz8FC5...
> properly. Maybe the bots don't clear out newly added files in that directory
> between runs. +machenbach do you have any thoughts?

See this bit of GN documentation:

https://chromium.googlesource.com/chromium/src/tools/gn/+/HEAD/docs/reference...

So archive_test262 will never do the right thing if the only change is the
deletion of a file. This needs re-thinking...

[Dan Ehrenberg, 2017-01-19 01:46:15]

The CQ bit was checked by littledan@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-19 01:46:22]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-19 02:11:26]

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

[commit-bot: I haz the power, 2017-01-19 02:11:29]

Dry run: This issue passed the CQ dry run.

[Michael Achenbach, 2017-01-19 08:06:38]

On 2017/01/19 00:48:53, adamk wrote:
> On 2017/01/18 23:23:20, Dan Ehrenberg wrote:
> > On 2017/01/18 21:05:12, jungshik at Google wrote:
> > > On 2017/01/18 20:46:36, jungshik at Google wrote:
> > > > Dan, I don't see test/test262/local-tests/test/intl402/6.2.2_b.js in my
> tree
> > > > (after git pull and gclient sync). Am I missing something? 
> > > 
> > > I found 
> > > test/test262/data/test/intl402/6.2.2_b.js. instead. However, it does not
> have
> > > 'de-tester-Tester'. 
> > > Anyway, we're failing that test. Regex in i18n.js does not check for
> > duplicated
> > > variant subtags ('tester' and 'Tester'), but 
> > > it has nothing to do with this CL. That is, it should fail even without
this
> > CL.
> > > 
> > > 
> > > 
> > > > 
> > > > Test: test262/intl402/6.2.2_b
> > > > Flags: --use-strict
> > > > Command: /b/s/w/iru18BgP/out/Release/d8 --test --random-seed=831298184
> > > > --use-strict --nohard-abort --nodead-code-elimination --nofold-constants
> > > > /b/s/w/iru18BgP/test/test262/data/harness/sta.js
> > > > /b/s/w/iru18BgP/test/test262/data/harness/assert.js
> > > > /b/s/w/iru18BgP/test/test262/harness-adapt.js
> > > > /b/s/w/iru18BgP/test/test262/data/harness/testIntl.js
> > > > /b/s/w/iru18BgP/test/test262/local-tests/test/intl402/6.2.2_b.js
> > > > --invoke-weak-callbacks --omit-quit
> > > > 
> > > 
> > > > Run #1
> > > > Exit code: 1
> > > > Result: FAIL
> > > > Expected outcomes: PASS
> > > > Duration: 00:00:111
> > > > 
> > > > Stdout:
> > > > /b/s/w/iru18BgP/test/test262/data/harness/testIntl.js:30: Test262Error:
> > > Invalid
> > > > language tag de-tester-Tester was not rejected. (Testing with Collator.)
> > > >             throw e;
> > > >             ^
> > 
> > That test is out for review in
> > https://codereview.chromium.org/2639333003/ . Seems like there must be some
> sort
> > of infrastructure issue that I didn't handle in
> >
>
https://docs.google.com/document/d/16bj7AIDgZLv4WOsUEzQ5NzcEN9_xo095e88Pz8FC5...
> > properly. Maybe the bots don't clear out newly added files in that directory
> > between runs. +machenbach do you have any thoughts?
> 
> See this bit of GN documentation:
> 
>
https://chromium.googlesource.com/chromium/src/tools/gn/+/HEAD/docs/reference...
> 
> So archive_test262 will never do the right thing if the only change is the
> deletion of a file. This needs re-thinking...

That's probably right. I'm not even sure if the approach ever worked
sufficiently with gyp. But we can't do without it. Without the archiving, jobs
time out on swarming due to download slow down. The built-in archiving on
upload/download is still not delivered on swarming, but we can try to find out.

Another solution would be to check-in a source-list-file and provide a script to
update it each time somebody wants to add/remove files in test262.

[Dan Ehrenberg, 2017-01-19 19:36:04]

lgtm

[jungshik at Google, 2017-01-19 21:09:14]

The CQ bit was checked by jshin@chromium.org

[jungshik at Google, 2017-01-19 21:09:15]

The patchset sent to the CQ was uploaded after l-g-t-m from yangguo@chromium.org
Link to the patchset: https://codereview.chromium.org/2588963002/#ps140001
(title: "add fail tests for Lower,UpperCase")

[commit-bot: I haz the power, 2017-01-19 21:09:21]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[jungshik at Google, 2017-01-19 21:09:29]

The CQ bit was unchecked by jshin@chromium.org

[jungshik at Google, 2017-01-19 21:11:27]

Description was changed from

==========
Turn on icu_case_mapping by default

Update string-capitalize expected result because now it
passes all the tests in the file.
Mark fast/js/string-capitalization as failing with no_i18n.

Relanding after revert because the failure was taken care of
by Adam's CL at https://codereview.chromium.org/2597543002 .

3rd langing after a crash is taken care of in
https://codereview.chromium.org/2621393002

In addition to the previous CLs that landed and reverted,
drop String.prototype.to(Locale){Upper,Lower}Case from the
whitelist of built-in functions for side-effect-free-debugging.

BUG=v8:4477, v8:4476
TEST=test262/{built-ins,intl402}/Strings/*, webkit/fast/js/*,
     mjsunit/string-case, intl/general/case*

Cr-Original-Commit-Position: refs/heads/master@{#41834}
Committed:
https://chromium.googlesource.com/v8/v8/+/7c79e23c34ea971947eedc6e42d8a882617...
Review-Url: https://codereview.chromium.org/2588963002
Cr-Commit-Position: refs/heads/master@{#41883}
Committed:
https://chromium.googlesource.com/v8/v8/+/a42c8c67dece5328896b13d37c5c846a2a0...
==========

to

==========
Turn on icu_case_mapping by default

Update string-capitalize expected result because now it
passes all the tests in the file.
Mark fast/js/string-capitalization as failing with no_i18n.

Relanding after revert because the failure was taken care of
by Adam's CL at https://codereview.chromium.org/2597543002 .

3rd langing after a crash is taken care of in
https://codereview.chromium.org/2621393002

In addition to the previous version of this CL (PS #3) that landed and
reverted, drop String.prototype.to(Locale){Upper,Lower}Case from the
whitelist of built-in functions for side-effect-free-debugging.

BUG=v8:4477, v8:4476
TEST=test262/{built-ins,intl402}/Strings/*, webkit/fast/js/*,
     mjsunit/string-case, intl/general/case*

Cr-Original-Commit-Position: refs/heads/master@{#41834}
Committed:
https://chromium.googlesource.com/v8/v8/+/7c79e23c34ea971947eedc6e42d8a882617...
Review-Url: https://codereview.chromium.org/2588963002
Cr-Commit-Position: refs/heads/master@{#41883}
Committed:
https://chromium.googlesource.com/v8/v8/+/a42c8c67dece5328896b13d37c5c846a2a0...
==========

[jungshik at Google, 2017-01-19 21:12:02]

Description was changed from

==========
Turn on icu_case_mapping by default

Update string-capitalize expected result because now it
passes all the tests in the file.
Mark fast/js/string-capitalization as failing with no_i18n.

Relanding after revert because the failure was taken care of
by Adam's CL at https://codereview.chromium.org/2597543002 .

3rd langing after a crash is taken care of in
https://codereview.chromium.org/2621393002

In addition to the previous version of this CL (PS #3) that landed and
reverted, drop String.prototype.to(Locale){Upper,Lower}Case from the
whitelist of built-in functions for side-effect-free-debugging.

BUG=v8:4477, v8:4476
TEST=test262/{built-ins,intl402}/Strings/*, webkit/fast/js/*,
     mjsunit/string-case, intl/general/case*

Cr-Original-Commit-Position: refs/heads/master@{#41834}
Committed:
https://chromium.googlesource.com/v8/v8/+/7c79e23c34ea971947eedc6e42d8a882617...
Review-Url: https://codereview.chromium.org/2588963002
Cr-Commit-Position: refs/heads/master@{#41883}
Committed:
https://chromium.googlesource.com/v8/v8/+/a42c8c67dece5328896b13d37c5c846a2a0...
==========

to

==========
Turn on icu_case_mapping by default

Update string-capitalize expected result because now it
passes all the tests in the file.
Mark fast/js/string-capitalization as failing with no_i18n.

Relanding after revert because the failure was taken care of
by Adam's CL at https://codereview.chromium.org/2597543002 .

3rd langing after a crash is taken care of in
https://codereview.chromium.org/2621393002

In addition to the previous version of this CL (PS #4) that landed and
reverted, drop String.prototype.to(Locale){Upper,Lower}Case from the
whitelist of built-in functions for side-effect-free-debugging.

BUG=v8:4477, v8:4476
TEST=test262/{built-ins,intl402}/Strings/*, webkit/fast/js/*,
     mjsunit/string-case, intl/general/case*

Cr-Original-Commit-Position: refs/heads/master@{#41834}
Committed:
https://chromium.googlesource.com/v8/v8/+/7c79e23c34ea971947eedc6e42d8a882617...
Review-Url: https://codereview.chromium.org/2588963002
Cr-Commit-Position: refs/heads/master@{#41883}
Committed:
https://chromium.googlesource.com/v8/v8/+/a42c8c67dece5328896b13d37c5c846a2a0...
==========

[jungshik at Google, 2017-01-19 21:12:12]

The CQ bit was checked by jshin@chromium.org

[commit-bot: I haz the power, 2017-01-19 21:12:21]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-19 21:45:15]

CQ is committing da patch.

Bot data: {"patchset_id": 140001, "attempt_start_ts": 1484860332744600,
"parent_rev": "5ccc719a3169f4b05843cfc69de8e1b9e8e67e7e", "commit_rev":
"c70ec473baf72dc38e1def98d097b7fbf2ec544a"}

[commit-bot: I haz the power, 2017-01-19 21:45:21]

Description was changed from

==========
Turn on icu_case_mapping by default

Update string-capitalize expected result because now it
passes all the tests in the file.
Mark fast/js/string-capitalization as failing with no_i18n.

Relanding after revert because the failure was taken care of
by Adam's CL at https://codereview.chromium.org/2597543002 .

3rd langing after a crash is taken care of in
https://codereview.chromium.org/2621393002

In addition to the previous version of this CL (PS #4) that landed and
reverted, drop String.prototype.to(Locale){Upper,Lower}Case from the
whitelist of built-in functions for side-effect-free-debugging.

BUG=v8:4477, v8:4476
TEST=test262/{built-ins,intl402}/Strings/*, webkit/fast/js/*,
     mjsunit/string-case, intl/general/case*

Cr-Original-Commit-Position: refs/heads/master@{#41834}
Committed:
https://chromium.googlesource.com/v8/v8/+/7c79e23c34ea971947eedc6e42d8a882617...
Review-Url: https://codereview.chromium.org/2588963002
Cr-Commit-Position: refs/heads/master@{#41883}
Committed:
https://chromium.googlesource.com/v8/v8/+/a42c8c67dece5328896b13d37c5c846a2a0...
==========

to

==========
Turn on icu_case_mapping by default

Update string-capitalize expected result because now it
passes all the tests in the file.
Mark fast/js/string-capitalization as failing with no_i18n.

Relanding after revert because the failure was taken care of
by Adam's CL at https://codereview.chromium.org/2597543002 .

3rd langing after a crash is taken care of in
https://codereview.chromium.org/2621393002

In addition to the previous version of this CL (PS #4) that landed and
reverted, drop String.prototype.to(Locale){Upper,Lower}Case from the
whitelist of built-in functions for side-effect-free-debugging.

BUG=v8:4477, v8:4476
TEST=test262/{built-ins,intl402}/Strings/*, webkit/fast/js/*,
     mjsunit/string-case, intl/general/case*

Cr-Original-Original-Commit-Position: refs/heads/master@{#41834}
Committed:
https://chromium.googlesource.com/v8/v8/+/7c79e23c34ea971947eedc6e42d8a882617...
Review-Url: https://codereview.chromium.org/2588963002
Cr-Original-Commit-Position: refs/heads/master@{#41883}
Committed:
https://chromium.googlesource.com/v8/v8/+/a42c8c67dece5328896b13d37c5c846a2a0...
Review-Url: https://codereview.chromium.org/2588963002
Cr-Commit-Position: refs/heads/master@{#42524}
Committed:
https://chromium.googlesource.com/v8/v8/+/c70ec473baf72dc38e1def98d097b7fbf2e...
==========

[commit-bot: I haz the power, 2017-01-19 21:45:22]

Committed patchset #8 (id:140001) as
https://chromium.googlesource.com/v8/v8/+/c70ec473baf72dc38e1def98d097b7fbf2e...