This adds new methods to String object:

Add new Harmony methods to String.prototype object.

String.prototype.repeat
String.prototype.startsWith
String.prototype.endsWith
String.prototype.contains

http://people.mozilla.org/~jorendorff/es6-draft.html

BUG=v8:2796, v8:2797, v8:2798, v8:2799
TEST=mjsunit/string-repeat,mjsunit/string-startswith,mjsunit/string-endswith,mjsunit/string-contains
R=mstarzinger@chromium.org

Committed: http://code.google.com/p/v8/source/detail?r=15960

[Michael Starzinger, 2013-07-30 09:04:15]

I'll take a look at this. For the record, this CL is a combination of three
previous CLs that already contain a first round of comments:

https://codereview.chromium.org/20928002/
https://codereview.chromium.org/20673003/
https://codereview.chromium.org/20818005/

[Michael Starzinger, 2013-07-30 10:58:39]

Yep, looking good already. Semantics look OK. One last round of nits and then we
should be able to land this CL.

https://codereview.chromium.org/21014007/diff/3001/src/flag-definitions.h
File src/flag-definitions.h (right):

https://codereview.chromium.org/21014007/diff/3001/src/flag-definitions.h#new...
src/flag-definitions.h:176: DEFINE_bool(harmony_string, false, "enable harmony
string")
nit: s/harmony_string/harmony_strings/ (just for the flag) for consistency.

https://codereview.chromium.org/21014007/diff/3001/src/harmony-string.js
File src/harmony-string.js (right):

https://codereview.chromium.org/21014007/diff/3001/src/harmony-string.js#newc...
src/harmony-string.js:34: 
nit: Please add the separation marker as present in other builtin JS files here.

https://codereview.chromium.org/21014007/diff/3001/src/harmony-string.js#newc...
src/harmony-string.js:49: var i;
nit: Please move the variable declaration of "i" into the for-statement (i.e.
"for (var i = ...").

https://codereview.chromium.org/21014007/diff/3001/src/harmony-string.js#newc...
src/harmony-string.js:109: return %StringLastIndexOf(s, ss, start) === end -
ss_len;
nit: s/end - ss_len/start/ here, makes it easier to read.

https://codereview.chromium.org/21014007/diff/3001/src/harmony-string.js#newc...
src/harmony-string.js:138: 
nit: Please add the separation marker as present in other builtin JS files here.

https://codereview.chromium.org/21014007/diff/3001/test/mjsunit/string-contai...
File test/mjsunit/string-contains.js (right):

https://codereview.chromium.org/21014007/diff/3001/test/mjsunit/string-contai...
test/mjsunit/string-contains.js:27: 
This test package needs a "// Flags: --harmony-string" line here, otherwise the
tests won't pass. Also we should move this file into the ./test/mjsunit/harmony
directory. This comment applies to all four test files.

[ioseb, 2013-07-30 11:34:56]

On 2013/07/30 10:58:39, Michael Starzinger wrote:
> Yep, looking good already. Semantics look OK. One last round of nits and then
we
> should be able to land this CL.

Cool! Submitted the patch with changes.

> 
> https://codereview.chromium.org/21014007/diff/3001/src/flag-definitions.h
> File src/flag-definitions.h (right):
> 
>
https://codereview.chromium.org/21014007/diff/3001/src/flag-definitions.h#new...
> src/flag-definitions.h:176: DEFINE_bool(harmony_string, false, "enable harmony
> string")
> nit: s/harmony_string/harmony_strings/ (just for the flag) for consistency.
> 
> https://codereview.chromium.org/21014007/diff/3001/src/harmony-string.js
> File src/harmony-string.js (right):
> 
>
https://codereview.chromium.org/21014007/diff/3001/src/harmony-string.js#newc...
> src/harmony-string.js:34: 
> nit: Please add the separation marker as present in other builtin JS files
here.
> 
>
https://codereview.chromium.org/21014007/diff/3001/src/harmony-string.js#newc...
> src/harmony-string.js:49: var i;
> nit: Please move the variable declaration of "i" into the for-statement (i.e.
> "for (var i = ...").
> 
>
https://codereview.chromium.org/21014007/diff/3001/src/harmony-string.js#newc...
> src/harmony-string.js:109: return %StringLastIndexOf(s, ss, start) === end -
> ss_len;
> nit: s/end - ss_len/start/ here, makes it easier to read.
> 
>
https://codereview.chromium.org/21014007/diff/3001/src/harmony-string.js#newc...
> src/harmony-string.js:138: 
> nit: Please add the separation marker as present in other builtin JS files
here.
> 
>
https://codereview.chromium.org/21014007/diff/3001/test/mjsunit/string-contai...
> File test/mjsunit/string-contains.js (right):
> 
>
https://codereview.chromium.org/21014007/diff/3001/test/mjsunit/string-contai...
> test/mjsunit/string-contains.js:27: 
> This test package needs a "// Flags: --harmony-string" line here, otherwise
the
> tests won't pass. Also we should move this file into the
./test/mjsunit/harmony
> directory. This comment applies to all four test files.

[Michael Starzinger, 2013-07-30 11:44:01]

LGTM. I'll land this CL for you later today. Thanks for the contribution!

[ioseb, 2013-07-30 11:44:31]

Thank you!

On 2013/07/30 11:44:01, Michael Starzinger wrote:
> LGTM. I'll land this CL for you later today. Thanks for the contribution!

[Michael Starzinger, 2013-07-30 16:33:18]

Committed patchset #3 manually as r15960 (presubmit successful).