`responsePolicyContainer` in "determine navigation params policy container" should be cloned for consistency

[Lubrsi]

What is the issue with the HTML Standard?

In determine navigation params policy container, step 4 states:

If responsePolicyContainer is not null, then return responsePolicyContainer.

The other steps in this algorithm either return a clone of a policy container, or a brand new policy container. responsePolicyContainer should be cloned to be consistent with the other steps.

This was noticed when implementing this algorithm in Ladybird:
https://github.com/LadybirdBrowser/ladybird/blob/1843a54df77f1b118bed07d92cafad4fef616f09/Libraries/LibWeb/HTML/Navigable.cpp#L640-L643

[domenic]

This generally appears to be kind of a mess. E.g. https://html.spec.whatwg.org/#beginning-navigation:determining-navigation-params-policy-container passes in a clone for the initiatorPolicyContainer argument, which is unnecessary. Other call sites do not.

I can't remember why we would or would not clone the policy containers here, so it might be equally valid to remove all the cloning.

[annevk]

We need to clone policies when observing them "live" could result in race conditions. Imagine the referrer policy changing half-way through a fetch. That's something we want to avoid. (And realistically they'll have to pass a process boundary so some kind of copy will be needed anyway. So best to enforce that at a clear point in time.)

[domenic]

Sure, it's just not clear that this particular algorithm represents the relevant process hop. It's called from three very different call sites, all of which appear to be way after the initial step that goes in parallel.

If we wanted to be more disciplined about snapshotting, I guess we would add initiatorPolicyContainer and parentPolicyContainer to the source and target snapshot params, respectively, before we go in parallel. responsePolicyContainer comes from the network so I don't think it can change; cloning it is probably not necessary. historyPolicyContainer is presumably stored in the browser process. Should it be cloned?

I guess this would all be a lot clearer if we knew which parts of the policy container were mutable, if any. I guess CSP, due to <meta>? Anything else?

[annevk]

CSP and referrer policy are mutable. Not sure about embedder policy. Other policies are currently not listed as covered, though that's prolly wrong?