[webkit-reviews] review denied: [Bug 108726] Call XSSAuditor's didBlockScript() for the threaded HTML parser : [Attachment 186189] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sat Feb 2 00:13:47 PST 2013


Adam Barth <abarth at webkit.org> has denied Tony Gentilcore
<tonyg at chromium.org>'s request for review:
Bug 108726: Call XSSAuditor's didBlockScript() for the threaded HTML parser
https://bugs.webkit.org/show_bug.cgi?id=108726

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

------- Additional Comments from Adam Barth <abarth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=186189&action=review


> Source/WebCore/html/parser/CompactHTMLToken.cpp:98
> +    *m_didBlockScriptRequest = *other.didBlockScriptRequest();

This doesn't seem right.  OwnPtr isn't copyable.  We need to implement some
sort of fancier VectorTraits to teach Vector how to move the CompactHTMLToken. 
There should be examples already in the codebase of how to do this.

> Source/WebCore/html/parser/CompactHTMLToken.h:80
> +    DidBlockScriptRequest* didBlockScriptRequest() const;

The "did" in this function name is very strange.  Can we rename
DidBlockScriptRequest to XSSInfo ?

>> Source/WebCore/html/parser/XSSAuditorDelegate.cpp:44
>> +	return isStringSafeToSendToAnotherThread(m_reportURL.string())
> 
> I'm skeptical this is going to work but am not sure what to do here. Is it
going to be safe to send KURL across threads? How do we ASSERT that?

Maybe we should add a isSafeToSendToAnotherThread function to KURL?


More information about the webkit-reviews mailing list