Guard bookmark folder traversal against cyclic folder relations

[theabhishekchandra]

Task/Issue URL: #5928
Tech Design URL (if applicable):
API Proposals URL(s) (if applicable): None

Description

A bookmark folder that ends up inside one of its own descendants (for example FolderA contains FolderB and FolderB contains FolderA) makes every recursive walk over the relations table loop until the stack overflows, and the app is killed with a StackOverflowError. Three walks were affected:

• traverseFolderWithDepth, used by the folder picker (getFolderTree)
• traverseBranch, used when fetching or deleting a folder branch (getFolderBranch / deleteFolderBranch)
• populateNode in RealSavedSitesExporter, used by the bookmarks HTML export
• getRequestEntriesFor in SavedSitesSyncDataProvider, which walks folder contents to build the sync upload
• traverseParents in RealSyncSavedSitesRepository, which walks up to the root after sync deduplication

Each walk now carries the set of folder ids already on its path and skips any folder it has seen, so a cycle is visited once and the walk terminates. Output for normal, acyclic trees is unchanged.

insert(folder) also now replaces the folder's existing parent relation instead of adding a second one. Relation's primary key is autogenerated, so OnConflictStrategy.REPLACE never deduplicated it, and re-inserting a folder could create the multi-parent state that leads to cycles.

Sync is also the most likely way such a loop appears in the first place, since folder moves made on two devices can merge into a state where each folder contains the other, so the two sync walks are guarded too and covered by unit tests.

Reproduced on a physical device by inserting one cyclic row into relations: the folder picker, the export and the folder delete each crashed with several thousand frames of the corresponding traversal. All three complete with this change on the same data.

Related: #9704 guards traverseBranch only. This change covers all five recursions plus the insert path.

Steps to test this PR

Cyclic folder relations no longer crash

• On a debug build, bookmark a site, create FolderA and FolderB inside it, then add a row to relations with folderId = FolderB.id and entityId = FolderA.id (e.g. pull app.db, edit with sqlite3, push back while the app is force-stopped)
• Open Bookmarks → overflow on a bookmark → Edit → Location: the picker lists Bookmarks / FolderA / FolderB instead of the app being killed
• Bookmarks → overflow → Export: an HTML file is written with each folder listed once
• Bookmarks → overflow on FolderA → Delete → confirm, wait for the snackbar to expire: the folder branch is removed, the app stays alive, and no dangling relations remain

Normal folder operations are unchanged

• Create a folder, move a bookmark into it via the picker, export and re-import the file, delete a folder and tap Undo: all behave as before

UI changes

Before	After
Folder picker never renders, app is killed	Folder picker lists the cyclic folders and the app stays alive

---

Note

Medium Risk
Touches core bookmark tree, export, and sync traversal; behavior for valid trees should be unchanged, but incorrect visit logic could omit folders or leave bad relations after insert.

Overview
Prevents StackOverflowError when bookmark folder relations form a cycle (e.g. Folder A contains B and B contains A). Recursive walks over the relations table now keep a visited-folder set and skip folders already on the current path, so each folder is handled once and traversal stops.

The same cycle guard is applied across folder picker (getFolderTree / traverseFolderWithDepth), branch fetch/delete (traverseBranch), HTML export (populateNode), sync payload building (getRequestEntriesFor), and parent walks for dedup sync (traverseParents in RealSyncSavedSitesRepository).

insert(BookmarkFolder) now deletes the folder’s existing parent relation before inserting the new one, so re-inserting a folder cannot accumulate multiple parents (a path that could create cycles, since Relation uses an autogenerated PK and REPLACE did not dedupe).

Tests cover cyclic data for those flows plus no duplicate relation when the same folder is inserted twice.

^{Reviewed by Cursor Bugbot for commit f34a049. Bugbot is set up for automated code reviews on this repo. Configure here.}