Don't let FindFramesToNavigate recurse into subframes with no FNE or from inactive frames.

This CL follows up on the session history changes in
https://chromium-review.googlesource.com/c/chromium/src/+/2181973.
Prior to that CL, inactive frames or frames with no
FrameNavigationEntry returned early from FindFramesToNavigate(),
without recursing down into subframes.  After that CL, we will now
recurse into subframes and keep looking for navigations in those
cases.  This is harmless, since an inactive frame's children should
all be inactive as well, and subframes of a frame with no FNE also
won't have FNEs, but it's also redundant.  This CL modifies the new
flow through DetermineActionForHistoryNavigation() to match the old
behavior and stop looking for navigations in subframes in those cases.

Bug: 705550
Change-Id: Ida707fc9e108f7102b26cf4e44f4d9761e7dd7a7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2225353
Commit-Queue: Alex Moshchuk <alexmos@chromium.org>
Auto-Submit: Alex Moshchuk <alexmos@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#774702}

content/browser/frame_host/navigation_controller_impl.cc

@@ -2747,8 +2747,11 @@ NavigationControllerImpl::DetermineActionForHistoryNavigation(
   // - If the frame is in back-forward cache, it's not allowed to navigate as it
   // should remain frozen. Ignore the request and evict the document from
   // back-forward cache.
+  //
+  // If the frame is inactive, there's no need to recurse into subframes, which
+  // should all be inactive as well.
   if (frame->current_frame_host()->IsInactiveAndDisallowReactivation())
-    return HistoryNavigationAction::kNone;
+    return HistoryNavigationAction::kStopLooking;
 
   // If there's no last committed entry, there is no previous history entry to
   // compare against, so fall back to a different-document load.  Note that we
@@ -2780,13 +2783,16 @@ NavigationControllerImpl::DetermineActionForHistoryNavigation(
   // NavigationEntry, but we shouldn't delete or change what's loaded in
   // it.
   //
+  // Note that in this case, there is no need to keep looking for navigations
+  // in subframes, which would be missing FrameNavigationEntries as well.
+  //
   // It's important to check this before checking |old_item| below, since both
   // might be null, and in that case we still shouldn't change what's loaded in
   // this frame.  Note that scheduling any loads assumes that |new_item| is
   // non-null.  See https://crbug.com/1088354.
   FrameNavigationEntry* new_item = pending_entry_->GetFrameEntry(frame);
   if (!new_item)
-    return HistoryNavigationAction::kNone;
+    return HistoryNavigationAction::kStopLooking;
 
   // If there is no old FrameNavigationEntry, schedule a different-document
   // load.
@@ -2830,10 +2836,10 @@ NavigationControllerImpl::DetermineActionForHistoryNavigation(
   }
 
   // If the item sequence numbers match, there is no need to navigate this
-  // frame.
+  // frame.  Keep looking for navigations in this frame's children.
   DCHECK_EQ(new_item->document_sequence_number(),
             old_item->document_sequence_number());
-  return HistoryNavigationAction::kNone;
+  return HistoryNavigationAction::kKeepLooking;
 }
 
 void NavigationControllerImpl::FindFramesToNavigate(
@@ -2886,6 +2892,8 @@ void NavigationControllerImpl::FindFramesToNavigate(
     // For a different document, the subframes will be destroyed, so there's
     // no need to consider them.
     return;
+  } else if (action == HistoryNavigationAction::kStopLooking) {
+    return;
   }
 
   for (size_t i = 0; i < frame->child_count(); i++) {

content/browser/frame_host/navigation_controller_impl.h

@@ -339,7 +339,8 @@ class CONTENT_EXPORT NavigationControllerImpl : public NavigationController {
   // Defines possible actions that are returned by
   // DetermineActionForHistoryNavigation().
   enum class HistoryNavigationAction {
-    kNone,
+    kStopLooking,
+    kKeepLooking,
     kSameDocument,
     kDifferentDocument,
   };