RESOLVED FIXED 111249
Remove two unnecessary mallocs from the main-thread-parser code path 
https://bugs.webkit.org/show_bug.cgi?id=111249
Summary Remove two unnecessary mallocs from the main-thread-parser code path
Eric Seidel (no email)
Reported 2013-03-02 02:34:01 PST
Remove two unnecessary mallocs from the main-thread-parser code path
Attachments
Patch (4.31 KB, patch)
2013-03-02 02:52 PST, Eric Seidel (no email) 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Eric Seidel (no email)
Comment 1 2013-03-02 02:52:06 PST
Created attachment 191104 [details] Patch
Eric Seidel (no email)
Comment 2 2013-03-02 02:53:44 PST
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.
Adam Barth
Comment 3 2013-03-02 10:02:28 PST
Comment on attachment 191104 [details] 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.
Adam Barth
Comment 4 2013-03-02 10:04:11 PST
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.
Eric Seidel (no email)
Comment 5 2013-03-02 12:52:22 PST
(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.
Eric Seidel (no email)
Comment 6 2013-03-02 12:52:39 PST
(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.
Eric Seidel (no email)
Comment 7 2013-03-02 12:53:22 PST
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")
WebKit Review Bot
Comment 8 2013-03-02 13:08:57 PST
Comment on attachment 191104 [details] Patch Clearing flags on attachment: 191104 Committed r144544: <http://trac.webkit.org/changeset/144544>
WebKit Review Bot
Comment 9 2013-03-02 13:09:01 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.