-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
Fix Iframe loading and closing #31973
Description
As part of debugging at new failing test in #31505, I noted some(known?) problems with how iframes are loaded.
The test in question is /html/browsers/browsing-the-web/navigating-across-documents/initial-empty-document/load-pageshow-events-iframe-contentWindow.html, which asserts that no load events are fired for an iframe which has no navigable("load" & "pageshow" events do not fire on contentWindow of iframe that stays on the initial empty document).
We fail all tests, meaning we fire load events, for all subtests, with the exception of "load & pageshow events do not fire on contentWindow of <iframe> element created with src=''". The test fails now also with #31505, hence my investigation, but I think I may want to let it fail with all others, and instead fix all tests in one go.
Quick recap on how this relates to the spec:
- Entry point for processing are iframe HTML element insertion steps, which call into process the iframe attributes.
- We then hit the "2 otherwise"(meaning it has a url), which then either:
- Immediately returns if there is no src, which means not firing any load event.
- Immediately run the load steps for an "about:blank" iframe, which means firing a load event on the element, not the content window
- Navigate the iframe otherwise, which means eventually firing the load event on the content window as per completely-finish-loading
So, in other words, there are three:
- in the case of no src, no event should fire at all, and no navigation should happen.
- in the case of "about:blank", a load event should synchronously fire on the element, but no navigation should happen.
- Otherwise, we should navigate the iframe, and then only fire the load event when the load completes(so that's asynchronous).
What we are currently doing, and why it is wrong:
bind_to_tree is the entry point, where we do two things via a delayed task:
create_nested_browsing_contextprocess_the_iframe_attributes(ProcessingMode::FirstTime).
Both end-up calling into start_new_pipeline(with the exception of the no src case, which returns), where 1 uses PipelineType::InitialAboutBlank, and 2 PipelineType::Navigation.
PipelineType::InitialAboutBlank results in a kind of synchronous loading of the iframe doc via ScriptThread::process_attach_layout, which eventually calls into document.maybe_queue_document_completion(), which then sends a msg to the constellation by way of document.notify_constellation_load. It does not end there: the constellation will then respond with a DispatchIFrameLoadEvent message, which will then result in calling the iframe load event steps.
PipelineType::Navigation starts with a ScriptMsg::ScriptLoadedURLInIFrame sent to the constellation, resulting in what I assume is a "normal" navigation workflow.
This is wrong because we are:
- For an about:blank, firing the load event on the element asynchronously.
- In all cases(except the early return no src) doing a navigation, which results in firing the load event on the document.
So somehow we need to restructure this, where:
- When binding to the tree
- If url is null, do nothing(or send some new message to the constellation that deals with it, just noting a new empty pipeline matching a src-less iframe)
- If url is about:blank, synchronously run the load steps, and then send the
PipelineType::InitialAboutBlankmsg. - Otherwise, send the
PipelineType::Navigationmessage. But probably this require doing some setup for the pipeline that is currently done withPipelineType::InitialAboutBlank.