[Webkit-unassigned] [Bug 131704] Simple ES6 feature:String prototype additions

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=131704





--- Comment #28 from Diego Pino <dpino at igalia.com>  2014-08-29 03:35:32 PST ---
(From update of attachment 237340)
View in context: https://bugs.webkit.org/attachment.cgi?id=237340&action=review

> Source/WTF/wtf/text/StringImpl.cpp:1454
> +    ASSERT(!(endOffset <= matchString.length() ^ startOffset <= 0));

This reads as if 'endOffset' is greater or equals than 'matchLength', 'startOffset' should be positive or zero. On the other hand, if 'endOffset' is lower or equals than 'matchLength', 'startOffset' should be negative.

> LayoutTests/js/script-tests/string-contains.js:56
> +sideEffect = 0;

It happened that offset.valueOf() was executed, so instead of making matchString.valueOf() to launch an exception, I did it the other way around. The offset.valueOf() launches an exception and check matchString.valueOf() was not run. I also changed the body of the JavaScript functions so arg(1) (offset) is fetched before arg(0) (matchString).

> LayoutTests/js/string-contains-expected.txt:42
> +PASS 'foo bar'.contains('bar', 0x7fffffff) is false

I didn't know what's the best way to validate an ASSERT. All functions return 0 when value of startOffset is negative. Now looking at it, startsWith and contains are returning false because the offset is larger than the length of the string. In the case of endsWith I think is working as 0x80000010 - 3 = 0x80000007 which is a negative number in int32_t.

I'm not sure about the idiom for checking for an overflow in startsWith and contains. I think it should check that the sum of the two values should be greater than any of them:

ASSERT(startOffset + matchString.length() >= startOffset);

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.