Summary: | CSP policy violations should log to the console | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Adam Barth <abarth> | ||||||||||||
Component: | DOM | Assignee: | Nobody <webkit-unassigned> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | commit-queue, dglazkov, eric, webkit.review.bot | ||||||||||||
Priority: | P2 | ||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||
Hardware: | All | ||||||||||||||
OS: | All | ||||||||||||||
Bug Depends on: | |||||||||||||||
Bug Blocks: | 53572 | ||||||||||||||
Attachments: |
|
Description
Adam Barth
2011-04-15 00:29:08 PDT
Created attachment 90312 [details]
Patch
Comment on attachment 90312 [details]
Patch
These tests are flaky as written (order dependent). Will fix.
Attachment 90312 [details] did not build on chromium: Build output: http://queues.webkit.org/results/8473567 Created attachment 90318 [details]
Patch
Attachment 90312 [details] did not build on chromium: Build output: http://queues.webkit.org/results/8473573 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. Created attachment 90397 [details]
Patch for landing
> I would have added an origin() or securityOrigin() private method. BUt this is OK.
I did everything except this last one.
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 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
Created attachment 90473 [details]
Patch for landing
Comment on attachment 90473 [details] Patch for landing Clearing flags on attachment: 90473 Committed r84457: <http://trac.webkit.org/changeset/84457> All reviewed patches have been landed. Closing bug. |