prerender: Ensure only speculative RFHs are created provisionally in renderer

A missing conditional meant that main RFHs for prerender hosts always
began as provisional in the renderer. This was mostly harmless, since a
prerender host isn't used unless a navigation commits in it—but certain
features, like devtools's network overrides, would cause a renderer
crash because of this mismatch.

Bug: 379933490
Change-Id: I532fbe8f8f33ffc468f870df58b8e0912f82084c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6231570
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1417737}

content/browser/renderer_host/render_view_host_impl.cc

@@ -542,9 +542,19 @@ bool RenderViewHostImpl::CreateRenderView(
               mojom::CreateProvisionalLocalMainFrameParams::New(
                   std::move(local_frame_params),
                   frame_tree_node->current_frame_host()->GetFrameToken()));
-    } else if (frame_tree_->is_prerendering()) {
-      // During a prerender navigation, a local main frame for a new
-      // RenderViewHost must always start as a provisinonal RenderFrame in the
+    } else if (frame_tree_->is_prerendering() &&
+               (!base::FeatureList::IsEnabled(
+                    features::kPrerenderMoreCorrectSpeculativeRFHCreation) ||
+                main_rfh->lifecycle_state() ==
+                    RenderFrameHostImpl::LifecycleStateImpl::kSpeculative)) {
+      // During prerender, the browser may need to create new speculative local
+      // main frames. Normally, creating a speculative local main frame is a
+      // two step process: the browser first creates a RenderViewHost with a
+      // main RenderFrameProxyHost and then creates the speculative main
+      // RenderFrameHost.
+      //
+      // Prerender skips the RenderFrameProxyHost creation step, but the new
+      // RenderViewHost must still start with a provisional RenderFrame in the
       // renderer. Otherwise, discarding a speculative RFH during prerender
       // navigation causes the browser and the renderer to go out of sync. See
       // https://crbug.com/40076091 for more background and details.

content/common/features.cc

@@ -317,6 +317,15 @@ BASE_FEATURE(kPreloadingConfig,
              "PreloadingConfig",
              base::FEATURE_ENABLED_BY_DEFAULT);
 
+// A misunderstanding when fixing crbug.com/40076091 meant that non-speculative
+// RFHs were being created with a provisional RenderFrame in the renderer. This
+// is nominally harmless, but can crash prerenders if devtool's network
+// overrides feature is enabled. Guarded by a feature since fixing this new bug
+// might reintroduce the previous crashes.
+BASE_FEATURE(kPrerenderMoreCorrectSpeculativeRFHCreation,
+             "PrerenderMoreCorrectSpeculativeRFHCreation",
+             base::FEATURE_ENABLED_BY_DEFAULT);
+
 // This feature makes it so that having pending views increase the priority of a
 // RenderProcessHost even when there is a priority override.
 BASE_FEATURE(kPriorityOverridePendingViews,

content/common/features.h

@@ -77,6 +77,8 @@ CONTENT_EXPORT BASE_DECLARE_FEATURE_PARAM(size_t, kMultipleSpareRPHsCount);
 CONTENT_EXPORT BASE_DECLARE_FEATURE(kPermissionsPolicyVerificationInContent);
 #endif
 CONTENT_EXPORT BASE_DECLARE_FEATURE(kPreloadingConfig);
+CONTENT_EXPORT BASE_DECLARE_FEATURE(
+    kPrerenderMoreCorrectSpeculativeRFHCreation);
 CONTENT_EXPORT BASE_DECLARE_FEATURE(kPriorityOverridePendingViews);
 CONTENT_EXPORT BASE_DECLARE_FEATURE(kPrivacySandboxAdsAPIsM1Override);
 CONTENT_EXPORT BASE_DECLARE_FEATURE(kProcessReuseOnPrerenderCOOPSwap);