Bug 65639 – Make atomic XML token

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Vicki Pfau]

Make atomic XML token

[Vicki Pfau]

Created attachment 102820 [details]
Patch

[Early Warning System Bot]

Comment on attachment 102820 [details]
Patch

Attachment 102820 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/9303123

[WebKit Review Bot]

Comment on attachment 102820 [details]
Patch

Attachment 102820 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/9301186

[Vicki Pfau]

Created attachment 102822 [details]
Patch

[Adam Barth]

Comment on attachment 102822 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=102822&action=review

This looks great.  Did you run the parser benchmark?

> Source/WebCore/html/parser/HTMLTokenizer.cpp:51
> +// This has to go in a .cpp file, as the linker doesn't like it being included more than once.
> +// We don't have an HTMLToken.cpp though, so this is the next best place.

Maybe we should add HTMLToken.cpp?  That seems sort of wasteful for just these three methods, but there could be more in the future.

> Source/WebCore/xml/parser/MarkupTokenBase.h:377
> +// FIXME: This class should eventually be named HTMLToken once we move the
> +// exiting HTMLToken to be internal to the HTMLTokenizer.

This FIXME should probably be removed.  Turns out making this class internal isn't worthwhile.

> Source/WebCore/xml/parser/XMLToken.h:436
> +    { }

These should each be on their own line.

[Vicki Pfau]

(In reply to comment #5)
> (From update of attachment 102822 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=102822&action=review
> 
> This looks great.  Did you run the parser benchmark?

I didn't until you reminded me, but they still appear to be on par. There doesn't seem to be any speed loss or gain.

> > Source/WebCore/html/parser/HTMLTokenizer.cpp:51
> > +// This has to go in a .cpp file, as the linker doesn't like it being included more than once.
> > +// We don't have an HTMLToken.cpp though, so this is the next best place.
> 
> Maybe we should add HTMLToken.cpp?  That seems sort of wasteful for just these three methods, but there could be more in the future.

Maybe so, but we can always do that later if we add more methods. I'm not sure it makes sense to add the files now.

[Vicki Pfau]

Created attachment 102836 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 102836 [details]
Patch for landing

Clearing flags on attachment: 102836

Committed r92325: <http://trac.webkit.org/changeset/92325>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.