Fix: Browser context leak causes Chrome crash and cascading test failures

toolproof/Cargo.toml

@@ -27,7 +27,7 @@ console = "0.16"
 dialoguer = { version = "0.12", features = ["fuzzy-select"] }
 async-trait = "0.1.88"
 pagebrowse = "0.1.1"
-chromiumoxide = "0.7"
+chromiumoxide = "0.9"
 clap = { version = "4", features = ["cargo"] }
 schematic = { version = "0.18.12", features = ["yaml"] }
 strip-ansi-escapes = "0.2.1"

toolproof/src/civilization.rs

@@ -40,11 +40,27 @@ impl<'u> Civilization<'u> {
     pub async fn shutdown(mut self) {
         self.stop_servers().await;
 
-        if let Some(BrowserWindow::Chrome(window)) = self.window {
-            window
-                .close()
-                .await
-                .expect("Failed to close browser window");
+        if let Some(BrowserWindow::Chrome {
+            page,
+            context_id,
+            browser,
+        }) = self.window
+        {
+            match tokio::time::timeout(Duration::from_secs(5), async {
+                if let Err(e) = page.close().await {
+                    eprintln!("[toolproof] Warning: Failed to close browser window: {e}");
+                }
+                if let Err(e) = browser.dispose_browser_context(context_id).await {
+                    eprintln!("[toolproof] Warning: Failed to dispose browser context: {e}");
+                }
+            })
+            .await
+            {
+                Ok(()) => {}
+                Err(_) => {
+                    eprintln!("[toolproof] Warning: Timed out cleaning up browser window");
+                }
+            }
         }
     }
 }

toolproof/src/definitions/browser/mod.rs

@@ -3,6 +3,7 @@ use std::sync::Arc;
 
 use async_trait::async_trait;
 use chromiumoxide::cdp::browser_protocol::page::CaptureScreenshotParams;
+use chromiumoxide::cdp::browser_protocol::browser::BrowserContextId;
 use chromiumoxide::cdp::browser_protocol::target::{
     CreateBrowserContextParams, CreateTargetParams,
 };
@@ -67,14 +68,12 @@ async fn try_launch_browser(mut max: usize, visible: bool) -> (Browser, chromium
     let mut launch = Err(CdpError::NotFound);
     while launch.is_err() && max > 0 {
         max -= 1;
-        let headless_mode = if visible {
-            chromiumoxide::browser::HeadlessMode::False
-        } else {
-            chromiumoxide::browser::HeadlessMode::New
-        };
+        let mut builder = BrowserConfig::builder();
+        if visible {
+            builder = builder.with_head();
+        }
         launch = Browser::launch(
-            BrowserConfig::builder()
-                .headless_mode(headless_mode)
+            builder
                 .user_data_dir(tempdir().expect("testing on a system with a temp dir"))
                 .viewport(Some(Viewport {
                     width: 1600,
@@ -145,7 +144,7 @@ impl BrowserTester {
                 browser_timeout,
                 ..
             } => {
-                let context = browser
+                let context_id = browser
                     .create_browser_context(CreateBrowserContextParams {
                         dispose_on_detach: Some(true),
                         proxy_server: None,
@@ -158,33 +157,45 @@ impl BrowserTester {
                     .new_page(CreateTargetParams {
                         url: "about:blank".to_string(),
                         for_tab: None,
+                        left: None,
+                        top: None,
                         width: None,
                         height: None,
-                        browser_context_id: Some(context),
+                        window_state: None,
+                        browser_context_id: Some(context_id.clone()),
                         enable_begin_frame_control: None,
                         new_window: None,
                         background: None,
+                        hidden: None,
                     })
                     .await
                     .unwrap();
                 page.evaluate_on_new_document(init_script(*browser_timeout))
                     .await
                     .expect("Could not set initialization js");
-                BrowserWindow::Chrome(page)
+                BrowserWindow::Chrome {
+                    page,
+                    context_id,
+                    browser: Arc::clone(browser),
+                }
             }
         }
     }
 }
 
 pub enum BrowserWindow {
-    Chrome(chromiumoxide::Page),
+    Chrome {
+        page: chromiumoxide::Page,
+        context_id: BrowserContextId,
+        browser: Arc<Browser>,
+    },
     Pagebrowse(PagebrowserWindow),
 }
 
 impl BrowserWindow {
     async fn navigate(&self, url: String, wait_for_load: bool) -> Result<(), ToolproofStepError> {
         match self {
-            BrowserWindow::Chrome(page) => {
+            BrowserWindow::Chrome { page, .. } => {
                 // TODO: This is implicitly always wait_for_load: true
                 page.goto(url)
                     .await
@@ -203,7 +214,7 @@ impl BrowserWindow {
         script: String,
     ) -> Result<Option<serde_json::Value>, ToolproofStepError> {
         match self {
-            BrowserWindow::Chrome(page) => {
+            BrowserWindow::Chrome { page, .. } => {
                 let res = page
                     .evaluate_function(format!("async function() {{{}}}", harnessed(script)))
                     .await
@@ -220,7 +231,7 @@ impl BrowserWindow {
 
     async fn screenshot_page(&self, filepath: PathBuf) -> Result<(), ToolproofStepError> {
         match self {
-            BrowserWindow::Chrome(page) => {
+            BrowserWindow::Chrome { page, .. } => {
                 let image_format = browser_specific::chrome_image_format(&filepath)?;
 
                 page.save_screenshot(
@@ -253,7 +264,7 @@ impl BrowserWindow {
         timeout_secs: u64,
     ) -> Result<(), ToolproofStepError> {
         match self {
-            BrowserWindow::Chrome(page) => {
+            BrowserWindow::Chrome { page, .. } => {
                 let image_format = browser_specific::chrome_image_format(&filepath)?;
 
                 let element = browser_specific::wait_for_chrome_element_selector(
@@ -284,7 +295,7 @@ impl BrowserWindow {
         timeout_secs: u64,
     ) -> Result<(), ToolproofStepError> {
         match self {
-            BrowserWindow::Chrome(page) => {
+            BrowserWindow::Chrome { page, .. } => {
                 let text = text.to_lowercase();
                 let selector_text = escape_xpath_string(&text);
                 let el_xpath = |el: &str| {
@@ -437,7 +448,7 @@ impl BrowserWindow {
         timeout_secs: u64,
     ) -> Result<(), ToolproofStepError> {
         match self {
-            BrowserWindow::Chrome(page) => {
+            BrowserWindow::Chrome { page, .. } => {
                 loop {
                     let element = browser_specific::wait_for_chrome_element_selector(
                         page,
@@ -523,7 +534,7 @@ impl BrowserWindow {
         timeout_secs: u64,
     ) -> Result<(), ToolproofStepError> {
         match self {
-            BrowserWindow::Chrome(page) => {
+            BrowserWindow::Chrome { page, .. } => {
                 loop {
                     let element = browser_specific::wait_for_chrome_element_selector(
                         page,
@@ -563,7 +574,7 @@ impl BrowserWindow {
 
     async fn press_key(&self, key: &str, timeout_secs: u64) -> Result<(), ToolproofStepError> {
         match self {
-            BrowserWindow::Chrome(page) => {
+            BrowserWindow::Chrome { page, .. } => {
                 let dom =
                     browser_specific::wait_for_chrome_element_selector(page, "body", timeout_secs)
                         .await?;

toolproof/src/main.rs

@@ -869,10 +869,12 @@ async fn main_inner() -> Result<(), ()> {
     let mut results = join_or_shutdown(hands, &shutdown_rx)
         .await?
         .into_iter()
-        .map(|outer_err| match outer_err {
-            Ok(Ok(success)) => Ok(success),
-            Ok(Err(e)) => Err(e),
-            Err(e) => panic!("Failed to await all tests: {e}"),
+        .filter_map(|outer_err| match outer_err {
+            Ok(inner) => Some(inner),
+            Err(e) => {
+                eprintln!("[toolproof] Error: A test task panicked: {e}");
+                None
+            }
         })
         .collect::<Vec<_>>();