99230 2012-10-12 18:33:00 -0700 StringPrototype::jsSpliceSubstringsWithSeparators() doesn't optimally handle 8 bit strings 2012-10-17 14:54:51 -0700 1 1 1 Unclassified WebKit JavaScriptCore 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 1 msaboff msaboff webkit.review.bot oldest_to_newest 741378 0 msaboff 2012-10-12 18:33:00 -0700 jsSpliceSubstringsWithSeparators() handles the case where both the source an separators are all 8 bit strings, but doesn't handle when they are mixed sizes. 741379 1 168528 msaboff 2012-10-12 18:36:18 -0700 Created attachment 168528 Patch 742322 2 168528 eric 2012-10-15 12:51:33 -0700 Comment on attachment 168528 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? 743671 3 msaboff 2012-10-16 16:17:52 -0700 (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. 744642 4 168528 ggaren 2012-10-17 14:48:19 -0700 Comment on attachment 168528 Patch r=me 744655 5 168528 webkit.review.bot 2012-10-17 14:54:47 -0700 Comment on attachment 168528 Patch Clearing flags on attachment: 168528 Committed r131648: <http://trac.webkit.org/changeset/131648> 744656 6 webkit.review.bot 2012-10-17 14:54:51 -0700 All reviewed patches have been landed. Closing bug. 168528 2012-10-12 18:36:18 -0700 2012-10-17 14:54:47 -0700 Patch 99230.patch text/plain 2321 msaboff SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMTMxMjQ4KQorKysgU291cmNl L0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE2IEBA CisyMDEyLTEwLTEyICBNaWNoYWVsIFNhYm9mZiAgPG1zYWJvZmZAYXBwbGUuY29tPgorCisgICAg ICAgIFN0cmluZ1Byb3RvdHlwZTo6anNTcGxpY2VTdWJzdHJpbmdzV2l0aFNlcGFyYXRvcnMoKSBk b2Vzbid0IG9wdGltYWxseSBoYW5kbGUgOCBiaXQgc3RyaW5ncworICAgICAgICBodHRwczovL2J1 Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9OTkyMzAKKworICAgICAgICBSZXZpZXdlZCBi eSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBBZGRlZCBjb2RlIHRvIHNlbGVjdCBjaGFyYWN0 ZXJzOCgpIG9yIGNoYXJhY3RlcnMxNigpIG9uIHRoZSBub3QgYWxsIDggYml0IHBhdGggZm9yIGJv dGggdGhlIAorICAgICAgICBwcm9jZXNzaW5nIG9mIHRoZSBzb3VyY2UgYW5kIHRoZSBzZXBhcmF0 b3JzLgorCisgICAgICAgICogcnVudGltZS9TdHJpbmdQcm90b3R5cGUuY3BwOgorICAgICAgICAo SlNDOjpqc1NwbGljZVN1YnN0cmluZ3NXaXRoU2VwYXJhdG9ycyk6CisKIDIwMTItMTAtMTIgIE1h cmsgSGFobmVuYmVyZyAgPG1oYWhuZW5iZXJnQGFwcGxlLmNvbT4KIAogICAgICAgICBSYWNlIGNv bmRpdGlvbiBkdXJpbmcgQ29weWluZ1BoYXNlIGNhbiBsZWFkIHRvIGRlYWRsb2NrCkluZGV4OiBT b3VyY2UvSmF2YVNjcmlwdENvcmUvcnVudGltZS9TdHJpbmdQcm90b3R5cGUuY3BwCj09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT0KLS0tIFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9ydW50aW1lL1N0cmluZ1Byb3RvdHlwZS5jcHAJ KHJldmlzaW9uIDEzMTIwMikKKysrIFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9ydW50aW1lL1N0cmlu Z1Byb3RvdHlwZS5jcHAJKHdvcmtpbmcgY29weSkKQEAgLTM4OSwxMyArMzg5LDE5IEBAIHN0YXRp YyBBTFdBWVNfSU5MSU5FIEpTVmFsdWUganNTcGxpY2VTdWIKICAgICBmb3IgKGludCBpID0gMDsg aSA8IG1heENvdW50OyBpKyspIHsKICAgICAgICAgaWYgKGkgPCByYW5nZUNvdW50KSB7CiAgICAg ICAgICAgICBpZiAoaW50IHNyY0xlbiA9IHN1YnN0cmluZ1Jhbmdlc1tpXS5sZW5ndGgpIHsKLSAg ICAgICAgICAgICAgICBTdHJpbmdJbXBsOjpjb3B5Q2hhcnMoYnVmZmVyICsgYnVmZmVyUG9zLCBz b3VyY2UuY2hhcmFjdGVycygpICsgc3Vic3RyaW5nUmFuZ2VzW2ldLnBvc2l0aW9uLCBzcmNMZW4p OworICAgICAgICAgICAgICAgIGlmIChzb3VyY2UuaXM4Qml0KCkpCisgICAgICAgICAgICAgICAg ICAgIFN0cmluZ0ltcGw6OmNvcHlDaGFycyhidWZmZXIgKyBidWZmZXJQb3MsIHNvdXJjZS5jaGFy YWN0ZXJzOCgpICsgc3Vic3RyaW5nUmFuZ2VzW2ldLnBvc2l0aW9uLCBzcmNMZW4pOworICAgICAg ICAgICAgICAgIGVsc2UKKyAgICAgICAgICAgICAgICAgICAgU3RyaW5nSW1wbDo6Y29weUNoYXJz KGJ1ZmZlciArIGJ1ZmZlclBvcywgc291cmNlLmNoYXJhY3RlcnMxNigpICsgc3Vic3RyaW5nUmFu Z2VzW2ldLnBvc2l0aW9uLCBzcmNMZW4pOwogICAgICAgICAgICAgICAgIGJ1ZmZlclBvcyArPSBz cmNMZW47CiAgICAgICAgICAgICB9CiAgICAgICAgIH0KICAgICAgICAgaWYgKGkgPCBzZXBhcmF0 b3JDb3VudCkgewogICAgICAgICAgICAgaWYgKGludCBzZXBMZW4gPSBzZXBhcmF0b3JzW2ldLmxl bmd0aCgpKSB7Ci0gICAgICAgICAgICAgICAgU3RyaW5nSW1wbDo6Y29weUNoYXJzKGJ1ZmZlciAr IGJ1ZmZlclBvcywgc2VwYXJhdG9yc1tpXS5jaGFyYWN0ZXJzKCksIHNlcExlbik7CisgICAgICAg ICAgICAgICAgaWYgKHNlcGFyYXRvcnNbaV0uaXM4Qml0KCkpCisgICAgICAgICAgICAgICAgICAg IFN0cmluZ0ltcGw6OmNvcHlDaGFycyhidWZmZXIgKyBidWZmZXJQb3MsIHNlcGFyYXRvcnNbaV0u Y2hhcmFjdGVyczgoKSwgc2VwTGVuKTsKKyAgICAgICAgICAgICAgICBlbHNlCisgICAgICAgICAg ICAgICAgICAgIFN0cmluZ0ltcGw6OmNvcHlDaGFycyhidWZmZXIgKyBidWZmZXJQb3MsIHNlcGFy YXRvcnNbaV0uY2hhcmFjdGVyczE2KCksIHNlcExlbik7CiAgICAgICAgICAgICAgICAgYnVmZmVy UG9zICs9IHNlcExlbjsKICAgICAgICAgICAgIH0KICAgICAgICAgfQo=