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.

Sign up for GitHub

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

Jump to bottom

Crash in FrameInfo.init (safeRequest property) #981

Merged
merged 1 commit into from
Feb 20, 2023
Merged

Crash in FrameInfo.init (safeRequest property) #981

merged 1 commit into from
Feb 20, 2023

Conversation

mallexxx
Copy link
Collaborator

@mallexxx mallexxx commented Feb 20, 2023 •
edited
Loading

Task/Issue URL: https://app.asana.com/0/1199230911884351/1203965979591356/f
BSK PR: duckduckgo/BrowserServicesKit#234
Tech Design URL:
CC: @tomasstrba

Description:

  • Added WKFrameInfo.safeRequest property instead of wrongly-non-nullable .request
  • Added runtime checks for WKFrameInfo.safeRequest and WKNavigationAction.sourceFrame usage in Debug config
  • Fixed swizzled method original implementation calls by adding "dynamic" method dispatch modifier

Steps to test this PR:
0. Validate both Debug&Release builds

  1. Go to https://www.seriouseats.com/ask-the-food-lab-do-i-need-to-use-kosher-salt
  2. Click through the "Buy on ..." product links, validate there‘s no crash when WKFrameInfo.request is accessed
  3. Validate microphone permission works on https://permission.site
Internal references:

Pull Request Review Checklist
Software Engineering Expectations
Technical Design Template
When ready for review, remember to post the PR in MM

@mallexxx mallexxx requested a review from ayoy February 20, 2023 05:46
@mallexxx mallexxx mentioned this pull request Feb 20, 2023
Merged
ayoy
ayoy approved these changes Feb 20, 2023
View reviewed changes
Copy link
Collaborator

@ayoy ayoy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! 🏅

@mallexxx mallexxx merged commit 7678b99 into alex/dist-nav-delegate Feb 20, 2023
@mallexxx mallexxx deleted the alex/wkframeinfo-safe-request branch February 20, 2023 10:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ayoy ayoy ayoy approved these changes

Assignees

@ayoy ayoy

Labels
includes BSK changes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mallexxx @ayoy