55645 2011-03-02 19:02:59 -0800 DatasetDOMStringMap::item and ::contains copies attribute name string 2011-03-29 15:10:33 -0700 1 1 1 Unclassified WebKit DOM 528+ (Nightly build) All All RESOLVED FIXED P2 Enhancement --- 1 eae eae abarth commit-queue dglazkov mitz sam oldest_to_newest 361304 0 eae 2011-03-02 19:02:59 -0800 DatasetDOMStringMap::item and DatasetDOMStringMap::contains both use propertyNameMatchesAttributeName to match dataset property names with attribute names. The current implementation does this matching by creating a new string that's converted from a attribute name to a property name. It also loops over the attribute string twice (first to check if it's valid and then to create the copy). This is unnecessary and rather inefficient. 361306 1 84510 eae 2011-03-02 19:05:38 -0800 Created attachment 84510 Patch 375545 2 84510 dglazkov 2011-03-29 11:03:23 -0700 Comment on attachment 84510 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=84510&action=review Ask abarth to take a gander, I am not very well-versed in character encodings and the challenges sorrounding them. I am just scared of them :) > Source/WebCore/dom/DatasetDOMStringMap.cpp:-80 > - String convertedName = convertAttributeNameToPropertyName(attributeName); Do we still need this helper? > Source/WebCore/dom/DatasetDOMStringMap.cpp:95 > + if (attribute[a] == '-' && a + 1 < attributeLength && attribute[a + 1] != '-') > + wordBoundary = true; > + else { > + if ((wordBoundary ? toASCIIUpper(attribute[a]) : attribute[a]) != property[p]) > + return false; > + p++; > + wordBoundary = false; > + } > + a++; Will this still work with non-ASCII characters? 375584 3 84510 eae 2011-03-29 11:31:23 -0700 Comment on attachment 84510 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=84510&action=review >> Source/WebCore/dom/DatasetDOMStringMap.cpp:-80 >> - String convertedName = convertAttributeNameToPropertyName(attributeName); > > Do we still need this helper? Yeah, at least for now. It's used in one more place. >> Source/WebCore/dom/DatasetDOMStringMap.cpp:95 >> + a++; > > Will this still work with non-ASCII characters? No, but then again neither will convertAttributeNameToPropertyName. 375586 4 84510 dglazkov 2011-03-29 11:33:47 -0700 Comment on attachment 84510 Patch alrighty. 375796 5 commit-queue 2011-03-29 15:07:37 -0700 The commit-queue encountered the following flaky tests while processing attachment 84510: http/tests/inspector/console-websocket-error.html bug 57392 (authors: pfeldman@chromium.org and yutak@chromium.org) The commit-queue is continuing to process your patch. 375800 6 84510 commit-queue 2011-03-29 15:10:28 -0700 Comment on attachment 84510 Patch Clearing flags on attachment: 84510 Committed r82332: <http://trac.webkit.org/changeset/82332> 375801 7 commit-queue 2011-03-29 15:10:33 -0700 All reviewed patches have been landed. Closing bug. 84510 2011-03-02 19:05:38 -0800 2011-03-29 15:10:28 -0700 Patch 55645.patch text/plain 4523 eae SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDgwMTk2KQorKysgU291cmNlL1dlYkNvcmUvQ2hhbmdl TG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTYgQEAKKzIwMTEtMDMtMDIgIEVtaWwgQSBF a2x1bmQgIDxlYWVAY2hyb21pdW0ub3JnPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAo T09QUyEpLgorCisgICAgICAgIERhdGFzZXRET01TdHJpbmdNYXA6Oml0ZW0gYW5kIDo6Y29udGFp bnMgY29waWVzIGF0dHJpYnV0ZSBuYW1lIHN0cmluZworICAgICAgICBodHRwczovL2J1Z3Mud2Vi a2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9NTU2NDUKKworICAgICAgICBDaGFuZ2UgcHJvcGVydHlO YW1lTWF0Y2hlc0F0dHJpYnV0ZU5hbWUgdG8gbWF0Y2ggd2l0aG91dCBjcmVhdGluZyBhIGNvcHkK KyAgICAgICAgb2YgdGhlIHN0cmluZy4KKworICAgICAgICAqIGRvbS9EYXRhc2V0RE9NU3RyaW5n TWFwLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OnByb3BlcnR5TmFtZU1hdGNoZXNBdHRyaWJ1dGVO YW1lKToKKwogMjAxMS0wMy0wMiAgTGV2aSBXZWludHJhdWIgIDxsZXZpd0BjaHJvbWl1bS5vcmc+ CiAKICAgICAgICAgUmV2aWV3ZWQgYnkgUnlvc3VrZSBOaXdhLgpJbmRleDogU291cmNlL1dlYkNv cmUvZG9tL0RhdGFzZXRET01TdHJpbmdNYXAuY3BwCj09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9XZWJD b3JlL2RvbS9EYXRhc2V0RE9NU3RyaW5nTWFwLmNwcAkocmV2aXNpb24gODAxNzkpCisrKyBTb3Vy Y2UvV2ViQ29yZS9kb20vRGF0YXNldERPTVN0cmluZ01hcC5jcHAJKHdvcmtpbmcgY29weSkKQEAg LTcyLDEzICs3MiwzMCBAQCBzdGF0aWMgU3RyaW5nIGNvbnZlcnRBdHRyaWJ1dGVOYW1lVG9Qcm9w CiAKIHN0YXRpYyBib29sIHByb3BlcnR5TmFtZU1hdGNoZXNBdHRyaWJ1dGVOYW1lKGNvbnN0IFN0 cmluZyYgcHJvcGVydHlOYW1lLCBjb25zdCBTdHJpbmcmIGF0dHJpYnV0ZU5hbWUpCiB7Ci0gICAg Ly8gRklYTUU6IFRoaXMgc2hvdWxkIGJlIGFibGUgdG8gbWF0Y2ggd2l0aG91dCBjcmVhdGluZyBh IG5ldyBzdHJpbmcuCi0KLSAgICBpZiAoIWlzVmFsaWRBdHRyaWJ1dGVOYW1lKGF0dHJpYnV0ZU5h bWUpKQorICAgIGlmICghYXR0cmlidXRlTmFtZS5zdGFydHNXaXRoKCJkYXRhLSIpKQogICAgICAg ICByZXR1cm4gZmFsc2U7CiAKLSAgICBTdHJpbmcgY29udmVydGVkTmFtZSA9IGNvbnZlcnRBdHRy aWJ1dGVOYW1lVG9Qcm9wZXJ0eU5hbWUoYXR0cmlidXRlTmFtZSk7Ci0gICAgcmV0dXJuIChjb252 ZXJ0ZWROYW1lID09IHByb3BlcnR5TmFtZSk7CisgICAgY29uc3QgVUNoYXIqIHByb3BlcnR5ID0g cHJvcGVydHlOYW1lLmNoYXJhY3RlcnMoKTsKKyAgICBjb25zdCBVQ2hhciogYXR0cmlidXRlID0g YXR0cmlidXRlTmFtZS5jaGFyYWN0ZXJzKCk7CisgICAgdW5zaWduZWQgcHJvcGVydHlMZW5ndGgg PSBwcm9wZXJ0eU5hbWUubGVuZ3RoKCk7CisgICAgdW5zaWduZWQgYXR0cmlidXRlTGVuZ3RoID0g YXR0cmlidXRlTmFtZS5sZW5ndGgoKTsKKyAgIAorICAgIHVuc2lnbmVkIGEgPSA1OworICAgIHVu c2lnbmVkIHAgPSAwOworICAgIGJvb2wgd29yZEJvdW5kYXJ5ID0gZmFsc2U7CisgICAgd2hpbGUg KGEgPCBhdHRyaWJ1dGVMZW5ndGggJiYgcCA8IHByb3BlcnR5TGVuZ3RoKSB7CisgICAgICAgIGlm IChhdHRyaWJ1dGVbYV0gPT0gJy0nICYmIGEgKyAxIDwgYXR0cmlidXRlTGVuZ3RoICYmIGF0dHJp YnV0ZVthICsgMV0gIT0gJy0nKQorICAgICAgICAgICAgd29yZEJvdW5kYXJ5ID0gdHJ1ZTsKKyAg ICAgICAgZWxzZSB7CisgICAgICAgICAgICBpZiAoKHdvcmRCb3VuZGFyeSA/IHRvQVNDSUlVcHBl cihhdHRyaWJ1dGVbYV0pIDogYXR0cmlidXRlW2FdKSAhPSBwcm9wZXJ0eVtwXSkKKyAgICAgICAg ICAgICAgICByZXR1cm4gZmFsc2U7CisgICAgICAgICAgICBwKys7CisgICAgICAgICAgICB3b3Jk Qm91bmRhcnkgPSBmYWxzZTsKKyAgICAgICAgfQorICAgICAgICBhKys7CisgICAgfQorCisgICAg cmV0dXJuIChhID09IGF0dHJpYnV0ZUxlbmd0aCAmJiBwID09IHByb3BlcnR5TGVuZ3RoKTsKIH0K IAogc3RhdGljIGJvb2wgaXNWYWxpZFByb3BlcnR5TmFtZShjb25zdCBTdHJpbmcmIG5hbWUpCklu ZGV4OiBMYXlvdXRUZXN0cy9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gTGF5b3V0VGVzdHMvQ2hh bmdlTG9nCShyZXZpc2lvbiA4MDE5NikKKysrIExheW91dFRlc3RzL0NoYW5nZUxvZwkod29ya2lu ZyBjb3B5KQpAQCAtMSwzICsxLDE0IEBACisyMDExLTAzLTAyICBFbWlsIEEgRWtsdW5kICA8ZWFl QGNocm9taXVtLm9yZz4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKwor ICAgICAgICBEYXRhc2V0RE9NU3RyaW5nTWFwOjppdGVtIGFuZCA6OmNvbnRhaW5zIGNvcGllcyBh dHRyaWJ1dGUgbmFtZSBzdHJpbmcKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hv d19idWcuY2dpP2lkPTU1NjQ1CisKKyAgICAgICAgKiBmYXN0L2RvbS9zY3JpcHQtdGVzdHMvZGF0 YXNldC1leHBlY3RlZC50eHQ6CisgICAgICAgICogZmFzdC9kb20vc2NyaXB0LXRlc3RzL2RhdGFz ZXQuanM6CisgICAgICAgICAgICBBZGRlZCBleHRyYSB0ZXN0IGNhc2VzIGZvciBnZXR0aW5nIGZp ZWxkcyBieSBwcm9wZXJ0eSBuYW1lLgorCiAyMDExLTAzLTAyICBNaWhhaSBQYXJwYXJpdGEgIDxt aWhhaXBAY2hyb21pdW0ub3JnPgogCiAgICAgICAgIFJldmlld2VkIGJ5IFRvbnkgR2VudGlsY29y ZS4KSW5kZXg6IExheW91dFRlc3RzL2Zhc3QvZG9tL2RhdGFzZXQtZXhwZWN0ZWQudHh0Cj09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT0KLS0tIExheW91dFRlc3RzL2Zhc3QvZG9tL2RhdGFzZXQtZXhwZWN0ZWQudHh0CShyZXZp c2lvbiA4MDE3OSkKKysrIExheW91dFRlc3RzL2Zhc3QvZG9tL2RhdGFzZXQtZXhwZWN0ZWQudHh0 CSh3b3JraW5nIGNvcHkpCkBAIC04LDYgKzgsMTAgQEAgUEFTUyB0ZXN0R2V0KCdkYXRhLWZvby1i YXInLCAnZm9vQmFyJykgaQogUEFTUyB0ZXN0R2V0KCdkYXRhLS0nLCAnLScpIGlzIHRydWUKIFBB U1MgdGVzdEdldCgnZGF0YS0tZm9vJywgJ0ZvbycpIGlzIHRydWUKIFBBU1MgdGVzdEdldCgnZGF0 YS0tLWZvbycsICctRm9vJykgaXMgdHJ1ZQorUEFTUyB0ZXN0R2V0KCdkYXRhLS0tZm9vLS1iYXIn LCAnLUZvby1CYXInKSBpcyB0cnVlCitQQVNTIHRlc3RHZXQoJ2RhdGEtLS1mb28tLS1iYXInLCAn LUZvby0tQmFyJykgaXMgdHJ1ZQorUEFTUyB0ZXN0R2V0KCdkYXRhLWZvby0nLCAnZm9vLScpIGlz IHRydWUKK1BBU1MgdGVzdEdldCgnZGF0YS1mb28tLScsICdmb28tLScpIGlzIHRydWUKIFBBU1Mg dGVzdEdldCgnZGF0YS1Gb28nLCAnZm9vJykgaXMgdHJ1ZQogUEFTUyB0ZXN0R2V0KCdkYXRhLScs ICcnKSBpcyB0cnVlCiBQQVNTIHRlc3RHZXQoJ2RhdGEtw6AnLCAnw6AnKSBpcyB0cnVlCkluZGV4 OiBMYXlvdXRUZXN0cy9mYXN0L2RvbS9zY3JpcHQtdGVzdHMvZGF0YXNldC5qcwo9PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 Ci0tLSBMYXlvdXRUZXN0cy9mYXN0L2RvbS9zY3JpcHQtdGVzdHMvZGF0YXNldC5qcwkocmV2aXNp b24gODAxNzkpCisrKyBMYXlvdXRUZXN0cy9mYXN0L2RvbS9zY3JpcHQtdGVzdHMvZGF0YXNldC5q cwkod29ya2luZyBjb3B5KQpAQCAtMTIsNiArMTIsMTAgQEAgc2hvdWxkQmVUcnVlKCJ0ZXN0R2V0 KCdkYXRhLWZvby1iYXInLCAnZgogc2hvdWxkQmVUcnVlKCJ0ZXN0R2V0KCdkYXRhLS0nLCAnLScp Iik7CiBzaG91bGRCZVRydWUoInRlc3RHZXQoJ2RhdGEtLWZvbycsICdGb28nKSIpOwogc2hvdWxk QmVUcnVlKCJ0ZXN0R2V0KCdkYXRhLS0tZm9vJywgJy1Gb28nKSIpOworc2hvdWxkQmVUcnVlKCJ0 ZXN0R2V0KCdkYXRhLS0tZm9vLS1iYXInLCAnLUZvby1CYXInKSIpOworc2hvdWxkQmVUcnVlKCJ0 ZXN0R2V0KCdkYXRhLS0tZm9vLS0tYmFyJywgJy1Gb28tLUJhcicpIik7CitzaG91bGRCZVRydWUo InRlc3RHZXQoJ2RhdGEtZm9vLScsICdmb28tJykiKTsKK3Nob3VsZEJlVHJ1ZSgidGVzdEdldCgn ZGF0YS1mb28tLScsICdmb28tLScpIik7CiBzaG91bGRCZVRydWUoInRlc3RHZXQoJ2RhdGEtRm9v JywgJ2ZvbycpIik7IC8vIEhUTUwgbG93ZXJjYXNlcyBhbGwgYXR0cmlidXRlcy4KIHNob3VsZEJl VHJ1ZSgidGVzdEdldCgnZGF0YS0nLCAnJykiKTsKIHNob3VsZEJlVHJ1ZSgidGVzdEdldCgnZGF0 YS1ceEUwJywgJ1x4RTAnKSIpOwo=