Bug 108557

Summary: Continue making XSSAuditor thread safe: Remove unsafe AtomicString compares
Product: WebKit Reporter: Tony Gentilcore <tonyg>
Component: New BugsAssignee: Tony Gentilcore <tonyg>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, dbates, ojan.autocc, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 106127, 107603    
Attachments:
Description Flags
Patch
none
Patch none

Description Tony Gentilcore 2013-01-31 17:37:44 PST
Continue making XSSAuditor thread safe: Remove unsafe AtomicString compares
Comment 1 Tony Gentilcore 2013-01-31 17:39:24 PST
Created attachment 185906 [details]
Patch
Comment 2 Tony Gentilcore 2013-01-31 17:40:12 PST
All the other compares use equalIgnoringNullity() which looks thread safe to me. Please let me know if I'm missing anything.
Comment 3 Eric Seidel (no email) 2013-01-31 17:46:12 PST
Comment on attachment 185906 [details]
Patch

We need to fix AtomicStrings to work on the parser thread, and make QualifiedName know how to ASSERT when x-thread compares are made.

Separately, Adam would tell you we should share these functions until we do.  I'm not sure where to put them w/o causing yourself an enourmous add-a-file headache.
Comment 4 Adam Barth 2013-01-31 20:24:55 PST
Comment on attachment 185906 [details]
Patch

This patch is fine, but we should share this code, as Eric says.  :)
Comment 5 Tony Gentilcore 2013-02-01 09:54:44 PST
(In reply to comment #4)
> (From update of attachment 185906 [details])
> This patch is fine, but we should share this code, as Eric says.  :)

Any preferences on where to put these methods?
Comment 6 Adam Barth 2013-02-01 11:41:27 PST
I'd probably put them in a new header in the WebCore/html/parser directory.  We can move them to a more general location later if that's needed.
Comment 7 Tony Gentilcore 2013-02-01 11:42:18 PST
(In reply to comment #6)
> I'd probably put them in a new header in the WebCore/html/parser directory.  We can move them to a more general location later if that's needed.

ThreadUtils.h?
Comment 8 Adam Barth 2013-02-01 11:56:24 PST
Maybe we should add them to the existing HTMLParserIdioms.h ?
Comment 9 Tony Gentilcore 2013-02-01 12:44:02 PST
Created attachment 186109 [details]
Patch
Comment 10 WebKit Review Bot 2013-02-02 00:26:55 PST
Comment on attachment 186109 [details]
Patch

Clearing flags on attachment: 186109

Committed r141686: <http://trac.webkit.org/changeset/141686>
Comment 11 WebKit Review Bot 2013-02-02 00:26:59 PST
All reviewed patches have been landed.  Closing bug.