OOPIF: Handle cross-site frames being blocked by X-Frame-Options or CSP.

content/browser/frame_host/render_frame_host_impl.cc

Index: content/browser/frame_host/render_frame_host_impl.cc
diff --git a/content/browser/frame_host/render_frame_host_impl.cc b/content/browser/frame_host/render_frame_host_impl.cc
index a6e002acc8ee2c2af88a7be8ff6f2e36f50c9960..5e6b025c7b633bce0380d60858f126a78e03a27b 100644
--- a/content/browser/frame_host/render_frame_host_impl.cc
+++ b/content/browser/frame_host/render_frame_host_impl.cc
@@ -550,6 +550,8 @@ bool RenderFrameHostImpl::OnMessageReceived(const IPC::Message &msg) {
                         OnDidChangeSandboxFlags)
     IPC_MESSAGE_HANDLER(FrameHostMsg_DidChangeFrameOwnerProperties,
                         OnDidChangeFrameOwnerProperties)
+    IPC_MESSAGE_HANDLER(FrameHostMsg_DidCancelLoadAfterXFrameOptionsOrCSPDenied,
+                        OnDidCancelLoadAfterXFrameOptionsOrCSPDenied)
     IPC_MESSAGE_HANDLER(FrameHostMsg_UpdateTitle, OnUpdateTitle)
     IPC_MESSAGE_HANDLER(FrameHostMsg_UpdateEncoding, OnUpdateEncoding)
     IPC_MESSAGE_HANDLER(FrameHostMsg_BeginNavigation,
@@ -1516,6 +1518,29 @@ void RenderFrameHostImpl::OnDidChangeFrameOwnerProperties(
   }
 }
 
+void RenderFrameHostImpl::OnDidCancelLoadAfterXFrameOptionsOrCSPDenied() {
+  // When a frame is blocked by X-Frame-Options or CSP frame-ancestors, the
+  // (empty) blocked frame needs to get a unique origin, which ensures that it
+  // appears as a normal cross-origin document, and is the desired behavior
+  // according to spec: https://www.w3.org/TR/CSP2/#directive-frame-ancestors
+  // This sets the unique origin on both the browser and renderer sides.  The
+  // IPC to renderer may be required when a cross-process subframe is blocked,
+  // since the blocking currently occurs in the pending RenderFrame, but the
+  // actual blocked (empty) frame will be left in the current RenderFrame in a
+  // different process.
+  //
+  // TODO(mkwst, alexmos): This will probably be called directly rather than
+  // from an renderer IPC once X-Frame-Options and CSP enforcement moves to the
+  // browser process (https://crbug.com/555418).
+  frame_tree_node_->SetCurrentOrigin(url::Origin());

[alexmos, 2016/02/25 21:59:12]

I think this matters in default Chrome even without --site-per-process, so I
always send this IPC and do this.  Previously, we would've returned the last
committed origin for a blocked frame, whereas I think it's more correct to
return a unique origin, which would match Blink's behavior.

[Charlie Reis, 2016/02/26 01:13:22]

Acknowledged.

+
+  if (this == frame_tree_node_->render_manager()->pending_frame_host()) {
+    RenderFrameHost* current_rfh = frame_tree_node_->current_frame_host();
+    current_rfh->Send(new FrameMsg_CancelLoadAfterXFrameOptionsOrCSPDenied(
+        current_rfh->GetRoutingID()));
+  }
+}
+
 void RenderFrameHostImpl::OnUpdateTitle(
     const base::string16& title,
     blink::WebTextDirection title_direction) {
@@ -1557,11 +1582,22 @@ void RenderFrameHostImpl::OnDispatchLoad() {
   RenderFrameProxyHost* proxy =
       frame_tree_node()->render_manager()->GetProxyToParent();
   if (!proxy) {
+    // A valid special case where the proxy won't exist occurs when a frame
+    // gets blocked due to X-Frame-Options or CSP while it is still pending.
+    // (The proxy in the parent isn't created until commit.)  In that case, it
+    // is ok to ignore this load event dispatch, since it will be done as part
+    // of forwarding the blocked notification (see
+    // FrameMsg_CancelLoadAfterXFrameOptionsOrCSPDenied).
+    //
+    // TODO(mkwst, alexmos): This won't be necessary once X-Frame-Options and
+    // CSP enforcement moves to the browser process (https://crbug.com/555418).
+    if (this != frame_tree_node_->current_frame_host())

[alexmos, 2016/02/25 21:59:12]

This is pretty ugly, but I tried to detect whether or not to send this IPC from
Blink in the blocked case, and I thought that was even uglier.  At least this is
in one place and easy to rip out once we're done with 555418.  Also, dispatching
loads on <iframe> elements can now probably be done together with forwarding
DidStopLoading, as opposed to this separate IPC, in which case this function
will disappear entirely (but this is probably best left to a separate CL).

[Charlie Reis, 2016/02/26 21:26:41]

Acknowledged.

+      return;
+
     bad_message::ReceivedBadMessage(GetProcess(),
                                     bad_message::RFH_NO_PROXY_TO_PARENT);
     return;
   }
-
   proxy->Send(new FrameMsg_DispatchLoad(proxy->GetRoutingID()));
 }