ryanmcgrath / cacao

Rust bindings for AppKit (macOS) and UIKit (iOS/tvOS). Experimental, but working!
MIT License
1.85k stars 68 forks source link

Browser Example Broken #122

Open abhillman opened 3 months ago

abhillman commented 3 months ago

Observed:

image

Expected:

image

Notes:

trunk gives the above "observed" screenshot. The "expected" screenshot is given with 0.4.0-beta2 and this diff (else compilation fails):

diff --git a/examples/browser/toolbar.rs b/examples/browser/toolbar.rs
index b4416dd..f3bcf68 100644
--- a/examples/browser/toolbar.rs
+++ b/examples/browser/toolbar.rs
@@ -35,12 +35,12 @@ impl BrowserToolbar {
         let back_button = Button::new("Back");
         let mut back_item = ToolbarItem::new(BACK_BUTTON);
         back_item.set_button(back_button);
-        back_item.set_action(|| Action::Back.dispatch());
+        back_item.set_action(|_| Action::Back.dispatch());

         let forwards_button = Button::new("Forwards");
         let mut forwards_item = ToolbarItem::new(FWDS_BUTTON);
         forwards_item.set_button(forwards_button);
-        forwards_item.set_action(|| Action::Forwards.dispatch());
+        forwards_item.set_action(|_| Action::Forwards.dispatch());

         let url_bar = TextField::with(URLBar);
         let url_bar_item = ToolbarItem::new(URL_BAR);

https://github.com/ryanmcgrath/cacao/pull/30 appears to be the commit that breaks the browser example.

ryanmcgrath commented 3 months ago

Hmmm, are you saying that the webview isn't loading anything, or that back doesn't work?

And what version of macOS are you testing against...?

abhillman commented 3 months ago

Hmmm, are you saying that the webview isn't loading anything, or that back doesn't work?

The webview is not loading anything aginst trunk, but does load against 0.4.0-beta2.

And what version of macOS are you testing against...?

$ sw_vers
ProductName:        macOS
ProductVersion:     14.5 # Sonoma 14.5
BuildVersion:       23F79

$ uname -a
Darwin ok.local 23.5.0 Darwin Kernel Version 23.5.0: Wed May  1 20:17:33 PDT 2024; root:xnu-10063.121.3~5/RELEASE_ARM64_T6031 arm64
ryanmcgrath commented 3 months ago

Hmmm, might be down to a difference in obj/objc2. @madsmtm might know more, otherwise I'd have to find time to dig in - and I'll be busy + traveling the next two weeks.

If you wanted to tinker and debug, a webview not loading sounds like something not getting retained somewhere - either the webview itself, or the webview's delegate. It's where I'd start looking at least.

abhillman commented 3 months ago

[edit: submitted before seeing your last message @ryanmcgrath]

Glad to take a deeper dive in case you have any rough ideas and would enjoy any support.

I have taken a cursory look at the changes to webview between trunk and 0.4.0-beta2 https://github.com/ryanmcgrath/cacao/compare/0.4.0-beta2..trunk, but nothing major jumps out.

abhillman commented 3 months ago

If you wanted to tinker and debug, a webview not loading sounds like something not getting retained somewhere - either the webview itself, or the webview's delegate. It's where I'd start looking at least.

Agreed regarding retainment. I'm not sure if meaningful, but I did see some differences in the webview configuration after objc2 was introduced; i.e. a "strong pointer" against 0.4.0-beta2 but something different against trunk:

Screenshot 2024-06-30 at 4 55 28 PM Screenshot 2024-06-30 at 5 15 52 PM

It's not clear to me without more studying if this is meaningful.

abhillman commented 3 months ago

Indeed perhaps @madsmtm may have some thoughts...

madsmtm commented 2 months ago

The old StrongPtr is the same as the objc2::rc::Id, there's no difference there. The issue is indeed very likely that something that was previously (erroneously) leaked is now no longer getting retained long enough.

If you could put your code up somewhere, then it'd be easier to diagnose? Otherwise, I'd suggest to try sprinkle std::mem::forget(obj) in various parts of your code.