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

content/browser/frame_host/navigator_impl.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/frame_host/navigator_impl.h"
 
 #include <utility>
 
 #include "base/metrics/histogram.h"
 #include "base/time/time.h"
@@ skipping 209 matching lines @@
   bool should_preserve_entry = controller_->IsUnmodifiedBlankTab() ||
       delegate_->ShouldPreserveAbortedURLs();
   if (controller_->GetPendingEntry() != controller_->GetVisibleEntry() ||
       !should_preserve_entry) {
     controller_->DiscardPendingEntry(true);
 
     // Also force the UI to refresh.
     controller_->delegate()->NotifyNavigationStateChanged(INVALIDATE_TYPE_URL);
   }
 
+  // If this failure came from a pending RenderFrameHost rather than the
+  // current one, the current RenderFrameHost needs to be notified to stop
+  // its load.  Typically, the pending RenderFrameHost should be created only
+  // once the browser process is sure that it will commit, but there is a case
+  // where this is currently not true, which is when a cross-process frame is
+  // blocked by X-Frame-Options or CSP frame-ancestors.  This check will also
+  // guard against any future such cases.
+  //
+  // TODO(mkwst, alexmos): The case above should disappear when X-Frame-Options
+  // and CSP enforcement moves to the browser process
+  // (https://crbug.com/555418).
+  RenderFrameHostManager* manager =
+      render_frame_host->frame_tree_node()->render_manager();
+  if (render_frame_host == manager->pending_frame_host())

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

Here, one case I was thinking about is if A embeds a subframe B (which gets
blocked), but A navigates the subframe to C in parallel to B getting blocked. 
I'm not sure if it's possible to get this race, as we'll probably clean up the
old pending RFH for B before creating the new pending RFH for C (right?).  But
regardless, this code should do the right thing even if the race is possible
(which is to not stop the new load in process A).

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

Right, the RFHM will destroy the RFH for B before creating the one for C.

That said, ensuring that we're not hearing this from some other RFH makes sense.
 It could be one pending deletion, or even the current one.

+    manager->current_frame_host()->Stop();

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

I did this more generically, as opposed to stopping the load as part of
OnCancelLoadAfterXFrameOptionsOrCSPDenied, to prevent similar future cases from
falling through, as we discussed.

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

Note also that I'm not cleaning up the pending RFH in which the frame was
blocked, which seems consistent in general with how DidFail behaves (IIUC, the
pending RFH will be cleaned up on next navigation; there's a comment about that
above with a TODO for you :).  I actually tried cleaning up the pending RFH, but
ran into various trouble and left it alone for now.

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

Camille, is RFHI::Stop() the right way to handle this?

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

Right.  I'm sad to hear that there's still trouble waiting there, since I was
hoping it would be easier after so much time had passed.  Anyway, that's
unrelated to this.

+
   if (delegate_)
     delegate_->DidFailProvisionalLoadWithError(render_frame_host, params);
 }
 
 void NavigatorImpl::DidFailLoadWithError(
     RenderFrameHostImpl* render_frame_host,
     const GURL& url,
     int error_code,
     const base::string16& error_description,
     bool was_ignored_by_handler) {
@@ skipping 825 matching lines @@
       entry->set_should_replace_entry(pending_entry->should_replace_entry());
       entry->SetRedirectChain(pending_entry->GetRedirectChain());
     }
     controller_->SetPendingEntry(std::move(entry));
     if (delegate_)
       delegate_->NotifyChangedNavigationState(content::INVALIDATE_TYPE_URL);
   }
 }
 
 }  // namespace content