Auto merge of #27163 - alarsyo:23053-layout-queries-disconnected-frames-panic, r=jdm

Return Option for Window's layout channel

<!-- Please describe your changes on the following line: -->

`Window::layout_chan()` now returns an `Option<Sender<Msg>>`, returning `None` if the window is dead.

FIX #26969
FIX #26429
FIX #21208
FIX #19092
FIX #22559
FIX #22584
FIX #22652

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #23053

<!-- Either: -->
- [x] There are tests for these changes

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

This is my first contribution, I'm trying to figure things out!

This fix passes the test case shown in #23053, however I don't know what the behavior should be in `Document` and `ScriptThread` if `Window::is_alive()` is false : simply ignore it, don't do anything ? Or is this something that should not happen now that we return false in `Window::force_reflow()` ?

I'm not sure about the directory where the test case should go, any advice?

components/script/dom/document.rs

@@ -804,10 +804,10 @@ impl Document {
         self.quirks_mode.set(mode);
 
         if mode == QuirksMode::Quirks {
-            self.window
-                .layout_chan()
-                .send(Msg::SetQuirksMode(mode))
-                .unwrap();
+            match self.window.layout_chan() {
+                Some(chan) => chan.send(Msg::SetQuirksMode(mode)).unwrap(),
+                None => warn!("Layout channel unavailable"),
+            }
         }
     }
 
@@ -3724,13 +3724,15 @@ impl Document {
             })
             .cloned();
 
-        self.window
-            .layout_chan()
+        match self.window.layout_chan() {
+            Some(chan) => chan
                 .send(Msg::AddStylesheet(
                     sheet.clone(),
                     insertion_point.as_ref().map(|s| s.sheet.clone()),
                 ))
-            .unwrap();
+                .unwrap(),
+            None => return warn!("Layout channel unavailable"),
+        }
 
         DocumentOrShadowRoot::add_stylesheet(
             owner,
@@ -3744,10 +3746,10 @@ impl Document {
     /// Remove a stylesheet owned by `owner` from the list of document sheets.
     #[allow(unrooted_must_root)] // Owner needs to be rooted already necessarily.
     pub fn remove_stylesheet(&self, owner: &Element, s: &Arc<Stylesheet>) {
-        self.window
-            .layout_chan()
-            .send(Msg::RemoveStylesheet(s.clone()))
-            .unwrap();
+        match self.window.layout_chan() {
+            Some(chan) => chan.send(Msg::RemoveStylesheet(s.clone())).unwrap(),
+            None => return warn!("Layout channel unavailable"),
+        }
 
         DocumentOrShadowRoot::remove_stylesheet(
             owner,

components/script/dom/window.rs

@@ -224,6 +224,9 @@ pub struct Window {
     js_runtime: DomRefCell<Option<Rc<Runtime>>>,
 
     /// A handle for communicating messages to the layout thread.
+    ///
+    /// This channel shouldn't be accessed directly, but through `Window::layout_chan()`,
+    /// which returns `None` if there's no layout thread anymore.
     #[ignore_malloc_size_of = "channels are hard"]
     layout_chan: Sender<Msg>,
 
@@ -1553,12 +1556,15 @@ impl Window {
         // TODO Step 1
         // TODO(mrobinson, #18709): Add smooth scrolling support to WebRender so that we can
         // properly process ScrollBehavior here.
-        self.layout_chan
+        match self.layout_chan() {
+            Some(chan) => chan
                 .send(Msg::UpdateScrollStateFromScript(ScrollState {
                     scroll_id,
                     scroll_offset: Vector2D::new(-x, -y),
                 }))
-            .unwrap();
+                .unwrap(),
+            None => warn!("Layout channel unavailable"),
+        }
     }
 
     pub fn update_viewport_for_scroll(&self, x: f32, y: f32) {
@@ -1696,9 +1702,12 @@ impl Window {
             animations: document.animations().sets.clone(),
         };
 
-        self.layout_chan
+        match self.layout_chan() {
+            Some(layout_chan) => layout_chan
                 .send(Msg::Reflow(reflow))
-            .expect("Layout thread disconnected.");
+                .expect("Layout thread disconnected"),
+            None => return false,
+        };
 
         debug!("script: layout forked");
 
@@ -2102,8 +2111,12 @@ impl Window {
         self.Document().url()
     }
 
-    pub fn layout_chan(&self) -> &Sender<Msg> {
-        &self.layout_chan
+    pub fn layout_chan(&self) -> Option<&Sender<Msg>> {
+        if self.is_alive() {
+            Some(&self.layout_chan)
+        } else {
+            None
+        }
     }
 
     pub fn windowproxy_handler(&self) -> WindowProxyHandler {

components/script/script_thread.rs

@@ -1172,9 +1172,13 @@ impl ScriptThread {
                 );
             },
         };
-        let _ = window
-            .layout_chan()
-            .send(Msg::RegisterPaint(name, properties, painter));
+
+        match window.layout_chan() {
+            Some(chan) => chan
+                .send(Msg::RegisterPaint(name, properties, painter))
+                .unwrap(),
+            None => warn!("Layout channel unavailable"),
+        }
     }
 
     pub fn push_new_element_queue() {
@@ -2467,12 +2471,12 @@ impl ScriptThread {
         });
 
         // Pick a layout thread, any layout thread
-        let current_layout_chan = self
+        let current_layout_chan: Option<Sender<Msg>> = self
             .documents
             .borrow()
             .iter()
             .next()
-            .map(|(_, document)| document.window().layout_chan().clone())
+            .and_then(|(_, document)| document.window().layout_chan().cloned())
             .or_else(|| {
                 self.incomplete_loads
                     .borrow()
@@ -2879,7 +2883,10 @@ impl ScriptThread {
             let load = self.incomplete_loads.borrow_mut().remove(idx);
             load.layout_chan.clone()
         } else if let Some(ref document) = document {
-            document.window().layout_chan().clone()
+            match document.window().layout_chan() {
+                Some(chan) => chan.clone(),
+                None => return warn!("Layout channel unavailable"),
+            }
         } else {
             return warn!("Exiting nonexistant pipeline {}.", id);
         };

tests/wpt/mozilla/meta/MANIFEST.json

@@ -13361,6 +13361,13 @@
       {}
      ]
     ],
+    "detached_layout.html": [
+     "b9fe090cc29c147f493732327e36b06702c4f846",
+     [
+      null,
+      {}
+     ]
+    ],
     "deterministic-raf.html": [
      "441664829a14379ebc92306f42ab0bad6581257e",
      [

tests/wpt/mozilla/tests/mozilla/detached_layout.html

@@ -0,0 +1,23 @@
+<!doctype html>
+<meta charset="utf-8">
+<title>Detached layout doesn't panic</title>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<iframe></iframe>
+<script>
+    setup({ single_test: true });
+
+    var i = document.querySelector('iframe');
+    var d = i.contentDocument;
+    var e = d.createElement('div');
+    d.body.appendChild(e);
+    i.remove();
+
+    setTimeout(function () {
+        let r = e.getBoundingClientRect();
+        assert_equals(r.width, 0, "rectangle should be zero-sized");
+        assert_equals(r.height, 0, "rectangle should be zero-sized");
+
+        done();
+    }, 10);
+</script>