Add a mechanism for users to opt-out of CSP & XSS violations reports.

Add a mechanism for users to opt-out of CSP & XSS violations reports.

Added a setting to allow the users to opt out of sending CSP & XSS violation
reports. This just has Blink side changes.

BUG=392146

[Mayur Kankanwadi, 2014-09-23 11:20:14]

mayurk.vk@samsung.com changed reviewers:
+ mkwst@chromium.org

[Mike West, 2014-09-24 09:07:55]

This looks pretty reasonable. Two comments inline.

https://codereview.chromium.org/591413003/diff/1/Source/core/frame/Settings.in
File Source/core/frame/Settings.in (right):

https://codereview.chromium.org/591413003/diff/1/Source/core/frame/Settings.i...
Source/core/frame/Settings.in:301: CSPViolationReportsUploadOptOut initial=false
I'd suggest changing this to 'CSPViolationReportsEnabled initial=true'. There's
no hard and fast rule, as you'll probably notice from skimming through the
settings in this document, but I prefer settings in this form, rather than
dealing with a situation where "true" means "don't do this thing".

https://codereview.chromium.org/591413003/diff/1/Source/core/frame/csp/Conten...
File Source/core/frame/csp/ContentSecurityPolicy.cpp (right):

https://codereview.chromium.org/591413003/diff/1/Source/core/frame/csp/Conten...
Source/core/frame/csp/ContentSecurityPolicy.cpp:652: if
(document->settings()->CSPViolationReportsUploadOptOut())
This should happen after the DOM Event fires on line 663. You could fold it into
the `if (reportEndpoints.isEmpty())` clause, for instance.

[Mayur Kankanwadi, 2014-09-25 08:40:37]

Thanks for the review.
Incorporated the comments.
PTAL.
Thanks.

https://codereview.chromium.org/591413003/diff/1/Source/core/frame/Settings.in
File Source/core/frame/Settings.in (right):

https://codereview.chromium.org/591413003/diff/1/Source/core/frame/Settings.i...
Source/core/frame/Settings.in:301: CSPViolationReportsUploadOptOut initial=false
On 2014/09/24 09:07:55, Mike West wrote:
> I'd suggest changing this to 'CSPViolationReportsEnabled initial=true'.
There's
> no hard and fast rule, as you'll probably notice from skimming through the
> settings in this document, but I prefer settings in this form, rather than
> dealing with a situation where "true" means "don't do this thing".

Done.

https://codereview.chromium.org/591413003/diff/1/Source/core/frame/Settings.i...
Source/core/frame/Settings.in:301: CSPViolationReportsUploadOptOut initial=false
On 2014/09/24 09:07:55, Mike West wrote:
> I'd suggest changing this to 'CSPViolationReportsEnabled initial=true'.
There's
> no hard and fast rule, as you'll probably notice from skimming through the
> settings in this document, but I prefer settings in this form, rather than
> dealing with a situation where "true" means "don't do this thing".

Done.

https://codereview.chromium.org/591413003/diff/1/Source/core/frame/csp/Conten...
File Source/core/frame/csp/ContentSecurityPolicy.cpp (right):

https://codereview.chromium.org/591413003/diff/1/Source/core/frame/csp/Conten...
Source/core/frame/csp/ContentSecurityPolicy.cpp:652: if
(document->settings()->CSPViolationReportsUploadOptOut())
On 2014/09/24 09:07:55, Mike West wrote:
> This should happen after the DOM Event fires on line 663. You could fold it
into
> the `if (reportEndpoints.isEmpty())` clause, for instance.

Done.

[Mike West, 2014-09-25 16:59:38]

LGTM, if you promise to add a TestRunner method in a followup patch on the
Chromium side (see //content/shell/renderer/test_runner/test_runner.h), and then
to add layout tests which verify that reports aren't set if the setting is
toggled in a followup patch on the Blink side.

You'll still need an OWNER of public and source/web to review the CL, of course.

https://codereview.chromium.org/591413003/diff/20001/Source/core/frame/Settin...
File Source/core/frame/Settings.in (right):

https://codereview.chromium.org/591413003/diff/20001/Source/core/frame/Settin...
Source/core/frame/Settings.in:302: xssViolationReportsEnabled initial=true
Nit: Either uppercase or lowercase both "CSP" and "XSS". I don't care which you
choose, but please be consistent.

[Mayur Kankanwadi, 2014-09-25 17:57:32]

mayurk.vk@samsung.com changed reviewers:
+ abarth@chromium.org, jamesr@chromium.org

[Mayur Kankanwadi, 2014-09-25 17:58:44]

Hi Adam & James,
Please review the public and source/web folder related changes in this CL.
Thanks.

On 2014/09/25 16:59:38, Mike West wrote:
> LGTM, if you promise to add a TestRunner method in a followup patch on the
> Chromium side (see //content/shell/renderer/test_runner/test_runner.h), and
then
> to add layout tests which verify that reports aren't set if the setting is
> toggled in a followup patch on the Blink side.
> 
> You'll still need an OWNER of public and source/web to review the CL, of
course.
> 
>
https://codereview.chromium.org/591413003/diff/20001/Source/core/frame/Settin...
> File Source/core/frame/Settings.in (right):
> 
>
https://codereview.chromium.org/591413003/diff/20001/Source/core/frame/Settin...
> Source/core/frame/Settings.in:302: xssViolationReportsEnabled initial=true
> Nit: Either uppercase or lowercase both "CSP" and "XSS". I don't care which
you
> choose, but please be consistent.

[Mayur Kankanwadi, 2014-09-26 05:29:12]

Hi Mike,
Thanks for the review.
I will follow up this issue and complete the steps mentioned.
Let's see where this heads. :)

> LGTM, if you promise to add a TestRunner method in a followup patch on the
> Chromium side (see //content/shell/renderer/test_runner/test_runner.h), and
then
> to add layout tests which verify that reports aren't set if the setting is
> toggled in a followup patch on the Blink side.
> 
> You'll still need an OWNER of public and source/web to review the CL, of
course.
> 
>
https://codereview.chromium.org/591413003/diff/20001/Source/core/frame/Settin...
> File Source/core/frame/Settings.in (right):
> 
>
https://codereview.chromium.org/591413003/diff/20001/Source/core/frame/Settin...
> Source/core/frame/Settings.in:302: xssViolationReportsEnabled initial=true
> Nit: Either uppercase or lowercase both "CSP" and "XSS". I don't care which
you
> choose, but please be consistent.