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

Bugfix FXIOS-5344 [v109] fixes search highlights getting interrupted #12578

Merged

Conversation

tarikeshaq
Copy link
Contributor

@tarikeshaq tarikeshaq commented Dec 4, 2022

Bugfix for #12530

Moves the places.interruptReader() to only run if search highlights are disabled and we are running the new places history. The interrupt helps make sure we aren't running too many long-running queries back to back against places.db without interruption.

Additionally, optimizes the search highlights case. Previously, we would always run the normal history query, but then discard its result if search highlights are enabled. Now, we only run the search query if search highlights are disabled.

Some more context on the bug in the issue.

Possibly for a follow-up:

  • We should also interrupt the reader in the case of history highlights by introducing an interrupt in the search highlights flow
  • Even better, but more complicated, the search highlights should probably have their own reader connection. We have lots of logic in application-services that serves the case of multiple readers. We can have a reader per use case. (this becomes more important once we use AS history)

cc @lmarceau @badboy

@lmarceau lmarceau self-requested a review December 5, 2022 14:35
Client/Frontend/Browser/SearchLoader.swift Outdated Show resolved Hide resolved
Client/Frontend/Browser/SearchLoader.swift Outdated Show resolved Hide resolved
Client/Frontend/Browser/SearchLoader.swift Outdated Show resolved Hide resolved
Client/Frontend/Browser/SearchLoader.swift Outdated Show resolved Hide resolved
@lmarceau
Copy link
Contributor

lmarceau commented Dec 6, 2022

Build was green here
Screen Shot 2022-12-06 at 2 50 22 PM

@lmarceau lmarceau merged commit 7a57e75 into mozilla-mobile:main Dec 7, 2022
@tarikeshaq tarikeshaq deleted the fixes-search-highlights-interrupt branch December 7, 2022 19:08
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