Re: document sequence numbers and prerendering

[Fergal Daly]

On Fri, 29 Aug 2025 at 09:51, Fergal Daly <fer...@google.com> wrote:

[reposting having joined loading-dev group]

On Fri, 29 Aug 2025 at 09:47, Fergal Daly <fer...@chromium.org> wrote:

On Fri, 29 Aug 2025 at 09:46, Fergal Daly <fer...@google.com> wrote:

[fixing loading-dev]

On Fri, 29 Aug 2025 at 09:45, Fergal Daly <fer...@chromium.org> wrote:

[-prerendering-dev, +loading-dev]

FYI, adding a

CHECK_NE(params.document_sequence_number, -1);

to NavigationControllerImpl::RendererDidNavigate and then running

content_browsertests:*/*BackForward*.*/*

causes 19 tests to fail and they all involve prerendering,

F

On Thu, 28 Aug 2025 at 21:52, Fergal Daly <fer...@google.com> wrote:

Prerender folks,

I've been trying to debug a BFCache issue and I came across something unexpected that appears to be related to prerendering.

BFCache relies on document_sequence_number when creating the BackForwardCacheMetrics object. The goal is that a contiguous sequence of history entries that all have the same main frame document will all have the same BackForwardCacheMetrics object. This logic is in CreateOrReuseBackForwardCacheMetricsForNavigation.

What I'm seeing is that params.document_sequence_number is often -1. If I disable prerendering, it's never -1.

It happens reliably with www.google.com and example.com.

The outcome of this is two-fold:

- someone navigating to www.google.com then example.com will have them sharing a metrics object. This can cause over-eviction of BFCache entries.

- someone navigating to example.com then example/com#foo will get 2 distinct metrics objects instead of sharing them. It's hard to figure out when this results in a real problem but it's definitely not WAI.

Is this intentional? I don't see anything in the docs in the .mojom file. If it's intentional, where can we find the actual DSN?

Thanks,

F

[Hiroki Nakagawa]

Hi Fergal,

Thanks for reaching out.

For prerender activation navigation, params.document_sequence_number is set in NavigationRequest::MakeDidCommitProvisionalLoadParamsForActivation() [cs].

I guess the number should have been copied from the prerendering navigation entry there. Otherwise, the number remains the default value, that is, -1. I'm now creating a fix.

Thanks,

Hiroki

[Fergal Daly]

On Fri, 29 Aug 2025 at 17:59, Hiroki Nakagawa <nhi...@chromium.org> wrote:

Hi Fergal,

Thanks for reaching out.

For prerender activation navigation, params.document_sequence_number is set in NavigationRequest::MakeDidCommitProvisionalLoadParamsForActivation() [cs].

I guess the number should have been copied from the prerendering navigation entry there. Otherwise, the number remains the default value, that is, -1. I'm now creating a fix.

Thanks. I have no idea what impact this is having on BFCache. I also wonder if what else is impacted by the missing DSNs.

I added a CHECK_NE(params->document_sequence_number, -1) to RenderFrameHostImpl::DidCommitNavigationInternal. 724 tests fail. Filtering out prerender tests, we're still left with

All/AmpPageLoadMetricsBrowserTest.AmpMainFrame/1
All/AmpPageLoadMetricsBrowserTest.AmpSubframe/1
All/AmpPageLoadMetricsBrowserTest.NoAmp/1
All/HTTPCacheSearchPreloadUnifiedBrowserTest.BackwardHitHttpCache/0
All/HTTPCacheSearchPreloadUnifiedBrowserTest.BackwardHitHttpCache/1
All/InvisiblePageLazyLoadingImageBrowserTest.LazyLoading/all_invisible_page
All/NavigationControllerBrowserTest.ErrorPageNavigationWithoutNavigationRequestGetsKilled/RDAllFrames_BFCacheDisabled
All/NavigationControllerBrowserTest.ErrorPageNavigationWithoutNavigationRequestGetsKilled/RDAllFrames_BFCacheEnabled
All/NavigationControllerBrowserTest.ErrorPageNavigationWithoutNavigationRequestGetsKilled/RDCrashedFrame_BFCacheDisabled
All/NavigationControllerBrowserTest.ErrorPageNavigationWithoutNavigationRequestGetsKilled/RDCrashedFrame_BFCacheEnabled
NavigationControllerTest.PushStateUpdatesTitleAndFavicon
NavigationControllerTest.PushStateWithOnlyInitialEntry
NavigationControllerTest.SameDocument_Replace
RenderFrameHostImplTest.NavigationApiInterceptBrowserInitiated
RenderFrameHostImplTest.NavigationApiInterceptShowLoadingUi
SafeSearchPolicyTest.ForceGoogleSafeSearch
SearchPreloadUnifiedBrowserTest.ChunkedResponseBody
SearchPreloadUnifiedBrowserTest.OpenPrefetchedResponseInBackgroundedTab
SearchPreloadUnifiedBrowserTest.SetLoaderTimeCorrectly
SearchPreloadUnifiedBrowserTest.TriggerAndActivate
SecurityExploitBrowserTest.NonInitialAboutBlankRendererKill
WebContentsImplTest.UpdateTitle

I'm trying again with the CHECK_NE moved to NavigationControllerImpl::RendererDidNavigate and rebased on top of your fix.

F