Along the same lines as http://webk.it/86848, the error message for CSP-blocked `setTimeout` and `setInterval` calls don't give developers enough information to debug the issue. We should include a stack trace with the error so they can jump directly to the offending line. I've got a patch together that does this, but I'm a bit worried that it's making the CSP interface pretty complex (especially in conjunction with 86848, and some CSP 1.1 work). I'd also like to add a test to verify that a call stack is present, but I'm not exactly sure how I can track that down from JavaScript. I dug through the inspector tests currently in the system, but nothing seemed to fit the use case. Suggestions?
Created attachment 142866 [details] Patch
Comment on attachment 142866 [details] Patch Attachment 142866 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/12729378
(In reply to comment #0) > I'd also like to add a test to verify that a call stack is present, but I'm not exactly sure how I can track that down from JavaScript. I dug through the inspector tests currently in the system, but nothing seemed to fit the use case. Suggestions? I think that InspectorTest.dumpConsoleMessages() should do the trick (see LayoutTests/inspector/console/)
After digging a bit, it looks like the InspectorTest framework isn't playing nicely with CSP. If I inject `<meta http-equiv="X-WebKit-CSP" content="script-src 'unsafe-inline' 'self'">` into http/tests/inspector/console-cd.html, for example, it times out. I'm not exactly sure where it's dying, but it's entirely possible that it relies on `eval` (in the form of `RuntimeAgent.evaluate`).
The code in this patch looks fine, but we should have a test. I'm not sure what to do about the test harness using eval...
(In reply to comment #5) > The code in this patch looks fine, but we should have a test. I'm not sure what to do about the test harness using eval... I would recommend to add a function to window.internals that returns the recorded console messages, doing basically the same as InspectorTest.dumpConsoleMessages, just without depending on InspectorTest
(In reply to comment #6) > (In reply to comment #5) > > The code in this patch looks fine, but we should have a test. I'm not sure what to do about the test harness using eval... > > I would recommend to add a function to window.internals that returns the recorded console messages, doing basically the same as InspectorTest.dumpConsoleMessages, just without depending on InspectorTest +1
As it turns out, this isn't an `eval` problem. It's an inspector test issue. Long story short, if I copy my test into the `http/tests/inspector` directory, things work as expected. If I don't, I get timeouts. Jochen suggested that there might be a separate inspector agent process that's only spun up if certain tests are executed. It looks like `http/tests/inspector-enabled` is there for exactly this purpose; I'm adding tests there.
Created attachment 143014 [details] Patch
Comment on attachment 143014 [details] Patch Attachment 143014 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/12733602
Comment on attachment 143014 [details] Patch Looks like you've got some sort of problem on apple-win.
Yeah, it looks like the compiler is picking the wrong `isAllowedByAll` and throwing type errors. Would you mind terribly if I simply renamed the function to something like `isAllowedByAllWithCallStack` instead of overloading the `isAllowedByAll` that takes a KURL? We'd end up with something like CSP::isAllowedByAll, CSP::isAllowedByAllWithCallStack, and CSP::isAllowedByAllWithKURL, which is ugly, but simpler to understand than digging through Windows' template specialization rules. (It might, of course, just be something simple that I'm overlooking...)
That's fine. These functions are only used in this file, so it's ok if their names a bit wonky.
Created attachment 143070 [details] Patch
Comment on attachment 143070 [details] Patch Clearing flags on attachment: 143070 Committed r117826: <http://trac.webkit.org/changeset/117826>
All reviewed patches have been landed. Closing bug.