[webkit-reviews] review granted: [Bug 62971] view-source doesn't colorize </script> correctly : [Attachment 120621] 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 62971: view-source doesn't colorize </script> correctly
https://bugs.webkit.org/show_bug.cgi?id=62971

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

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


LGTM if the perf is OK.

> Source/WebCore/ChangeLog:35
> +	       previous segment of source.  We copy them into our accumulation
> +	       buffer and adjust the token base offset to account for the extra

> +	       characters.

Did you perf test this change?	Could you please mention the effect (if any) in
the ChangeLog?

> Source/WebCore/ChangeLog:56
> +	     - Previously, we cleared the temporary buffer lazily when we
needed
> +	       to add new characters to it.  Now we clear it eagerly so that
it's
> +	       length tells us whether we're currently using it to store
> +	       characters.

Moar perf fear.

> Source/WebCore/html/parser/HTMLTokenizer.cpp:168
> +    m_temporaryBuffer.clear();

Is temporary buffer a spec name?  Seems we could give htis a more descriptive
name about what it contains or how it's used...

> Source/WebCore/html/parser/HTMLTokenizer.cpp:1590
> +    characters.append('<');
> +    characters.append('/');

Can we assert that we're in the "waiting for end tag state"?  Seems this should
at least have a comment that the only time we buffer characters is when
searching for an end tag.