[webkit-reviews] review granted: [Bug 109750] StartTagScanner should be thread-safe : [Attachment 188186] 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 109750: StartTagScanner should be thread-safe
https://bugs.webkit.org/show_bug.cgi?id=109750

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

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


LGTM.

>>> Source/WebCore/html/parser/HTMLPreloadScanner.cpp:112
>>>		 AtomicString attributeName(iter->name);
>> 
>> I believe this will crash on the background thread, no?  It looks like you
don't even use it as Atomic, but making it Atomic does prevent a copy in teh
common case on the main thread.
> 
> Yeah, processAttributes(const HTMLToken::AttributeList& attributes) is going
to be main thread only.  I plan to add a processAttributes that takes a
CompactHTMLToken::AttributeList or whatever the actual type is called.	I can
add an ASSERT(isMainThread()) to this function if that would be helpful.

I think ASSERTing when we should be on the main thread is helpful, yes.