Exclude selected tab from return hatch query to prevent stale data#8224
Merged
YoussefKeyrouz merged 1 commit intodevelopfrom Apr 9, 2026
Merged
Exclude selected tab from return hatch query to prevent stale data#8224YoussefKeyrouz merged 1 commit intodevelopfrom
YoussefKeyrouz merged 1 commit intodevelopfrom
Conversation
Collaborator
Author
This stack of pull requests is managed by Graphite. Learn more about stacking. |
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit c234744. Configure here.
c234744 to
36fc18d
Compare
karlenDimla
approved these changes
Apr 9, 2026
36fc18d to
7556583
Compare
YoussefKeyrouz
added a commit
that referenced
this pull request
Apr 10, 2026
… to NTP (#8235) Task/Issue URL: https://app.asana.com/1/137249556945/project/488551667048375/task/1213988415500467?focus=true ### Description This PR has 2 commits. I recommend reviewing them separately. Commit #1: revert the fix made in #8224. It introduced an undesirable side effect when tapping the hatch to navigate away. The older tab flashes quickly before navigation Commit #2: Fixing the actual root cause for the Hatch logo incorrectly loading the wrong image after quick back navigation. ### Details on the fix (In commit #2): The return hatch's favicon load uses a retry mechanism that launches fire-and-forget coroutines on the view's lifecycleScope. When the hatch re-renders with a different tab's data, the stale retries keep running and overwrite the ImageView with the old favicon. Even if we cancel the favicon coroutine, it does not cancel the retries if they are launched as separate coroutines. So the existing load method cannot be used. 1. Added a method to the FavIconManager that does the same thing with the retry but they are canceleable 2. Canceled the favicon load when new loads are triggered or when the coroutine is detached. ### Steps to test this PR - Open a tab and navigate to a site (e.g., wikipedia.org), let it fully load - Open a new tab - Navigate to a tracker-heavy site ideally new site where the fav icon is not cached yet (I had good success with nba.com) - Press back quickly while the page is still loading - Observe the return hatch on the new tab page. - If done a the right timing the escape hatch will incorrectly show the NBA logo instead of wikipedia. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Changes favicon loading to a new cancellable retry path and alters the "last accessed tab" query to include selected tabs, which can affect NTP return-hatch behavior and any callers relying on the previous non-selected semantics. > > **Overview** > Fixes the New Tab Page return hatch showing a stale/incorrect favicon by introducing `FaviconManager.loadToViewFromLocalWithRetry`, where all retries run in the calling coroutine and are cancelled when the job is cancelled (the old `loadToViewFromLocalWithPlaceholder` is now deprecated). > > Updates `NewTabReturnHatchView` to cancel any in-flight favicon load on re-render/detach and to use the new retry API. Separately, renames and changes tab-access APIs from *last accessed non-selected tab* to `lastAccessedTab`/`flowLastAccessedTab` (and updates tests) by removing the exclusion of currently selected tabs in the Room query. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 2b529e8. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Task/Issue URL: https://app.asana.com/1/137249556945/project/1212810093780571/task/1213988415500467?focus=true
Description
The return hatch query (flowLastAccessedTab) could return the currently selected tab, displaying the site that was loading even after the user navigated back to the NTP. Concurrent DB writes from tracker events could overwrite the null URL written by navigateHome(), leaving the tab with stale data permanently.
Root cause
onSiteChanged()is called frequently during page load (every tracker event triggers it), each call asynchronously writing the site URL to the tab database. When the user presses back, navigateHome() clears the URL asynchronously, but stale writes from tracker events can land after the null write, briefly restoring the URL in the database. The return hatch observes these changes via a Room Flow and renders the stale data.Fix
Excluding the selected tab from the query eliminates the race entirely regardless of DB write ordering, the current tab's data is never shown in the hatch.
Also renamed and updated the query to flowLastAccessedNonSelectedTab, so it better reflects what the query is doing and avoids confusion.
Steps to test this Bug
This PR should fix that
Note
Low Risk
Low risk: a small change to a Room query and associated API rename to prevent the return hatch from observing stale data; main risk is unintended behavior changes in which tab is considered "last accessed".
Overview
Prevents the new-tab "return hatch" from ever showing the currently selected tab by updating the "last accessed" tab queries to exclude
tabIds present intab_selection, eliminating a race where late DB writes could restore stale URL/title data.Renames the affected DAO/repository APIs from
lastAccessedTab/flowLastAccessedTabtolastAccessedNonSelectedTab/flowLastAccessedNonSelectedTaband updates call sites and tests accordingly.Reviewed by Cursor Bugbot for commit 7556583. Bugbot is set up for automated code reviews on this repo. Configure here.