Notify the renderer if a history navigation has no subframe items.

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 48f0183ed0ba72e1ae632028c1a41d224b87eec7..fc62a575c2b9dba49fe5aeb7ebfa848d3be57a7e 100644
--- a/content/renderer/render_frame_impl.cc
+++ b/content/renderer/render_frame_impl.cc
@@ -1043,6 +1043,7 @@ RenderFrameImpl::RenderFrameImpl(const CreateParams& params)
       render_view_(params.render_view->AsWeakPtr()),
       routing_id_(params.routing_id),
       proxy_routing_id_(MSG_ROUTING_NONE),
+      browser_has_subtree_history_items_(false),

[alexmos, 2016/09/22 01:44:06]

I was thinking whether "true" might be a safer default, given that the previous
behavior was to always ask the browser.  But I guess it's doesn't really matter
given that this will always be set for history navigations before any child
frames are created.

[Charlie Reis, 2016/09/22 21:00:37]

Right.  I think this is a smaller delta from where we're heading, when the
browser will tell us exactly which frame unique names it should be consulted for
during a history navigation.

 #if defined(ENABLE_PLUGINS)
       plugin_power_saver_helper_(nullptr),
       plugin_find_handler_(nullptr),
@@ -1173,6 +1174,11 @@ void RenderFrameImpl::Initialize() {
   RenderFrameImpl* parent_frame = RenderFrameImpl::FromWebFrame(
       frame_->parent());
   if (parent_frame) {
+    // Inherit knowledge of whether we need to consult the browser process for
+    // a history item on the first navigation.  This is inherited by further
+    // subframes and cleared at didStopLoading.
+    browser_has_subtree_history_items_ =
+        parent_frame->browser_has_subtree_history_items_;
     is_using_lofi_ = parent_frame->IsUsingLoFi();
     effective_connection_type_ = parent_frame->getEffectiveConnectionType();
   }
@@ -4834,6 +4840,12 @@ void RenderFrameImpl::didStartLoading(bool to_different_document) {
 void RenderFrameImpl::didStopLoading() {
   TRACE_EVENT1("navigation,rail", "RenderFrameImpl::didStopLoading",
                "id", routing_id_);
+
+  // Any subframes created after this point won't be considered part of the
+  // current history navigation (if this was one), so we don't need to track
+  // this state anymore.
+  browser_has_subtree_history_items_ = false;

[alexmos, 2016/09/22 01:44:06]

This will be reset in all frames, since main frames and subframes all will
eventually fire didStopLoading(), right?  What if we have a main frame with a
subframe, the subframe finishes loading and then creates a new (grandchild)
frame while the main frame hasn't finished loading?  The grandchild is not
considered part of the current history navigation, right?

[Charlie Reis, 2016/09/22 21:00:37]

Exactly.  The newly created child frame checks to see if its parent is still
loading before trying to find a history item for itself.  See this comment/code
in FrameLoaderClientImpl for where it happens:
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/web/FrameLoade...

As a side note, didStopLoading happens bottom up.  A frame won't hear it until
all of its subframes are done, so it's possible for the main frame to keep its
bit around much longer than children/grandchildren.

+
   render_view_->FrameDidStopLoading(frame_);
   Send(new FrameHostMsg_DidStopLoading(routing_id_));
 }
@@ -4978,10 +4990,12 @@ WebNavigationPolicy RenderFrameImpl::decidePolicyForNavigation(
   }
 
   // In OOPIF-enabled modes, back/forward navigations in newly created subframes
-  // should be sent to the browser in case there is a matching
-  // FrameNavigationEntry.  If none is found, fall back to the default url.
+  // should be sent to the browser if there is a chance there is a matching
+  // FrameNavigationEntry.  If none is found (or if the browser has indicated it
+  // has no subtree history items), fall back to loading the default url.
   if (SiteIsolationPolicy::UseSubframeNavigationEntries() &&
-      info.isHistoryNavigationInNewChildFrame && is_content_initiated) {
+      info.isHistoryNavigationInNewChildFrame && is_content_initiated &&
+      browser_has_subtree_history_items_) {
     // 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.
     if (!info.isClientRedirect) {
@@ -5577,6 +5591,11 @@ void RenderFrameImpl::NavigateInternal(
                         : blink::WebFrameLoadType::BackForward;
         should_load_request = true;
 
+        // Remember whether we should consult the browser process for any
+        // subframes created during this history navigation.
+        browser_has_subtree_history_items_ =
+            request_params.has_subtree_history_items;
+
         if (history_load_type == blink::WebHistorySameDocumentLoad) {
           // If this is marked as a same document load but we haven't committed
           // anything, treat it as a new load.  The browser shouldn't let this