Replace Closed event with CloseRequested and Destroyed (#476)

* Replace Closed event with CloseRequested and Destroyed

Implements #434

The existing Closed event had ambiguous meaning, both in name and in
cross-platform behavior. Closed is now split into two more precise events:

* CloseRequested - the window has been requested to close, most commonly by
having clicked the window's close button. Whether or not you respond by
closing the window is up to you.

* Destroyed - the window has been destroyed, and can no longer be safely
used.

Most notably, now you can reliably implement classic patterns like
prompting the user to save their work before closing, and have the
opportunity to perform any necessary cleanup.

Migrating to the new API is straightforward. In most cases, you can simply
replace all existing usages of Closed with CloseRequested. For more
information, see the example programs, particularly handling_close and
multiwindow.

iOS applications must replace all usages of Closed with Destroyed, and
require no other changes.

CHANGELOG.md

@@ -13,6 +13,7 @@
 - Corrected `get_position` on macOS to return outer frame position, not content area position.
 - Corrected `set_position` on macOS to set outer frame position, not content area position.
 - Added `get_inner_position` method to `Window`, which gets the position of the window's client area. This is implemented on all applicable platforms (all desktop platforms other than Wayland, where this isn't possible).
+- **Breaking:** the `Closed` event has been replaced by `CloseRequested` and `Destroyed`. To migrate, you typically just need to replace all usages of `Closed` with `CloseRequested`; see example programs for more info. The exception is iOS, where `Closed` must be replaced by `Destroyed`.
 
 # Version 0.12.0 (2018-04-06)
 

README.md

@@ -33,9 +33,10 @@ fn main() {
 
     events_loop.run_forever(|event| {
         match event {
-            winit::Event::WindowEvent { event: winit::WindowEvent::Closed, .. } => {
-                winit::ControlFlow::Break
-            },
+            winit::Event::WindowEvent {
+              event: winit::WindowEvent::CloseRequested,
+              ..
+            } => winit::ControlFlow::Break,
             _ => winit::ControlFlow::Continue,
         }
     });

examples/cursor.rs

@@ -22,7 +22,7 @@ fn main() {
                     cursor_idx = 0;
                 }
             },
-            Event::WindowEvent { event: WindowEvent::Closed, .. } => {
+            Event::WindowEvent { event: WindowEvent::CloseRequested, .. } => {
                 return ControlFlow::Break;
             },
             _ => ()

examples/fullscreen.rs

@@ -40,7 +40,7 @@ fn main() {
 
         match event {
             Event::WindowEvent { event, .. } => match event {
-                WindowEvent::Closed => return ControlFlow::Break,
+                WindowEvent::CloseRequested => return ControlFlow::Break,
                 WindowEvent::KeyboardInput {
                     input:
                         winit::KeyboardInput {

examples/grabbing.rs

@@ -28,7 +28,7 @@ fn main() {
                         }
                     },
 
-                    WindowEvent::Closed => return ControlFlow::Break,
+                    WindowEvent::CloseRequested => return ControlFlow::Break,
 
                     a @ WindowEvent::CursorMoved { .. } => {
                         println!("{:?}", a);

examples/handling_close.rs

@@ -0,0 +1,74 @@
+extern crate winit;
+
+fn main() {
+    let mut events_loop = winit::EventsLoop::new();
+
+    let _window = winit::WindowBuilder::new()
+        .with_title("Your faithful window")
+        .build(&events_loop)
+        .unwrap();
+
+    let mut close_requested = false;
+
+    events_loop.run_forever(|event| {
+        use winit::WindowEvent::*;
+        use winit::ElementState::Released;
+        use winit::VirtualKeyCode::{N, Y};
+
+        match event {
+            winit::Event::WindowEvent { event, .. } => match event {
+                CloseRequested => {
+                    // `CloseRequested` is sent when the close button on the window is pressed (or
+                    // through whatever other mechanisms the window manager provides for closing a
+                    // window). If you don't handle this event, the close button won't actually do
+                    // anything.
+
+                    // A common thing to do here is prompt the user if they have unsaved work.
+                    // Creating a proper dialog box for that is far beyond the scope of this
+                    // example, so here we'll just respond to the Y and N keys.
+                    println!("Are you ready to bid your window farewell? [Y/N]");
+                    close_requested = true;
+
+                    // In applications where you can safely close the window without further
+                    // action from the user, this is generally where you'd handle cleanup before
+                    // closing the window. How to close the window is detailed in the handler for
+                    // the Y key.
+                }
+                KeyboardInput {
+                    input:
+                        winit::KeyboardInput {
+                            virtual_keycode: Some(virtual_code),
+                            state: Released,
+                            ..
+                        },
+                    ..
+                } => match virtual_code {
+                    Y => {
+                        if close_requested {
+                            // This is where you'll want to do any cleanup you need.
+                            println!("Buh-bye!");
+
+                            // For a single-window application like this, you'd normally just
+                            // break out of the event loop here. If you wanted to keep running the
+                            // event loop (i.e. if it's a multi-window application), you need to
+                            // drop the window. That closes it, and results in `Destroyed` being
+                            // sent.
+                            return winit::ControlFlow::Break;
+                        }
+                    }
+                    N => {
+                        if close_requested {
+                            println!("Your window will continue to stay by your side.");
+                            close_requested = false;
+                        }
+                    }
+                    _ => (),
+                },
+                _ => (),
+            },
+            _ => (),
+        }
+
+        winit::ControlFlow::Continue
+    });
+}

examples/min_max_size.rs

@@ -14,7 +14,7 @@ fn main() {
         println!("{:?}", event);
 
         match event {
-            winit::Event::WindowEvent { event: winit::WindowEvent::Closed, .. } => winit::ControlFlow::Break,
+            winit::Event::WindowEvent { event: winit::WindowEvent::CloseRequested, .. } => winit::ControlFlow::Break,
             _ => winit::ControlFlow::Continue,
         }
     });

examples/multiwindow.rs

@@ -1,32 +1,31 @@
 extern crate winit;
 
+use std::collections::HashMap;
+
 fn main() {
     let mut events_loop = winit::EventsLoop::new();
 
-    let window1 = winit::Window::new(&events_loop).unwrap();
-    let window2 = winit::Window::new(&events_loop).unwrap();
-    let window3 = winit::Window::new(&events_loop).unwrap();
-
-    let mut num_windows = 3;
+    let mut windows = HashMap::new();
+    for _ in 0..3 {
+        let window = winit::Window::new(&events_loop).unwrap();
+        windows.insert(window.id(), window);
+    }
 
     events_loop.run_forever(|event| {
         match event {
-            winit::Event::WindowEvent { event: winit::WindowEvent::Closed, window_id } => {
-                if window_id == window1.id() {
-                    println!("Window 1 has been closed")
-                } else if window_id == window2.id() {
-                    println!("Window 2 has been closed")
-                } else if window_id == window3.id() {
-                    println!("Window 3 has been closed");
-                } else {
-                    unreachable!()
-                }
+            winit::Event::WindowEvent {
+                event: winit::WindowEvent::CloseRequested,
+                window_id,
+            } => {
+                println!("Window {:?} has received the signal to close", window_id);
 
-                num_windows -= 1;
-                if num_windows == 0 {
+                // This drops the window, causing it to close.
+                windows.remove(&window_id);
+
+                if windows.is_empty() {
                     return winit::ControlFlow::Break;
                 }
-            },
+            }
             _ => (),
         }
         winit::ControlFlow::Continue

examples/proxy.rs

@@ -21,7 +21,7 @@ fn main() {
     events_loop.run_forever(|event| {
         println!("{:?}", event);
         match event {
-            winit::Event::WindowEvent { event: winit::WindowEvent::Closed, .. } =>
+            winit::Event::WindowEvent { event: winit::WindowEvent::CloseRequested, .. } =>
                 winit::ControlFlow::Break,
             _ => winit::ControlFlow::Continue,
         }

examples/transparent.rs

@@ -13,7 +13,7 @@ fn main() {
         println!("{:?}", event);
 
         match event {
-            winit::Event::WindowEvent { event: winit::WindowEvent::Closed, .. } => winit::ControlFlow::Break,
+            winit::Event::WindowEvent { event: winit::WindowEvent::CloseRequested, .. } => winit::ControlFlow::Break,
             _ => winit::ControlFlow::Continue,
         }
     });

examples/window.rs

@@ -12,9 +12,10 @@ fn main() {
         println!("{:?}", event);
 
         match event {
-            winit::Event::WindowEvent { event: winit::WindowEvent::Closed, .. } => {
-                winit::ControlFlow::Break
-            },
+            winit::Event::WindowEvent {
+                event: winit::WindowEvent::CloseRequested,
+                ..
+            } => winit::ControlFlow::Break,
             _ => winit::ControlFlow::Continue,
         }
     });

src/events.rs

@@ -30,8 +30,11 @@ pub enum WindowEvent {
     /// The position of the window has changed.
     Moved(i32, i32),
 
-    /// The window has been closed.
-    Closed,
+    /// The window has been requested to close.
+    CloseRequested,
+
+    /// The window has been destroyed.
+    Destroyed,
 
     /// A file has been dropped into the window.
     DroppedFile(PathBuf),

src/lib.rs

@@ -60,8 +60,8 @@
 //!
 //! events_loop.run_forever(|event| {
 //!     match event {
-//!         Event::WindowEvent { event: WindowEvent::Closed, .. } => {
-//!             println!("The window was closed ; stopping");
+//!         Event::WindowEvent { event: WindowEvent::CloseRequested, .. } => {
+//!             println!("The close button was pressed; stopping");
 //!             ControlFlow::Break
 //!         },
 //!         _ => ControlFlow::Continue,
@@ -127,7 +127,7 @@ pub mod os;
 ///
 /// events_loop.run_forever(|event| {
 ///     match event {
-///         Event::WindowEvent { event: WindowEvent::Closed, .. } => {
+///         Event::WindowEvent { event: WindowEvent::CloseRequested, .. } => {
 ///             ControlFlow::Break
 ///         },
 ///         _ => ControlFlow::Continue,

src/platform/ios/mod.rs

@@ -51,12 +51,12 @@
 //!  - applicationWillResignActive is Focused(false)
 //!  - applicationDidEnterBackground is Suspended(true)
 //!  - applicationWillEnterForeground is Suspended(false)
-//!  - applicationWillTerminate is Closed
+//!  - applicationWillTerminate is Destroyed
 //!
-//! Keep in mind that after Closed event is received every attempt to draw with
+//! Keep in mind that after Destroyed event is received every attempt to draw with
 //! opengl will result in segfault.
 //!
-//! Also note that app will not receive Closed event if suspended, it will be SIGKILL'ed
+//! Also note that app will not receive Destroyed event if suspended, it will be SIGKILL'ed
 
 #![cfg(target_os = "ios")]
 
@@ -459,7 +459,7 @@ fn create_delegate_class() {
             // immidiatly after jump
             state.events_queue.push_front(Event::WindowEvent {
                 window_id: RootEventId(WindowId),
-                event: WindowEvent::Closed,
+                event: WindowEvent::Destroyed,
             });
             longjmp(mem::transmute(&mut jmpbuf),1);
         }

src/platform/linux/wayland/event_loop.rs

@@ -335,8 +335,11 @@ impl EventsLoop {
         {
             let mut cleanup_needed = self.cleanup_needed.lock().unwrap();
             if *cleanup_needed {
-                evq.state().get_mut(&self.store).cleanup();
+                let pruned = evq.state().get_mut(&self.store).cleanup();
                 *cleanup_needed = false;
+                for wid in pruned {
+                    sink.send_event(::WindowEvent::Destroyed, wid);
+                }
             }
         }
         // process pending resize/refresh
@@ -355,7 +358,7 @@ impl EventsLoop {
                     sink.send_event(::WindowEvent::Refresh, wid);
                 }
                 if closed {
-                    sink.send_event(::WindowEvent::Closed, wid);
+                    sink.send_event(::WindowEvent::CloseRequested, wid);
                 }
             }
         )

src/platform/linux/wayland/window.rs

@@ -262,16 +262,19 @@ impl WindowStore {
         None
     }
 
-    pub fn cleanup(&mut self) {
+    pub fn cleanup(&mut self) -> Vec<WindowId> {
+        let mut pruned = Vec::new();
         self.windows.retain(|w| {
             if *w.kill_switch.lock().unwrap() {
                 // window is dead, cleanup
+                pruned.push(make_wid(&w.surface));
                 w.surface.destroy();
                 false
             } else {
                 true
             }
         });
+        pruned
     }
 
     pub fn for_each<F>(&mut self, mut f: F)

src/platform/linux/x11/mod.rs

@@ -245,15 +245,7 @@ impl EventsLoop {
                 let window_id = mkwid(window);
 
                 if client_msg.data.get_long(0) as ffi::Atom == self.wm_delete_window {
-                    callback(Event::WindowEvent { window_id, event: WindowEvent::Closed });
-
-                    if let Some(_) = self.windows.lock().unwrap().remove(&WindowId(window)) {
-                        unsafe {
-                            (self.display.xlib.XDestroyWindow)(self.display.display, window);
-                        }
-                        self.display.check_errors()
-                            .expect("Failed to destroy window");
-                    }
+                    callback(Event::WindowEvent { window_id, event: WindowEvent::CloseRequested });
                 } else if client_msg.message_type == self.dnd.atoms.enter {
                     let source_window = client_msg.data.get_long(0) as c_ulong;
                     let flags = client_msg.data.get_long(1);
@@ -430,11 +422,20 @@ impl EventsLoop {
                 let xev: &ffi::XDestroyWindowEvent = xev.as_ref();
 
                 let window = xev.window;
+                let window_id = mkwid(window);
 
+                // In the event that the window's been destroyed without being dropped first, we
+                // cleanup again here.
+                self.windows.lock().unwrap().remove(&WindowId(window));
+
+                // Since all XIM stuff needs to happen from the same thread, we destroy the input
+                // context here instead of when dropping the window.
                 self.ime
                     .borrow_mut()
                     .remove_context(window)
                     .expect("Failed to destroy input context");
+
+                callback(Event::WindowEvent { window_id, event: WindowEvent::Destroyed });
             }
 
             ffi::Expose => {
@@ -1059,27 +1060,12 @@ impl Window {
 impl Drop for Window {
     fn drop(&mut self) {
         if let (Some(windows), Some(display)) = (self.windows.upgrade(), self.display.upgrade()) {
-            // It's possible for the Window object to outlive the actual window, so we need to
-            // check for that, lest the program explode with BadWindow errors soon after this.
-            let window_closed = windows
-                .lock()
-                .unwrap()
-                .get(&self.window.id())
-                .is_none();
-            if !window_closed { unsafe {
-                let wm_protocols_atom = util::get_atom(&display, b"WM_PROTOCOLS\0")
-                    .expect("Failed to call XInternAtom (WM_PROTOCOLS)");
-                let wm_delete_atom = util::get_atom(&display, b"WM_DELETE_WINDOW\0")
-                    .expect("Failed to call XInternAtom (WM_DELETE_WINDOW)");
-                util::send_client_msg(
-                    &display,
-                    self.window.id().0,
-                    self.window.id().0,
-                    wm_protocols_atom,
-                    None,
-                    (wm_delete_atom as _, ffi::CurrentTime as _, 0, 0, 0),
-                ).expect("Failed to send window deletion message");
-            } }
+            if let Some(_) = windows.lock().unwrap().remove(&self.window.id()) {
+                unsafe {
+                    (display.xlib.XDestroyWindow)(display.display, self.window.id().0);
+                    display.check_errors().expect("Failed to destroy window");
+                }
+            }
         }
     }
 }

src/platform/macos/events_loop.rs

@@ -89,7 +89,7 @@ impl Shared {
 
     // Removes the window with the given `Id` from the `windows` list.
     //
-    // This is called when a window is either `Closed` or `Drop`ped.
+    // This is called in response to `windowWillClose`.
     pub fn find_and_remove_window(&self, id: super::window::Id) {
         if let Ok(mut windows) = self.windows.lock() {
             windows.retain(|w| match w.upgrade() {

src/platform/macos/window.rs

@@ -165,7 +165,17 @@ impl WindowDelegate {
             unsafe {
                 let state: *mut c_void = *this.get_ivar("winitState");
                 let state = &mut *(state as *mut DelegateState);
-                emit_event(state, WindowEvent::Closed);
+                emit_event(state, WindowEvent::CloseRequested);
+            }
+            NO
+        }
+
+        extern fn window_will_close(this: &Object, _: Sel, _: id) {
+            unsafe {
+                let state: *mut c_void = *this.get_ivar("winitState");
+                let state = &mut *(state as *mut DelegateState);
+
+                emit_event(state, WindowEvent::Destroyed);
 
                 // Remove the window from the shared state.
                 if let Some(shared) = state.shared.upgrade() {
@@ -173,7 +183,6 @@ impl WindowDelegate {
                     shared.find_and_remove_window(window_id);
                 }
             }
-            YES
         }
 
         extern fn window_did_resize(this: &Object, _: Sel, _: id) {
@@ -364,6 +373,8 @@ impl WindowDelegate {
             // Add callback methods
             decl.add_method(sel!(windowShouldClose:),
                 window_should_close as extern fn(&Object, Sel, id) -> BOOL);
+            decl.add_method(sel!(windowWillClose:),
+                window_will_close as extern fn(&Object, Sel, id));
             decl.add_method(sel!(windowDidResize:),
                 window_did_resize as extern fn(&Object, Sel, id));
             decl.add_method(sel!(windowDidChangeScreen:),
@@ -468,12 +479,6 @@ unsafe fn get_current_monitor() -> RootMonitorId {
 
 impl Drop for Window2 {
     fn drop(&mut self) {
-        // Remove this window from the `EventLoop`s list of windows.
-        let id = self.id();
-        if let Some(shared) = self.delegate.state.shared.upgrade() {
-            shared.find_and_remove_window(id);
-        }
-
         // Close the window if it has not yet been closed.
         let nswindow = *self.window;
         if nswindow != nil {

src/platform/windows/events_loop.rs

@@ -326,6 +326,13 @@ lazy_static! {
             winuser::RegisterWindowMessageA("Winit::ExecMsg\0".as_ptr() as *const i8)
         }
     };
+    // Message sent by a `Window` when it wants to be destroyed by the main thread.
+    // WPARAM and LPARAM are unused.
+    pub static ref DESTROY_MSG_ID: u32 = {
+        unsafe {
+            winuser::RegisterWindowMessageA("Winit::DestroyMsg\0".as_ptr() as *const i8)
+        }
+    };
 }
 
 // There's no parameters passed to the callback function, so it needs to get its context stashed
@@ -386,16 +393,25 @@ pub unsafe extern "system" fn callback(window: HWND, msg: UINT,
 {
     match msg {
         winuser::WM_CLOSE => {
-            use events::WindowEvent::Closed;
+            use events::WindowEvent::CloseRequested;
             send_event(Event::WindowEvent {
                 window_id: SuperWindowId(WindowId(window)),
-                event: Closed
+                event: CloseRequested
             });
+            0
+        },
+
+        winuser::WM_DESTROY => {
+            use events::WindowEvent::Destroyed;
             CONTEXT_STASH.with(|context_stash| {
                 let mut context_stash = context_stash.borrow_mut();
                 context_stash.as_mut().unwrap().windows.remove(&window);
             });
-            winuser::DefWindowProcW(window, msg, wparam, lparam)
+            send_event(Event::WindowEvent {
+                window_id: SuperWindowId(WindowId(window)),
+                event: Destroyed
+            });
+            0
         },
 
         winuser::WM_PAINT => {
@@ -922,7 +938,12 @@ pub unsafe extern "system" fn callback(window: HWND, msg: UINT,
         },
 
         _ => {
+            if msg == *DESTROY_MSG_ID {
+                winuser::DestroyWindow(window);
+                0
+            } else {
                 winuser::DefWindowProcW(window, msg, wparam, lparam)
             }
         }
     }
+}

src/platform/windows/window.rs

@@ -11,7 +11,7 @@ use std::sync::Mutex;
 use std::sync::mpsc::channel;
 use std::cell::Cell;
 
-use platform::platform::events_loop;
+use platform::platform::events_loop::{self, DESTROY_MSG_ID};
 use platform::platform::EventsLoop;
 use platform::platform::PlatformSpecificWindowBuilderAttributes;
 use platform::platform::WindowId;
@@ -620,9 +620,9 @@ impl Drop for Window {
     #[inline]
     fn drop(&mut self) {
         unsafe {
-            // We are sending WM_CLOSE, and our callback will process this by calling DefWindowProcW,
-            // which in turn will send a WM_DESTROY.
-            winuser::PostMessageW(self.window.0, winuser::WM_CLOSE, 0, 0);
+            // The window must be destroyed from the same thread that created it, so we send a
+            // custom message to be handled by our callback to do the actual work.
+            winuser::PostMessageW(self.window.0, *DESTROY_MSG_ID, 0, 0);
         }
     }
 }