Require setting the activation policy on the event loop (#1922)

* Require setting the activation policy on the event loop

* Run cargo fmt

* Update changelog

* Fixes and tweaks from review

* Correct comment in app_state.rs

Co-authored-by: Mads Marquart <mads@marquart.dk>

.gitignore

@@ -7,3 +7,4 @@ rls/
 *.ts
 *.js
 #*#
+.DS_Store

CHANGELOG.md

@@ -1,5 +1,6 @@
 # Unreleased
 
+- **Breaking:** On macOS, replace `WindowBuilderExtMacOS::with_activation_policy` with `EventLoopExtMacOS::set_activation_policy`
 - On macOS, wait with activating the application until the application has initialized.
 - On macOS, fix creating new windows when the application has a main menu.
 - On Windows, fix fractional deltas for mouse wheel device events.

src/platform/macos.rs

@@ -4,8 +4,9 @@ use std::os::raw::c_void;
 
 use crate::{
     dpi::LogicalSize,
-    event_loop::EventLoopWindowTarget,
+    event_loop::{EventLoop, EventLoopWindowTarget},
     monitor::MonitorHandle,
+    platform_impl::get_aux_state_mut,
     window::{Window, WindowBuilder},
 };
 
@@ -100,8 +101,6 @@ impl Default for ActivationPolicy {
 ///  - `with_titlebar_buttons_hidden`
 ///  - `with_fullsize_content_view`
 pub trait WindowBuilderExtMacOS {
-    /// Sets the activation policy for the window being built.
-    fn with_activation_policy(self, activation_policy: ActivationPolicy) -> WindowBuilder;
     /// Enables click-and-drag behavior for the entire window, not just the titlebar.
     fn with_movable_by_window_background(self, movable_by_window_background: bool)
         -> WindowBuilder;
@@ -122,12 +121,6 @@ pub trait WindowBuilderExtMacOS {
 }
 
 impl WindowBuilderExtMacOS for WindowBuilder {
-    #[inline]
-    fn with_activation_policy(mut self, activation_policy: ActivationPolicy) -> WindowBuilder {
-        self.platform_specific.activation_policy = activation_policy;
-        self
-    }
-
     #[inline]
     fn with_movable_by_window_background(
         mut self,
@@ -186,6 +179,23 @@ impl WindowBuilderExtMacOS for WindowBuilder {
     }
 }
 
+pub trait EventLoopExtMacOS {
+    /// Sets the activation policy for the application. It is set to
+    /// `NSApplicationActivationPolicyRegular` by default.
+    ///
+    /// This function only takes effect if it's called before calling [`run`](crate::event_loop::EventLoop::run) or
+    /// [`run_return`](crate::platform::run_return::EventLoopExtRunReturn::run_return)
+    fn set_activation_policy(&mut self, activation_policy: ActivationPolicy);
+}
+impl<T> EventLoopExtMacOS for EventLoop<T> {
+    #[inline]
+    fn set_activation_policy(&mut self, activation_policy: ActivationPolicy) {
+        unsafe {
+            get_aux_state_mut(&**self.event_loop.delegate).activation_policy = activation_policy;
+        }
+    }
+}
+
 /// Additional methods on `MonitorHandle` that are specific to MacOS.
 pub trait MonitorHandleExtMacOS {
     /// Returns the identifier of the monitor for Cocoa.

src/platform_impl/macos/app_delegate.rs

@@ -1,9 +1,23 @@
-use super::app_state::AppState;
+use crate::{platform::macos::ActivationPolicy, platform_impl::platform::app_state::AppState};
+
 use cocoa::base::id;
 use objc::{
     declare::ClassDecl,
     runtime::{Class, Object, Sel},
 };
+use std::{
+    cell::{RefCell, RefMut},
+    os::raw::c_void,
+};
+
+static AUX_DELEGATE_STATE_NAME: &str = "auxState";
+
+pub struct AuxDelegateState {
+    /// We store this value in order to be able to defer setting the activation policy until
+    /// after the app has finished launching. If the activation policy is set earlier, the
+    /// menubar is initially unresponsive on macOS 10.15 for example.
+    pub activation_policy: ActivationPolicy,
+}
 
 pub struct AppDelegateClass(pub *const Class);
 unsafe impl Send for AppDelegateClass {}
@@ -14,17 +28,51 @@ lazy_static! {
         let superclass = class!(NSResponder);
         let mut decl = ClassDecl::new("WinitAppDelegate", superclass).unwrap();
 
+        decl.add_class_method(sel!(new), new as extern "C" fn(&Class, Sel) -> id);
+        decl.add_method(sel!(dealloc), dealloc as extern "C" fn(&Object, Sel));
+
         decl.add_method(
             sel!(applicationDidFinishLaunching:),
             did_finish_launching as extern "C" fn(&Object, Sel, id),
         );
+        decl.add_ivar::<*mut c_void>(AUX_DELEGATE_STATE_NAME);
 
         AppDelegateClass(decl.register())
     };
 }
 
-extern "C" fn did_finish_launching(_: &Object, _: Sel, _: id) {
+/// Safety: Assumes that Object is an instance of APP_DELEGATE_CLASS
+pub unsafe fn get_aux_state_mut(this: &Object) -> RefMut<'_, AuxDelegateState> {
+    let ptr: *mut c_void = *this.get_ivar(AUX_DELEGATE_STATE_NAME);
+    // Watch out that this needs to be the correct type
+    (*(ptr as *mut RefCell<AuxDelegateState>)).borrow_mut()
+}
+
+extern "C" fn new(class: &Class, _: Sel) -> id {
+    unsafe {
+        let this: id = msg_send![class, alloc];
+        let this: id = msg_send![this, init];
+        (*this).set_ivar(
+            AUX_DELEGATE_STATE_NAME,
+            Box::into_raw(Box::new(RefCell::new(AuxDelegateState {
+                activation_policy: ActivationPolicy::Regular,
+            }))) as *mut c_void,
+        );
+        this
+    }
+}
+
+extern "C" fn dealloc(this: &Object, _: Sel) {
+    unsafe {
+        let state_ptr: *mut c_void = *(this.get_ivar(AUX_DELEGATE_STATE_NAME));
+        // As soon as the box is constructed it is immediately dropped, releasing the underlying
+        // memory
+        Box::from_raw(state_ptr as *mut RefCell<AuxDelegateState>);
+    }
+}
+
+extern "C" fn did_finish_launching(this: &Object, _: Sel, _: id) {
     trace!("Triggered `applicationDidFinishLaunching`");
-    AppState::launched();
+    AppState::launched(this);
     trace!("Completed `applicationDidFinishLaunching`");
 }

src/platform_impl/macos/app_state.rs

@@ -19,17 +19,23 @@ use cocoa::{
 };
 use objc::runtime::YES;
 
+use objc::runtime::Object;
+
 use crate::{
     dpi::LogicalSize,
     event::{Event, StartCause, WindowEvent},
     event_loop::{ControlFlow, EventLoopWindowTarget as RootWindowTarget},
-    platform_impl::platform::{
+    platform::macos::ActivationPolicy,
-        event::{EventProxy, EventWrapper},
+    platform_impl::{
-        event_loop::{post_dummy_event, PanicInfo},
+        get_aux_state_mut,
-        menu,
+        platform::{
-        observer::{CFRunLoopGetMain, CFRunLoopWakeUp, EventLoopWaker},
+            event::{EventProxy, EventWrapper},
-        util::{IdRef, Never},
+            event_loop::{post_dummy_event, PanicInfo},
-        window::get_window_id,
+            menu,
+            observer::{CFRunLoopGetMain, CFRunLoopWakeUp, EventLoopWaker},
+            util::{IdRef, Never},
+            window::get_window_id,
+        },
     },
     window::WindowId,
 };
@@ -273,7 +279,8 @@ impl AppState {
         HANDLER.callback.lock().unwrap().take();
     }
 
-    pub fn launched() {
+    pub fn launched(app_delegate: &Object) {
+        apply_activation_policy(app_delegate);
         unsafe {
             let ns_app = NSApp();
             window_activation_hack(ns_app);
@@ -457,3 +464,18 @@ unsafe fn window_activation_hack(ns_app: id) {
         }
     }
 }
+fn apply_activation_policy(app_delegate: &Object) {
+    unsafe {
+        use cocoa::appkit::NSApplicationActivationPolicy::*;
+        let ns_app = NSApp();
+        // We need to delay setting the activation policy and activating the app
+        // until `applicationDidFinishLaunching` has been called. Otherwise the
+        // menu bar won't be interactable.
+        let act_pol = get_aux_state_mut(app_delegate).activation_policy;
+        ns_app.setActivationPolicy_(match act_pol {
+            ActivationPolicy::Regular => NSApplicationActivationPolicyRegular,
+            ActivationPolicy::Accessory => NSApplicationActivationPolicyAccessory,
+            ActivationPolicy::Prohibited => NSApplicationActivationPolicyProhibited,
+        });
+    }
+}

src/platform_impl/macos/event_loop.rs

@@ -87,6 +87,8 @@ impl<T: 'static> EventLoopWindowTarget<T> {
 }
 
 pub struct EventLoop<T: 'static> {
+    pub(crate) delegate: IdRef,
+
     window_target: Rc<RootWindowTarget<T>>,
     panic_info: Rc<PanicInfo>,
 
@@ -97,7 +99,6 @@ pub struct EventLoop<T: 'static> {
     /// into a strong reference in order to call the callback but then the
     /// strong reference should be dropped as soon as possible.
     _callback: Option<Rc<RefCell<dyn FnMut(Event<'_, T>, &RootWindowTarget<T>, &mut ControlFlow)>>>,
-    _delegate: IdRef,
 }
 
 impl<T> EventLoop<T> {
@@ -122,13 +123,13 @@ impl<T> EventLoop<T> {
         let panic_info: Rc<PanicInfo> = Default::default();
         setup_control_flow_observers(Rc::downgrade(&panic_info));
         EventLoop {
+            delegate,
             window_target: Rc::new(RootWindowTarget {
                 p: Default::default(),
                 _marker: PhantomData,
             }),
             panic_info,
             _callback: None,
-            _delegate: delegate,
         }
     }
 

src/platform_impl/macos/menu.rs

@@ -1,7 +1,4 @@
-use cocoa::appkit::{
+use cocoa::appkit::{NSApp, NSApplication, NSEventModifierFlags, NSMenu, NSMenuItem};
-    NSApp, NSApplication, NSApplicationActivationPolicyRegular, NSEventModifierFlags, NSMenu,
-    NSMenuItem,
-};
 use cocoa::base::{nil, selector};
 use cocoa::foundation::{NSAutoreleasePool, NSProcessInfo, NSString};
 use objc::{

src/platform_impl/macos/mod.rs

@@ -17,6 +17,7 @@ mod window_delegate;
 use std::{fmt, ops::Deref, sync::Arc};
 
 pub use self::{
+    app_delegate::{get_aux_state_mut, AuxDelegateState},
     event_loop::{EventLoop, EventLoopWindowTarget, Proxy as EventLoopProxy},
     monitor::{MonitorHandle, VideoMode},
     window::{Id as WindowId, PlatformSpecificWindowBuilderAttributes, UnownedWindow},

src/platform_impl/macos/window.rs

@@ -16,7 +16,7 @@ use crate::{
     error::{ExternalError, NotSupportedError, OsError as RootOsError},
     icon::Icon,
     monitor::{MonitorHandle as RootMonitorHandle, VideoMode as RootVideoMode},
-    platform::macos::{ActivationPolicy, WindowExtMacOS},
+    platform::macos::WindowExtMacOS,
     platform_impl::platform::{
         app_state::AppState,
         app_state::INTERRUPT_EVENT_LOOP_EXIT,
@@ -34,9 +34,8 @@ use crate::{
 };
 use cocoa::{
     appkit::{
-        self, CGFloat, NSApp, NSApplication, NSApplicationActivationPolicy,
+        self, CGFloat, NSApp, NSApplication, NSApplicationPresentationOptions, NSColor,
-        NSApplicationPresentationOptions, NSColor, NSRequestUserAttentionType, NSScreen, NSView,
+        NSRequestUserAttentionType, NSScreen, NSView, NSWindow, NSWindowButton, NSWindowStyleMask,
-        NSWindow, NSWindowButton, NSWindowStyleMask,
     },
     base::{id, nil},
     foundation::{NSAutoreleasePool, NSDictionary, NSPoint, NSRect, NSSize},
@@ -64,7 +63,6 @@ pub fn get_window_id(window_cocoa_id: id) -> Id {
 
 #[derive(Clone)]
 pub struct PlatformSpecificWindowBuilderAttributes {
-    pub activation_policy: ActivationPolicy,
     pub movable_by_window_background: bool,
     pub titlebar_transparent: bool,
     pub title_hidden: bool,
@@ -80,7 +78,6 @@ impl Default for PlatformSpecificWindowBuilderAttributes {
     #[inline]
     fn default() -> Self {
         Self {
-            activation_policy: Default::default(),
             movable_by_window_background: false,
             titlebar_transparent: false,
             title_hidden: false,
@@ -94,24 +91,6 @@ impl Default for PlatformSpecificWindowBuilderAttributes {
     }
 }
 
-fn create_app(activation_policy: ActivationPolicy) -> Option<id> {
-    unsafe {
-        let ns_app = NSApp();
-        if ns_app == nil {
-            None
-        } else {
-            // TODO: Move ActivationPolicy from an attribute on the window to something on the EventLoop
-            use self::NSApplicationActivationPolicy::*;
-            ns_app.setActivationPolicy_(match activation_policy {
-                ActivationPolicy::Regular => NSApplicationActivationPolicyRegular,
-                ActivationPolicy::Accessory => NSApplicationActivationPolicyAccessory,
-                ActivationPolicy::Prohibited => NSApplicationActivationPolicyProhibited,
-            });
-            Some(ns_app)
-        }
-    }
-}
-
 unsafe fn create_view(
     ns_window: id,
     pl_attribs: &PlatformSpecificWindowBuilderAttributes,
@@ -358,12 +337,6 @@ impl UnownedWindow {
         trace!("Creating new window");
 
         let pool = unsafe { NSAutoreleasePool::new(nil) };
-
-        create_app(pl_attribs.activation_policy).ok_or_else(|| {
-            unsafe { pool.drain() };
-            os_error!(OsError::CreationError("Couldn't create `NSApplication`"))
-        })?;
-
         let ns_window = create_window(&win_attribs, &pl_attribs).ok_or_else(|| {
             unsafe { pool.drain() };
             os_error!(OsError::CreationError("Couldn't create `NSWindow`"))