[webkit-reviews] review granted: [Bug 64398] Character reference parser for new XML parser : [Attachment 100743] Patch

[bugzilla-daemon at webkit.org]

Adam Barth <abarth at webkit.org> has granted Jeffrey Pfau <jeffrey at endrift.com>'s
request for review:
Bug 64398: Character reference parser for new XML parser
https://bugs.webkit.org/show_bug.cgi?id=64398

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

------- Additional Comments from Adam Barth <abarth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=100743&action=review


This looks great.  We could od some work to fixup some naming mistakes I made
when writing this code originally, but that's nto a big deal.  (The most
pervasive one is renaming "cc" to either currentCharacter or "c" as
appropriate.)

> Source/WebCore/xml/parser/CharacterReferenceParserInlineMethods.h:28
> +#ifndef CharacterReferenceParserInlineMethods_h
> +#define CharacterReferenceParserInlineMethods_h

I'd drop the word "Methods" from the name of this file/

> Source/WebCore/xml/parser/CharacterReferenceParserInlineMethods.h:51
> +template <typename ParserFunctions>
> +inline bool consumeCharacterReference(SegmentedString& source, Vector<UChar,
16>& decodedCharacter, bool& notEnoughCharacters, UChar
additionalAllowedCharacter)

jamesr says to get rid of this inline keyword.	That's probably the right.  The
compiler will probably just generate an unconditional jump anyway.

> Source/WebCore/xml/parser/XMLCharacterReferenceParser.cpp:45
> +	   return 0xFFFD;

bad indents in this file.

> Source/WebCore/xml/parser/XMLCharacterReferenceParser.cpp:66
> +	   ASSERT_NOT_REACHED();

Do you mean notImplemented() ?