Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Browser Example Broken #122

Open
abhillman opened this issue Jun 30, 2024 · 7 comments
Open

Browser Example Broken #122

abhillman opened this issue Jun 30, 2024 · 7 comments

Comments

@abhillman
Copy link

abhillman commented Jun 30, 2024

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);

#30 appears to be the commit that breaks the browser example.

@ryanmcgrath
Copy link
Owner

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
Copy link
Author

abhillman commented Jul 1, 2024

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
Copy link
Owner

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
Copy link
Author

abhillman commented Jul 1, 2024

[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
Copy link
Author

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
Copy link
Author

Indeed perhaps @madsmtm may have some thoughts...

@madsmtm
Copy link
Contributor

madsmtm commented Jul 15, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants