CSP: group policies in didAddContentSecurityPolicy.

third_party/WebKit/Source/core/frame/csp/ContentSecurityPolicy.cpp

Index: third_party/WebKit/Source/core/frame/csp/ContentSecurityPolicy.cpp
diff --git a/third_party/WebKit/Source/core/frame/csp/ContentSecurityPolicy.cpp b/third_party/WebKit/Source/core/frame/csp/ContentSecurityPolicy.cpp
index d2969188c519c1d01d8e902d268bb0e5d0797045..e166fbb82344f7c80a54e07e6210db373081eb87 100644
--- a/third_party/WebKit/Source/core/frame/csp/ContentSecurityPolicy.cpp
+++ b/third_party/WebKit/Source/core/frame/csp/ContentSecurityPolicy.cpp
@@ -325,9 +325,9 @@ void ContentSecurityPolicy::reportAccumulatedHeaders(
   // addAndReportPolicyFromHeaderValue for more details and context.
   DCHECK(client);
   for (const auto& policy : m_policies) {
-    client->didAddContentSecurityPolicy(
-        policy->header(), policy->headerType(), policy->headerSource(),
-        {policy->exposeForNavigationalChecks()});
+    client->didAddContentSecurityPolicy(policy->header(), policy->headerType(),
+                                        policy->headerSource(),
+                                        policy->exposeForNavigationalChecks());

[Mike West, 2017/03/22 09:45:53]

It seems like doing the opposite might be more performant. That is, we're
awaiting a vector, so why not build a vector of the page's 15 policies and send
them up, rather than doing 15 IPC hops?

[arthursonzogni, 2017/03/22 10:27:09]

I agree, I did this CL in order to improve the simplicity of the code. It is a
sort of follow up of the comments I had in:
https://codereview.chromium.org/2655463006/diff/470001/content/browser/frame_...

If the performance is an issue, maybe we could go deeper in the opposite side as
you said.

several IPC(one header, several policies) [Current]
several IPC(one header, one policy)       [My CL]
one IPC(several header, several policies) [The suggestion]

In the next CL, I will try this.

   }
 }
 
@@ -345,15 +345,18 @@ void ContentSecurityPolicy::addAndReportPolicyFromHeaderValue(
     // 2) enforce CSP in the browser process (long-term - see
     // https://crbug.com/376522).
     // TODO(arthursonzogni): policies are actually replicated (1) and some of
-    // them are (or will) be enforced on the browser process (2). Stop doing (1)
-    // when (2) is finished.
-
-    // Zero, one or several policies could be produced by only one header.
-    std::vector<blink::WebContentSecurityPolicyPolicy> policies;
-    for (size_t i = previousPolicyCount; i < m_policies.size(); ++i)
-      policies.push_back(m_policies[i]->exposeForNavigationalChecks());
-    document()->frame()->client()->didAddContentSecurityPolicy(
-        header, type, source, policies);
+    // them are enforced on the browser process (2). Stop doing (1) when (2) is
+    // finished.
+
+    // RFC2616, section 4.2 specifies that headers appearing multiple times can
+    // be combined with a comma. That's why a single header could causes several
+    // policies to be added.

[Mike West, 2017/03/22 09:45:53]

I don't think this is necessary; we say something along the same lines when
parsing the policies in `addPolicyFromHeaderValue`. Here, I think we can simply
take it for granted that more than one policy might exist.

[arthursonzogni, 2017/03/22 10:27:09]

Okay, I will remove this comment.

+    for (size_t i = previousPolicyCount; i < m_policies.size(); ++i) {
+      document()->frame()->client()->didAddContentSecurityPolicy(
+          m_policies[i]->header(), m_policies[i]->headerType(),
+          m_policies[i]->headerSource(),
+          m_policies[i]->exposeForNavigationalChecks());
+    }
   }
 }