Bug 99230 – StringPrototype::jsSpliceSubstringsWithSeparators() doesn't optimally handle 8 bit strings

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Michael Saboff]

jsSpliceSubstringsWithSeparators() handles the case where both the source an separators are all 8 bit strings, but doesn't handle when they are mixed sizes.

[Michael Saboff]

Created attachment 168528 [details]
Patch

[Eric Seidel (no email)]

Comment on attachment 168528 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=168528&action=review

> Source/JavaScriptCore/runtime/StringPrototype.cpp:404
> +                    StringImpl::copyChars(buffer + bufferPos, separators[i].characters8(), sepLen);
> +                else
> +                    StringImpl::copyChars(buffer + bufferPos, separators[i].characters16(), sepLen);

Why not just have a StringImpl::copyChars() which takes a StringImpl and does this if internally?

[Michael Saboff]

(In reply to comment #2)
> (From update of attachment 168528 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=168528&action=review
> 
> > Source/JavaScriptCore/runtime/StringPrototype.cpp:404
> > +                    StringImpl::copyChars(buffer + bufferPos, separators[i].characters8(), sepLen);
> > +                else
> > +                    StringImpl::copyChars(buffer + bufferPos, separators[i].characters16(), sepLen);
> 
> Why not just have a StringImpl::copyChars() which takes a StringImpl and does this if internally?

We would likely need a few flavors.  This flavor would use the string length, but at least another one would need a string offset and a reduced length.

[Geoffrey Garen]

Comment on attachment 168528 [details]
Patch

r=me

[WebKit Review Bot]

Comment on attachment 168528 [details]
Patch

Clearing flags on attachment: 168528

Committed r131648: <http://trac.webkit.org/changeset/131648>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.