Bug 109411 – Make WebVTTTokenizer stop inheriting from MarkupTokenizerBase

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

[Eric Seidel (no email)]

Make WebVTTTokenizer stop inheriting from MarkupTokenizerBase

[Eric Seidel (no email)]

Created attachment 187529 [details]
Patch

[Eric Seidel (no email)]

Created attachment 187609 [details]
Patch

[Eric Seidel (no email)]

Created attachment 187612 [details]
Patch

[kov's GTK+ EWS bot]

Comment on attachment 187612 [details]
Patch

Attachment 187612 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/16493297

[WebKit Review Bot]

Comment on attachment 187612 [details]
Patch

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

[Eric Seidel (no email)]

Sigh.  I was too tricky for some compilers.  Will fix.

[Tony Gentilcore]

Comment on attachment 187612 [details]
Patch

Need to fix compilation. Else looks good.

[Adam Barth]

Comment on attachment 187612 [details]
Patch

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

> Source/WebCore/html/parser/InputStreamPreprocessor.h:84
> +                if (m_tokenizer->shouldSkipNullCharacters()) {

Another option is to pass this bit in to peek rather than keeping a pointer to the tokenizer.  That would let your remove the template parameter.  Maybe in a followup patch?

[Eric Seidel (no email)]

Comment on attachment 187612 [details]
Patch

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

>> Source/WebCore/html/parser/InputStreamPreprocessor.h:84
>> +                if (m_tokenizer->shouldSkipNullCharacters()) {
> 
> Another option is to pass this bit in to peek rather than keeping a pointer to the tokenizer.  That would let your remove the template parameter.  Maybe in a followup patch?

Yup.  This function is ridiculously hot, so I didn't want to add that overhead to every callsite in the chnace that it changed perf.  We can try it in a separate patch.

[Eric Seidel (no email)]

Created attachment 187623 [details]
Patch

[Eric Seidel (no email)]

Comment on attachment 187623 [details]
Patch

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

> Source/WebCore/html/parser/HTMLInputStream.h:40
> +// The current segmented string is stored in InputStream. Each of afterInsertionPoint

Sublime got me.  Fixed.

[Eric Seidel (no email)]

Created attachment 187625 [details]
Patch

[WebKit Review Bot]

Comment on attachment 187625 [details]
Patch

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

[Eric Seidel (no email)]

Created attachment 187637 [details]
Patch

[WebKit Review Bot]

Comment on attachment 187637 [details]
Patch

Clearing flags on attachment: 187637

Committed r142497: <http://trac.webkit.org/changeset/142497>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.