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

[Dynamic Links] Reduce memory stress on WebKit API #8847

Merged
merged 3 commits into from
Oct 21, 2021
Merged

[Dynamic Links] Reduce memory stress on WebKit API #8847

merged 3 commits into from
Oct 21, 2021

Conversation

ncooke3
Copy link
Member

@ncooke3 ncooke3 commented Oct 21, 2021

There seems to be a memory issue on simulators where calling WebKit's loadHTMLString:baseURL: API crashes.

Apparently, WebKit is known to have some memory related bugs and my hunch is that the string that dynamic links passes to the above API is too large for some instances being run on a simulator.

This PR reduces the size of the string being passed to the API. The string should be evaluated the same as before this PR.

While I was not able to repro the issue and verify the fix, the fix got some positive feedback.

Fix #8655
@google-cla google-cla bot added the cla: yes label Oct 21, 2021
paulb777
paulb777 reviewed Oct 21, 2021
View reviewed changes
Copy link
Member

@paulb777 paulb777 left a comment

Choose a reason for hiding this comment

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

How do we know the language code being deleted isn't needed?
@google-oss-bot
Copy link

google-oss-bot commented Oct 21, 2021 •
edited
Loading

Coverage Report

Affected SDKs

  • FirebaseDynamicLinks-iOS-FirebaseDynamicLinks.framework

    SDK overall coverage changed from 60.41% (ef433a0) to 60.35% (53fd2e3) by -0.05%.

    Filename Base (ef433a0) Head (53fd2e3) Diff
    FIRDLDefaultRetrievalProcessV2.m 15.52% 14.04% -1.48%

Test Logs
@paulb777 paulb777 requested a review from eldhosembabu October 21, 2021 20:04
@paulb777 paulb777 added this to the 8.9.0 - M106 milestone Oct 21, 2021
@paulb777 paulb777 merged commit 97aff9e into master Oct 21, 2021
@paulb777 paulb777 deleted the nc/dynamiclinks-8655 branch October 21, 2021 21:10
@paulb777 paulb777 mentioned this pull request Oct 22, 2021
Closed
@firebase firebase locked and limited conversation to collaborators Nov 21, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api: dynamiclinks cla: yes
4 participants
@ncooke3 @google-oss-bot @paulb777 @eldhosembabu