Auto merge of #10910 - asajeffrey:remove-unwrap-from-glutin-app, r=aneeshusa

Removed sources of panic from ports/glutin.

Fixes #10547.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/10910)
<!-- Reviewable:end -->

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",

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[@]}"

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",

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"}

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<WindowID>) -> Rc<Window> {
     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)
 }

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) => {
+            Event::KeyboardInput(element_state, _scan_code, Some(virtual_key_code)) => {
-                if virtual_key_code.is_some() {
+                match virtual_key_code {
-                    let virtual_key_code = virtual_key_code.unwrap();
+                    VirtualKeyCode::LControl => self.toggle_modifier(LEFT_CONTROL),
-
+                    VirtualKeyCode::RControl => self.toggle_modifier(RIGHT_CONTROL),
-                    match (element_state, virtual_key_code) {
+                    VirtualKeyCode::LShift => self.toggle_modifier(LEFT_SHIFT),
-                        (_, VirtualKeyCode::LControl) => self.toggle_modifier(LEFT_CONTROL),
+                    VirtualKeyCode::RShift => self.toggle_modifier(RIGHT_SHIFT),
-                        (_, VirtualKeyCode::RControl) => self.toggle_modifier(RIGHT_CONTROL),
+                    VirtualKeyCode::LAlt => self.toggle_modifier(LEFT_ALT),
-                        (_, VirtualKeyCode::LShift) => self.toggle_modifier(LEFT_SHIFT),
+                    VirtualKeyCode::RAlt => self.toggle_modifier(RIGHT_ALT),
-                        (_, VirtualKeyCode::RShift) => self.toggle_modifier(RIGHT_SHIFT),
+                    VirtualKeyCode::LWin => self.toggle_modifier(LEFT_SUPER),
-                        (_, VirtualKeyCode::LAlt) => self.toggle_modifier(LEFT_ALT),
+                    VirtualKeyCode::RWin => self.toggle_modifier(RIGHT_SUPER),
-                        (_, VirtualKeyCode::RAlt) => self.toggle_modifier(RIGHT_ALT),
+                    _ => {
-                        (_, VirtualKeyCode::LWin) => self.toggle_modifier(LEFT_SUPER),
+                        if let Ok(key) = Window::glutin_key_to_script_key(virtual_key_code) {
-                        (_, VirtualKeyCode::RWin) => self.toggle_modifier(RIGHT_SUPER),
+                            let state = match element_state {
-                        (_, key_code) => {
+                                ElementState::Pressed => KeyState::Pressed,
-                            if let Ok(key) = Window::glutin_key_to_script_key(key_code) {
+                                ElementState::Released => KeyState::Released,
-                                let state = match element_state {
+                            };
-                                    ElementState::Pressed => KeyState::Pressed,
+                            let modifiers = Window::glutin_mods_to_script_mods(self.key_modifiers.get());
-                                    ElementState::Released => KeyState::Released,
+                            self.event_queue.borrow_mut().push(WindowEvent::KeyEvent(key, state, modifiers));
-                                };
-                                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<glutin::WindowProxy> {
 impl WindowMethods for Window {
     fn framebuffer_size(&self) -> TypedSize2D<DevicePixel, u32> {
         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<ScreenPx, f32> {
-        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<u32>, Point2D<i32>) {
-        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()
         }