From bd94242e723fec5ab8c1f3ebace66185ff69fd2e Mon Sep 17 00:00:00 2001
From: Mukilan Thiyagarajan <me@mukilan.in>
Date: Fri, 2 Jun 2023 17:48:56 +0530
Subject: [PATCH] Fix `mach test-wpt` to make crash tests work

There are two issues related to crash tests:
1. test-wpt is unable to find existing crash tests even when
   called with --test-types=crashtests. The fix here is to
   add crashtests to the default test suite types to python/wpt/run.py
2. When running in headless mode, crashes in style threads
   don't cause servo to crash because the logic in constellation.rs
   currently calls handle_panic only when the top-level browsing
   context id is some value. Since style pool threads are shared,
   they always generate Panic messages with None as top-level
   browsing context id.

Signed-off-by: Mukilan Thiyagarajan <me@mukilan.in>
---
 components/constellation/constellation.rs | 35 +++++++++++++----------
 python/wpt/run.py                         |  2 +-
 2 files changed, 21 insertions(+), 16 deletions(-)

diff --git a/components/constellation/constellation.rs b/components/constellation/constellation.rs
index 8ada8aa654d40..f17b0e888de45 100644
--- a/components/constellation/constellation.rs
+++ b/components/constellation/constellation.rs
@@ -1491,11 +1491,10 @@ where
             // Panic a top level browsing context.
             FromCompositorMsg::SendError(top_level_browsing_context_id, error) => {
                 debug!("constellation got SendError message");
-                if let Some(id) = top_level_browsing_context_id {
-                    self.handle_panic(id, error, None);
-                } else {
+                if top_level_browsing_context_id.is_none() {
                     warn!("constellation got a SendError message without top level id");
                 }
+                self.handle_panic(top_level_browsing_context_id, error, None);
             },
             // Send frame tree to WebRender. Make it visible.
             FromCompositorMsg::SelectBrowser(top_level_browsing_context_id) => {
@@ -2757,15 +2756,13 @@ where
             .pipelines
             .get(&pipeline_id)
             .map(|pipeline| pipeline.top_level_browsing_context_id);
-        if let Some(top_level_browsing_context_id) = top_level_browsing_context_id {
-            let reason = format!("Send failed ({})", err);
-            self.handle_panic(top_level_browsing_context_id, reason, None);
-        }
+        let reason = format!("Send failed ({})", err);
+        self.handle_panic(top_level_browsing_context_id, reason, None);
     }
 
     fn handle_panic(
         &mut self,
-        top_level_browsing_context_id: TopLevelBrowsingContextId,
+        top_level_browsing_context_id: Option<TopLevelBrowsingContextId>,
         reason: String,
         backtrace: Option<String>,
     ) {
@@ -2776,6 +2773,11 @@ where
             process::exit(1);
         }
 
+        let top_level_browsing_context_id = match top_level_browsing_context_id {
+            Some(id) => id,
+            None => return,
+        };
+
         debug!(
             "Panic handler for top-level browsing context {}: {}.",
             top_level_browsing_context_id, reason
@@ -2857,13 +2859,16 @@ where
         entry: LogEntry,
     ) {
         debug!("Received log entry {:?}.", entry);
-        match (entry, top_level_browsing_context_id) {
-            (LogEntry::Panic(reason, backtrace), Some(top_level_browsing_context_id)) => {
-                self.handle_panic(top_level_browsing_context_id, reason, Some(backtrace));
-            },
-            (LogEntry::Panic(reason, _), _) |
-            (LogEntry::Error(reason), _) |
-            (LogEntry::Warn(reason), _) => {
+        if let LogEntry::Panic(ref reason, ref backtrace) = entry {
+            self.handle_panic(
+                top_level_browsing_context_id,
+                reason.clone(),
+                Some(backtrace.clone()),
+            );
+        }
+
+        match entry {
+            LogEntry::Panic(reason, _) | LogEntry::Error(reason) | LogEntry::Warn(reason) => {
                 // VecDeque::truncate is unstable
                 if WARNINGS_BUFFER_SIZE <= self.handled_warnings.len() {
                     self.handled_warnings.pop_front();
diff --git a/python/wpt/run.py b/python/wpt/run.py
index e3fd368179023..f410968aa6660 100644
--- a/python/wpt/run.py
+++ b/python/wpt/run.py
@@ -102,7 +102,7 @@ def run_tests(**kwargs):
 
     if not kwargs.get("no_default_test_types"):
         test_types = {
-            "servo": ["testharness", "reftest", "wdspec"],
+            "servo": ["testharness", "reftest", "wdspec", "crashtest"],
             "servodriver": ["testharness", "reftest"],
         }
         product = kwargs.get("product") or "servo"