Bug 92056 – Clear the external characters pointer of an AtomicHTMLToken before the raw token is cleared.

• Component: DOM
• Status: RESOLVED FIXED

[Kwang Yul Seo]

AtomicHTMLToken keeps a pointer to the HTMLToken's buffer instead of copying the characters for performance. Clear the external characters pointer before the raw token is cleared to make sure that we won't have a dangling pointer.

[Kwang Yul Seo]

Created attachment 153938 [details]
Patch

[Kwang Yul Seo]

Adam, this is the follow-up patch you requested in Bug 91981.

[Adam Barth]

Comment on attachment 153938 [details]
Patch

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

> Source/WebCore/html/parser/HTMLTreeBuilder.cpp:459
> +    if (token->type() == HTMLTokenTypes::Character)
> +        token->clearExternalCharacters();

I would just do this unconditionally.  There isn't any harm in overwriting m_externalCharacters with 0 for other token types and it saves us this branch.

[Adam Barth]

Thanks for writing this patch.  :)

[Kwang Yul Seo]

(In reply to comment #3)
> > Source/WebCore/html/parser/HTMLTreeBuilder.cpp:459
> > +    if (token->type() == HTMLTokenTypes::Character)
> > +        token->clearExternalCharacters();
> 
> I would just do this unconditionally.  There isn't any harm in overwriting m_externalCharacters with 0 for other token types and it saves us this branch.

Thanks for the review! I will change it before I land the patch.

[Kwang Yul Seo]

Committed r123536: <http://trac.webkit.org/changeset/123536>

[Adam Barth]

This assert is wrong now that we're calling the function unconditionally.

        ASSERT(m_type == Token::Type::Character);

[Kwang Yul Seo]

(In reply to comment #7)
> This assert is wrong now that we're calling the function unconditionally.
> 
>         ASSERT(m_type == Token::Type::Character);

Oops, sorry. I will fix it.

[Kwang Yul Seo]

(In reply to comment #8)
> (In reply to comment #7)
> > This assert is wrong now that we're calling the function unconditionally.
> > 
> >         ASSERT(m_type == Token::Type::Character);
> 
> Oops, sorry. I will fix it.

Done. http://trac.webkit.org/changeset/123542