[webkit-reviews] review denied: [Bug 53685] Add CSP to Document and pass the X-WebKit-CSP header from the MainResourceLoader to the Document. : [Attachment 81279] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Feb 4 14:39:44 PST 2011


Adam Barth <abarth at webkit.org> has denied jochen at chromium.org's request for
review:
Bug 53685: Add CSP to Document and pass the X-WebKit-CSP header from the
MainResourceLoader to the Document.
https://bugs.webkit.org/show_bug.cgi?id=53685

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

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

Mostly organizational comments.  We want to put as much of the "guts" inside
ContentSecurityPolicy as possible.  The outside code should be as oblivious as
possible.

> Source/WebCore/dom/Document.cpp:4622
> +void Document::parseContentSecurityPolicyHeader(const String& header)
> +{
> +    // FIXME: Actually parse the header.
> +    m_contentSecurityPolicy.setEnabled(!header.isEmpty());
> +}

parse should be a method on ContentSecurityPolicy.  It's the one that's going
to do the heavy lifting.

> Source/WebCore/dom/Document.h:1093
> +    void parseContentSecurityPolicyHeader(const String&);

We should just have an accessor for m_contentSecurityPolicy

> Source/WebCore/loader/MainResourceLoader.cpp:362
> +	   m_frame->loader()->didReceiveContentSecurityPolicyHeader(content);

I'd just change this to call through to the
document->contentSecurityPolicy()->didReceiveHeader(content), which can then
call parse internally.


More information about the webkit-reviews mailing list