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

Tony Gentilcore
Reported 2013-01-31 17:37:44 PST
Continue making XSSAuditor thread safe: Remove unsafe AtomicString compares
Attachments
Patch (3.10 KB, patch)
2013-01-31 17:39 PST, Tony Gentilcore 		no flags
DetailsFormatted DiffDiff
Patch (5.71 KB, patch)
2013-02-01 12:44 PST, Tony Gentilcore 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Tony Gentilcore
Comment 1 2013-01-31 17:39:24 PST
Created attachment 185906 [details] Patch
Tony Gentilcore
Comment 2 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.
Eric Seidel (no email)
Comment 3 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.
Adam Barth
Comment 4 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. :)
Tony Gentilcore
Comment 5 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?
Adam Barth
Comment 6 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.
Tony Gentilcore
Comment 7 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?
Adam Barth
Comment 8 2013-02-01 11:56:24 PST
Maybe we should add them to the existing HTMLParserIdioms.h ?
Tony Gentilcore
Comment 9 2013-02-01 12:44:02 PST
Created attachment 186109 [details] Patch
WebKit Review Bot
Comment 10 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>
WebKit Review Bot
Comment 11 2013-02-02 00:26:59 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.