Skip to content

Commit 7f35382

Browse files
committed
Extra safety comments
1 parent 31ccb0b commit 7f35382

2 files changed

Lines changed: 26 additions & 9 deletions

File tree

‎src/macos/view.rs‎

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -84,8 +84,13 @@ macro_rules! add_simple_keyboard_class_method {
8484
}
8585

8686
pub(super) fn create_view(window_options: &WindowOpenOptions) -> Retained<NSView> {
87-
let class = create_view_class();
88-
let view: Allocated<NSView> = unsafe { msg_send![class, alloc] };
87+
let view: Allocated<NSView> = {
88+
// SAFETY: We don't access this reference after calling alloc
89+
let class = unsafe { create_view_class() };
90+
// SAFETY: This function is valid to call, and Allocated<NSView> is the correct type for the
91+
// returned pointer
92+
unsafe { msg_send![class, alloc] }
93+
};
8994

9095
let size = window_options.size;
9196
let view = NSView::initWithFrame(
@@ -95,7 +100,7 @@ pub(super) fn create_view(window_options: &WindowOpenOptions) -> Retained<NSView
95100

96101
let notification_center = NSNotificationCenter::defaultCenter();
97102

98-
// SAFETY: TODO
103+
// SAFETY: Our NSView class does have a handleNotification: method with the matching signature.
99104
unsafe {
100105
notification_center.addObserver_selector_name_object(
101106
&view,
@@ -111,14 +116,19 @@ pub(super) fn create_view(window_options: &WindowOpenOptions) -> Retained<NSView
111116
);
112117
}
113118

114-
// SAFETY: TODO
119+
// SAFETY: This static is a read-only constant
115120
let ns_filenames_pboard_type = unsafe { NSFilenamesPboardType };
116121
view.registerForDraggedTypes(&NSArray::from_slice(&[ns_filenames_pboard_type]));
117122

118123
view
119124
}
120125

121-
fn create_view_class() -> &'static AnyClass {
126+
/// # Safety
127+
///
128+
/// This class is going to be destroyed when its first instance gets deallocated.
129+
///
130+
/// The returned reference must NOT be used after that point.
131+
unsafe fn create_view_class() -> &'static AnyClass {
122132
// Use unique class names so that there are no conflicts between different
123133
// instances. The class is deleted when the view is released. Previously,
124134
// the class was stored in a OnceCell after creation. This way, we didn't
@@ -249,6 +259,7 @@ extern "C-unwind" fn become_first_responder(this: &NSView, _sel: Sel) -> Bool {
249259
};
250260

251261
if window.isKeyWindow() {
262+
// SAFETY: This is our own view instance
252263
let state = unsafe { WindowState::from_view(this) };
253264
state.trigger_deferrable_event(Event::Window(WindowEvent::Focused));
254265
}
@@ -257,12 +268,14 @@ extern "C-unwind" fn become_first_responder(this: &NSView, _sel: Sel) -> Bool {
257268
}
258269

259270
extern "C-unwind" fn resign_first_responder(this: &NSView, _sel: Sel) -> Bool {
271+
// SAFETY: This is our own view instance
260272
let state = unsafe { WindowState::from_view(this) };
261273
state.trigger_deferrable_event(Event::Window(WindowEvent::Unfocused));
262274
Bool::YES
263275
}
264276

265277
extern "C-unwind" fn window_should_close(this: &NSView, _: Sel, _sender: &AnyObject) -> Bool {
278+
// SAFETY: This is our own view instance
266279
let state = unsafe { WindowState::from_view(this) };
267280

268281
state.trigger_event(Event::Window(WindowEvent::WillClose));
@@ -279,8 +292,8 @@ extern "C-unwind" fn dealloc(this: &mut AnyObject, _sel: Sel) {
279292
let () = unsafe { msg_send![super(this, superclass), dealloc] };
280293
}
281294

282-
// Delete class
283-
// SAFETY: TODO: nope, this is NOT sound, as this invalidates any &AnyClass
295+
// SAFETY: This is safe as long as nobody holds a reference to this class.
296+
// On the Baseview side, this is enforced by the safety contract in `create_view_class`
284297
unsafe { objc_disposeClassPair(class as *const _ as *mut _) }
285298
}
286299

@@ -289,7 +302,7 @@ extern "C-unwind" fn view_did_change_backing_properties(this: &NSView, _: Sel, _
289302

290303
let scale_factor: f64 = ns_window.map(|w| w.backingScaleFactor()).unwrap_or(1.0);
291304

292-
// SAFETY: TODO
305+
// SAFETY: This is our own view instance
293306
let state = unsafe { WindowState::from_view(this) };
294307

295308
let bounds = this.bounds();
@@ -358,7 +371,7 @@ fn new_tracking_area(this: &NSView) -> Retained<NSTrackingArea> {
358371
/// default implementation.
359372
extern "C-unwind" fn hit_test(this: &NSView, _sel: Sel, point: NSPoint) -> Option<&NSView> {
360373
let superclass = this.class().superclass().unwrap();
361-
// SAFETY: TODO
374+
// SAFETY: Our superclass is NSView
362375
let super_result: Option<&NSView> =
363376
unsafe { msg_send![super(this, superclass), hitTest: point] };
364377
let super_result = super_result?;

‎src/macos/window.rs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -400,6 +400,10 @@ impl WindowState {
400400
/// This method returns a cloned `Rc<WindowState>` rather than just a `&WindowState`, since the
401401
/// original `Rc<WindowState>` owned by the `NSView` can be dropped at any time
402402
/// (including during an event handler).
403+
///
404+
/// # Safety
405+
///
406+
/// `view` MUST be our own NSView, as created by `create_view`
403407
pub(super) unsafe fn from_view(view: &NSView) -> Rc<WindowState> {
404408
let state_ptr = view
405409
.class()

0 commit comments

Comments
 (0)