Summary: | Continue making XSSAuditor thread safe: Remove unsafe AtomicString compares | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Tony Gentilcore <tonyg> | ||||||
Component: | New Bugs | Assignee: | 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
Tony Gentilcore
2013-01-31 17:37:44 PST
Created attachment 185906 [details]
Patch
All the other compares use equalIgnoringNullity() which looks thread safe to me. Please let me know if I'm missing anything. 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 on attachment 185906 [details]
Patch
This patch is fine, but we should share this code, as Eric says. :)
(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? 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. (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? Maybe we should add them to the existing HTMLParserIdioms.h ? Created attachment 186109 [details]
Patch
Comment on attachment 186109 [details] Patch Clearing flags on attachment: 186109 Committed r141686: <http://trac.webkit.org/changeset/141686> All reviewed patches have been landed. Closing bug. |