CSP: group policies in didAddContentSecurityPolicy.

content/renderer/render_frame_proxy.cc

Index: content/renderer/render_frame_proxy.cc
diff --git a/content/renderer/render_frame_proxy.cc b/content/renderer/render_frame_proxy.cc
index d6a89cef094a7691e07988a6f390865fcd5af42a..92790fcd62996dd163f5fbba1c2afe5f4955656e 100644
--- a/content/renderer/render_frame_proxy.cc
+++ b/content/renderer/render_frame_proxy.cc
@@ -231,8 +231,7 @@ void RenderFrameProxy::SetReplicatedState(const FrameReplicationState& state) {
     web_frame_->setHasReceivedUserGesture();
 
   web_frame_->resetReplicatedContentSecurityPolicy();
-  for (const auto& header : state.accumulated_csp_headers)
-    OnAddContentSecurityPolicy(header);
+  OnAddContentSecurityPolicies(state.accumulated_csp_headers);
 }
 
 // Update the proxy's SecurityContext and FrameOwner with new sandbox flags
@@ -276,8 +275,8 @@ bool RenderFrameProxy::OnMessageReceived(const IPC::Message& msg) {
     IPC_MESSAGE_HANDLER(FrameMsg_DidUpdateSandboxFlags, OnDidUpdateSandboxFlags)
     IPC_MESSAGE_HANDLER(FrameMsg_DispatchLoad, OnDispatchLoad)
     IPC_MESSAGE_HANDLER(FrameMsg_DidUpdateName, OnDidUpdateName)
-    IPC_MESSAGE_HANDLER(FrameMsg_AddContentSecurityPolicy,
-                        OnAddContentSecurityPolicy)
+    IPC_MESSAGE_HANDLER(FrameMsg_AddContentSecurityPolicies,
+                        OnAddContentSecurityPolicies)
     IPC_MESSAGE_HANDLER(FrameMsg_ResetContentSecurityPolicy,
                         OnResetContentSecurityPolicy)
     IPC_MESSAGE_HANDLER(FrameMsg_EnforceInsecureRequestPolicy,
@@ -351,11 +350,13 @@ void RenderFrameProxy::OnDidUpdateName(const std::string& name,
   unique_name_ = unique_name;
 }
 
-void RenderFrameProxy::OnAddContentSecurityPolicy(
-    const ContentSecurityPolicyHeader& header) {
-  web_frame_->addReplicatedContentSecurityPolicyHeader(
-      blink::WebString::fromUTF8(header.header_value), header.type,
-      header.source);
+void RenderFrameProxy::OnAddContentSecurityPolicies(
+    const std::vector<ContentSecurityPolicyHeader>& headers) {

[alexmos, 2017/03/25 01:46:27]

Is anything stopping us now from sending over the real policies to the proxies,
instead of reparsing policies from the headers?  That would be a nice cleanup to
do in a followup. 

[arthursonzogni, 2017/03/27 12:03:52]

We need this when PlzNavigate is disabled and OOPIF enabled. The browser-side
enforcement of the CSP works only with PlzNavigate.

I think that as soon as PlzNavigate is shipped, we will be able to remove the
replication of the CSP among the remote frame. That would be a nice cleanup.
Alternatively, it would be possible to enable the browser-side enforcement of
the CSP when PlzNavigate is disabled. FYI, I am currently blocked on making this
patch:
https://codereview.chromium.org/2698623006/
working without PlzNavigate.

[alexmos, 2017/03/27 18:27:22]

I agree we still need this for OOPIF without PlzNavigate, I was just thinking
whether FrameMsg_AddContentSecurityPolicies could pass not
ContentSecurityPolicyHeaders, but rather a vector of
content::ContentSecurityPolicy.  With the former, we end up reparsing the same
policy multiple times in each OOPIF renderer, and with the latter we'd avoid
that.  Back when we did the CSP header replication, we didn't have
content::ContentSecurityPolicy, but now we do.  But that'll probably involve
modifying FrameReplicationState as well, and you're probably right that it's
just easier to wait for PlzNavigate to ship and remove this altogether.

[arthursonzogni, 2017/03/28 11:12:21]

Yes, I think it would be possible to convert back the
content::ContentSecurityPolicy to the blink::CSPDirectiveList if we wanted to,
such that it isn't need to parse the header again.
I don't know. Let's say we will do nothing for the moment and wait for the CSP
replication not to be needed anymore and removed.

+  for (const auto& header : headers) {
+    web_frame_->addReplicatedContentSecurityPolicyHeader(
+        blink::WebString::fromUTF8(header.header_value), header.type,
+        header.source);
+  }
 }
 
 void RenderFrameProxy::OnResetContentSecurityPolicy() {