Bug 85561

Created attachment 140338 [details] Patch

Comment on attachment 140338 [details] Patch Attachment 140338 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/12633291

I think the mac build failure isn't real.

Comment on attachment 140338 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=140338&action=review > Source/WebCore/page/ContentSecurityPolicy.cpp:921 > + for (PolicyList::const_iterator iter = m_policies.begin(); iter != m_policies.end(); ++iter) { > + if (!(*iter)->allowJavaScriptURLs()) > + return false; > + } > + return true; Really? Can't a helper or templates save us here? Some sort of function pointer?

> Really? Can't a helper or templates save us here? Some sort of function pointer? We can do it with macros, but that's pretty ugly. There might be a way to do it with templates... Do you know how?

Comment on attachment 140338 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=140338&action=review > Source/WebCore/page/ContentSecurityPolicy.cpp:892 > + for (PolicyList::const_iterator iter = other->m_policies.begin(); iter != other->m_policies.end(); ++iter) > + didReceiveHeader((*iter)->header(), (*iter)->headerType()); Our usual name for this is “it” rather than “iter” although I don’t think either is great. Normally we iterate a vector using indices rather than iterators, and use iterators only for compatibility with generic algorithms. >> Source/WebCore/page/ContentSecurityPolicy.cpp:921 >> + return true; > > Really? Can't a helper or templates save us here? Some sort of function pointer? I think we can do this cleanly with a template. > Source/WebCore/page/ContentSecurityPolicy.h:57 > + // FIXME: These functions are wrong becuase they assume that there is only one header. What’s the plan for those clients? > Source/WebCore/page/ContentSecurityPolicy.h:79 > + typedef Vector<OwnPtr<CSPDirectiveList> > PolicyList; PolicyVector? Unless the term list is a term of art here.

(In reply to comment #6) > (From update of attachment 140338 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=140338&action=review > > > Source/WebCore/page/ContentSecurityPolicy.h:57 > > + // FIXME: These functions are wrong becuase they assume that there is only one header. > > What’s the plan for those clients? I'm going to change these functions to return a vector. There's some amount of plumbing involved, which is why I'm saving it for a future patch.

Comment on attachment 140338 [details] Patch /me will attempt to templatize.

Created attachment 140438 [details] Patch

Created attachment 140443 [details] Patch

Comment on attachment 140443 [details] Patch Infinitely better. THank you.

Comment on attachment 140443 [details] Patch Clearing flags on attachment: 140443 Committed r116254: <http://trac.webkit.org/changeset/116254>

All reviewed patches have been landed. Closing bug.