111249 2013-03-02 02:34:01 -0800 Remove two unnecessary mallocs from the main-thread-parser code path 2013-03-03 01:04:58 -0800 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 106127 111264 1 eric eric abarth dbates esprehn+autocc ojan.autocc webkit.review.bot oldest_to_newest 845782 0 eric 2013-03-02 02:34:01 -0800 Remove two unnecessary mallocs from the main-thread-parser code path 845785 1 191104 eric 2013-03-02 02:52:06 -0800 Created attachment 191104 Patch 845786 2 eric 2013-03-02 02:53:44 -0800 It actually ended up being 3 unnecessary mallocs, one of which affects both code paths. I also killed nameString() since it was just getting us in trouble. 845813 3 191104 abarth 2013-03-02 10:02:28 -0800 Comment on attachment 191104 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=191104&action=review > Source/WebCore/html/parser/XSSAuditor.cpp:116 > +// If other files need this, we should move this to HTMLParserIdioms.h We should move it to HTMLParserIdioms.h anyway since that's where the the other threadSafeMatch function is. 845814 4 abarth 2013-03-02 10:04:11 -0800 You might also be interested in the mallocs in XSSAuditor::eraseDangerousAttributesIfInjected: bool valueContainsJavaScriptURL = !isInlineEventHandler && protocolIsJavaScript(stripLeadingAndTrailingHTMLSpaces(String(attribute.value))); We shouldn't need to call malloc at all on that line, but we can end up calling it twice. 845851 5 eric 2013-03-02 12:52:22 -0800 (In reply to comment #3) > (From update of attachment 191104 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=191104&action=review > > > Source/WebCore/html/parser/XSSAuditor.cpp:116 > > +// If other files need this, we should move this to HTMLParserIdioms.h > > We should move it to HTMLParserIdioms.h anyway since that's where the the other threadSafeMatch function is. The only problem with that is that then it would add QualifiedName.h to that header (because it's a template). I think we should hold off for now. 845852 6 eric 2013-03-02 12:52:39 -0800 (In reply to comment #4) > You might also be interested in the mallocs in XSSAuditor::eraseDangerousAttributesIfInjected: > > bool valueContainsJavaScriptURL = !isInlineEventHandler && protocolIsJavaScript(stripLeadingAndTrailingHTMLSpaces(String(attribute.value))); > > We shouldn't need to call malloc at all on that line, but we can end up calling it twice. Yeah, I'll take a look at the XSS auditor in more detail soon. 845853 7 eric 2013-03-02 12:53:22 -0800 I'm happy to move the function if you feel strongly (I originally put it there, but moved it here to avoid the #include "QualifiedName.h") 845858 8 191104 webkit.review.bot 2013-03-02 13:08:57 -0800 Comment on attachment 191104 Patch Clearing flags on attachment: 191104 Committed r144544: <http://trac.webkit.org/changeset/144544> 845859 9 webkit.review.bot 2013-03-02 13:09:01 -0800 All reviewed patches have been landed. Closing bug. 191104 2013-03-02 02:52:06 -0800 2013-03-02 13:08:57 -0800 Patch bug-111249-20130302024821.patch text/plain 4415 eric U3VidmVyc2lvbiBSZXZpc2lvbjogMTQ0NTA3CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggNjM3ZTBjZTJkYTZhOTgz M2UxMWYwNDEyNTUzY2IwYjY5ZWY4MzE5ZS4uMjU2MjMxYzQ1NjEzM2I2M2EwNDYzMmFlMDAxMWM1 ZjA5MDA2YzIyNiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDI5IEBACisyMDEzLTAzLTAyICBFcmlj IFNlaWRlbCAgPGVyaWNAd2Via2l0Lm9yZz4KKworICAgICAgICBSZW1vdmUgdHdvIHVubmVjZXNz YXJ5IG1hbGxvY3MgZnJvbSB0aGUgbWFpbi10aHJlYWQtcGFyc2VyIGNvZGUgcGF0aAorICAgICAg ICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTExMjQ5CisKKyAgICAg ICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgSSBub3RpY2VkIHRoZXNl IHdoaWxlIGZpeGluZyB1cCBvdXIgVmVjdG9yIC0+IFN0cmluZyBjb252ZXJzaW9ucworICAgICAg ICBidXQgbmV2ZXIgd2VudCBiYWNrIHRvIGZpeCB0aGUgRklYTUUuCisKKyAgICAgICAgQXRvbWlj U3RyaW5nKFZlY3RvcjxVQ2hhciwgY2FwYWNpdHk+KSBpcyBzbWFydCBlbm91Z2ggdG8gYXZvaWQg bWFsbG9jaW5nCisgICAgICAgIGlmIHRoZSByZXByZXNlbnRlZCBzdHJpbmcgaXMgYWxyZWFkeSBp biB0aGUgQXRvbWljU3RyaW5nIHRhYmxlLiAgSXQKKyAgICAgICAgYWxzbyBoYW5kbGVzIDhiaXQg dnMuIDE2Yml0IGFuZCBlbXB0eSgpIGp1c3QgbGlrZSBuYW1lU3RyaW5nKCkgZG9lcy4KKworICAg ICAgICBJIGFsc28gcmVtb3ZlZCBhIDNyZCBjYWxsZXIgdG8gbmFtZVN0cmluZygpIGluIHRoZSBY U1NBdWRpdG9yIHdoaWNoCisgICAgICAgIHdhcyBjYXVzaW5nIGFuIHVubmVjZXNzYXJ5IG1hbGxv YyBpbiBib3RoIHRoZSBtYWluIGFuZCBiYWNrZ3JvdW5kCisgICAgICAgIHRocmVhZCBwYXJzZXIg cGF0aHMuCisKKyAgICAgICAgKiBodG1sL3BhcnNlci9BdG9taWNIVE1MVG9rZW4uaDoKKyAgICAg ICAgKFdlYkNvcmU6OkF0b21pY0hUTUxUb2tlbjo6QXRvbWljSFRNTFRva2VuKToKKyAgICAgICAg KiBodG1sL3BhcnNlci9IVE1MVG9rZW4uaDoKKyAgICAgICAgKiBodG1sL3BhcnNlci9YU1NBdWRp dG9yLmNwcDoKKyAgICAgICAgKFdlYkNvcmUpOgorICAgICAgICAoV2ViQ29yZTo6dGhyZWFkU2Fm ZU1hdGNoKToKKyAgICAgICAgKFdlYkNvcmU6Omhhc05hbWUpOgorCiAyMDEzLTAzLTAxICBBbmR5 IEVzdGVzICA8YWVzdGVzQGFwcGxlLmNvbT4KIAogICAgICAgICBSRUdSRVNTSU9OIChyMTI1ODA5 KTogQ0ZTdHJpbmdzIGNyZWF0ZWQgdmlhIFN0cmluZ0ltcGw6OmNyZWF0ZUNGU3RyaW5nKCkgIG1p Z2h0IHJlZmVyZW5jZSBmcmVlZCBtZW1vcnkgd2hlbiBPYmplY3RpdmUtQyBnYXJiYWdlIGNvbGxl Y3Rpb24gaXMgZW5hYmxlZApkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvaHRtbC9wYXJzZXIv QXRvbWljSFRNTFRva2VuLmggYi9Tb3VyY2UvV2ViQ29yZS9odG1sL3BhcnNlci9BdG9taWNIVE1M VG9rZW4uaAppbmRleCAwNDc3Yzg3YmYwY2JjMzE4NTI2MjEyZTkzN2Y4M2VhMjZhMjdkMTY2Li4x MzgzNzEwMDU1YzVlY2RkNGJkZTA0OGVmMzY5YWZmMWZlOTRlOGZmIDEwMDY0NAotLS0gYS9Tb3Vy Y2UvV2ViQ29yZS9odG1sL3BhcnNlci9BdG9taWNIVE1MVG9rZW4uaAorKysgYi9Tb3VyY2UvV2Vi Q29yZS9odG1sL3BhcnNlci9BdG9taWNIVE1MVG9rZW4uaApAQCAtMTUzLDcgKzE1Myw3IEBAIHBy aXZhdGU6CiAgICAgICAgICAgICBBU1NFUlRfTk9UX1JFQUNIRUQoKTsKICAgICAgICAgICAgIGJy ZWFrOwogICAgICAgICBjYXNlIEhUTUxUb2tlbjo6RE9DVFlQRToKLSAgICAgICAgICAgIG1fbmFt ZSA9IEF0b21pY1N0cmluZyh0b2tlbi5uYW1lU3RyaW5nKCkpOyAvLyBGSVhNRTogU2hvdWxkIGJl IEF0b21pY1N0cmluZyh0b2tlbi5uYW1lKCkpIHRvIGF2b2lkIG1hbGxvY2luZyBldmVyeSB0aW1l LgorICAgICAgICAgICAgbV9uYW1lID0gQXRvbWljU3RyaW5nKHRva2VuLm5hbWUoKSk7CiAgICAg ICAgICAgICBtX2RvY3R5cGVEYXRhID0gdG9rZW4ucmVsZWFzZURvY3R5cGVEYXRhKCk7CiAgICAg ICAgICAgICBicmVhazsKICAgICAgICAgY2FzZSBIVE1MVG9rZW46OkVuZE9mRmlsZToKQEAgLTE2 MSw3ICsxNjEsNyBAQCBwcml2YXRlOgogICAgICAgICBjYXNlIEhUTUxUb2tlbjo6U3RhcnRUYWc6 CiAgICAgICAgIGNhc2UgSFRNTFRva2VuOjpFbmRUYWc6IHsKICAgICAgICAgICAgIG1fc2VsZkNs b3NpbmcgPSB0b2tlbi5zZWxmQ2xvc2luZygpOwotICAgICAgICAgICAgbV9uYW1lID0gQXRvbWlj U3RyaW5nKHRva2VuLm5hbWVTdHJpbmcoKSk7IC8vIEZJWE1FOiBTaG91bGQgYmUgQXRvbWljU3Ry aW5nKHRva2VuLm5hbWUoKSkgdG8gYXZvaWQgbWFsbG9jaW5nIGV2ZXJ5IHRpbWUuCisgICAgICAg ICAgICBtX25hbWUgPSBBdG9taWNTdHJpbmcodG9rZW4ubmFtZSgpKTsKICAgICAgICAgICAgIGlu aXRpYWxpemVBdHRyaWJ1dGVzKHRva2VuLmF0dHJpYnV0ZXMoKSk7CiAgICAgICAgICAgICBicmVh azsKICAgICAgICAgfQpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvaHRtbC9wYXJzZXIvSFRN TFRva2VuLmggYi9Tb3VyY2UvV2ViQ29yZS9odG1sL3BhcnNlci9IVE1MVG9rZW4uaAppbmRleCA5 MjgzODJiNzMzZGY3YmVhNTE1MGFiZTUxMzA1Mjc2M2UxZWE4YThlLi5mM2VhMjgyMDUxNDAxY2Q1 OGM2YTk2ZmJiY2M2ZjIxODMzOTFkY2EwIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9odG1s L3BhcnNlci9IVE1MVG9rZW4uaAorKysgYi9Tb3VyY2UvV2ViQ29yZS9odG1sL3BhcnNlci9IVE1M VG9rZW4uaApAQCAtMTUxLDE2ICsxNTEsNiBAQCBwdWJsaWM6CiAgICAgICAgIG1fb3JBbGxEYXRh IHw9IGNoYXJhY3RlcjsKICAgICB9CiAKLSAgICAvLyBGSVhNRTogUmVuYW1lIHRoaXMgdG8gY29w eU5hbWVBc1N0cmluZygpLgotICAgIFN0cmluZyBuYW1lU3RyaW5nKCkgY29uc3QKLSAgICB7Ci0g ICAgICAgIGlmICghbV9kYXRhLnNpemUoKSkKLSAgICAgICAgICAgIHJldHVybiBlbXB0eVN0cmlu ZygpOwotICAgICAgICBpZiAoaXNBbGw4Qml0RGF0YSgpKQotICAgICAgICAgICAgcmV0dXJuIFN0 cmluZzo6bWFrZThCaXRGcm9tMTZCaXRTb3VyY2UobV9kYXRhLmRhdGEoKSwgbV9kYXRhLnNpemUo KSk7Ci0gICAgICAgIHJldHVybiBTdHJpbmcobV9kYXRhKTsKLSAgICB9Ci0KICAgICAvKiBET0NU WVBFIFRva2VucyAqLwogCiAgICAgYm9vbCBmb3JjZVF1aXJrcygpIGNvbnN0CmRpZmYgLS1naXQg YS9Tb3VyY2UvV2ViQ29yZS9odG1sL3BhcnNlci9YU1NBdWRpdG9yLmNwcCBiL1NvdXJjZS9XZWJD b3JlL2h0bWwvcGFyc2VyL1hTU0F1ZGl0b3IuY3BwCmluZGV4IGQ2YTQwZmYxODlmNjFlMmNkNTE3 MjViZjQyMDdkNWQ1Yzg1NWIxMGEuLmQ4MWQ2ZjE5ZDRlZmUzNjlmYjdhZDc5NTY0ZDZmNzc0ZjIz OTViY2YgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL2h0bWwvcGFyc2VyL1hTU0F1ZGl0b3Iu Y3BwCisrKyBiL1NvdXJjZS9XZWJDb3JlL2h0bWwvcGFyc2VyL1hTU0F1ZGl0b3IuY3BwCkBAIC0x MTMsOSArMTEzLDE2IEBAIHN0YXRpYyBib29sIHN0YXJ0c011bHRpTGluZUNvbW1lbnRBdChjb25z dCBTdHJpbmcmIHN0cmluZywgc2l6ZV90IHN0YXJ0KQogICAgIHJldHVybiAoc3RhcnQgKyAxIDwg c3RyaW5nLmxlbmd0aCgpICYmIHN0cmluZ1tzdGFydF0gPT0gJy8nICYmIHN0cmluZ1tzdGFydCsx XSA9PSAnKicpOwogfQogCisvLyBJZiBvdGhlciBmaWxlcyBuZWVkIHRoaXMsIHdlIHNob3VsZCBt b3ZlIHRoaXMgdG8gSFRNTFBhcnNlcklkaW9tcy5oCit0ZW1wbGF0ZTxzaXplX3QgaW5saW5lQ2Fw YWNpdHk+Citib29sIHRocmVhZFNhZmVNYXRjaChjb25zdCBWZWN0b3I8VUNoYXIsIGlubGluZUNh cGFjaXR5PiYgdmVjdG9yLCBjb25zdCBRdWFsaWZpZWROYW1lJiBxbmFtZSkKK3sKKyAgICByZXR1 cm4gZXF1YWxJZ25vcmluZ051bGxpdHkodmVjdG9yLCBxbmFtZS5sb2NhbE5hbWUoKS5pbXBsKCkp OworfQorCiBzdGF0aWMgYm9vbCBoYXNOYW1lKGNvbnN0IEhUTUxUb2tlbiYgdG9rZW4sIGNvbnN0 IFF1YWxpZmllZE5hbWUmIG5hbWUpCiB7Ci0gICAgcmV0dXJuIHRocmVhZFNhZmVNYXRjaCh0b2tl bi5uYW1lU3RyaW5nKCksIG5hbWUpOworICAgIHJldHVybiB0aHJlYWRTYWZlTWF0Y2godG9rZW4u bmFtZSgpLCBuYW1lKTsKIH0KIAogc3RhdGljIGJvb2wgZmluZEF0dHJpYnV0ZVdpdGhOYW1lKGNv bnN0IEhUTUxUb2tlbiYgdG9rZW4sIGNvbnN0IFF1YWxpZmllZE5hbWUmIG5hbWUsIHNpemVfdCYg aW5kZXhPZk1hdGNoaW5nQXR0cmlidXRlKQo=