Remove LifecycleStateImpl kSpeculative -> kReadyToBeDeleted transition

In the case of Speculative RenderFrameHosts deletion, we don't run any
unload handlers. We remove the LifecycleStateImpl transition from
kSpeculative -> kReadyToBeDeleted and the RenderFrameHost is
deleted directly without doing any LifecycleState transitions for
simplicity.

BUG=1183639

Change-Id: I386f75f736cc425001a6b1386a370ed50a38edc3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3116418
Auto-Submit: Sreeja Kamishetty <sreejakshetty@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Alexander Timin <altimin@chromium.org>
Commit-Queue: Sreeja Kamishetty <sreejakshetty@chromium.org>
Cr-Commit-Position: refs/heads/main@{#918528}

content/browser/renderer_host/back_forward_cache_impl.cc

@@ -545,8 +545,9 @@ BackForwardCacheImpl::CanPotentiallyStorePageLater(RenderFrameHostImpl* rfh) {
                   kBackForwardCacheDisabledForDelegate);
   }
 
-  const bool is_prerendering = rfh->GetLifecycleState() ==
-                               RenderFrameHost::LifecycleState::kPrerendering;
+  const bool is_prerendering =
+      rfh->lifecycle_state() ==
+      RenderFrameHostImpl::LifecycleStateImpl::kPrerendering;
   if (!IsBackForwardCacheEnabled() || is_disabled_for_testing_ ||
       is_prerendering) {
     result.No(

content/browser/renderer_host/render_frame_host_impl.cc

@@ -1662,7 +1662,8 @@ RenderFrameHostImpl::~RenderFrameHostImpl() {
   // being deleted, since it must have already been unset.
   if (was_created && render_view_host_->GetMainFrame() != this) {
     CHECK(IsPendingDeletion() || IsInBackForwardCache() ||
-          lifecycle_state() == LifecycleStateImpl::kPrerendering);
+          lifecycle_state() == LifecycleStateImpl::kPrerendering ||
+          lifecycle_state() == LifecycleStateImpl::kSpeculative);
   }
 
   GetAgentSchedulingGroup().RemoveRoute(routing_id_);
@@ -2856,9 +2857,9 @@ void RenderFrameHostImpl::RenderProcessExited(
     // This happens when this RenderFrameHost is pending deletion and is
     // waiting on one of its children to run its unload handler. While running
     // it, it can request its parent to detach itself.
-    if (lifecycle_state() != LifecycleStateImpl::kReadyToBeDeleted) {
+    if (lifecycle_state() != LifecycleStateImpl::kReadyToBeDeleted)
       SetLifecycleState(LifecycleStateImpl::kReadyToBeDeleted);
-    }
+
     DCHECK(children_.empty());
     PendingDeletionCheckCompleted();
     // |this| is deleted. Don't add any more code at this point in the function.
@@ -3104,6 +3105,14 @@ void RenderFrameHostImpl::DeleteRenderFrame(
     }
   }
 
+  // In case of speculative RenderFrameHosts deletion, we don't run any unload
+  // handlers and RenderFrameHost is deleted directly without any lifecycle
+  // state transitions.
+  if (lifecycle_state() == LifecycleStateImpl::kSpeculative) {
+    DCHECK(!has_unload_handlers());
+    return;
+  }
+
   // In case of BackForwardCache, page is evicted directly from the cache and
   // deleted immediately, without waiting for unload handlers.
   SetLifecycleState(ShouldWaitForUnloadHandlers()
@@ -4305,7 +4314,7 @@ void RenderFrameHostImpl::UndoCommitNavigation(RenderFrameProxyHost& proxy,
         proxy.GetFrameToken(), proxy.CreateAndBindRemoteMainFrameInterfaces());
   }
 
-  SetLifecycleStateToReadyToBeDeleted();
+  SetLifecycleState(LifecycleStateImpl::kReadyToBeDeleted);
 }
 
 void RenderFrameHostImpl::MaybeDispatchDidFinishLoadOnPrerenderActivation() {
@@ -12045,8 +12054,8 @@ void RenderFrameHostImpl::SetLifecycleState(LifecycleStateImpl state) {
           // transitions happen to this state during its lifetime.
           StateTransitions<LifecycleStateImpl>({
               {LifecycleStateImpl::kSpeculative,
-               {LifecycleStateImpl::kActive, LifecycleStateImpl::kPendingCommit,
-                LifecycleStateImpl::kReadyToBeDeleted}},
+               {LifecycleStateImpl::kActive,
+                LifecycleStateImpl::kPendingCommit}},
 
               {LifecycleStateImpl::kPendingCommit,
                {LifecycleStateImpl::kPrerendering, LifecycleStateImpl::kActive,

content/browser/renderer_host/render_frame_host_impl.h

@@ -934,7 +934,9 @@ class CONTENT_EXPORT RenderFrameHostImpl
     // instead of following the full navigation path (known as "early commit").
     // This happens when current RenderFrameHost is not live. The work to
     // remove this transition is tracked in crbug.com/1072817.
-    // - kReadyToBeDeleted -- The navigation redirects or gets cancelled.
+    //
+    // Speculative RenderFrameHost deletion happens without running any unload
+    // handlers and with LifecycleStateImpl remaining in kSpeculative state.
     //
     // Note that the term speculative is used, because the navigation might be
     // canceled or redirected and the RenderFrameHost might get deleted before
@@ -1058,8 +1060,8 @@ class CONTENT_EXPORT RenderFrameHostImpl
     // This state corresponds to when RenderFrameHost has completed running the
     // unload handlers. Once all the descendant frames in other processes are
     // gone, this RenderFrameHost will delete itself. Transition to this state
-    // may happen from one of kSpeculative, kPrerendering, kActive,
-    // kInBackForwardCache or kRunningUnloadHandlers states.
+    // may happen from one of kPrerendering, kActive, kInBackForwardCache or
+    // kRunningUnloadHandlers states.
     kReadyToBeDeleted,
   };
   LifecycleStateImpl lifecycle_state() const { return lifecycle_state_; }

content/browser/renderer_host/render_frame_host_manager.cc

@@ -1291,7 +1291,14 @@ void RenderFrameHostManager::DiscardSpeculativeRenderFrameHostForShutdown() {
   // RenderFrameProxy is detached, it also detaches any associated provisional
   // RenderFrame, whether this due to a child frame being removed from the
   // frame tree or the entire RenderView being torn down.
-  speculative_render_frame_host_->SetLifecycleStateToReadyToBeDeleted();
+  //
+  // When the LifecycleStateImpl is kSpeculative, there is no need to transition
+  // to kReadyToBeDeleted as speculative RenderFrameHosts don't run any unload
+  // handlers but gets deleted by reset directly in kSpeculative state.
+  if (speculative_render_frame_host_->lifecycle_state() ==
+      LifecycleStateImpl::kPendingCommit) {
+    speculative_render_frame_host_->SetLifecycleStateToReadyToBeDeleted();
+  }
   // TODO(dcheng): Figure out why `RenderFrameDeleted()` doesn't seem to be
   // called on child `RenderFrameHost`s at shutdown. This is currently limited
   // to main frame-only because that is how it has worked for some time:

docs/render-frame-host-lifecycle-state.gv

@@ -3,7 +3,7 @@
 //
 // See tools/state_transitions/README.md
 digraph createflow {
-  kSpeculative -> {kActive, kPendingCommit, kReadyToBeDeleted};
+  kSpeculative -> {kActive, kPendingCommit};
   kPendingCommit -> {kPrerendering, kActive, kReadyToBeDeleted};
   kPrerendering -> {kActive, kRunningUnloadHandlers, kReadyToBeDeleted};
   kActive -> {kInBackForwardCache, kRunningUnloadHandlers, kReadyToBeDeleted};