From 4b74223a3fa202c45b1d3de04ce6c7087572f496 Mon Sep 17 00:00:00 2001 From: Fabian Freund Date: Wed, 27 May 2026 07:04:33 +0200 Subject: [PATCH] improve sync logic --- .../features/tabs/data/database/daos/tab.dart | 121 +++++++++++++----- .../drift/tabs/tab_hierarchy_order_test.dart | 29 +++++ 2 files changed, 120 insertions(+), 30 deletions(-) diff --git a/apps/weblibre/lib/features/geckoview/features/tabs/data/database/daos/tab.dart b/apps/weblibre/lib/features/geckoview/features/tabs/data/database/daos/tab.dart index 72eda939..c00715de 100644 --- a/apps/weblibre/lib/features/geckoview/features/tabs/data/database/daos/tab.dart +++ b/apps/weblibre/lib/features/geckoview/features/tabs/data/database/daos/tab.dart @@ -46,6 +46,7 @@ class SyncTabsResult { @DriftAccessor() class TabDao extends DatabaseAccessor with $TabDaoMixin { final _undoHistory = {}; + final _pendingParentIds = {}; Timer? _clearHistoryTimer; static const closedTabTombstoneTtl = Duration(hours: 24); @@ -167,6 +168,54 @@ class TabDao extends DatabaseAccessor with $TabDaoMixin { ); } + Future _resolvePendingParents() async { + if (_pendingParentIds.isEmpty) return; + + final pendingChildren = selectOnly(db.tab) + ..addColumns([db.tab.id]) + ..where( + db.tab.id.isIn(_pendingParentIds.keys) & + db.tab.parentId.isNull() & + db.tab.source.isNotValue(TabSource.manual.index), + ); + final pendingChildIds = (await pendingChildren.get()) + .map((row) => row.read(db.tab.id)!) + .toSet(); + + if (pendingChildIds.isEmpty) { + _pendingParentIds.clear(); + return; + } + + final resolvableParents = await getExistingTabIds( + pendingChildIds.map((childId) => _pendingParentIds[childId]!).toSet(), + ).get().then((ids) => ids.toSet()); + + await batch((batch) { + for (final childId in pendingChildIds) { + final parentId = _pendingParentIds[childId]; + if (parentId == null || !resolvableParents.contains(parentId)) { + continue; + } + + batch.update( + db.tab, + TabCompanion( + parentId: Value(parentId), + source: const Value(TabSource.manual), + ), + where: (t) => t.id.equals(childId), + ); + } + }); + + _pendingParentIds.removeWhere( + (childId, parentId) => + !pendingChildIds.contains(childId) || + resolvableParents.contains(parentId), + ); + } + Future _generateOrderKey({ required Value parentId, required Value containerId, @@ -1081,35 +1130,33 @@ class TabDao extends DatabaseAccessor with $TabDaoMixin { // change from Gecko. final parentSyncEligibleIds = next.isEmpty ? const {} - : { - for (final tab in await (select( - db.tab, - )..where((t) => t.id.isIn(next.keys))).get()) - if (tab.source != TabSource.manual && tab.parentId == null) - tab.id, - }; + : await (() async { + final query = selectOnly(db.tab) + ..addColumns([db.tab.id, db.tab.source]) + ..where(db.tab.id.isIn(next.keys) & db.tab.parentId.isNull()); + return { + for (final row in await query.get()) + if (row.readWithConverter(db.tab.source) != TabSource.manual) + row.read(db.tab.id)!, + }; + })(); - // Collect parent IDs that need database validation - final parentIdsToValidate = {}; - final validatedParentIds = {}; - - for (final state in next.values) { - if (parentSyncEligibleIds.contains(state.id) && - state.parentId != null) { - if (next.containsKey(state.parentId)) { - // Parent exists in current state - validatedParentIds[state.id] = state.parentId; - } else { - // Need to validate against database - parentIdsToValidate.add(state.parentId!); - } - } - } - - // Batch validate parent IDs that aren't in the current state + // Validate every candidate parent against the database — a parentId + // present in `next` is not proof the row has been persisted yet (e.g. + // engine state snapshot arriving before the matching tab-list insert). + // Without the DB check we could write a parent_id pointing at a row + // that does not exist, triggering the self-referential FK violation + // and aborting the whole batch. + final parentIdsToValidate = { + for (final state in next.values) + if (parentSyncEligibleIds.contains(state.id) && + state.parentId != null) + state.parentId!, + }; final existingParentIds = await getExistingTabIds( parentIdsToValidate, ).get().then((ids) => ids.toSet()); + final validatedParentIds = {}; final containerRepairCandidates = { for (final state in next.values) @@ -1137,17 +1184,22 @@ class TabDao extends DatabaseAccessor with $TabDaoMixin { entry.key: containerId, }; - // Complete validation map for (final state in next.values) { - if (!parentSyncEligibleIds.contains(state.id) || - validatedParentIds.containsKey(state.id) || - state.parentId == null) { + if (!parentSyncEligibleIds.contains(state.id)) { + _pendingParentIds.remove(state.id); + continue; + } + + if (state.parentId == null) { + _pendingParentIds.remove(state.id); continue; } - // This parent ID needed database validation if (existingParentIds.contains(state.parentId)) { validatedParentIds[state.id] = state.parentId; + _pendingParentIds.remove(state.id); + } else { + _pendingParentIds[state.id] = state.parentId!; } } @@ -1220,6 +1272,13 @@ class TabDao extends DatabaseAccessor with $TabDaoMixin { }); } + final retainedTabIds = retainTabIds.toSet(); + _pendingParentIds.removeWhere( + (childId, parentId) => + !retainedTabIds.contains(childId) || + !retainedTabIds.contains(parentId), + ); + var currentOrderKey = await db.containerDao .generateLeadingOrderKey(null) .getSingle(); @@ -1242,6 +1301,8 @@ class TabDao extends DatabaseAccessor with $TabDaoMixin { onConflict: DoNothing(), ); + await _resolvePendingParents(); + return SyncTabsResult( deletedIsolationContextIds: deletedIsolationContextIds, deletedCount: deleted.length, diff --git a/apps/weblibre/test/drift/tabs/tab_hierarchy_order_test.dart b/apps/weblibre/test/drift/tabs/tab_hierarchy_order_test.dart index 71e49933..e347a742 100644 --- a/apps/weblibre/test/drift/tabs/tab_hierarchy_order_test.dart +++ b/apps/weblibre/test/drift/tabs/tab_hierarchy_order_test.dart @@ -150,6 +150,35 @@ void main() { }, ); + test( + 'tab-list sync resolves an unresolved engine parent after inserting the parent row', + () async { + await _insertTabs(db, const [ + _TabFixture('child', source: TabSource.addedEvent), + ]); + + await db.tabDao.updateTabs(null, { + 'child': _tabState('child', parentId: 'late-parent'), + }); + + final unresolvedChild = await db.tabDao + .getTabDataById('child') + .getSingleOrNull(); + expect(unresolvedChild, isNotNull); + expect(unresolvedChild!.parentId, isNull); + expect(unresolvedChild.source, TabSource.addedEvent); + + await db.tabDao.syncTabs(retainTabIds: const ['late-parent', 'child']); + + final resolvedChild = await db.tabDao + .getTabDataById('child') + .getSingleOrNull(); + expect(resolvedChild, isNotNull); + expect(resolvedChild!.parentId, 'late-parent'); + expect(resolvedChild.source, TabSource.manual); + }, + ); + test( 'content-state sync ignores parent-only changes with unresolved parents', () async {