[webkit-reviews] review denied: [Bug 40557] Add a preload scanner for the HTML5 parser : [Attachment 58714] Patch

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has denied Adam Barth <abarth at webkit.org>'s
request for review:
Bug 40557: Add a preload scanner for the HTML5 parser
https://bugs.webkit.org/show_bug.cgi?id=40557

Attachment 58714: Patch
https://bugs.webkit.org/attachment.cgi?id=58714&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
You said I should r- this.

But here are some comments anyway:
WebCore/html/HTML5PreloadScanner.cpp:51
 +	// FIXME: We should re-use these tokens in HTML5DocumentParser if the
We should save and re-use these tokens (otherwise I think you're meaning we
should use them right there and then)

WebCore/html/HTML5PreloadScanner.cpp:59
 +  void HTML5PreloadScanner::processToken()
This function is quite large.

WebCore/html/HTML5PreloadScanner.cpp:64
 +	AtomicString tagName(m_token.name().data(), m_token.name().size());
Strange the token doesn't expose a method to do this.  Like
makeAtomicTagName()?

WebCore/html/HTML5PreloadScanner.cpp:78
 +	for (HTML5Token::AttributeList::const_iterator iter =
m_token.attributes().begin();
Can this loop be a separate function?

WebCore/html/HTML5PreloadScanner.cpp:74
 +	String urlToLoad;
Should these be a struct?

WebCore/html/HTML5PreloadScanner.cpp:103
 +	if (tagName == scriptTag)
If the above was a struct, then this could be a separate "startPreloads()"
function or something, which was passed the struct.