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

Refactor FXIOS-5488 [v110] Remove deferred from SQLiteHistoryFavicons #12776

Conversation

lmarceau
Copy link
Contributor

FXIOS-5488 #12773

Looking at #12771, we can't rule out that deferred isn't linked into this crash. Seeing #12527 this also becomes suspicious.

One easy change is to remove deferred from that code in 109. This module is enclosed and so this shouldn't make things worst. Made sure that we call UI code from the main thread. This is a patch in the meantime that we're working on the long term favicon solution.

@lmarceau lmarceau requested a review from OrlaM December 20, 2022 22:01
Copy link
Contributor

@OrlaM OrlaM left a comment

Choose a reason for hiding this comment

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

Did some testing locally and everything seems to be working as expected.

@lmarceau
Copy link
Contributor Author

Build is green here

@lmarceau lmarceau changed the title Refactor FXIOS-5488 [v109] Remove deferred from SQLiteHistoryFavicons Refactor FXIOS-5488 [v110] Remove deferred from SQLiteHistoryFavicons Dec 21, 2022
@lmarceau lmarceau merged commit 942509a into mozilla-mobile:main Dec 21, 2022
@lmarceau lmarceau deleted the lm/FXIOS-5488-Remove-deferred-from-sqlhistory branch December 21, 2022 16:35
@Isamas
Copy link

Isamas commented Dec 26, 2022

Hello, as i don't really understand this discussion above, to say the least ;) Can some of you code wizzards let me know when I'll be able to reconnect my Firefox account on my mobile device? I tried again yesterday and of course it crashed again. Do I need to wait until the next upgrade? And when will that be? It's quite difficult to use Firefox on iphone without passwords. And I don't always have access to computer. Many thanks for your answer.

@lmarceau
Copy link
Contributor Author

lmarceau commented Jan 3, 2023

Hello @Isamas,
It's preferable to comment on issues rather than pull requests. Is there an existing issue for the problem you are mentioning (you're saying you can't login? or you can't sync?) Other than that, I would normally expect Firefox account to work on mobile.

If there's no existing issue for the problem you're encountering, please create one here

@Isamas
Copy link

Isamas commented Jan 3, 2023 via email

@lmarceau
Copy link
Contributor Author

lmarceau commented Jan 3, 2023

If you open the actual link and do not reply from email, you'll see you are at the moment commenting on the Pull request #12776 (where code changes happen) and not on the actual issue.

From your comment I am deducing that you're referring to issue #12771. Please do not comment further on the Pull request. Thanks!

@Isamas
Copy link

Isamas commented Jan 3, 2023

Ah ok. Sorry. Tks

@OrlaM
Copy link
Contributor

OrlaM commented Jan 4, 2023

@Mergifyio backport release/v109.0

@mergify
Copy link
Contributor

mergify bot commented Jan 4, 2023

backport release/v109.0

✅ Backports have been created

mergify bot pushed a commit that referenced this pull request Jan 4, 2023
OrlaM pushed a commit that referenced this pull request Jan 4, 2023
@Isamas
Copy link

Isamas commented Jan 5, 2023

It seems to work! Many thanks 💝

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.

3 participants