Make with_parent_window safe

examples/child_window.rs

@@ -17,7 +17,7 @@ fn main() -> Result<(), impl std::error::Error> {
         dpi::{LogicalPosition, LogicalSize, Position},
         event::{ElementState, Event, KeyEvent, WindowEvent},
         event_loop::{EventLoop, EventLoopWindowTarget},
-        raw_window_handle::HasRawWindowHandle,
+        raw_window_handle::HasWindowHandle,
         window::{Window, WindowBuilder, WindowId},
     };
 
@@ -26,14 +26,13 @@ fn main() -> Result<(), impl std::error::Error> {
         event_loop: &EventLoopWindowTarget<()>,
         windows: &mut HashMap<WindowId, Window>,
     ) {
-        let parent = parent.raw_window_handle().unwrap();
+        let parent = parent.window_handle().unwrap();
         let mut builder = WindowBuilder::new()
             .with_title("child window")
             .with_inner_size(LogicalSize::new(200.0f32, 200.0f32))
             .with_position(Position::Logical(LogicalPosition::new(0.0, 0.0)))
             .with_visible(true);
-        // `with_parent_window` is unsafe. Parent window must be a valid window.
-        builder = unsafe { builder.with_parent_window(Some(parent)) };
+        builder = builder.with_parent_window(Some(parent));
         let child_window = builder.build(event_loop).unwrap();
 
         let id = child_window.id();

src/platform_impl/android/mod.rs

@@ -751,6 +751,18 @@ impl DeviceId {
 #[derive(Clone, Copy, Debug, Default, Eq, PartialEq)]
 pub struct PlatformSpecificWindowBuilderAttributes;
 
+#[derive(Debug, Clone)]
+pub(crate) struct OwnedWindowHandle {}
+
+impl OwnedWindowHandle {
+    #[cfg(feature = "rwh_06")]
+    pub(crate) fn new_parent_window(_handle: rwh_06::WindowHandle<'_>) -> Self {
+        // Parent windows are currently unsupported, though owned window
+        // handles would be implementable.
+        Self {}
+    }
+}
+
 pub(crate) struct Window {
     app: AndroidApp,
     redraw_requester: RedrawRequester,

src/platform_impl/ios/mod.rs

@@ -73,7 +73,7 @@ pub(crate) use self::{
         EventLoop, EventLoopProxy, EventLoopWindowTarget, PlatformSpecificEventLoopAttributes,
     },
     monitor::{MonitorHandle, VideoMode},
-    window::{PlatformSpecificWindowBuilderAttributes, Window, WindowId},
+    window::{OwnedWindowHandle, PlatformSpecificWindowBuilderAttributes, Window, WindowId},
 };
 
 use self::uikit::UIScreen;

src/platform_impl/ios/window.rs

@@ -25,6 +25,19 @@ use crate::{
     },
 };
 
+#[derive(Debug, Clone)]
+pub(crate) struct OwnedWindowHandle {}
+
+impl OwnedWindowHandle {
+    #[cfg(feature = "rwh_06")]
+    pub(crate) fn new_parent_window(_handle: rwh_06::WindowHandle<'_>) -> Self {
+        // Parent windows are currently unsupported, though owned window
+        // handles would be implementable (would work similar to macOS).
+        warn!("parent windows are unsupported on iOS");
+        Self {}
+    }
+}
+
 pub struct Inner {
     window: Id<WinitUIWindow>,
     view_controller: Id<WinitViewController>,

src/platform_impl/linux/mod.rs

@@ -141,6 +141,43 @@ impl fmt::Display for OsError {
     }
 }
 
+#[derive(Debug, Clone)]
+#[allow(dead_code)]
+pub(crate) enum OwnedWindowHandle {
+    #[cfg(x11_platform)]
+    X(x11rb::protocol::xproto::Window),
+    #[cfg(wayland_platform)]
+    Wayland,
+}
+
+impl OwnedWindowHandle {
+    #[cfg(feature = "rwh_06")]
+    pub(crate) fn new_parent_window(handle: rwh_06::WindowHandle<'_>) -> Self {
+        // TODO: Do we need to do something extra to extend the lifetime of
+        // the window lives beyond the passed-in handle?
+        match handle.as_raw() {
+            #[cfg(x11_platform)]
+            rwh_06::RawWindowHandle::Xlib(handle) => {
+                Self::X(handle.window as x11rb::protocol::xproto::Window)
+            }
+            #[cfg(x11_platform)]
+            rwh_06::RawWindowHandle::Xcb(handle) => Self::X(handle.window.get()),
+            #[cfg(wayland_platform)]
+            rwh_06::RawWindowHandle::Wayland(_handle) => {
+                // Wayland does not currently support parent windows, but it
+                // could support owned handles.
+                Self::Wayland
+            }
+            #[cfg(not(x11_platform))]
+            handle => panic!("invalid window handle {handle:?} on Wayland"),
+            #[cfg(not(wayland_platform))]
+            handle => panic!("invalid window handle {handle:?} on X11"),
+            #[cfg(all(x11_platform, wayland_platform))]
+            handle => panic!("invalid window handle {handle:?} on X11 or Wayland"),
+        }
+    }
+}
+
 pub(crate) enum Window {
     #[cfg(x11_platform)]
     X(x11::Window),

src/platform_impl/linux/x11/window.rs

@@ -30,6 +30,7 @@ use crate::{
             atoms::*, xinput_fp1616_to_float, MonitorHandle as X11MonitorHandle, WakeSender,
             X11Error,
         },
+        OwnedWindowHandle as PlatformOwnedWindowHandle,
         Fullscreen, MonitorHandle as PlatformMonitorHandle, OsError, PlatformCustomCursor,
         PlatformIcon, PlatformSpecificWindowBuilderAttributes, VideoMode as PlatformVideoMode,
     },
@@ -157,15 +158,12 @@ impl UnownedWindow {
     ) -> Result<UnownedWindow, RootOsError> {
         let xconn = &event_loop.xconn;
         let atoms = xconn.atoms();
-        #[cfg(feature = "rwh_06")]
-        let root = match window_attrs.parent_window.0 {
-            Some(rwh_06::RawWindowHandle::Xlib(handle)) => handle.window as xproto::Window,
-            Some(rwh_06::RawWindowHandle::Xcb(handle)) => handle.window.get(),
-            Some(raw) => unreachable!("Invalid raw window handle {raw:?} on X11"),
+        let root = match window_attrs.parent_window {
+            Some(PlatformOwnedWindowHandle::X(handle)) => handle,
+            #[cfg(wayland_platform)]
+            Some(handle) => panic!("invalid window handle {handle:?} on X11"),
             None => event_loop.root,
         };
-        #[cfg(not(feature = "rwh_06"))]
-        let root = event_loop.root;
 
         let mut monitors = leap!(xconn.available_monitors());
         let guessed_monitor = if monitors.is_empty() {

src/platform_impl/macos/mod.rs

@@ -28,7 +28,7 @@ pub(crate) use self::{
 use crate::event::DeviceId as RootDeviceId;
 
 pub(crate) use self::cursor::CustomCursor as PlatformCustomCursor;
-pub(crate) use self::window::Window;
+pub(crate) use self::window::{OwnedWindowHandle, Window};
 pub(crate) use crate::cursor::OnlyCursorImageBuilder as PlatformCustomCursorBuilder;
 pub(crate) use crate::icon::NoIcon as PlatformIcon;
 pub(crate) use crate::platform_impl::Fullscreen;

src/platform_impl/macos/window.rs

@@ -129,6 +129,54 @@ impl From<u64> for WindowId {
     }
 }
 
+#[derive(Debug)]
+pub(crate) struct OwnedWindowHandle {
+    ns_view: MainThreadBound<Id<NSView>>,
+}
+
+impl Clone for OwnedWindowHandle {
+    fn clone(&self) -> Self {
+        Self {
+            ns_view: MainThreadMarker::run_on_main(|mtm| {
+                MainThreadBound::new(self.ns_view.get(mtm).clone(), mtm)
+            }),
+        }
+    }
+}
+
+impl OwnedWindowHandle {
+    #[cfg(feature = "rwh_06")]
+    pub(crate) fn new_parent_window(handle: rwh_06::WindowHandle<'_>) -> Self {
+        let mtm =
+            MainThreadMarker::new().expect("can only have handles on macOS on the main thread");
+        let ns_view = match handle.as_raw() {
+            rwh_06::RawWindowHandle::AppKit(handle) => {
+                // SAFETY: Taking `WindowHandle<'_>` ensures that the pointer is valid.
+                // Unwrap is fine, since the pointer comes from `NonNull`.
+                unsafe { Id::retain(handle.ns_view.as_ptr().cast()) }.unwrap()
+            }
+            handle => panic!("invalid window handle {handle:?} on macOS"),
+        };
+        Self {
+            ns_view: MainThreadBound::new(ns_view, mtm),
+        }
+    }
+
+    #[cfg(feature = "rwh_06")]
+    pub(crate) fn raw_window_handle(&self) -> Result<rwh_06::RawWindowHandle, rwh_06::HandleError> {
+        if let Some(mtm) = MainThreadMarker::new() {
+            let window_handle = rwh_06::AppKitWindowHandle::new({
+                let ptr = Id::as_ptr(self.ns_view.get(mtm)) as *mut _;
+                std::ptr::NonNull::new(ptr).expect("Id<T> should never be null")
+            });
+            let handle = rwh_06::RawWindowHandle::AppKit(window_handle);
+            Ok(handle)
+        } else {
+            Err(rwh_06::HandleError::Unavailable)
+        }
+    }
+}
+
 #[derive(Clone)]
 pub struct PlatformSpecificWindowBuilderAttributes {
     pub movable_by_window_background: bool,
@@ -439,25 +487,16 @@ impl WinitWindow {
         })
         .ok_or_else(|| os_error!(OsError::CreationError("Couldn't create `NSWindow`")))?;
 
-        #[cfg(feature = "rwh_06")]
-        match attrs.parent_window.0 {
-            Some(rwh_06::RawWindowHandle::AppKit(handle)) => {
-                // SAFETY: Caller ensures the pointer is valid or NULL
-                // Unwrap is fine, since the pointer comes from `NonNull`.
-                let parent_view: Id<NSView> =
-                    unsafe { Id::retain(handle.ns_view.as_ptr().cast()) }.unwrap();
-                let parent = parent_view.window().ok_or_else(|| {
-                    os_error!(OsError::CreationError(
-                        "parent view should be installed in a window"
-                    ))
-                })?;
-
-                // SAFETY: We know that there are no parent -> child -> parent cycles since the only place in `winit`
-                // where we allow making a window a child window is right here, just after it's been created.
-                unsafe { parent.addChildWindow_ordered(&this, NSWindowAbove) };
-            }
-            Some(raw) => panic!("Invalid raw window handle {raw:?} on macOS"),
-            None => (),
+        if let Some(parent_window) = attrs.parent_window {
+            let parent = parent_window.ns_view.get(mtm).window().ok_or_else(|| {
+                os_error!(OsError::CreationError(
+                    "parent view should be installed in a window"
+                ))
+            })?;
+
+            // SAFETY: We know that there are no parent -> child -> parent cycles since the only place in `winit`
+            // where we allow making a window a child window is right here, just after it's been created.
+            unsafe { parent.addChildWindow_ordered(&this, NSWindowAbove) };
         }
 
         let view = WinitView::new(&this, pl_attrs.accepts_first_mouse);

src/platform_impl/orbital/mod.rs

@@ -9,7 +9,7 @@ use crate::dpi::{PhysicalPosition, PhysicalSize};
 pub use self::event_loop::{EventLoop, EventLoopProxy, EventLoopWindowTarget};
 mod event_loop;
 
-pub use self::window::Window;
+pub(crate) use self::window::{OwnedWindowHandle, Window};
 mod window;
 
 struct RedoxSocket {

src/platform_impl/orbital/window.rs

@@ -25,6 +25,18 @@ const ORBITAL_FLAG_BORDERLESS: char = 'l';
 const ORBITAL_FLAG_RESIZABLE: char = 'r';
 const ORBITAL_FLAG_TRANSPARENT: char = 't';
 
+#[derive(Debug, Clone)]
+pub(crate) struct OwnedWindowHandle {}
+
+impl OwnedWindowHandle {
+    #[cfg(feature = "rwh_06")]
+    pub(crate) fn new_parent_window(_handle: rwh_06::WindowHandle<'_>) -> Self {
+        // Parent windows are currently unsupported, though owned window
+        // handles would be implementable.
+        Self {}
+    }
+}
+
 pub struct Window {
     window_socket: Arc<RedoxSocket>,
     redraws: Arc<Mutex<VecDeque<WindowId>>>,

src/platform_impl/web/mod.rs

@@ -35,7 +35,9 @@ pub(crate) use self::event_loop::{
     EventLoop, EventLoopProxy, EventLoopWindowTarget, PlatformSpecificEventLoopAttributes,
 };
 pub use self::monitor::{MonitorHandle, VideoMode};
-pub use self::window::{PlatformSpecificWindowBuilderAttributes, Window, WindowId};
+pub(crate) use self::window::{
+    OwnedWindowHandle, PlatformSpecificWindowBuilderAttributes, Window, WindowId,
+};
 
 pub(crate) use self::keyboard::KeyEventExtra;
 pub(crate) use crate::icon::NoIcon as PlatformIcon;

src/platform_impl/web/window.rs

@@ -17,6 +17,18 @@ use std::cell::RefCell;
 use std::collections::VecDeque;
 use std::rc::Rc;
 
+#[derive(Debug, Clone)]
+pub(crate) struct OwnedWindowHandle {}
+
+impl OwnedWindowHandle {
+    #[cfg(feature = "rwh_06")]
+    pub(crate) fn new_parent_window(_handle: rwh_06::WindowHandle<'_>) -> Self {
+        // Parent windows are currently unsupported, though owned window
+        // handles would be implementable.
+        Self {}
+    }
+}
+
 pub struct Window {
     inner: Dispatcher<Inner>,
 }

src/platform_impl/windows/mod.rs

@@ -12,7 +12,7 @@ pub(crate) use self::{
     },
     icon::{SelectedCursor, WinIcon},
     monitor::{MonitorHandle, VideoMode},
-    window::Window,
+    window::{OwnedWindowHandle, Window},
 };
 
 pub use self::icon::WinIcon as PlatformIcon;

src/platform_impl/windows/window.rs

@@ -81,6 +81,23 @@ use crate::{
     },
 };
 
+#[derive(Debug, Clone)]
+pub(crate) struct OwnedWindowHandle {
+    hwnd: HWND,
+}
+
+impl OwnedWindowHandle {
+    #[cfg(feature = "rwh_06")]
+    pub(crate) fn new_parent_window(handle: rwh_06::WindowHandle<'_>) -> Self {
+        // TODO: Do we need to do something to extend the lifetime of the window handle?
+        let hwnd = match handle.as_raw() {
+            rwh_06::RawWindowHandle::Win32(handle) => handle.hwnd.get() as HWND,
+            handle => panic!("invalid window handle {handle:?} on Windows"),
+        };
+        Self { hwnd }
+    }
+}
+
 /// The Win32 implementation of the main `Window` object.
 pub(crate) struct Window {
     /// Main handle for the window.
@@ -1301,33 +1318,25 @@ where
     // so the diffing later can work.
     window_flags.set(WindowFlags::CLOSABLE, true);
 
-    let mut fallback_parent = || match pl_attribs.owner {
-        Some(parent) => {
-            window_flags.set(WindowFlags::POPUP, true);
-            Some(parent)
+    let parent = if let Some(parent_window) = &attributes.parent_window {
+        window_flags.set(WindowFlags::CHILD, true);
+        if pl_attribs.menu.is_some() {
+            warn!("Setting a menu on a child window is unsupported");
         }
-        None => {
-            window_flags.set(WindowFlags::ON_TASKBAR, true);
-            None
-        }
-    };
-
-    #[cfg(feature = "rwh_06")]
-    let parent = match attributes.parent_window.0 {
-        Some(rwh_06::RawWindowHandle::Win32(handle)) => {
-            window_flags.set(WindowFlags::CHILD, true);
-            if pl_attribs.menu.is_some() {
-                warn!("Setting a menu on a child window is unsupported");
+        Some(parent_window.hwnd)
+    } else {
+        match pl_attribs.owner {
+            Some(parent) => {
+                window_flags.set(WindowFlags::POPUP, true);
+                Some(parent)
+            }
+            None => {
+                window_flags.set(WindowFlags::ON_TASKBAR, true);
+                None
             }
-            Some(handle.hwnd.get() as HWND)
         }
-        Some(raw) => unreachable!("Invalid raw window handle {raw:?} on Windows"),
-        None => fallback_parent(),
     };
 
-    #[cfg(not(feature = "rwh_06"))]
-    let parent = fallback_parent();
-
     let mut initdata = InitData {
         event_loop,
         attributes,