Remove two unnecessary mallocs from the main-thread-parser code path

• Component: New Bugs
• Status: RESOLVED FIXED

[Eric Seidel (no email)]

Remove two unnecessary mallocs from the main-thread-parser code path

[Eric Seidel (no email)]

Created attachment 191104 [details]
Patch

[Eric Seidel (no email)]

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 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]

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)]

(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)]

(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)]

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 on attachment 191104 [details]
Patch

Clearing flags on attachment: 191104

Committed r144544: <http://trac.webkit.org/changeset/144544>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.