[M110] Reset navigations using swapped out pending delete RFH when un…

…loading

We used to delete all navigations happening in a FrameTreeNode when
a RFH in that FrameTreeNode gets into the "pending deletion" state,
but with crrev.com/c/4067080 we stopped doing that for cases where
the RFH is pending deletion because it got swapped out by another RFH
after a navigation commits (so now we only cancel all navigations on
FrameTreeNode detach).

However, this might introduce problems with existing NavigationRequests
that uses the swapped out RFH. Pending-commit navigations that are
owned by the swapped out RFH is already deleted from
RenderFrameHostManager::UnloadOldFrame(), but non-pending-commit
navigations that point to the swapped out RFH might still be around.
When the non-pending-commit navigation tries to access the RFH (which
is possible from e.g. NavigationThrottles running OnWillProcessResponse
method), the RFH might already be deleted, causing a use-after-free.

This CL makes it so that we also delete non-pending-commit navigations
owned by the FrameTreeNode if it points to the swapped out RFH. We will
also delete all navigations happening in child frames of the swapped
out RFH, as those frames will be detached and deleted.

Note that this is not a full revert of crrev.com/c/4067080, because we
will only delete the navigations that are using the swapped out RFH,
and not navigations that use other RFHs.

(cherry picked from commit 7f10cfd)

Bug: 1401933
Change-Id: I94fc25a4643ea0508783064a2feb615fe7c748c4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4122380
Commit-Queue: Rakina Zata Amni <rakina@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1087482}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4127320
Auto-Submit: Rakina Zata Amni <rakina@chromium.org>
Commit-Queue: Nasko Oskov <nasko@chromium.org>
Cr-Commit-Position: refs/branch-heads/5481@{#99}
Cr-Branched-From: 130f3e4-refs/heads/main@{#1084008}

content/browser/renderer_host/render_frame_host_impl.cc

@@ -8575,6 +8575,14 @@ void RenderFrameHostImpl::StartPendingDeletionOnSubtree(
     // frame detach if the kStopCancellingNavigationsOnCommitAndNewNavigation
     // flag is enabled, or for all pending deletion cases otherwise.
     ResetAllNavigationsInSubtreeForFrameDetach();
+  } else {
+    CHECK(
+        pending_deletion_reason == PendingDeletionReason::kSwappedOut ||
+        base::FeatureList::IsEnabled(kAvoidUnnecessaryNavigationCancellations));
+    // The pending deletion state is caused by swapping out the RFH. Reset only
+    // the navigations that are owned by or will be using the swapped out RFH,
+    // and also reset all navigations happening in the descendant frames.
+    ResetNavigationsUsingSwappedOutRFHAndAllNavigationsInSubtree();
   }
 
   for (std::unique_ptr<FrameTreeNode>& child_frame : children_) {
@@ -8658,6 +8666,34 @@ void RenderFrameHostImpl::PendingDeletionCheckCompletedOnSubtree() {
   }
 }
 
+void RenderFrameHostImpl::
+    ResetNavigationsUsingSwappedOutRFHAndAllNavigationsInSubtree() {
+  // Only delete the navigation owned by the swapped out RFH or those that
+  // intend to use the current RFH.
+  ResetOwnedNavigationRequests(
+      NavigationDiscardReason::kRenderFrameHostDestruction);
+  if (frame_tree_node_->navigation_request() &&
+      frame_tree_node_->navigation_request()->state() >=
+          NavigationRequest::WILL_PROCESS_RESPONSE &&
+      frame_tree_node_->navigation_request()->GetRenderFrameHost() == this) {
+    // It's possible for a RenderFrameHost to already have been picked for a
+    // navigation but the NavigationRequest's ownership hasn't been moved to the
+    // RenderFrameHost yet, if the navigation is deferred by a
+    // NavigationThrottle or CommitDeferringCondition. We need to reset the
+    // NavigationRequest to prevent it from trying to commit in the pending
+    // deletion RFH.
+    frame_tree_node_->ResetNavigationRequest(
+        NavigationDiscardReason::kRenderFrameHostDestruction);
+  }
+
+  // For the child frames, we should delete all ongoing navigations instead of
+  // just the one using the current RFH, because the child frames will be
+  // deleted when this RFH gets unloaded.
+  for (auto& child : children_) {
+    child->current_frame_host()->ResetAllNavigationsInSubtreeForFrameDetach();
+  }
+}
+
 void RenderFrameHostImpl::ResetAllNavigationsInSubtreeForFrameDetach() {
   for (auto& child : children_) {
     child->current_frame_host()->ResetAllNavigationsInSubtreeForFrameDetach();

content/browser/renderer_host/render_frame_host_impl.h

@@ -3497,6 +3497,16 @@ class CONTENT_EXPORT RenderFrameHostImpl
   // this frame's subtree.
   void PendingDeletionCheckCompletedOnSubtree();
 
+  // In this RenderFramehost, cancels every:
+  // - Non-pending commit NavigationRequest owned by the FrameTreeNode that
+  // intends to commit in this RFH
+  // - Pending commit NavigationRequest owned by the RenderFrameHost
+  // In this RenderFrameHost's children, calls
+  // `ResetAllNavigationsInSubtreeForFrameDetach()` to cancel all navigations
+  // that are ongoing in the descendant FrameTreeNodes.
+  // This function should only be called on swapped out RenderFrameHosts.
+  void ResetNavigationsUsingSwappedOutRFHAndAllNavigationsInSubtree();
+
   // In this RenderFrameHost and its children, removes every:
   // - Non-pending commit NavigationRequest owned by the FrameTreeNode
   // - Pending commit NavigationRequest owned by the RenderFrameHost

third_party/blink/web_tests/http/tests/inspector-protocol/target/oopif-no-discovery.js

@@ -23,6 +23,10 @@
   testRunner.log('Target id should match: ' + (attachedEvent.targetInfo.targetId === detachedEvent.targetId));
 
   testRunner.log('Navigating back to out-of-process iframe...');
+  // Wait for a little while before continuing, to ensure that the new
+  // navigation won't get cancelled by the previous navigation's commit in the
+  // browser process.
+  await new Promise(resolve => setTimeout(resolve, 100));
 
   const attachedPromise2 = dp.Target.onceAttachedToTarget();
   dp.Page.navigate({frameId, url: 'http://devtools.oopif.test:8000/inspector-protocol/resources/iframe.html'});