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

Remove Deferred usage from BookmarksSaver #23787

Open

data-sync-user opened this issue Dec 16, 2024 · 2 comments

Collaborator

data-sync-user commented Dec 16, 2024 •

edited

Loading

[WIP]

Replace usage of Deferred with completion handlers in RustPlaces.swift

Acceptance Criteria:

┆Issue is synchronized with this Jira Task

data-sync-user changed the title ~~Remove Deferred from Places.swift~~ Remove Deferred usage from BookmarksSaver

Collaborator Author

data-sync-user commented Dec 16, 2024

➤ Laurie Marceau commented:

A comparable PR work is https://github.com/mozilla-mobile/firefox-ios/pull/18920/files ( https://github.com/mozilla-mobile/firefox-ios/pull/18920/files|smart-link ) . Basically we can add new methods that uses completion handlers instead of Deferred in RustPlaces, then use those new methods in the BookmarksSaver.swift. So we’re not removing Deferred from RustPlaces, but providing alternative so our new code can access Places without Deferred. A similar PR was done in RustPlaces with https://github.com/mozilla-mobile/firefox-ios/pull/11408/files#diff-4831bd77ff9fc5a2aad43d89be90fce6becbde4bd71db26792b2d373c2f57b1e ( https://github.com/mozilla-mobile/firefox-ios/pull/11408/files#diff-4831bd77ff9fc5a2aad43d89be90fce6becbde4bd71db26792b2d373c2f57b1e|smart-link ) .

Collaborator Author

data-sync-user commented Dec 18, 2024

➤ Laurie Marceau commented:

As discussed in https://mozilla.slack.com/archives/C05C9RET70F/p1734556299304639 ( https://mozilla.slack.com/archives/C05C9RET70F/p1734556299304639|smart-link ) , we can investigate to use async await instead of completion handlers, to avoid the usage of withCheckedContinuation due to unknown crashes with this functionality in production. Let’s try to be on the safer side if we can avoid those entirely

PARAIPAN9 mentioned this issue

Refactor FXIOS-10924 - Remove Deferred from FolderHierarchyFetcher #23900

Open

6 tasks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment