script: when handling page headers, stop if pipeline closed already (#38739)

Previously, the script-thread would assert the pipeline was closed if no
pending load was found, but it did not check whether the pipeline was
closed before processing the page headers. Since incomplete loads are
removed only when page headers are processed, this means the page
headers were processed even if the pipeline had been closed before the
page headers were available. If the pipeline had been closed as part of
exiting the constellation, it was possible for the constellation to have
exited by the time the page headers became available(since the
script-thread closes a pipeline independently from ongoing navigation
fetches), which would produce a panic on trying to communicate with the
constellation to obtain the browsing context info.

Note: due to the nature of the problem, I cannot verify that this fixes
the crash test, although logically this appears to make sense, and a
couple of days of WPT runs should tell us more.

Testing: A crash test was added; unfortunately the crash was
intermittent.
Fixes: https://github.com/servo/servo/issues/36747

---------

Signed-off-by: gterzian <2792687+gterzian@users.noreply.github.com>

components/script/script_thread.rs

@@ -2840,52 +2840,51 @@ impl ScriptThread {
         metadata: Option<Metadata>,
         can_gc: CanGc,
     ) -> Option<DomRoot<ServoParser>> {
-        let idx = self
+        if self.closed_pipelines.borrow().contains(id) {
+            // If the pipeline closed, do not process the headers.
+            return None;
+        }
+
+        let Some(idx) = self
             .incomplete_loads
             .borrow()
             .iter()
-            .position(|load| load.pipeline_id == *id);
+            .position(|load| load.pipeline_id == *id)
-        // The matching in progress load structure may not exist if
+        else {
-        // the pipeline exited before the page load completed.
+            unreachable!("Pipeline shouldn't have finished loading.");
-        match idx {
+        };
-            Some(idx) => {
-                // https://html.spec.whatwg.org/multipage/#process-a-navigate-response
-                // 2. If response's status is 204 or 205, then abort these steps.
-                let is20x = match metadata {
-                    Some(ref metadata) => metadata.status.in_range(204..=205),
-                    _ => false,
-                };
 
-                if is20x {
+        // https://html.spec.whatwg.org/multipage/#process-a-navigate-response
-                    // If we have an existing window that is being navigated:
+        // 2. If response's status is 204 or 205, then abort these steps.
-                    if let Some(window) = self.documents.borrow().find_window(*id) {
+        let is_204_205 = match metadata {
-                        let window_proxy = window.window_proxy();
+            Some(ref metadata) => metadata.status.in_range(204..=205),
-                        // https://html.spec.whatwg.org/multipage/
+            _ => false,
-                        // #navigating-across-documents:delaying-load-events-mode-2
+        };
-                        if window_proxy.parent().is_some() {
-                            // The user agent must take this nested browsing context
-                            // out of the delaying load events mode
-                            // when this navigation algorithm later matures,
-                            // or when it terminates (whether due to having run all the steps,
-                            // or being canceled, or being aborted), whichever happens first.
-                            window_proxy.stop_delaying_load_events_mode();
-                        }
-                    }
-                    self.senders
-                        .pipeline_to_constellation_sender
-                        .send((*id, ScriptToConstellationMessage::AbortLoadUrl))
-                        .unwrap();
-                    return None;
-                };
 
-                let load = self.incomplete_loads.borrow_mut().remove(idx);
+        if is_204_205 {
-                metadata.map(|meta| self.load(meta, load, can_gc))
+            // If we have an existing window that is being navigated:
-            },
+            if let Some(window) = self.documents.borrow().find_window(*id) {
-            None => {
+                let window_proxy = window.window_proxy();
-                assert!(self.closed_pipelines.borrow().contains(id));
+                // https://html.spec.whatwg.org/multipage/
-                None
+                // #navigating-across-documents:delaying-load-events-mode-2
-            },
+                if window_proxy.parent().is_some() {
-        }
+                    // The user agent must take this nested browsing context
+                    // out of the delaying load events mode
+                    // when this navigation algorithm later matures,
+                    // or when it terminates (whether due to having run all the steps,
+                    // or being canceled, or being aborted), whichever happens first.
+                    window_proxy.stop_delaying_load_events_mode();
+                }
+            }
+            self.senders
+                .pipeline_to_constellation_sender
+                .send((*id, ScriptToConstellationMessage::AbortLoadUrl))
+                .unwrap();
+            return None;
+        };
+
+        let load = self.incomplete_loads.borrow_mut().remove(idx);
+        metadata.map(|meta| self.load(meta, load, can_gc))
     }
 
     /// Handles a request for the window title.

tests/wpt/meta/trusted-types/trusted-types-navigation.html.ini

@@ -73,7 +73,21 @@
 
 
 [trusted-types-navigation.html?26-30]
-  expected: CRASH
+  [Navigate a window via form-submission with javascript:-urls w/ default policy in enforcing mode.]
+    expected: FAIL
+
+  [Navigate a window via form-submission with javascript:-urls in report-only mode.]
+    expected: FAIL
+
+  [Navigate a window via form-submission with javascript:-urls w/ default policy in report-only mode.]
+    expected: FAIL
+
+  [Navigate a frame via form-submission with javascript:-urls in enforcing mode.]
+    expected: FAIL
+
+  [Navigate a frame via form-submission with javascript:-urls w/ default policy in enforcing mode.]
+    expected: FAIL
+
 
 [trusted-types-navigation.html?06-10]
   expected: CRASH
@@ -94,7 +108,22 @@
 
 
 [trusted-types-navigation.html?11-15]
-  expected: CRASH
+  expected: TIMEOUT
+  [Navigate a window via anchor with javascript:-urls w/ a default policy making the URL invalid in enforcing mode.]
+    expected: FAIL
+
+  [Navigate a window via anchor with javascript:-urls w/ a default policy making the URL invalid in report-only mode.]
+    expected: FAIL
+
+  [Navigate a window via area with javascript:-urls in enforcing mode.]
+    expected: FAIL
+
+  [Navigate a window via area with javascript:-urls w/ default policy in enforcing mode.]
+    expected: TIMEOUT
+
+  [Navigate a window via area with javascript:-urls in report-only mode.]
+    expected: NOTRUN
+
 
 [trusted-types-navigation.html?16-20]
   expected: TIMEOUT
@@ -132,4 +161,17 @@
 
 
 [trusted-types-navigation.html?01-05]
-  expected: CRASH
+  [Navigate a window via anchor with javascript:-urls in enforcing mode.]
+    expected: FAIL
+
+  [Navigate a window via anchor with javascript:-urls w/ default policy in enforcing mode.]
+    expected: FAIL
+
+  [Navigate a window via anchor with javascript:-urls in report-only mode.]
+    expected: FAIL
+
+  [Navigate a window via anchor with javascript:-urls w/ default policy in report-only mode.]
+    expected: FAIL
+
+  [Navigate a frame via anchor with javascript:-urls in enforcing mode.]
+    expected: FAIL

tests/wpt/mozilla/meta/MANIFEST.json

@@ -32,6 +32,13 @@
       {}
      ]
     ],
+    "form-navigate-loop-crash.html": [
+     "1068ccc38eed223cf83a4cbd8fed336b83c0b745",
+     [
+      null,
+      {}
+     ]
+    ],
     "form_reset-crash.html": [
      "b23cbf6aefdef8231e2cc4cb0e6416195d5bdf71",
      [

tests/wpt/mozilla/tests/mozilla/form-navigate-loop-crash.html

@@ -0,0 +1,10 @@
+<!DOCTYPE html>
+<script>
+  window.addEventListener("load", _ => {
+    form.submit();
+    window.frames.stop();
+    iframe.src = "data:text/html,";
+  });
+</script>
+<form id="form">
+<iframe id="iframe"></iframe>