Bug 112681

Created attachment 193819 [details] Patch

Comment on attachment 193819 [details] Patch Clearing r? until I've done a few rounds with the EWS bots, because I hate adding files. I'll be shocked if this compiles everywhere. :)

Comment on attachment 193819 [details] Patch Hrm. I am pleasantly shocked. :) r?ing. Would one of you fine folks on CC mind taking a look?

Comment on attachment 193819 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=193819&action=review This looks fine, but I'd like to look at it one more time to make sure the web-visible changes are properly hidden when CSP.next is disabled at runtime. > Source/WebCore/dom/Document.idl:344 > + [NotEnumerable, Conditional=CSP_NEXT] attribute EventListener onsecuritypolicyviolation; This attribute needs to be enabled at runtime or else it will be visible to the web. > Source/WebCore/dom/SecurityPolicyViolationEvent.h:36 > + }; No need for a ; here. > Source/WebCore/dom/SecurityPolicyViolationEvent.h:55 > + static PassRefPtr<SecurityPolicyViolationEvent> create(const String& documentURI, const String& referrer, const String& blockedURI, const String& violatedDirective, const String& effectiveDirective, const String& originalPolicy, const String& sourceURL, int lineNumber) I'd skip this one too. Even C++ code is better off using SecurityPolicyViolationEventInit. > Source/WebCore/dom/SecurityPolicyViolationEvent.h:65 > + void initSecurityPolicyViolationEvent(const AtomicString& eventType, bool canBubble, bool cancelable, const String& documentURI, const String& referrer, const String& blockedURI, const String& violatedDirective, const String& effectiveDirective, const String& originalPolicy, const String& sourceURL, int lineNumber) We should skip this function. We only implement these sorts of functions for legacy events. > Source/WebCore/dom/SecurityPolicyViolationEvent.idl:29 > + void initSecurityPolicyViolationEvent(in [Optional=DefaultIsUndefined] DOMString type, We should remove this function from the API. We're implementing new events in the DOM4 Events style. > Source/WebCore/page/DOMWindow.idl:573 > + [Conditional=CSP_NEXT] attribute SecurityPolicyViolationEventConstructor SecurityPolicyViolationEvent; This attribute should also be enabled at runtime.

Created attachment 193867 [details] Patch

(In reply to comment #5) > Created an attachment (id=193867) [details] > Patch Thanks, Adam. The runtime flag is important, thanks for remembering it. I've taken another pass; would you mind taking another look?

Comment on attachment 193867 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=193867&action=review > Source/WebCore/dom/SecurityPolicyViolationEvent.h:69 > + virtual const AtomicString& interfaceName() const { return eventNames().interfaceForSecurityPolicyViolationEvent; } Does this whole header need a CSP_NEXT ifdef? > Source/WebCore/dom/SecurityPolicyViolationEvent.h:76 > + SecurityPolicyViolationEvent(const String& documentURI, const String& referrer, const String& blockedURI, const String& violatedDirective, const String& effectiveDirective, const String& originalPolicy, const String& sourceURL, int lineNumber) This function now has no callers.

Created attachment 193919 [details] Patch

(In reply to comment #8) > Created an attachment (id=193919) [details] > Patch Addressed the bits of feedback from the last round, and I'll throw this to the EWS bots one more time, since there was a merge conflict in the xcode project. Assuming that the bots are happy, I'll CQ it in the morning. Thanks!

Comment on attachment 193919 [details] Patch Bots are happy. CQing. :)

Comment on attachment 193919 [details] Patch Clearing flags on attachment: 193919 Committed r146305: <http://trac.webkit.org/changeset/146305>

All reviewed patches have been landed. Closing bug.