[webkit-reviews] review granted: [Bug 88193] Duplicated CSP directives should throw a more accurate console error. : [Attachment 145490] Patch

[bugzilla-daemon at webkit.org]

Adam Barth <abarth at webkit.org> has granted Mike West <mkwst at chromium.org>'s
request for review:
Bug 88193: Duplicated CSP directives should throw a more accurate console
error.
https://bugs.webkit.org/show_bug.cgi?id=88193

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

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


This looks fine.  A bunch of style nits below.

> Source/WebCore/page/ContentSecurityPolicy.cpp:520
> -    PassOwnPtr<CSPDirective> createCSPDirective(const String& name, const
String& value);
> +    void setCSPDirective(OwnPtr<CSPDirective>&, const String& name, const
String& value);

This isn't a common pattern in WebKit, but it's fine.  In this case, the OwnPtr
is an in/out parameter for this function.  We usually put output parameters at
the end of the argument list.

> Source/WebCore/page/ContentSecurityPolicy.cpp:830
> +	 logDuplicateDirective(name);
> +	 return;

Four-space indent.

> Source/WebCore/page/ContentSecurityPolicy.cpp:852
> +    else

Prefer early return.

> Source/WebCore/page/ContentSecurityPolicy.cpp:859
> +	 logDuplicateDirective(name);

Four-space indent.

> Source/WebCore/page/ContentSecurityPolicy.cpp:860
> +    else {

Prefer early return.