Spec descendant frames' ongoing navigations block disableUntrustedNetwork promise from being resolved

[xiaochen-z]

fix: #205

This PR only implements the change in algorithm "recalculate the untrusted network status of all fenced frame descendants".

The part that this algorithm should be called whenever a navigation commits will be done in a follow-up PR. I'm still investigating how to spec this. See previous comment on this.

---

Preview | Diff

[blu25]

LGTM, thanks!

[xiaochen-z]

+@domfarolino for review, thank you.

[domfarolino]

Pretty much lgtm. This covers traversals as well btw, which I think is the goal too? Does that match the implementation? And are there WPTs?

[xiaochen-z]

Thanks!

This covers traversals as well btw, which I think is the goal too? Does that match the implementation?

From what I read from the spec, when ongoing navigation is "traversal", it means the navigable is traversing its history. For example, FF going back to a previously navigated URL. The FF may end up with a different page and having its untrusted network status reset to enabled.

This is the race condition that the PR targets: When a parent frame tries to disable untrusted networks, it checks the child frame and finds it has untrusted network disabled. Without checking descendant frames' ongoing navigations, the parent frame goes ahead marking its untrusted network also disabled. But if the child frame has an ongoing navigation, including the history traversal, it resets the child frame's untrusted network to enabled as the frame navigates to a different page.

The implementation checks this via FrameTreeNode::HasNavigation(), which is set during normal navigation as well as history traversal. So the spec matches the implementation.

And are there WPTs?

No. I'm adding WPTs for this. Tracking in bug: crbug/397377177.

[xiaochen-z]

WPTs added in https://chromium-review.googlesource.com/c/chromium/src/+/6280958. However they were added to wpt_internal/ where other network revocation WPTs live. I suppose it is not possible to link internal WPTs in the spec?

[blu25]

WPTs added in https://chromium-review.googlesource.com/c/chromium/src/+/6280958. However they were added to wpt_internal/ where other network revocation WPTs live. I suppose it is not possible to link internal WPTs in the spec?

On a separate PR, @domfarolino had suggested offline to add a non-normative section that links to the internal WPT directory with an explanation of why they're internal. Not sure if it's worth doing here or just adding the WPTs later once we move them to the external directory.

[domfarolino]

I think that'd be good.

[xiaochen-z]

Thanks! There is an issue block below already stated that the internal WPTs will be upstreamed and listed there once the required test-only function is added to the WPT web driver.

fenced-frame/spec.bs

Lines 2154 to 2158 in 6fa197f

	Issue: This will require a RFC to add a test-only function to the WPT web driver.
	(<a href="https://github.com/WICG/fenced-frame/issues/192">WICG/fenced-frame#192</a>)
	Once that web driver changes is made, existing Chromium-internal web platform tests for
	{{Fence/disableUntrustedNetwork()}} need to be upstreamed and linked here.
	(<a href="https://github.com/WICG/fenced-frame/issues/207">WICG/fenced-frame#207</a>)

[domfarolino]

Sure but maybe this PR can link to the specific test that's relevant to the changes made in this patch.