Panic under macOS with latest winit version

[jnb]

Using the latest winit and wry, we get a panic from winit on macOS when backgrounding (command-tabbing) the application:

thread 'main' panicked at [...]/winit/src/platform_impl/apple/appkit/window_delegate.rs:236:18:
called `Result::unwrap()` on an `Err` value: <WryWebViewParent: 0x12e405040>

This is because winit checks that the contentView is of type WinitView in WinitWindowDelegate::view:

pub(super) fn view(&self) -> Retained<WinitView> {
    // The view inside WinitWindow should always be set and be `WinitView`.
    self.window().contentView().unwrap().downcast().unwrap()
}

But wry replaces the content view with WryWebViewParent (unless we're explicitly using a child view).

(And command-tabbing invokes WinitWindowDelegate::window_did_resign_key, which invokes WinitWindowDelegate::view from above.)

Minimal(ish) reproducible example: https://gist.github.com/jnb/53568af99de5d3c40bf7dc2646aee93a

Editorializing: replacing the content view seems to be breaking a pretty fundamental assumption of winit? So it's not clear to me how wry can work on macOS without requiring the use of child view (which would be unfortunate, because resizing child views causes a lot of jank).

[amrbashir]

we had a lot of problems in the past with using a subview, probably regarding key events to the webview (I don't remember which tbh, maybe @pewsheen or @FabianLars or @lucasfernog has better idea) and we were kinda forced to replace the content view.

I am not a macOS developer, but maybe @madsmtm have an idea? could we possibly inherit the WinitView in our view and so both winit and wry can coexist?

few references:

• On macOS, add WKWebview as subview of existing NSView directly #745
• Menu shortcut does not work on macOS since wry 0.22 #749
• On macOS, remove doCommandBySelector in view tao#620
• [bug] Accelerators with single character in CustomMenuItem won't work tauri#5592
• On macOS, call next responder in view's keyDown and doCommandbySelector tao#623

There is probably more history there, we really need to document this.

[madsmtm]

Yeah, replacing the content view is unsupported by Winit (and was UB in v0.29, so it's nice that we at least catch this now).

I'm somewhat reluctant of exposing WinitView so that users can subclass it easily, because it'd expose our dependency on objc2. (If you really wanted to, I guess you could grab the class name yourself and create a class with extern_class! that Wry could subclass, but even the class name is subject to change).

Besides, fundamentally, I think replacing the content view is basically always the wrong choice. Like, sure, Tauri might want the entire window to be filled by Wry, but I can easily imagine applications that would rather embed Wry into a subview somewhere.

I'd rather work with you to get the "add view as subview" use-case working in Winit. Maybe we need to call super in more of the methods we override, similar to what you've done in tauri-apps/tao#623? Will be easier for me to comment on how if you know exactly which issues were caused by adding a subview?

requiring the use of child view (which would be unfortunate, because resizing child views causes a lot of jank).

Resizing can indeed be perilous, but not impossible; for the GPU ecosystem, I've written raw-window-metal which adds a CAMetalLayer subclasses which uses Key-Value Observing to keep in-sync with the parent layer; I suspect something equivalent would be possible in Wry.

[jnb]

For what it's worth, I've been using a subview for the webview (i.e. Webview::new_as_child), and it's been working well for my use-case, with the following two exceptions:

• The cursor can't be changed from js. This is not great, but is tolerable.
• The resize performance of the webview is pretty terrible. This is much closer to be a deal-breaker for me, to the extent that I've reluctantly been investigating non-wry approaches. The non-subview webview does indeed have much better resize performance.

[madsmtm]

For the resizing being non-performant, part of that is also "just" a Winit issue: rust-windowing/winit#2640.

The cursor can't be changed from js. This is not great, but is tolerable.

Hmm, might need us to call invalidateCursorRectsForView more? Or maybe we should call super in resetCursorRects?

In what manner doesn't it work? A crash? Or is the request just ignored?

Just to verify, how are you changing the cursor? With Winit's Window::set_cursor?

[jnb]

Just to verify, how are you changing the cursor? With Winit's Window::set_cursor?

No, I'm just setting style.cursor from Javascript. I haven't done any more investigation than just noting that this doesn't have the intended effect (the cursor remains unchanged). I'm assuming that that this Just Works when not running the webview as a subview, but haven't verified.