Auto merge of #28560 - yvt:fix-nosrc-iframe-load-2, r=jdm

The "process the iframe attributes" steps shouldn't enqueue the iframe load event steps (anymore)

Fixes `HtmlIFrameElement::process_the_iframe_attributes` (which implements [the "process the `iframe` attributes" steps][1]) queueing an element task to execute [the iframe load event steps][2], which causes another `load` event to be fired on the iframe in addition to the one asynchronously generated by [the "create a new nested browsing context" steps][3].

The following timeline shows what happens when a `src`-less `iframe` element is parser-inserted and which step this PR intends to remove. (This does not necessarily match Servo's current behavior exactly - Most notably, `T2`'s steps might run late because the "create a nested browsing context" steps' implementation actually [parses][5] a blank HTML document.)

```diff
  T0 (networking task source)
   | Insert the `iframe` element, causing the following steps to be executed:
   |  | Create a new nested browsing context
   |  |  | Create a new browsing context
   |  |  |  | Completely finish loading the `iframe` element's `Document`
   |  |  |  |  | Queue element task T2.
   |  |
   |  | Process the `iframe` attributes
-  |  |  | Queue element task T1

- T1 (DOM manipulation task source)
-  | Execute the `iframe` load event steps
-  |  | Fire `load` for the `iframe` element

  T2 (DOM manipulation task source)
   | Execute the `iframe` load event steps
   |  | Fire `load` for the `iframe` element
```

This bug likely originates from [a bug][4] in the specification.

[1]: https://html.spec.whatwg.org/multipage/#process-the-iframe-attributes
[2]: https://html.spec.whatwg.org/multipage/#iframe-load-event-steps
[3]: https://html.spec.whatwg.org/multipage/#creating-a-new-nested-browsing-context
[4]: https://github.com/whatwg/html/pull/5797
[5]: https://html.spec.whatwg.org/multipage/#the-end

---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #24501, #15727, and the `cross-origin-objects-on-new-window.html` part of #26299

---
- [x] There are tests for these changes OR
- [ ] These changes do not require tests because ___

6 files changed, 42 insertions, 44 deletions

diff --git a/components/script/dom/htmliframeelement.rs b/components/script/dom/htmliframeelement.rs
index 6f5264999be..1fe04c04518 100644
--- a/components/script/dom/htmliframeelement.rs
+++ b/components/script/dom/htmliframeelement.rs
@@ -25,7 +25,6 @@ use crate::dom::virtualmethods::VirtualMethods;
 use crate::dom::window::ReflowReason;
 use crate::dom::windowproxy::WindowProxy;
 use crate::script_thread::ScriptThread;
-use crate::task_source::TaskSource;
 use dom_struct::dom_struct;
 use html5ever::{LocalName, Prefix};
 use ipc_channel::ipc;
@@ -57,9 +56,9 @@ bitflags! {
 }
 
 #[derive(PartialEq)]
-pub enum NavigationType {
+enum PipelineType {
     InitialAboutBlank,
-    Regular,
+    Navigation,
 }
 
 #[derive(PartialEq)]
@@ -106,8 +105,16 @@ impl HTMLIFrameElement {
 
     pub fn navigate_or_reload_child_browsing_context(
         &self,
+        load_data: LoadData,
+        replace: HistoryEntryReplacement,
+    ) {
+        self.start_new_pipeline(load_data, PipelineType::Navigation, replace);
+    }
+
+    fn start_new_pipeline(
+        &self,
         mut load_data: LoadData,
-        nav_type: NavigationType,
+        pipeline_type: PipelineType,
         replace: HistoryEntryReplacement,
     ) {
         let sandboxed = if self.is_sandboxed() {
@@ -117,12 +124,12 @@ impl HTMLIFrameElement {
         };
 
         let browsing_context_id = match self.browsing_context_id() {
-            None => return warn!("Navigating unattached iframe."),
+            None => return warn!("Attempted to start a new pipeline on an unattached iframe."),
             Some(id) => id,
         };
 
         let top_level_browsing_context_id = match self.top_level_browsing_context_id() {
-            None => return warn!("Navigating unattached iframe."),
+            None => return warn!("Attempted to start a new pipeline on an unattached iframe."),
             Some(id) => id,
         };
 
@@ -181,8 +188,8 @@ impl HTMLIFrameElement {
             device_pixel_ratio: window.device_pixel_ratio(),
         };
 
-        match nav_type {
-            NavigationType::InitialAboutBlank => {
+        match pipeline_type {
+            PipelineType::InitialAboutBlank => {
                 let (pipeline_sender, pipeline_receiver) = ipc::channel().unwrap();
 
                 self.about_blank_pipeline_id.set(Some(new_pipeline_id));
@@ -213,7 +220,7 @@ impl HTMLIFrameElement {
                 self.pipeline_id.set(Some(new_pipeline_id));
                 ScriptThread::process_attach_layout(new_layout_info, document.origin().clone());
             },
-            NavigationType::Regular => {
+            PipelineType::Navigation => {
                 let load_info = IFrameLoadInfoWithData {
                     info: load_info,
                     load_data: load_data,
@@ -231,6 +238,7 @@ impl HTMLIFrameElement {
 
     /// <https://html.spec.whatwg.org/multipage/#process-the-iframe-attributes>
     fn process_the_iframe_attributes(&self, mode: ProcessingMode) {
+        // > 1. If `element`'s `srcdoc` attribute is specified, then:
         if self
             .upcast::<Element>()
             .has_attribute(&local_name!("srcdoc"))
@@ -251,7 +259,6 @@ impl HTMLIFrameElement {
             load_data.srcdoc = String::from(element.get_string_attribute(&local_name!("srcdoc")));
             self.navigate_or_reload_child_browsing_context(
                 load_data,
-                NavigationType::Regular,
                 HistoryEntryReplacement::Disabled,
             );
             return;
@@ -273,22 +280,16 @@ impl HTMLIFrameElement {
             }
         }
 
-        // https://github.com/whatwg/html/issues/490
         if mode == ProcessingMode::FirstTime &&
             !self.upcast::<Element>().has_attribute(&local_name!("src"))
         {
-            let this = Trusted::new(self);
-            let pipeline_id = self.pipeline_id().unwrap();
-            // FIXME(nox): Why are errors silenced here?
-            let _ = window.task_manager().dom_manipulation_task_source().queue(
-                task!(iframe_load_event_steps: move || {
-                    this.root().iframe_load_event_steps(pipeline_id);
-                }),
-                window.upcast(),
-            );
             return;
         }
 
+        // > 2. Otherwise, if `element` has a `src` attribute specified, or
+        // >    `initialInsertion` is false, then run the shared attribute
+        // >    processing steps for `iframe` and `frame` elements given
+        // >    `element`.
         let url = self.get_url();
 
         // TODO(#25748):
@@ -342,11 +343,25 @@ impl HTMLIFrameElement {
         } else {
             HistoryEntryReplacement::Disabled
         };
-        self.navigate_or_reload_child_browsing_context(load_data, NavigationType::Regular, replace);
+        self.navigate_or_reload_child_browsing_context(load_data, replace);
     }
 
     fn create_nested_browsing_context(&self) {
-        // Synchronously create a new context and navigate it to about:blank.
+        // Synchronously create a new browsing context, which will present
+        // `about:blank`. (This is not a navigation.)
+        //
+        // The pipeline started here will synchronously "completely finish
+        // loading", which will then asynchronously call
+        // `iframe_load_event_steps`.
+        //
+        // The precise event timing differs between implementations and
+        // remains controversial:
+        //
+        //  - [Unclear "iframe load event steps" for initial load of about:blank
+        //    in an iframe #490](https://github.com/whatwg/html/issues/490)
+        //  - [load event handling for iframes with no src may not be web
+        //    compatible #4965](https://github.com/whatwg/html/issues/4965)
+        //
         let url = ServoUrl::parse("about:blank").unwrap();
         let document = document_from_node(self);
         let window = window_from_node(self);
@@ -366,9 +381,9 @@ impl HTMLIFrameElement {
         self.top_level_browsing_context_id
             .set(Some(top_level_browsing_context_id));
         self.browsing_context_id.set(Some(browsing_context_id));
-        self.navigate_or_reload_child_browsing_context(
+        self.start_new_pipeline(
             load_data,
-            NavigationType::InitialAboutBlank,
+            PipelineType::InitialAboutBlank,
             HistoryEntryReplacement::Disabled,
         );
     }
diff --git a/components/script/script_thread.rs b/components/script/script_thread.rs
index c1161674fc2..506cd5d6bf2 100644
--- a/components/script/script_thread.rs
+++ b/components/script/script_thread.rs
@@ -45,7 +45,7 @@ use crate::dom::element::Element;
 use crate::dom::event::{Event, EventBubbles, EventCancelable};
 use crate::dom::globalscope::GlobalScope;
 use crate::dom::htmlanchorelement::HTMLAnchorElement;
-use crate::dom::htmliframeelement::{HTMLIFrameElement, NavigationType};
+use crate::dom::htmliframeelement::HTMLIFrameElement;
 use crate::dom::identityhub::Identities;
 use crate::dom::mutationobserver::MutationObserver;
 use crate::dom::node::{window_from_node, Node, ShadowIncluding};
@@ -3705,11 +3705,7 @@ impl ScriptThread {
             .borrow()
             .find_iframe(parent_pipeline_id, browsing_context_id);
         if let Some(iframe) = iframe {
-            iframe.navigate_or_reload_child_browsing_context(
-                load_data,
-                NavigationType::Regular,
-                replace,
-            );
+            iframe.navigate_or_reload_child_browsing_context(load_data, replace);
         }
     }
 
diff --git a/tests/wpt/metadata/html/browsers/windows/noreferrer-window-name.html.ini b/tests/wpt/metadata/html/browsers/windows/noreferrer-window-name.html.ini
index 711a7a94e41..9a82f602427 100644
--- a/tests/wpt/metadata/html/browsers/windows/noreferrer-window-name.html.ini
+++ b/tests/wpt/metadata/html/browsers/windows/noreferrer-window-name.html.ini
@@ -5,9 +5,6 @@
   [Following a noreferrer link with a named target should not cause creation of a window that can be targeted by another noreferrer link with the same named target]
     expected: TIMEOUT
 
-  [Targeting a rel=noreferrer link at an existing named subframe should work]
-    expected: FAIL
-
   [Targeting a rel=noreferrer link at an existing named window should work]
     expected: FAIL
 
diff --git a/tests/wpt/metadata/html/semantics/forms/form-submission-0/form-double-submit-2.html.ini b/tests/wpt/metadata/html/semantics/forms/form-submission-0/form-double-submit-2.html.ini
deleted file mode 100644
index 633a99517d3..00000000000
--- a/tests/wpt/metadata/html/semantics/forms/form-submission-0/form-double-submit-2.html.ini
+++ /dev/null
@@ -1,4 +0,0 @@
-[form-double-submit-2.html]
-  [preventDefault should allow onclick submit() to succeed]
-    expected: FAIL
-
diff --git a/tests/wpt/metadata/html/semantics/forms/form-submission-0/form-double-submit.html.ini b/tests/wpt/metadata/html/semantics/forms/form-submission-0/form-double-submit.html.ini
deleted file mode 100644
index b193c33c2b6..00000000000
--- a/tests/wpt/metadata/html/semantics/forms/form-submission-0/form-double-submit.html.ini
+++ /dev/null
@@ -1,4 +0,0 @@
-[form-double-submit.html]
-  [default submit action should supersede onclick submit()]
-    expected: FAIL
-
diff --git a/tests/wpt/metadata/xhr/open-url-multi-window-6.htm.ini b/tests/wpt/metadata/xhr/open-url-multi-window-6.htm.ini
index 59ebafd80c5..3a297970115 100644
--- a/tests/wpt/metadata/xhr/open-url-multi-window-6.htm.ini
+++ b/tests/wpt/metadata/xhr/open-url-multi-window-6.htm.ini
@@ -1,6 +1,4 @@
 [open-url-multi-window-6.htm]
-  type: testharness
-  expected: TIMEOUT
   [XMLHttpRequest: open() in document that is not fully active (but may be active) should throw]
-    expected: TIMEOUT
+    expected: FAIL