diff --git a/desktop/src/lib.rs b/desktop/src/lib.rs index 7292fa85d9..0666d93d65 100644 --- a/desktop/src/lib.rs +++ b/desktop/src/lib.rs @@ -94,36 +94,47 @@ pub fn start() -> ExitCode { } let acceleration = if prefs.disable_ui_acceleration { Acceleration::Disabled } else { Acceleration::Auto }; - let ui_context = ui_context.start(UiConfig { acceleration }).unwrap_or_else(|error| panic!("Failed to start the UI runtime: {error}")); - let ui = ui_context - .instance(&wgpu_context.device, &wgpu_context.queue) - .unwrap_or_else(|error| panic!("Failed to start the UI: {error}")); + let ui_context = match ui_context.start(UiConfig { acceleration }) { + Ok(context) => context, + Err(error) => { + tracing::error!("Failed to start the UI runtime: {error}"); + return ExitCode::FAILURE; + } + }; + let ui = match ui_context.instance(&wgpu_context.device, &wgpu_context.queue) { + Ok(ui) => ui, + Err(error) => { + tracing::error!("Failed to start the UI: {error}"); + return ExitCode::FAILURE; + } + }; tracing::info!("UI runtime started successfully"); { let ui = ui.clone(); let scheduler = app_event_scheduler.clone(); - std::thread::Builder::new() - .name("ui-events".to_string()) - .spawn(move || { - while let Some(event) = ui.recv() { - match event { - UiEvent::Ready => scheduler.schedule(AppEvent::WebCommunicationInitialized), - UiEvent::Frame(texture) => scheduler.schedule(AppEvent::UiUpdate(texture)), - UiEvent::Cursor(cursor) => scheduler.schedule(AppEvent::CursorChange(cursor)), - UiEvent::Message(message) => match wrapper::deserialize_editor_message(&message) { - Some(message) => scheduler.schedule(AppEvent::DesktopWrapperMessage(message)), - None => tracing::error!("Failed to deserialize web message"), - }, - UiEvent::InitFailed(error) => { - tracing::error!("UI initialization failed: {error}"); - scheduler.schedule(AppEvent::UiCrashed); - } - UiEvent::Crashed => scheduler.schedule(AppEvent::UiCrashed), + let spawned = std::thread::Builder::new().name("ui-events".to_string()).spawn(move || { + while let Some(event) = ui.recv() { + match event { + UiEvent::Ready => scheduler.schedule(AppEvent::WebCommunicationInitialized), + UiEvent::Frame(texture) => scheduler.schedule(AppEvent::UiUpdate(texture)), + UiEvent::Cursor(cursor) => scheduler.schedule(AppEvent::CursorChange(cursor)), + UiEvent::Message(message) => match wrapper::deserialize_editor_message(&message) { + Some(message) => scheduler.schedule(AppEvent::DesktopWrapperMessage(message)), + None => tracing::error!("Failed to deserialize web message"), + }, + UiEvent::InitFailed(error) => { + tracing::error!("UI initialization failed: {error}"); + scheduler.schedule(AppEvent::UiCrashed); } + UiEvent::Crashed => scheduler.schedule(AppEvent::UiCrashed), } - }) - .expect("Failed to spawn the UI event bridge thread"); + } + }); + if let Err(error) = spawned { + tracing::error!("Failed to spawn the UI event bridge thread: {error}"); + return ExitCode::FAILURE; + } } let app = App::new(ui.clone(), wgpu_context, app_event_receiver, app_event_scheduler, prefs, cli.files); @@ -158,14 +169,6 @@ pub fn start() -> ExitCode { _ => {} } - // Workaround for a Windows-specific exception that occurs when `app` is dropped. - // The issue causes the window to hang for a few seconds before closing. - // Appears to be related to CEF object destruction order. - // Calling `exit` bypasses rust teardown and lets Windows perform process cleanup. - // TODO: Identify and fix the underlying CEF shutdown issue so this workaround can be removed. - #[cfg(target_os = "windows")] - std::process::exit(0); - #[cfg(not(target_os = "windows"))] ExitCode::SUCCESS } diff --git a/desktop/ui/src/context.rs b/desktop/ui/src/context.rs index b98085814e..c54005c185 100644 --- a/desktop/ui/src/context.rs +++ b/desktop/ui/src/context.rs @@ -69,18 +69,27 @@ impl CefContext { let control_thread = std::thread::Builder::new() .name("cef-host-control".to_string()) .spawn(move || { - let result = control(CefContextHandle); + let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| control(CefContextHandle))); with_context(|context| { - context.browser.host().unwrap().close_browser(1); + if let Some(host) = context.browser.host() { + host.close_browser(1); + } }); run_on_ui_thread(cef::quit_message_loop); - let _ = result_sender.send(result); + match result { + Ok(result) => { + let _ = result_sender.send(result); + } + Err(panic) => std::panic::resume_unwind(panic), + } }) .expect("Failed to spawn the CEF control thread"); cef::run_message_loop(); drop(CONTEXT.take()); cef::shutdown(); - let _ = control_thread.join(); + if let Err(panic) = control_thread.join() { + std::panic::resume_unwind(panic); + } result_receiver.recv().expect("The CEF control thread ended without a result") } } @@ -95,11 +104,12 @@ pub(crate) fn execute_helper_process() -> std::process::ExitCode { fn bootstrap(helper: bool) -> Args { #[cfg(target_os = "macos")] - let _loader = { + { let loader = cef::library_loader::LibraryLoader::new(&std::env::current_exe().unwrap(), helper); assert!(loader.load()); - loader - }; + // LibraryLoader unloads the framework on drop + std::mem::forget(loader); + } #[cfg(not(target_os = "macos"))] let _ = helper; @@ -109,13 +119,13 @@ fn bootstrap(helper: bool) -> Args { fn initialize(args: &Args, instance_dir: &Path, accelerated_paint: bool) -> Result<(), InitError> { let mut app = App::new(BrowserProcessAppImpl::new(accelerated_paint)); - if cef::initialize(Some(args.as_main_args()), Some(&platform_settings(instance_dir)), Some(&mut app), std::ptr::null_mut()) != 1 { + if cef::initialize(Some(args.as_main_args()), Some(&platform_settings(instance_dir)?), Some(&mut app), std::ptr::null_mut()) != 1 { return Err(InitError::InitializationFailureCode(cef::get_exit_code() as u32)); } Ok(()) } -fn platform_settings(instance_dir: &Path) -> Settings { +fn platform_settings(instance_dir: &Path) -> Result { let log_severity = match std::env::var("GRAPHITE_BROWSER_LOG").unwrap_or_default().to_lowercase().as_str() { "debug" => cef_log_severity_t::LOGSEVERITY_VERBOSE, "info" => cef_log_severity_t::LOGSEVERITY_INFO, @@ -125,9 +135,12 @@ fn platform_settings(instance_dir: &Path) -> Settings { _ => cef_log_severity_t::LOGSEVERITY_FATAL, }; + let Some(root_cache_path) = instance_dir.to_str().map(CefString::from) else { + return Err(InitError::PathResolutionFailed(format!("non-UTF-8 instance directory path: {}", instance_dir.display()))); + }; let base = Settings { windowless_rendering_enabled: 1, - root_cache_path: instance_dir.to_str().map(CefString::from).unwrap(), + root_cache_path, cache_path: "".into(), disable_signal_handlers: 1, log_severity: LogSeverity::from(log_severity), @@ -136,24 +149,31 @@ fn platform_settings(instance_dir: &Path) -> Settings { #[cfg(target_os = "macos")] { - let exe = std::env::current_exe().expect("cannot get current exe path"); - let app_root = exe.parent().and_then(|p| p.parent()).expect("bad path structure").parent().expect("bad path structure"); - Settings { - main_bundle_path: app_root.to_str().map(CefString::from).unwrap(), + let exe = std::env::current_exe().map_err(|e| InitError::PathResolutionFailed(format!("cannot get current exe path: {e}")))?; + let app_root = exe + .parent() + .and_then(|p| p.parent()) + .and_then(|p| p.parent()) + .ok_or_else(|| InitError::PathResolutionFailed(format!("executable is not inside an app bundle: {}", exe.display())))?; + let Some(main_bundle_path) = app_root.to_str().map(CefString::from) else { + return Err(InitError::PathResolutionFailed(format!("invalid app bundle path: {}", app_root.display()))); + }; + Ok(Settings { + main_bundle_path, multi_threaded_message_loop: 0, external_message_pump: 0, no_sandbox: 1, // GPU helper crashes when running with sandbox ..base - } + }) } #[cfg(not(target_os = "macos"))] - Settings { + Ok(Settings { multi_threaded_message_loop: 1, #[cfg(target_os = "linux")] no_sandbox: 1, ..base - } + }) } fn create_browser(delegate: BrowserDelegate, frames: FrameStreamer, view_info_sender: Sender, instance_dir: TempDir, accelerated_paint: bool) -> Result { @@ -187,7 +207,9 @@ fn create_browser(delegate: BrowserDelegate, frames: FrameStreamer, view_info_se let mut scheme_handler_factory = SchemeHandlerFactory::new(SchemeHandlerFactoryImpl::new(delegate.clone())); incognito_request_context.clear_scheme_handler_factories(); - incognito_request_context.register_scheme_handler_factory(Some(&RESOURCE_SCHEME.into()), Some(&RESOURCE_DOMAIN.into()), Some(&mut scheme_handler_factory)); + if incognito_request_context.register_scheme_handler_factory(Some(&RESOURCE_SCHEME.into()), Some(&RESOURCE_DOMAIN.into()), Some(&mut scheme_handler_factory)) != 1 { + return Err(InitError::SchemeHandlerRegistrationFailed); + } let url = format!("{RESOURCE_SCHEME}://{RESOURCE_DOMAIN}/"); browser_host_create_browser_sync( @@ -220,6 +242,10 @@ pub(crate) enum InitError { BrowserCreationFailed, #[error("Request context creation failed")] RequestContextCreationFailed, + #[error("Failed to resolve a required path: {0}")] + PathResolutionFailed(String), + #[error("Scheme handler registration failed")] + SchemeHandlerRegistrationFailed, } #[derive(Clone)] @@ -261,7 +287,10 @@ impl BrowserContext { fn refresh_view_info(&self) { let view_info = self.delegate.view_info(); - let host = self.browser.host().unwrap(); + let Some(host) = self.browser.host() else { + tracing::error!("Browser host is not available, cannot refresh view info"); + return; + }; host.set_zoom_level(view_info.zoom()); host.was_resized(); @@ -278,7 +307,11 @@ impl BrowserContext { impl Drop for BrowserContext { fn drop(&mut self) { tracing::debug!("Shutting down CEF"); - self.browser.host().unwrap().close_browser(1); + if let Some(host) = self.browser.host() { + host.close_browser(1); + } else { + tracing::error!("Browser host is not available, cannot close browser"); + } } } @@ -298,7 +331,9 @@ where { let closure_task = ClosureTask::new(closure); let mut task = Task::new(closure_task); - post_task(ThreadId::from(cef_thread_id_t::TID_UI), Some(&mut task)); + if post_task(ThreadId::from(cef_thread_id_t::TID_UI), Some(&mut task)) != 1 { + tracing::error!("Failed to post a task to the CEF UI thread"); + } } fn with_context(closure: F) diff --git a/desktop/ui/src/dirs.rs b/desktop/ui/src/dirs.rs index 1bef4d549f..f4e8f2ff63 100644 --- a/desktop/ui/src/dirs.rs +++ b/desktop/ui/src/dirs.rs @@ -9,8 +9,8 @@ const APP_DIRECTORY_NAME: &str = "Graphite"; pub(crate) fn app_tmp_dir() -> PathBuf { let path = std::env::temp_dir().join(APP_DIRECTORY_NAME); - if !path.exists() { - fs::create_dir_all(&path).unwrap_or_else(|_| panic!("Failed to create directory at {path:?}")); + if let Err(e) = fs::create_dir_all(&path) { + tracing::error!("Failed to create temp directory at {path:?}: {e}"); } path } diff --git a/desktop/ui/src/frames/import/d3d11.rs b/desktop/ui/src/frames/import/d3d11.rs index 1702470e52..64bf574a25 100644 --- a/desktop/ui/src/frames/import/d3d11.rs +++ b/desktop/ui/src/frames/import/d3d11.rs @@ -1,6 +1,7 @@ use super::{TextureImportError, TextureImportResult, TextureImporter, texture_descriptor, wgpu_format}; use cef::sys::cef_color_type_t; use std::os::raw::c_void; +use wgpu::hal::api; pub struct D3D11Importer { pub handle: *mut c_void, @@ -15,16 +16,11 @@ impl TextureImporter for D3D11Importer { return Err(TextureImportError::InvalidHandle("Null D3D11 shared texture handle".to_string())); } - if is_d3d12_backend(device) { - match self.import_via_d3d12(device) { - Ok(texture) => { - tracing::trace!("Successfully imported D3D11 shared texture via D3D12"); - return Ok(texture); - } - Err(e) => { - tracing::warn!("Failed to import D3D11 via D3D12: {}, trying Vulkan fallback", e); - } - } + let is_d3d12_backend = unsafe { device.as_hal::().is_some() }; + + if is_d3d12_backend { + let texture = self.import_via_d3d12(device)?; + return Ok(texture); } let texture = self.import_via_vulkan(device)?; @@ -138,8 +134,3 @@ impl D3D11Importer { } } } - -fn is_d3d12_backend(device: &wgpu::Device) -> bool { - use wgpu::hal::api; - unsafe { device.as_hal::().is_some() } -} diff --git a/desktop/ui/src/frames/import/dmabuf.rs b/desktop/ui/src/frames/import/dmabuf.rs index a2cd5e5f33..5a72376cea 100644 --- a/desktop/ui/src/frames/import/dmabuf.rs +++ b/desktop/ui/src/frames/import/dmabuf.rs @@ -15,9 +15,19 @@ pub struct DmaBufImporter { impl TextureImporter for DmaBufImporter { fn import_to_wgpu(&self, device: &wgpu::Device) -> TextureImportResult { - if self.fds.is_empty() { - return Err(TextureImportError::InvalidHandle("No DMA-BUF plane fds".to_string())); + if self.fds.len() != 1 { + return Err(TextureImportError::InvalidHandle(format!("Expected exactly one DMA-BUF plane fd, got {}", self.fds.len()))); } + + if self.strides.len() != self.fds.len() || self.offsets.len() != self.fds.len() { + return Err(TextureImportError::InvalidHandle(format!( + "DMA-BUF plane count mismatch: {} fds, {} strides, {} offsets", + self.fds.len(), + self.strides.len(), + self.offsets.len() + ))); + } + let texture = self.import_via_vulkan(device)?; tracing::trace!("Successfully imported DMA-BUF texture via Vulkan"); Ok(texture) @@ -81,7 +91,7 @@ impl DmaBufImporter { fn create_vulkan_image_from_dmabuf(&self, hal_device: &::Device) -> Result<(vk::Image, vk::DeviceMemory), TextureImportError> { let device = hal_device.raw_device(); - let _instance = hal_device.shared_instance().raw_instance(); + let instance = hal_device.shared_instance().raw_instance(); if self.width == 0 || self.height == 0 { return Err(TextureImportError::InvalidHandle("Invalid DMA-BUF dimensions".to_string())); @@ -102,13 +112,25 @@ impl DmaBufImporter { .usage(vk::ImageUsageFlags::SAMPLED | vk::ImageUsageFlags::COLOR_ATTACHMENT | vk::ImageUsageFlags::TRANSFER_SRC) .sharing_mode(vk::SharingMode::EXCLUSIVE); - // Set up DRM format modifier - let plane_layouts = self.create_subresource_layouts(); + let plane_layouts = self + .offsets + .iter() + .zip(&self.strides) + .map(|(&offset, &stride)| vk::SubresourceLayout { + offset: offset as u64, + size: 0, // Will be calculated by driver + row_pitch: stride as u64, + array_pitch: 0, + depth_pitch: 0, + }) + .collect::>(); let mut drm_format_modifier = vk::ImageDrmFormatModifierExplicitCreateInfoEXT::default() .drm_format_modifier(self.modifier) .plane_layouts(&plane_layouts); - let image_create_info = image_create_info.push_next(&mut drm_format_modifier); + let mut external_memory_info = vk::ExternalMemoryImageCreateInfo::default().handle_types(vk::ExternalMemoryHandleTypeFlags::DMA_BUF_EXT); + + let image_create_info = image_create_info.push_next(&mut drm_format_modifier).push_next(&mut external_memory_info); let image = unsafe { device.create_image(&image_create_info, None).map_err(|e| TextureImportError::VulkanError { @@ -128,11 +150,25 @@ impl DmaBufImporter { }); } + let external_memory_fd = ash::khr::external_memory_fd::Device::new(instance, device); + let mut fd_properties = vk::MemoryFdPropertiesKHR::default(); + if let Err(e) = unsafe { external_memory_fd.get_memory_fd_properties(vk::ExternalMemoryHandleTypeFlags::DMA_BUF_EXT, dup_fd, &mut fd_properties) } { + // SAFETY: import failed and the fd is still ours, need to clean up the image and close the fd + unsafe { + device.destroy_image(image, None); + libc::close(dup_fd); + } + return Err(TextureImportError::VulkanError { + operation: format!("Failed to query DMA-BUF fd memory properties: {e:?}"), + }); + } + let mut import_memory_fd = vk::ImportMemoryFdInfoKHR::default().handle_type(vk::ExternalMemoryHandleTypeFlags::DMA_BUF_EXT).fd(dup_fd); - let memory_properties = unsafe { hal_device.shared_instance().raw_instance().get_physical_device_memory_properties(hal_device.raw_physical_device()) }; + let memory_properties = unsafe { instance.get_physical_device_memory_properties(hal_device.raw_physical_device()) }; - let Some(memory_type_index) = find_memory_type_index(memory_requirements.memory_type_bits, vk::MemoryPropertyFlags::empty(), &memory_properties) else { + let compatible_type_bits = memory_requirements.memory_type_bits & fd_properties.memory_type_bits; + let Some(memory_type_index) = find_memory_type_index(compatible_type_bits, vk::MemoryPropertyFlags::empty(), &memory_properties) else { // SAFETY: import failed and the fd is still ours, need to clean up the image and close the fd unsafe { device.destroy_image(image, None); @@ -175,18 +211,6 @@ impl DmaBufImporter { Ok((image, device_memory)) } - - fn create_subresource_layouts(&self) -> Vec { - (0..self.fds.len()) - .map(|i| vk::SubresourceLayout { - offset: self.offsets.get(i).copied().unwrap_or(0) as u64, - size: 0, // Will be calculated by driver - row_pitch: self.strides.get(i).copied().unwrap_or(0) as u64, - array_pitch: 0, - depth_pitch: 0, - }) - .collect() - } } fn vulkan_format(format: cef_color_type_t) -> Result { diff --git a/desktop/ui/src/frames/plane/linux.rs b/desktop/ui/src/frames/plane/linux.rs index 0d4b730142..2ae0d879d4 100644 --- a/desktop/ui/src/frames/plane/linux.rs +++ b/desktop/ui/src/frames/plane/linux.rs @@ -54,6 +54,12 @@ impl PlaneSender { } pub(crate) fn stage(&self, info: &cef::AcceleratedPaintInfo) -> Option { + let coded_size = &info.extra.coded_size; + if coded_size.width <= 0 || coded_size.height <= 0 { + tracing::error!("Accelerated paint delivered an invalid coded size: {}x{}", coded_size.width, coded_size.height); + return None; + } + let plane_count = (info.plane_count.max(0) as usize).min(info.planes.len()); let mut fds = Vec::with_capacity(plane_count); let mut strides = [0u32; 4]; @@ -76,8 +82,8 @@ impl PlaneSender { descriptor: FrameDescriptor { seq: 0, modifier: info.modifier, - width: info.extra.coded_size.width as u32, - height: info.extra.coded_size.height as u32, + width: coded_size.width as u32, + height: coded_size.height as u32, format: *info.format.as_ref() as u32, plane_count: plane_count as u32, strides, diff --git a/desktop/ui/src/frames/plane/mac.rs b/desktop/ui/src/frames/plane/mac.rs index 26126f3d47..92c5c98287 100644 --- a/desktop/ui/src/frames/plane/mac.rs +++ b/desktop/ui/src/frames/plane/mac.rs @@ -110,10 +110,17 @@ impl PlaneSender { } pub(crate) fn stage(&self, info: &cef::AcceleratedPaintInfo) -> Option { + let coded_size = &info.extra.coded_size; + if coded_size.width <= 0 || coded_size.height <= 0 { + tracing::error!("Accelerated paint delivered an invalid coded size: {}x{}", coded_size.width, coded_size.height); + return None; + } + let Some(surface) = std::ptr::NonNull::new(info.shared_texture_io_surface.cast::()) else { tracing::error!("Accelerated paint delivered a null IOSurface"); return None; }; + // SAFETY: CEF keeps the surface valid for the `on_accelerated_paint` callback. let port = unsafe { surface.as_ref() }.create_mach_port(); if port == MACH_PORT_NULL { @@ -123,8 +130,8 @@ impl PlaneSender { Some(StagedFrame { descriptor: FrameDescriptor { seq: 0, - width: info.extra.coded_size.width as u32, - height: info.extra.coded_size.height as u32, + width: coded_size.width as u32, + height: coded_size.height as u32, format: *info.format.as_ref() as u32, _pad: 0, }, @@ -155,9 +162,6 @@ impl PlaneSender { descriptor, }; - // Kernel takes ownership of the surface and moves it to the receiver. We must not drop. - std::mem::forget(frame.surface); - // SAFETY: message is a well-formed complex message of the declared size. let result = unsafe { mach_msg( @@ -173,6 +177,10 @@ impl PlaneSender { if result != MACH_MSG_SUCCESS { return Err(std::io::Error::other(format!("mach_msg send failed: {result:#x}"))); } + + // Kernel took ownership of the surface and moves it to the receiver. We must not drop. + std::mem::forget(frame.surface); + Ok(()) } } diff --git a/desktop/ui/src/frames/plane/win.rs b/desktop/ui/src/frames/plane/win.rs index a4c1dcc998..b3146dfaa1 100644 --- a/desktop/ui/src/frames/plane/win.rs +++ b/desktop/ui/src/frames/plane/win.rs @@ -67,6 +67,12 @@ impl PlaneSender { } pub(crate) fn stage(&self, info: &cef::AcceleratedPaintInfo) -> Option { + let coded_size = &info.extra.coded_size; + if coded_size.width <= 0 || coded_size.height <= 0 { + tracing::error!("Accelerated paint delivered an invalid coded size: {}x{}", coded_size.width, coded_size.height); + return None; + } + let handle = match self.main.duplicate_into(HANDLE(info.shared_texture_handle)) { Ok(handle) => handle, Err(e) => { @@ -74,10 +80,11 @@ impl PlaneSender { return None; } }; + Some(StagedFrame { handle: HandleInMain { handle, main: self.main.clone() }, - width: info.extra.coded_size.width as u32, - height: info.extra.coded_size.height as u32, + width: coded_size.width as u32, + height: coded_size.height as u32, format: *info.format.as_ref() as u32, }) } diff --git a/desktop/ui/src/frames/surface.rs b/desktop/ui/src/frames/surface.rs index 4c148df16f..045bada723 100644 --- a/desktop/ui/src/frames/surface.rs +++ b/desktop/ui/src/frames/surface.rs @@ -40,7 +40,7 @@ impl FrameSurface { view_formats: &[], })); } - let texture = slot.as_ref().expect("Texture was just created"); + let texture = slot.as_ref()?; self.queue.write_texture( wgpu::TexelCopyTextureInfo { diff --git a/desktop/ui/src/input.rs b/desktop/ui/src/input.rs index 43f8709fbc..b1c6e40d15 100644 --- a/desktop/ui/src/input.rs +++ b/desktop/ui/src/input.rs @@ -55,7 +55,7 @@ pub(crate) struct KeyData { /// (cursor position, click counting, modifiers) along the way. pub(crate) fn translate(input_state: &mut InputState, event: &WindowEvent) -> Vec { match event { - WindowEvent::PointerMoved { position, .. } | WindowEvent::PointerEntered { position, .. } => { + WindowEvent::PointerMoved { position, .. } => { if !input_state.cursor_move(position) { return Vec::new(); } @@ -64,6 +64,13 @@ pub(crate) fn translate(input_state: &mut InputState, event: &WindowEvent) -> Ve leave: false, }] } + WindowEvent::PointerEntered { position, .. } => { + let _ = input_state.cursor_move(position); + vec![InputEvent::MouseMove { + data: input_state.mouse_data(), + leave: false, + }] + } WindowEvent::PointerLeft { position, .. } => { if let Some(position) = position { let _ = input_state.cursor_move(position); @@ -73,7 +80,7 @@ pub(crate) fn translate(input_state: &mut InputState, event: &WindowEvent) -> Ve leave: true, }] } - WindowEvent::PointerButton { state, button, .. } => { + WindowEvent::PointerButton { state, button, position, .. } => { let mouse_button = match button { ButtonSource::Mouse(mouse_button) => mouse_button, _ => { @@ -81,6 +88,7 @@ pub(crate) fn translate(input_state: &mut InputState, event: &WindowEvent) -> Ve } }; + let _ = input_state.cursor_move(position); let click_count = input_state.mouse_input(mouse_button, state).into(); let up = matches!(state, ElementState::Released); let button = match mouse_button { diff --git a/desktop/ui/src/input/state.rs b/desktop/ui/src/input/state.rs index 8d6de8d167..aceedd6872 100644 --- a/desktop/ui/src/input/state.rs +++ b/desktop/ui/src/input/state.rs @@ -55,8 +55,8 @@ impl InputState { pub(crate) fn mouse_data(&self) -> MouseData { MouseData { - x: self.mouse_position.x as i32, - y: self.mouse_position.y as i32, + x: self.mouse_position.x, + y: self.mouse_position.y, modifiers: self.cef_mouse_modifiers().into(), } } @@ -64,14 +64,14 @@ impl InputState { #[derive(Default, Clone, Copy, Eq, PartialEq)] pub(crate) struct MousePosition { - x: usize, - y: usize, + x: i32, + y: i32, } impl From<&PhysicalPosition> for MousePosition { fn from(position: &PhysicalPosition) -> Self { Self { - x: position.x as usize, - y: position.y as usize, + x: position.x as i32, + y: position.y as i32, } } } @@ -133,8 +133,8 @@ impl ClickTracker { ElementState::Released => (record.up_count, record.up_position), }; - let dx = position.x.abs_diff(prev_position.x); - let dy = position.y.abs_diff(prev_position.y); + let dx = position.x.abs_diff(prev_position.x) as usize; + let dy = position.y.abs_diff(prev_position.y) as usize; let within_dist = dx <= MULTICLICK_ALLOWED_TRAVEL && dy <= MULTICLICK_ALLOWED_TRAVEL; let count = match (prev_count, within_time, within_dist) { diff --git a/desktop/ui/src/internal/render_handler.rs b/desktop/ui/src/internal/render_handler.rs index 86da6bf2bb..26f8c8d69d 100644 --- a/desktop/ui/src/internal/render_handler.rs +++ b/desktop/ui/src/internal/render_handler.rs @@ -51,7 +51,11 @@ impl ImplRenderHandler for RenderHandlerImpl { return; } - self.frames.stage_texture(info.unwrap()); + let Some(info) = info else { + tracing::error!("Accelerated paint callback received no info about the painted frame"); + return; + }; + self.frames.stage_texture(info); self.frames.publish(); } diff --git a/desktop/ui/src/lib.rs b/desktop/ui/src/lib.rs index 63d46cadb5..e413d53441 100644 --- a/desktop/ui/src/lib.rs +++ b/desktop/ui/src/lib.rs @@ -118,10 +118,6 @@ pub enum Acceleration { Disabled, } -#[derive(thiserror::Error, Debug)] -#[non_exhaustive] -pub enum InitError {} - #[derive(thiserror::Error, Debug)] #[non_exhaustive] pub enum UiError { diff --git a/desktop/ui/src/platform/linux.rs b/desktop/ui/src/platform/linux.rs index 2c77159e4f..685ab9095a 100644 --- a/desktop/ui/src/platform/linux.rs +++ b/desktop/ui/src/platform/linux.rs @@ -9,12 +9,12 @@ pub(crate) fn setup_command(command: &mut Command, #[cfg(feature = "accelerated_ // SAFETY: the closure runs in the forked child before exec and only makes async-signal-safe calls unsafe { command.pre_exec(move || { - // Tie the host's lifetime to the main process + // Tie the host lifetime to the main process if libc::prctl(libc::PR_SET_PDEATHSIG, libc::SIGKILL) != 0 { return Err(std::io::Error::last_os_error()); } if libc::getppid() != main_pid { - return Err(std::io::Error::other("main process died before PDEATHSIG was set")); + return Err(std::io::Error::from_raw_os_error(libc::ESRCH)); } // Move the host end of the frame socket to its advertised fd diff --git a/desktop/ui/src/remote/host.rs b/desktop/ui/src/remote/host.rs index 2537a7bebb..21607a87d0 100644 --- a/desktop/ui/src/remote/host.rs +++ b/desktop/ui/src/remote/host.rs @@ -90,6 +90,10 @@ pub(crate) fn run() { ControlOutcome::Disconnected => std::process::exit(0), } + // Workaround for a Windows-specific exception that occurs when `context` is dropped. + // Appears to be related to CEF object destruction order. + // Calling `exit` bypasses rust teardown and lets Windows perform process cleanup. + // TODO: Identify and fix the underlying CEF shutdown issue so this workaround can be removed. #[cfg(target_os = "windows")] std::process::exit(0); } diff --git a/desktop/ui/src/remote/spawn.rs b/desktop/ui/src/remote/spawn.rs index a9103a3fae..7588dd41ab 100644 --- a/desktop/ui/src/remote/spawn.rs +++ b/desktop/ui/src/remote/spawn.rs @@ -163,20 +163,23 @@ pub(crate) fn spawn_host(acceleration: bool) -> Result { plane::PlaneReceiver::new(main_end) }); - let (hello_sender, hello_receiver) = mpsc::channel(); - std::thread::Builder::new() - .name("cef-host-accept".to_string()) - .spawn(move || { - let _ = hello_sender.send(server.accept()); - }) - .expect("Failed to spawn CEF host accept thread"); + let (hello_tx, hello_rx) = mpsc::channel(); + let accept_thread = std::thread::Builder::new().name("cef-host-accept".to_string()).spawn(move || { + let _ = hello_tx.send(server.accept()); + }); + if let Err(e) = accept_thread { + let _ = child.kill(); + let _ = child.wait(); + return Err(UiError::Bootstrap(format!("failed to spawn the host accept thread: {e}"))); + } let deadline = Instant::now() + HOST_HELLO_TIMEOUT; let (event_receiver, hello) = loop { - match hello_receiver.recv_timeout(Duration::from_millis(100)) { + match hello_rx.recv_timeout(Duration::from_millis(100)) { Ok(Ok(accepted)) => break accepted, Ok(Err(e)) => { let _ = child.kill(); + let _ = child.wait(); return Err(UiError::Handshake(format!("failed to accept the host connection: {e}"))); } Err(RecvTimeoutError::Timeout) => { @@ -185,11 +188,13 @@ pub(crate) fn spawn_host(acceleration: bool) -> Result { } if Instant::now() >= deadline { let _ = child.kill(); + let _ = child.wait(); return Err(UiError::HandshakeTimeout); } } Err(RecvTimeoutError::Disconnected) => { let _ = child.kill(); + let _ = child.wait(); return Err(UiError::Handshake("the accept thread disappeared".to_string())); } } @@ -202,6 +207,7 @@ pub(crate) fn spawn_host(acceleration: bool) -> Result { } = hello else { let _ = child.kill(); + let _ = child.wait(); return Err(UiError::Handshake("the first message from the host was not Hello".to_string())); }; tracing::info!("CEF host process connected (pid {pid})"); @@ -247,7 +253,7 @@ pub(crate) fn start_instance(handle: &HostHandle, surface: FrameSurface, events: std::thread::Builder::new() .name("cef-frames".to_string()) .spawn(move || crate::frames::receive::plane_receiver_loop(receiver, consumer)) - .expect("Failed to spawn CEF frame receiver thread"); + .map_err(|e| UiError::Bootstrap(format!("failed to spawn the frame receiver thread: {e}")))?; } { @@ -258,7 +264,7 @@ pub(crate) fn start_instance(handle: &HostHandle, surface: FrameSurface, events: std::thread::Builder::new() .name("cef-host".to_string()) .spawn(move || event_receiver_loop(receiver, consumer, events, shutting_down, died_reported, shutdown_complete_sender)) - .expect("Failed to spawn CEF host event receiver thread"); + .map_err(|e| UiError::Bootstrap(format!("failed to spawn the host event receiver thread: {e}")))?; } { @@ -288,7 +294,7 @@ pub(crate) fn start_instance(handle: &HostHandle, surface: FrameSurface, events: } } }) - .expect("Failed to spawn CEF host supervisor thread"); + .map_err(|e| UiError::Bootstrap(format!("failed to spawn the host supervisor thread: {e}")))?; } Ok(shutdown_complete_receiver) diff --git a/desktop/ui/src/resources.rs b/desktop/ui/src/resources.rs index 4e638d14b8..a7992ef2ef 100644 --- a/desktop/ui/src/resources.rs +++ b/desktop/ui/src/resources.rs @@ -2,7 +2,7 @@ use std::fs::File; #[cfg(feature = "embedded_resources")] use std::io; use std::io::Read; -use std::path::PathBuf; +use std::path::{Component, PathBuf}; use std::sync::Arc; #[derive(Clone)] @@ -34,11 +34,21 @@ pub enum WebResources { } pub(crate) fn load(path: PathBuf) -> Option { + if path.components().any(|component| matches!(component, Component::ParentDir)) { + tracing::error!("Rejected resource path with a parent directory component: {path:?}"); + return None; + } + let resources = if cfg!(feature = "embedded_resources") { WebResources::Embedded } else { - let path = std::env::var("GRAPHITE_RESOURCES").expect("GRAPHITE_RESOURCES must point to the frontend assets when embedded resources are disabled"); - WebResources::External(path.into()) + match std::env::var("GRAPHITE_RESOURCES") { + Ok(dir) => WebResources::External(dir.into()), + Err(_) => { + tracing::error!("GRAPHITE_RESOURCES must point to the frontend assets when embedded resources are disabled"); + return None; + } + } }; let path = if path.as_os_str().is_empty() { PathBuf::from("index.html") } else { path };