Don't load subframe history items if a client redirect occurs during load.

content/renderer/render_frame_impl.cc

Index: content/renderer/render_frame_impl.cc
diff --git a/content/renderer/render_frame_impl.cc b/content/renderer/render_frame_impl.cc
index 1386b37783ecbea77602dfb670b435f43e1f6032..e52952d0ccb4d3f0b9cc3c5c4e070e52bc2f4b26 100644
--- a/content/renderer/render_frame_impl.cc
+++ b/content/renderer/render_frame_impl.cc
@@ -4958,11 +4958,21 @@ WebNavigationPolicy RenderFrameImpl::decidePolicyForNavigation(
   // FrameNavigationEntry.  If none is found, fall back to the default url.
   if (SiteIsolationPolicy::UseSubframeNavigationEntries() &&
       info.isHistoryNavigationInNewChildFrame && is_content_initiated) {
-    OpenURL(url, IsHttpPost(info.urlRequest),
-            GetRequestBodyForWebURLRequest(info.urlRequest), referrer,
-            info.defaultPolicy, info.replacesCurrentHistoryItem, true);
-    // Suppress the load in Blink but mark the frame as loading.
-    return blink::WebNavigationPolicyHandledByClient;
+    // Don't do this if |info| also says it is a client redirect, in which case
+    // JavaScript on the page is trying to interrupt the history navigation.

[nasko, 2016/08/04 16:00:38]

nit: s/on the page/in a different frame/ to be more precise that it isn't this
specific frame and in general I'd like us to avoid "page" as we are now in frame
granularity almost everywhere.

[Charlie Reis, 2016/08/04 16:46:27]

Will fix in the followup CL with OOPIF test, as you suggested to me offline.

+    if (!info.isClientRedirect) {

[Charlie Reis, 2016/08/04 14:48:46]

Before, we were punting both the original iframe load and the subsequent JS
navigation (which is treated as a client redirect) to the browser via OpenURL in
the new navigation path.  Now we avoid punting the client redirect and let it
proceed, and we'll try to ignore the history item when we get it in
NavigateInternal below.

[Nate Chapin, 2016/08/04 16:40:42]

It surprised me that info.isHistoryNavigationInNewChildFrame is true for the
client redirect. I see why, but that name fooled me :/

[Charlie Reis, 2016/08/04 16:46:27]

Yeah, I was surprised at first, too.  I suppose we could use the
isClientRedirect hint in FrameLoaderClientImpl::decidePolicyForNavigation to
avoid setting that to true, but then we wouldn't know to send the cancel IPC
below.

I'll think about ways to clarify that for the followup.

+      OpenURL(url, IsHttpPost(info.urlRequest),
+              GetRequestBodyForWebURLRequest(info.urlRequest), referrer,
+              info.defaultPolicy, info.replacesCurrentHistoryItem, true);
+      // Suppress the load in Blink but mark the frame as loading.
+      return blink::WebNavigationPolicyHandledByClient;
+    } else {
+      // Client redirects during an initial history load should attempt to
+      // cancel the history navigation.  They will create a provisional document
+      // loader, causing the history load to be ignored in NavigateInternal, and
+      // this IPC will try to cancel any cross-process history load.
+      Send(new FrameHostMsg_CancelInitialHistoryLoad(routing_id_));

[Charlie Reis, 2016/08/04 14:48:46]

This is only necessary if the history item was being sent to a different
process.  We'll try to cancel it before it commits (if possible).

+    }
   }
 
   // Use the frame's original request's URL rather than the document's URL for
@@ -5568,11 +5578,21 @@ void RenderFrameImpl::NavigateInternal(
                         : blink::WebFrameLoadType::BackForward;
         should_load_request = true;
 
+        // If this navigation is to a history item for a new child frame, we
+        // should cancel it if we were interrupted by a Javascript navigation
+        // that has already committed or has started a provisional loader.
+        if (request_params.is_history_navigation_in_new_child &&
+            (!current_history_item_.isNull() ||
+             frame_->provisionalDataSource())) {
+          should_load_request = false;
+          has_history_navigation_in_frame = false;
+        }
+
         // Generate the request for the load from the HistoryItem.
         // PlzNavigate: use the data sent by the browser for the url and the
         // HTTP state. The restoration of user state such as scroll position
         // will be done based on the history item during the load.
-        if (!browser_side_navigation) {
+        if (!browser_side_navigation && should_load_request) {
           request = frame_->requestFromHistoryItem(item_for_history_navigation,
                                                    cache_policy);
         }