Bug 58646 – CSP policy violations should log to the console

• Component: DOM
• Status: RESOLVED FIXED

[Adam Barth]

CSP policy violations should log to the console

[Adam Barth]

Created attachment 90312 [details]
Patch

[Adam Barth]

Comment on attachment 90312 [details]
Patch

These tests are flaky as written (order dependent).  Will fix.

[WebKit Review Bot]

Attachment 90312 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/8473567

[Adam Barth]

Created attachment 90318 [details]
Patch

[WebKit Review Bot]

Attachment 90312 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/8473573

[Eric Seidel (no email)]

Comment on attachment 90318 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=90318&action=review

Looks fine.  The only nit which really matters is the repeating of all that logging code.

> Source/WebCore/page/ContentSecurityPolicy.cpp:492
> +    if (!allowed) {

I might have reversed these ifs to flatten these blocks:
if (allowed)
    return allowed; (or simply true;)

But this is also OK.

> Source/WebCore/page/ContentSecurityPolicy.cpp:495
> +        if (Frame* frame = m_document->frame())
> +            frame->domWindow()->console()->addMessage(JSMessageSource, LogMessageType, ErrorMessageLevel, consoleMessage, 1, String());

Seems like these two lines want to be a helper.  We repeat them 7 times. :)

> Source/WebCore/page/ContentSecurityPolicy.cpp:702
> +        m_scriptSrc = adoptPtr(new CSPDirective(value, m_document->securityOrigin()));

I would have added an origin() or securityOrigin() private method.  BUt this is OK.

[Adam Barth]

Created attachment 90397 [details]
Patch for landing

[Adam Barth]

> I would have added an origin() or securityOrigin() private method.  BUt this is OK.

I did everything except this last one.

[WebKit Commit Bot]

Comment on attachment 90397 [details]
Patch for landing

Rejecting attachment 90397 [details] from commit-queue.

Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=eseidel-cq-sf', 'bu..." exit_code: 2

Last 500 characters of output:
mlmp .
http/tests/xmlhttprequest ................................................................................................................................................................................
http/tests/xmlhttprequest/web-apps ...............
http/tests/xmlhttprequest/workers ...........
http/tests/xmlviewer .
http/tests/xmlviewer/dumpAsText ............
747.48s total testing time

23330 test cases (99%) succeeded
1 test case (<1%) was new
15 test cases (<1%) had stderr output

Full output: http://queues.webkit.org/results/8468863

[WebKit Commit Bot]

Created attachment 90425 [details]
Archive of layout-test-results from eseidel-cq-sf

The attached test failures were seen while running run-webkit-tests on the commit-queue.
Bot: eseidel-cq-sf  Port: Mac  Platform: Mac OS X 10.6.4

[Adam Barth]

Created attachment 90473 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 90473 [details]
Patch for landing

Clearing flags on attachment: 90473

Committed r84457: <http://trac.webkit.org/changeset/84457>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.