[webkit-reviews] review granted: [Bug 106256] HTMLTreeBuilder should not depend on Frame : [Attachment 181569] Patch

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has granted Adam Barth <abarth at webkit.org>'s
request for review:
Bug 106256: HTMLTreeBuilder should not depend on Frame
https://bugs.webkit.org/show_bug.cgi?id=106256

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

------- Additional Comments from Eric Seidel <eric at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=181569&action=review


I'm slightly confused as to why you changed the test.  But otherwise this looks
fine.  I think you should move to a more explicit method name for getting the
right options before landing.

> Source/WebCore/html/parser/HTMLMetaCharsetParser.cpp:44
> +    : m_tokenizer(HTMLTokenizer::create(HTMLParserOptions())) // No
pre-HTML5 parser quirks.

This feels a bit odd.  Maybe we should give it a better name?  Like
HTMLParserOptions::defaultOptions()?  or
HTMLParserOptions::allOptionsDisabled()?

> Source/WebCore/html/parser/HTMLParserOptions.h:44
>	   , maximumDOMTreeDepth(0)

Is 0 the value you want for "default" in my previous comment?  That's part of
where my confusion comes from.	It's not clear what the empty/default
constructor is supposed to return there.