Fix crash in preloading scanning base tags with no href attribute for background parser
Created attachment 189193 [details] Patch
Comment on attachment 189193 [details] Patch I was afraid of this. I think this is a good change, but I don't really like how we're tying HTMLToken and CompactToken to the same API via the PreloadScanner template. :(
Comment on attachment 189193 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=189193&action=review > Source/WebCore/html/parser/CompactHTMLToken.h:86 > + Vector<Attribute> m_attributes; Attribute contains AtomicString and QualifiedName, which means it's not save to move between threads.
s/save/safe/
Comment on attachment 189193 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=189193&action=review >> Source/WebCore/html/parser/CompactHTMLToken.h:86 >> + Vector<Attribute> m_attributes; > > Attribute contains AtomicString and QualifiedName, which means it's not save to move between threads. This is CompactHTMLToken::Attribute which contains only Strings. I can change all occurrences of Attribute to CompactHTMLToken::Attribute to be more explicit to make it more readable.
(In reply to comment #2) > (From update of attachment 189193 [details]) > I was afraid of this. I think this is a good change, but I don't really like how we're tying HTMLToken and CompactToken to the same API via the PreloadScanner template. :( Would you prefer I undo it? I thought it was worth having the same API in order to avoid the code duplication. It's also possible a more similar API enhances readability.
(In reply to comment #6) > (In reply to comment #2) > > (From update of attachment 189193 [details] [details]) > > I was afraid of this. I think this is a good change, but I don't really like how we're tying HTMLToken and CompactToken to the same API via the PreloadScanner template. :( > > Would you prefer I undo it? I thought it was worth having the same API in order to avoid the code duplication. It's also possible a more similar API enhances readability. No, this is the right change. I just don't like the idea that we're limited to HTMLToken's API in CompactToken::Attribute. But when we run up against those limits, then we can re-visit.
Ah! Ignore me. :)
(In reply to comment #8) > Ah! Ignore me. :) Do you want to review now?
Comment on attachment 189193 [details] Patch By the power of justice.
Comment on attachment 189193 [details] Patch Clearing flags on attachment: 189193 Committed r143415: <http://trac.webkit.org/changeset/143415>
All reviewed patches have been landed. Closing bug.