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

Revert "fix(macos): check WKURLSchemeTask is valid before using" #1284

Merged
merged 1 commit into from
Jun 4, 2024

Conversation

wusyong
Copy link
Member

@wusyong wusyong commented Jun 4, 2024

Reverts #1282

Didn't realize there's another slimilar one in discussion.

@wusyong wusyong requested a review from a team as a code owner June 4, 2024 08:25
@wusyong wusyong merged commit ee1af1a into dev Jun 4, 2024
12 checks passed
@wusyong wusyong deleted the revert-1282-fix/macos-mitigate-async-crash branch June 4, 2024 08:31
@FabianLars
Copy link
Member

to be fair though, imo jason's solution was better because it didn't use so much objc 😂

@pewsheen
Copy link
Contributor

pewsheen commented Jun 4, 2024

to be fair though, imo jason's solution was better because it didn't use so much objc 😂

I was about to ask for your opinion lol

This still crashes in some circumstances, so we want to revert it first and try something else.

@pewsheen pewsheen restored the revert-1282-fix/macos-mitigate-async-crash branch June 4, 2024 08:38
@pewsheen pewsheen deleted the revert-1282-fix/macos-mitigate-async-crash branch June 4, 2024 08:38
@FabianLars
Copy link
Member

hmm, that's unfortunate. Did you check if the community pr also crashes in the same context?

@pewsheen
Copy link
Contributor

pewsheen commented Jun 4, 2024

hmm, that's unfortunate. Did you check if the community pr also crashes in the same context?

Community PR works, and I was testing the example that PR provided.
It looks like the task will be released even inside the didResponse method. Retain task until the response closure ends would fix it. I'll reopen the PR again with the fix.

update:
Community PR does not work for the issue on tauri

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

Successfully merging this pull request may close these issues.

4 participants