From 37511cc6164b7f37debbbd504207241117415fee Mon Sep 17 00:00:00 2001 From: Alan Jeffrey Date: Thu, 28 Apr 2016 17:21:48 -0500 Subject: [PATCH] Removed sources of panic from ports/glutin. --- components/servo/Cargo.lock | 1 + etc/ci/check_no_unwrap.sh | 4 +- ports/cef/Cargo.lock | 1 + ports/glutin/Cargo.toml | 1 + ports/glutin/lib.rs | 6 ++- ports/glutin/window.rs | 83 +++++++++++++++++++++++-------------- 6 files changed, 61 insertions(+), 35 deletions(-) diff --git a/components/servo/Cargo.lock b/components/servo/Cargo.lock index 660731f9a76..227b5e45d21 100644 --- a/components/servo/Cargo.lock +++ b/components/servo/Cargo.lock @@ -847,6 +847,7 @@ dependencies = [ "euclid 0.6.6 (registry+https://github.com/rust-lang/crates.io-index)", "gleam 0.2.18 (registry+https://github.com/rust-lang/crates.io-index)", "layers 0.2.5 (git+https://github.com/servo/rust-layers)", + "log 0.3.6 (registry+https://github.com/rust-lang/crates.io-index)", "msg 0.0.1", "net_traits 0.0.1", "script_traits 0.0.1", diff --git a/etc/ci/check_no_unwrap.sh b/etc/ci/check_no_unwrap.sh index 2d949cd591c..255e1ac8450 100755 --- a/etc/ci/check_no_unwrap.sh +++ b/etc/ci/check_no_unwrap.sh @@ -11,7 +11,9 @@ cd "$(git rev-parse --show-toplevel)" # cd into repo root so make sure paths wor # files that should not contain "unwrap" FILES=("components/compositing/compositor.rs" "components/compositing/pipeline.rs" - "components/constellation/constellation.rs") + "components/constellation/constellation.rs" + "ports/glutin/lib.rs" + "ports/glutin/window.rs") # make sure the files exist ls -1 "${FILES[@]}" diff --git a/ports/cef/Cargo.lock b/ports/cef/Cargo.lock index 096037b1913..6c052488a76 100644 --- a/ports/cef/Cargo.lock +++ b/ports/cef/Cargo.lock @@ -759,6 +759,7 @@ dependencies = [ "euclid 0.6.6 (registry+https://github.com/rust-lang/crates.io-index)", "gleam 0.2.18 (registry+https://github.com/rust-lang/crates.io-index)", "layers 0.2.5 (git+https://github.com/servo/rust-layers)", + "log 0.3.6 (registry+https://github.com/rust-lang/crates.io-index)", "msg 0.0.1", "net_traits 0.0.1", "script_traits 0.0.1", diff --git a/ports/glutin/Cargo.toml b/ports/glutin/Cargo.toml index 888454698f6..465f352c950 100644 --- a/ports/glutin/Cargo.toml +++ b/ports/glutin/Cargo.toml @@ -13,6 +13,7 @@ compositing = {path = "../../components/compositing"} euclid = {version = "0.6.4", features = ["plugins"]} gleam = "0.2.8" layers = {git = "https://github.com/servo/rust-layers", features = ["plugins"]} +log = "0.3.5" msg = {path = "../../components/msg"} net_traits = {path = "../../components/net_traits"} script_traits = {path = "../../components/script_traits"} diff --git a/ports/glutin/lib.rs b/ports/glutin/lib.rs index 5c984fbc691..0f557e14097 100644 --- a/ports/glutin/lib.rs +++ b/ports/glutin/lib.rs @@ -14,6 +14,7 @@ extern crate euclid; extern crate gleam; extern crate glutin; extern crate layers; +#[macro_use] extern crate log; extern crate msg; extern crate net_traits; extern crate script_traits; @@ -41,8 +42,9 @@ pub fn create_window(parent: Option) -> Rc { let opts = opts::get(); let foreground = opts.output_file.is_none() && !opts.headless; let scale_factor = ScaleFactor::new(opts.device_pixels_per_px.unwrap_or(1.0)); - let size = opts.initial_window_size.as_f32() * scale_factor; + let size_f32 = opts.initial_window_size.as_f32() * scale_factor; + let size_u32 = size_f32.as_uint().cast().expect("Window size should fit in a u32."); // Open a window. - Window::new(foreground, size.as_uint().cast().unwrap(), parent) + Window::new(foreground, size_u32, parent) } diff --git a/ports/glutin/window.rs b/ports/glutin/window.rs index f09489f0093..8197767d635 100644 --- a/ports/glutin/window.rs +++ b/ports/glutin/window.rs @@ -139,7 +139,7 @@ impl Window { builder = builder_with_platform_options(builder); - let mut glutin_window = builder.build().unwrap(); + let mut glutin_window = builder.build().expect("Failed to create window."); unsafe { glutin_window.make_current().expect("Failed to make context current!") } @@ -213,32 +213,31 @@ impl Window { fn handle_window_event(&self, event: glutin::Event) -> bool { match event { - Event::KeyboardInput(element_state, _scan_code, virtual_key_code) => { - if virtual_key_code.is_some() { - let virtual_key_code = virtual_key_code.unwrap(); - - match (element_state, virtual_key_code) { - (_, VirtualKeyCode::LControl) => self.toggle_modifier(LEFT_CONTROL), - (_, VirtualKeyCode::RControl) => self.toggle_modifier(RIGHT_CONTROL), - (_, VirtualKeyCode::LShift) => self.toggle_modifier(LEFT_SHIFT), - (_, VirtualKeyCode::RShift) => self.toggle_modifier(RIGHT_SHIFT), - (_, VirtualKeyCode::LAlt) => self.toggle_modifier(LEFT_ALT), - (_, VirtualKeyCode::RAlt) => self.toggle_modifier(RIGHT_ALT), - (_, VirtualKeyCode::LWin) => self.toggle_modifier(LEFT_SUPER), - (_, VirtualKeyCode::RWin) => self.toggle_modifier(RIGHT_SUPER), - (_, key_code) => { - if let Ok(key) = Window::glutin_key_to_script_key(key_code) { - let state = match element_state { - ElementState::Pressed => KeyState::Pressed, - ElementState::Released => KeyState::Released, - }; - let modifiers = Window::glutin_mods_to_script_mods(self.key_modifiers.get()); - self.event_queue.borrow_mut().push(WindowEvent::KeyEvent(key, state, modifiers)); - } + Event::KeyboardInput(element_state, _scan_code, Some(virtual_key_code)) => { + match virtual_key_code { + VirtualKeyCode::LControl => self.toggle_modifier(LEFT_CONTROL), + VirtualKeyCode::RControl => self.toggle_modifier(RIGHT_CONTROL), + VirtualKeyCode::LShift => self.toggle_modifier(LEFT_SHIFT), + VirtualKeyCode::RShift => self.toggle_modifier(RIGHT_SHIFT), + VirtualKeyCode::LAlt => self.toggle_modifier(LEFT_ALT), + VirtualKeyCode::RAlt => self.toggle_modifier(RIGHT_ALT), + VirtualKeyCode::LWin => self.toggle_modifier(LEFT_SUPER), + VirtualKeyCode::RWin => self.toggle_modifier(RIGHT_SUPER), + _ => { + if let Ok(key) = Window::glutin_key_to_script_key(virtual_key_code) { + let state = match element_state { + ElementState::Pressed => KeyState::Pressed, + ElementState::Released => KeyState::Released, + }; + let modifiers = Window::glutin_mods_to_script_mods(self.key_modifiers.get()); + self.event_queue.borrow_mut().push(WindowEvent::KeyEvent(key, state, modifiers)); } } } } + Event::KeyboardInput(_, _, None) => { + debug!("Keyboard input without virtual key."); + } Event::Resized(width, height) => { self.event_queue.borrow_mut().push(WindowEvent::Resize(Size2D::typed(width, height))); } @@ -346,7 +345,13 @@ impl Window { #[cfg(any(target_os = "macos", target_os = "windows"))] fn handle_next_event(&self) -> bool { - let event = self.window.wait_events().next().unwrap(); + let event = match self.window.wait_events().next() { + None => { + warn!("Window event stream closed."); + return false; + }, + Some(event) => event, + }; let mut close = self.handle_window_event(event); if !close { while let Some(event) = self.window.poll_events().next() { @@ -368,7 +373,13 @@ impl Window { // because it doesn't call X11 functions from another thread, so doesn't // hit the same issues explained below. if opts::get().use_webrender { - let event = self.window.wait_events().next().unwrap(); + let event = match self.window.wait_events().next() { + None => { + warn!("Window event stream closed."); + return false; + }, + Some(event) => event, + }; let mut close = self.handle_window_event(event); if !close { while let Some(event) = self.window.poll_events().next() { @@ -536,7 +547,7 @@ impl Window { } fn glutin_mods_to_script_mods(modifiers: KeyModifiers) -> constellation_msg::KeyModifiers { - let mut result = constellation_msg::KeyModifiers::from_bits(0).unwrap(); + let mut result = constellation_msg::KeyModifiers::from_bits(0).expect("infallible"); if modifiers.intersects(LEFT_SHIFT | RIGHT_SHIFT) { result.insert(SHIFT); } @@ -585,19 +596,23 @@ fn create_window_proxy(window: &Window) -> Option { impl WindowMethods for Window { fn framebuffer_size(&self) -> TypedSize2D { let scale_factor = self.window.hidpi_factor() as u32; - let (width, height) = self.window.get_inner_size().unwrap(); + // TODO(ajeffrey): can this fail? + let (width, height) = self.window.get_inner_size().expect("Failed to get window inner size."); Size2D::typed(width * scale_factor, height * scale_factor) } fn size(&self) -> TypedSize2D { - let (width, height) = self.window.get_inner_size().unwrap(); + // TODO(ajeffrey): can this fail? + let (width, height) = self.window.get_inner_size().expect("Failed to get window inner size."); Size2D::typed(width as f32, height as f32) } fn client_window(&self) -> (Size2D, Point2D) { - let (width, height) = self.window.get_outer_size().unwrap(); + // TODO(ajeffrey): can this fail? + let (width, height) = self.window.get_outer_size().expect("Failed to get window outer size."); let size = Size2D::new(width, height); - let (x, y) = self.window.get_position().unwrap(); + // TODO(ajeffrey): can this fail? + let (x, y) = self.window.get_position().expect("Failed to get window position."); let origin = Point2D::new(x as i32, y as i32); (size, origin) } @@ -611,7 +626,9 @@ impl WindowMethods for Window { } fn present(&self) { - self.window.swap_buffers().unwrap(); + if let Err(err) = self.window.swap_buffers() { + warn!("Failed to swap window buffers ({}).", err); + } } fn create_compositor_channel(&self) @@ -834,7 +851,9 @@ unsafe impl Send for GlutinCompositorProxy {} impl CompositorProxy for GlutinCompositorProxy { fn send(&self, msg: compositor_thread::Msg) { // Send a message and kick the OS event loop awake. - self.sender.send(msg).unwrap(); + if let Err(err) = self.sender.send(msg) { + warn!("Failed to send response ({}).", err); + } if let Some(ref window_proxy) = self.window_proxy { window_proxy.wakeup_event_loop() }