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.
Created attachment 153938 [details] Patch
Adam, this is the follow-up patch you requested in Bug 91981.
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.
Thanks for writing this patch. :)
(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.
Committed r123536: <http://trac.webkit.org/changeset/123536>
This assert is wrong now that we're calling the function unconditionally. ASSERT(m_type == Token::Type::Character);
(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.
(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