CSP: group policies in didAddContentSecurityPolicy.

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

 /*
  * Copyright (C) 2011 Google, Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 307 matching lines @@
   }
 }
 
 void ContentSecurityPolicy::reportAccumulatedHeaders(
     LocalFrameClient* client) const {
   // Notify the embedder about headers that have accumulated before the
   // navigation got committed.  See comments in
   // 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.

   }
 }
 
 void ContentSecurityPolicy::addAndReportPolicyFromHeaderValue(
     const String& header,
     ContentSecurityPolicyHeaderType type,
     ContentSecurityPolicyHeaderSource source) {
   size_t previousPolicyCount = m_policies.size();
   addPolicyFromHeaderValue(header, type, source);
   if (document() && document()->frame()) {
     // Notify about the new header, so that it can be reported back to the
     // browser process.  This is needed in order to:
     // 1) replicate CSP directives (i.e. frame-src) to OOPIFs (only for now /
     // short-term).
     // 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.
+    // them are 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);
+    // 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());
+    }
   }
 }
 
 void ContentSecurityPolicy::setOverrideAllowInlineStyle(bool value) {
   m_overrideInlineStyleAllowed = value;
 }
 
 void ContentSecurityPolicy::setOverrideURLForSelf(const KURL& url) {
   // Create a temporary CSPSource so that 'self' expressions can be resolved
   // before we bind to an execution context (for 'frame-ancestor' resolution,
@@ skipping 1277 matching lines @@
   if (SecurityOrigin::shouldUseInnerURL(url)) {
     return SchemeRegistry::schemeShouldBypassContentSecurityPolicy(
         SecurityOrigin::extractInnerURL(url).protocol(), area);
   } else {
     return SchemeRegistry::schemeShouldBypassContentSecurityPolicy(
         url.protocol(), area);
   }
 }
 
 }  // namespace blink