constellation: crash to a new “sad tab” error page (servo#30290)

* constellation: crash to a new “sad tab” page

* check in resources/crash.html

* use a separate enum variant instead of keying on reason

* fmt + tidy

* rename Resource::Crash to Resource::CrashHTML

* clean up crash page and add details (reason + backtrace)

* avoid repeating crash errors in script::script_thread warn log

* make new LoadData init more idiomatic

* clarify comments and new fields

* fix doc comment style

components/constellation/constellation.rs

@@ -2782,7 +2782,7 @@ where
 
         self.embedder_proxy.send((
             Some(top_level_browsing_context_id),
-            EmbedderMsg::Panic(reason, backtrace),
+            EmbedderMsg::Panic(reason.clone(), backtrace.clone()),
         ));
 
         let browsing_context = match self.browsing_contexts.get(&browsing_context_id) {
@@ -2797,7 +2797,6 @@ where
             Some(p) => p,
             None => return warn!("failed pipeline is missing"),
         };
-        let pipeline_url = pipeline.url.clone();
         let opener = pipeline.opener;
 
         self.close_browsing_context_children(
@@ -2806,23 +2805,27 @@ where
             ExitPipelineMode::Force,
         );
 
-        let failure_url = ServoUrl::parse("about:failure").expect("infallible");
-
-        if pipeline_url == failure_url {
-            return error!("about:failure failed");
+        let old_pipeline_id = pipeline_id;
+        let old_load_data = match self.pipelines.get(&pipeline_id) {
+            Some(pipeline) => pipeline.load_data.clone(),
+            None => return warn!("failed pipeline is missing"),
+        };
+        if old_load_data.crash.is_some() {
+            return error!("crash page crashed");
         }
 
-        warn!("creating replacement pipeline for about:failure");
+        warn!("creating replacement pipeline for crash page");
 
         let new_pipeline_id = PipelineId::new();
-        let load_data = LoadData::new(
-            LoadOrigin::Constellation,
-            failure_url,
-            None,
-            Referrer::NoReferrer,
-            None,
-            None,
-        );
+        let new_load_data = LoadData {
+            crash: Some(
+                backtrace
+                    .map(|b| format!("{}\n{}", reason, b))
+                    .unwrap_or(reason),
+            ),
+            ..old_load_data.clone()
+        };
+
         let sandbox = IFrameSandboxState::IFrameSandboxed;
         let is_private = false;
         self.new_pipeline(
@@ -2832,7 +2835,7 @@ where
             None,
             opener,
             window_size,
-            load_data,
+            new_load_data,
             sandbox,
             is_private,
             is_visible,
@@ -2841,7 +2844,9 @@ where
             top_level_browsing_context_id,
             browsing_context_id,
             new_pipeline_id,
-            replace: None,
+            // Pipeline already closed by close_browsing_context_children, so we can pass Yes here
+            // to avoid closing again in handle_activate_document_msg (though it would be harmless)
+            replace: Some(NeedsToReload::Yes(old_pipeline_id, old_load_data)),
             new_browsing_context_info: None,
             window_size,
         });

components/embedder_traits/resources.rs

@@ -61,6 +61,7 @@ pub enum Resource {
     RippyPNG,
     MediaControlsCSS,
     MediaControlsJS,
+    CrashHTML,
 }
 
 pub trait ResourceReaderMethods {
@@ -96,6 +97,7 @@ fn resources_for_tests() -> Box<dyn ResourceReaderMethods + Sync + Send> {
                 Resource::RippyPNG => "rippy.png",
                 Resource::MediaControlsCSS => "media-controls.css",
                 Resource::MediaControlsJS => "media-controls.js",
+                Resource::CrashHTML => "crash.html",
             };
             let mut path = env::current_exe().unwrap();
             path = path.canonicalize().unwrap();

components/net/fetch/methods.rs

@@ -195,6 +195,13 @@ pub async fn main_fetch(
     // Step 1.
     let mut response = None;
 
+    // Servo internal: return a crash error when a crash error page is needed
+    if let Some(ref details) = request.crash {
+        response = Some(Response::network_error(NetworkError::Crash(
+            details.clone(),
+        )));
+    }
+
     // Step 2.
     if request.local_urls_only {
         if !matches!(

components/net_traits/lib.rs

@@ -758,8 +758,10 @@ pub enum NetworkError {
     /// Could be any of the internal errors, like unsupported scheme, connection errors, etc.
     Internal(String),
     LoadCancelled,
-    /// SSL validation error that has to be handled in the HTML parser
+    /// SSL validation error, to be converted to Resource::BadCertHTML in the HTML parser.
     SslValidation(String, Vec<u8>),
+    /// Crash error, to be converted to Resource::Crash in the HTML parser.
+    Crash(String),
 }
 
 impl NetworkError {

components/net_traits/request.rs

@@ -254,6 +254,8 @@ pub struct RequestBuilder {
     pub initiator: Initiator,
     pub https_state: HttpsState,
     pub response_tainting: ResponseTainting,
+    /// Servo internal: if crash details are present, trigger a crash error page with these details.
+    pub crash: Option<String>,
 }
 
 impl RequestBuilder {
@@ -284,6 +286,7 @@ impl RequestBuilder {
             csp_list: None,
             https_state: HttpsState::None,
             response_tainting: ResponseTainting::Basic,
+            crash: None,
         }
     }
 
@@ -382,6 +385,11 @@ impl RequestBuilder {
         self
     }
 
+    pub fn crash(mut self, crash: Option<String>) -> Self {
+        self.crash = crash;
+        self
+    }
+
     pub fn build(self) -> Request {
         let mut request = Request::new(
             self.url.clone(),
@@ -415,6 +423,7 @@ impl RequestBuilder {
         request.parser_metadata = self.parser_metadata;
         request.csp_list = self.csp_list;
         request.response_tainting = self.response_tainting;
+        request.crash = self.crash;
         request
     }
 }
@@ -488,6 +497,8 @@ pub struct Request {
     #[ignore_malloc_size_of = "Defined in rust-content-security-policy"]
     pub csp_list: Option<CspList>,
     pub https_state: HttpsState,
+    /// Servo internal: if crash details are present, trigger a crash error page with these details.
+    pub crash: Option<String>,
 }
 
 impl Request {
@@ -528,6 +539,7 @@ impl Request {
             response_tainting: ResponseTainting::Basic,
             csp_list: None,
             https_state: https_state,
+            crash: None,
         }
     }
 

components/script/dom/servoparser/mod.rs

@@ -774,28 +774,29 @@ impl FetchResponseListener for ParserContext {
     fn process_request_eof(&mut self) {}
 
     fn process_response(&mut self, meta_result: Result<FetchMetadata, NetworkError>) {
-        let mut ssl_error = None;
-        let mut network_error = None;
-        let metadata = match meta_result {
-            Ok(meta) => Some(match meta {
-                FetchMetadata::Unfiltered(m) => m,
-                FetchMetadata::Filtered { unsafe_, .. } => unsafe_,
-            }),
-            Err(NetworkError::SslValidation(reason, cert_bytes)) => {
-                ssl_error = Some((reason, cert_bytes));
-                let mut meta = Metadata::default(self.url.clone());
-                let mime: Option<Mime> = "text/html".parse().ok();
-                meta.set_content_type(mime.as_ref());
-                Some(meta)
-            },
-            Err(NetworkError::Internal(reason)) => {
-                network_error = Some(reason);
-                let mut meta = Metadata::default(self.url.clone());
-                let mime: Option<Mime> = "text/html".parse().ok();
-                meta.set_content_type(mime.as_ref());
-                Some(meta)
-            },
-            Err(_) => None,
+        let (metadata, error) = match meta_result {
+            Ok(meta) => (
+                Some(match meta {
+                    FetchMetadata::Unfiltered(m) => m,
+                    FetchMetadata::Filtered { unsafe_, .. } => unsafe_,
+                }),
+                None,
+            ),
+            Err(error) => (
+                // Check variant without moving
+                match &error {
+                    NetworkError::SslValidation(..) |
+                    NetworkError::Internal(..) |
+                    NetworkError::Crash(..) => {
+                        let mut meta = Metadata::default(self.url.clone());
+                        let mime: Option<Mime> = "text/html".parse().ok();
+                        meta.set_content_type(mime.as_ref());
+                        Some(meta)
+                    },
+                    _ => None,
+                },
+                Some(error),
+            ),
         };
         let content_type: Option<Mime> = metadata
             .clone()
@@ -876,8 +877,8 @@ impl FetchResponseListener for ParserContext {
                 parser.parse_sync();
                 parser.tokenizer.borrow_mut().set_plaintext_state();
             },
-            (mime::TEXT, mime::HTML, _) => {
-                if let Some((reason, bytes)) = ssl_error {
+            (mime::TEXT, mime::HTML, _) => match error {
+                Some(NetworkError::SslValidation(reason, bytes)) => {
                     self.is_synthesized_document = true;
                     let page = resources::read_string(Resource::BadCertHTML);
                     let page = page.replace("${reason}", &reason);
@@ -887,14 +888,23 @@ impl FetchResponseListener for ParserContext {
                         page.replace("${secret}", &net_traits::PRIVILEGED_SECRET.to_string());
                     parser.push_string_input_chunk(page);
                     parser.parse_sync();
-                }
-                if let Some(reason) = network_error {
+                },
+                Some(NetworkError::Internal(reason)) => {
                     self.is_synthesized_document = true;
                     let page = resources::read_string(Resource::NetErrorHTML);
                     let page = page.replace("${reason}", &reason);
                     parser.push_string_input_chunk(page);
                     parser.parse_sync();
-                }
+                },
+                Some(NetworkError::Crash(details)) => {
+                    self.is_synthesized_document = true;
+                    let page = resources::read_string(Resource::CrashHTML);
+                    let page = page.replace("${details}", &details);
+                    parser.push_string_input_chunk(page);
+                    parser.parse_sync();
+                },
+                Some(_) => {},
+                None => {},
             },
             (mime::TEXT, mime::XML, _) |
             (mime::APPLICATION, mime::XML, _) |

components/script/fetch.rs

@@ -129,6 +129,7 @@ fn request_init_from_request(request: NetTraitsRequest) -> RequestBuilder {
         csp_list: None,
         https_state: request.https_state,
         response_tainting: request.response_tainting,
+        crash: None,
     }
 }
 

components/script/script_thread.rs

@@ -3843,7 +3843,8 @@ impl ScriptThread {
             .headers(load_data.headers)
             .body(load_data.data)
             .redirect_mode(RedirectMode::Manual)
-            .origin(incomplete.origin.immutable().clone());
+            .origin(incomplete.origin.immutable().clone())
+            .crash(load_data.crash);
 
         let context = ParserContext::new(id, load_data.url);
         self.incomplete_parser_contexts
@@ -3869,6 +3870,7 @@ impl ScriptThread {
     ) {
         match fetch_metadata {
             Ok(_) => (),
+            Err(NetworkError::Crash(..)) => (),
             Err(ref e) => {
                 warn!("Network error: {:?}", e);
             },

components/script_traits/lib.rs

@@ -184,6 +184,9 @@ pub struct LoadData {
     pub srcdoc: String,
     /// The inherited context is Secure, None if not inherited
     pub inherited_secure_context: Option<bool>,
+
+    /// Servo internal: if crash details are present, trigger a crash error page with these details.
+    pub crash: Option<String>,
 }
 
 /// The result of evaluating a javascript scheme url.
@@ -218,6 +221,7 @@ impl LoadData {
             referrer_policy: referrer_policy,
             srcdoc: "".to_string(),
             inherited_secure_context,
+            crash: None,
         }
     }
 }

ports/gstplugin/resources.rs

@@ -35,6 +35,7 @@ fn filename(file: Resource) -> &'static str {
         Resource::RippyPNG => "rippy.png",
         Resource::MediaControlsCSS => "media-controls.css",
         Resource::MediaControlsJS => "media-controls.js",
+        Resource::CrashHTML => "crash.html",
     }
 }
 

ports/libsimpleservo/api/src/lib.rs

@@ -920,6 +920,7 @@ impl ResourceReaderMethods for ResourceReaderInstance {
             Resource::MediaControlsJS => {
                 &include_bytes!("../../../../resources/media-controls.js")[..]
             },
+            Resource::CrashHTML => &include_bytes!("../../../../resources/crash.html")[..],
         })
     }
 

ports/servoshell/resources.rs

@@ -30,6 +30,7 @@ fn filename(file: Resource) -> &'static str {
         Resource::RippyPNG => "rippy.png",
         Resource::MediaControlsCSS => "media-controls.css",
         Resource::MediaControlsJS => "media-controls.js",
+        Resource::CrashHTML => "crash.html",
     }
 }
 

resources/crash.html

@@ -0,0 +1,8 @@
+<p>Servo crashed!</p>
+
+<!-- NOTE: unlike in Firefox and Chrome, this reloads POST as GET -->
+<!-- see whatwg/html#6600 + whatwg/html#3215 -->
+<button onclick="location.reload()">Reload</button>
+
+<pre><plaintext>
+${details}