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 da647fa1..72eda939 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 @@ -489,11 +489,13 @@ class TabDao extends DatabaseAccessor with $TabDaoMixin { // hierarchical views. Value parentValue = const Value.absent(); Value containerValue = const Value.absent(); + Value rootSource = const Value.absent(); switch (parentChange) { case TabParentUnchanged(): break; case TabParentDetach(): parentValue = const Value(null); + rootSource = const Value(TabSource.manual); case TabParentToSpecific(:final parentTabId): final parent = anchors[parentTabId] ?? @@ -501,6 +503,7 @@ class TabDao extends DatabaseAccessor with $TabDaoMixin { if (parent != null) { parentValue = Value(parentTabId); containerValue = Value(parent.containerId); + rootSource = const Value(TabSource.manual); } } @@ -510,6 +513,7 @@ class TabDao extends DatabaseAccessor with $TabDaoMixin { batch.update( db.tab, TabCompanion( + source: isRoot ? rootSource : const Value.absent(), orderKey: Value(orderKeys[i]), // parent_id change applies only to the moving root; // descendants keep their existing parent_id pointers. @@ -592,9 +596,12 @@ class TabDao extends DatabaseAccessor with $TabDaoMixin { if (newParentId == null) { // Detach: keep existing order_keys. The row becomes a root in its // current slot and the subtree under it stays intact. - await _updateByIdStatement( - tabId, - ).write(const TabCompanion(parentId: Value(null))); + await _updateByIdStatement(tabId).write( + const TabCompanion( + source: Value(TabSource.manual), + parentId: Value(null), + ), + ); return true; } @@ -662,6 +669,7 @@ class TabDao extends DatabaseAccessor with $TabDaoMixin { batch.update( db.tab, TabCompanion( + source: const Value(TabSource.manual), parentId: Value(newParentId), orderKey: Value(orderKeys[0]), ), @@ -728,6 +736,7 @@ class TabDao extends DatabaseAccessor with $TabDaoMixin { batch.update( db.tab, TabCompanion( + source: const Value(TabSource.manual), parentId: Value(parent.parentId), orderKey: Value(parent.orderKey), ), @@ -736,6 +745,7 @@ class TabDao extends DatabaseAccessor with $TabDaoMixin { batch.update( db.tab, TabCompanion( + source: const Value(TabSource.manual), parentId: Value(childId), orderKey: Value(newParentOrderKey), ), @@ -1062,14 +1072,29 @@ class TabDao extends DatabaseAccessor with $TabDaoMixin { Map next, ) { return db.transaction(() async { + // Gecko exposes a parentId in content-state events, but local DB tab + // hierarchy becomes authoritative once a row has a DB parent or has + // been manually created/reparented. Only use Gecko parent data to seed + // engine-event rows that have not been claimed by local hierarchy yet. + // Keep retrying while the row remains unclaimed so out-of-order tab-list + // inserts can seed the parent later without needing another parentId + // 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, + }; + // Collect parent IDs that need database validation final parentIdsToValidate = {}; final validatedParentIds = {}; for (final state in next.values) { - final previousState = previous?[state.id]; - - if (previousState?.parentId != state.parentId && + if (parentSyncEligibleIds.contains(state.id) && state.parentId != null) { if (next.containsKey(state.parentId)) { // Parent exists in current state @@ -1114,50 +1139,52 @@ class TabDao extends DatabaseAccessor with $TabDaoMixin { // Complete validation map for (final state in next.values) { - final previousState = previous?[state.id]; + if (!parentSyncEligibleIds.contains(state.id) || + validatedParentIds.containsKey(state.id) || + state.parentId == null) { + continue; + } - if (previousState?.parentId != state.parentId) { - if (!validatedParentIds.containsKey(state.id)) { - // This parent ID needed database validation - if (state.parentId != null && - existingParentIds.contains(state.parentId)) { - validatedParentIds[state.id] = state.parentId; - } else { - validatedParentIds[state.id] = null; - } - } + // This parent ID needed database validation + if (existingParentIds.contains(state.parentId)) { + validatedParentIds[state.id] = state.parentId; } } await batch((batch) { for (final state in next.values) { final previousState = previous?[state.id]; + final hasParentUpdate = validatedParentIds.containsKey(state.id); + final hasContainerRepair = repairedContainerIds.containsKey(state.id); + final hasUrlChange = previousState?.url != state.url; + final hasTitleChange = previousState?.title != state.title; + final hasTabModeChange = previousState?.tabMode != state.tabMode; - if (previousState == null || - previousState.url != state.url || - previousState.title != state.title || - previousState.parentId != state.parentId || - previousState.tabMode != state.tabMode || - repairedContainerIds.containsKey(state.id)) { + if (hasUrlChange || + hasTitleChange || + hasTabModeChange || + hasParentUpdate || + hasContainerRepair) { batch.update( db.tab, TabCompanion( - parentId: validatedParentIds.containsKey(state.id) + parentId: hasParentUpdate ? Value(validatedParentIds[state.id]) : const Value.absent(), + source: hasParentUpdate + ? const Value(TabSource.manual) + : const Value.absent(), containerId: repairedContainerIds[state.id].mapNotNull(Value.new) ?? const Value.absent(), - url: (previousState?.url != state.url) - ? Value(state.url) - : const Value.absent(), - title: (previousState?.title != state.title) + url: hasUrlChange ? Value(state.url) : const Value.absent(), + title: hasTitleChange ? Value(state.title) : const Value.absent(), - tabMode: (previousState?.tabMode != state.tabMode) + tabMode: hasTabModeChange ? Value(state.tabMode.toDbValue()) : const Value.absent(), - isolationContextId: (previousState?.tabMode != state.tabMode) + isolationContextId: hasTabModeChange ? Value(state.isolationContextId) : const Value.absent(), ), 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 6646bed1..71e49933 100644 --- a/apps/weblibre/test/drift/tabs/tab_hierarchy_order_test.dart +++ b/apps/weblibre/test/drift/tabs/tab_hierarchy_order_test.dart @@ -1,10 +1,12 @@ -import 'package:drift/drift.dart'; +import 'package:drift/drift.dart' show Value; import 'package:drift/native.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:lexo_rank/lexo_rank.dart'; import 'package:weblibre/data/database/functions/lexo_rank_functions.dart'; import 'package:weblibre/data/database/functions/url_functions.dart'; +import 'package:weblibre/features/geckoview/domain/entities/states/tab.dart'; import 'package:weblibre/features/geckoview/features/tabs/data/database/database.dart'; +import 'package:weblibre/features/geckoview/features/tabs/data/entities/tab_source.dart'; void main() { late TabDatabase db; @@ -93,14 +95,216 @@ void main() { ]); }, ); + + test('content-state sync seeds parent for an unclaimed engine row', () async { + await _insertTabs(db, const [ + _TabFixture('gecko-parent', source: TabSource.addedEvent), + _TabFixture('child', source: TabSource.addedEvent), + ]); + + await db.tabDao.updateTabs(null, { + 'child': _tabState('child', parentId: 'gecko-parent'), + }); + + final child = await db.tabDao.getTabDataById('child').getSingleOrNull(); + expect(child, isNotNull); + expect(child!.parentId, 'gecko-parent'); + expect(child.source, TabSource.manual); + }); + + test( + 'content-state sync retries an unresolved engine parent when the row arrives later', + () async { + await _insertTabs(db, const [ + _TabFixture('child', source: TabSource.addedEvent), + ]); + + final initialState = { + 'child': _tabState('child', parentId: 'late-parent'), + }; + + await db.tabDao.updateTabs(null, initialState); + + final unresolvedChild = await db.tabDao + .getTabDataById('child') + .getSingleOrNull(); + expect(unresolvedChild, isNotNull); + expect(unresolvedChild!.parentId, isNull); + expect(unresolvedChild.source, TabSource.addedEvent); + + await _insertTabs(db, const [ + _TabFixture('late-parent', source: TabSource.addedEvent), + ]); + + await db.tabDao.updateTabs(initialState, { + 'child': _tabState('child', parentId: 'late-parent'), + 'late-parent': _tabState('late-parent'), + }); + + 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 { + await _insertTabs(db, const [ + _TabFixture('child', source: TabSource.addedEvent), + ]); + + final previousState = _tabState('child'); + + await db.tabDao.updateTabs( + {'child': previousState}, + {'child': previousState.copyWith(parentId: 'missing-parent')}, + ); + + final child = await db.tabDao.getTabDataById('child').getSingleOrNull(); + expect(child, isNotNull); + expect(child!.parentId, isNull); + expect(child.source, TabSource.addedEvent); + }, + ); + + test( + 'content-state sync does not overwrite a locally managed parent', + () async { + await _insertTabs(db, const [ + _TabFixture('local-parent'), + _TabFixture('gecko-parent', source: TabSource.addedEvent), + _TabFixture('child', parentId: 'local-parent'), + ]); + + await db.tabDao.updateTabs(null, { + 'child': _tabState('child', parentId: 'gecko-parent'), + }); + + final child = await db.tabDao.getTabDataById('child').getSingleOrNull(); + expect(child, isNotNull); + expect(child!.parentId, 'local-parent'); + expect(child.source, TabSource.manual); + }, + ); + + test( + 'content-state sync ignores parent-only changes for locally managed rows', + () async { + await _insertTabs(db, const [ + _TabFixture('local-parent'), + _TabFixture('gecko-parent', source: TabSource.addedEvent), + _TabFixture('child', parentId: 'local-parent'), + ]); + + final previousState = _tabState('child', parentId: 'local-parent'); + + await db.tabDao.updateTabs( + {'child': previousState}, + {'child': previousState.copyWith(parentId: 'gecko-parent')}, + ); + + final child = await db.tabDao.getTabDataById('child').getSingleOrNull(); + expect(child, isNotNull); + expect(child!.parentId, 'local-parent'); + expect(child.source, TabSource.manual); + }, + ); + + test( + 'content-state sync preserves an existing parent on engine rows', + () async { + await _insertTabs(db, const [ + _TabFixture('existing-parent', source: TabSource.addedEvent), + _TabFixture('gecko-parent', source: TabSource.addedEvent), + _TabFixture( + 'child', + parentId: 'existing-parent', + source: TabSource.addedEvent, + ), + ]); + + await db.tabDao.updateTabs(null, { + 'child': _tabState('child', parentId: 'gecko-parent'), + }); + + final child = await db.tabDao.getTabDataById('child').getSingleOrNull(); + expect(child, isNotNull); + expect(child!.parentId, 'existing-parent'); + expect(child.source, TabSource.addedEvent); + }, + ); + + test('reorder-only moves do not claim manual hierarchy authority', () async { + await _insertTabs(db, const [ + _TabFixture('other'), + _TabFixture('child', source: TabSource.addedEvent), + _TabFixture('gecko-parent', source: TabSource.addedEvent), + ]); + + await db.tabDao.reorderTabs( + movingTabIds: const ['child'], + previousTabId: null, + nextTabId: 'other', + ); + + final reorderedChild = await db.tabDao + .getTabDataById('child') + .getSingleOrNull(); + expect(reorderedChild, isNotNull); + expect(reorderedChild!.source, TabSource.addedEvent); + expect(reorderedChild.parentId, isNull); + + await db.tabDao.updateTabs(null, { + 'child': _tabState('child', parentId: 'gecko-parent'), + }); + + final seededChild = await db.tabDao + .getTabDataById('child') + .getSingleOrNull(); + expect(seededChild, isNotNull); + expect(seededChild!.parentId, 'gecko-parent'); + expect(seededChild.source, TabSource.manual); + }); + + test('tab-list sync preserves an existing local parent', () async { + await _insertTabs(db, const [ + _TabFixture('parent'), + _TabFixture('child', parentId: 'parent'), + ]); + + await db.tabDao.syncTabs(retainTabIds: const ['parent', 'child']); + + final child = await db.tabDao.getTabDataById('child').getSingleOrNull(); + expect(child, isNotNull); + expect(child!.parentId, 'parent'); + }); + + test('deleting a parent rewires children to the grandparent', () async { + await _insertTabs(db, const [ + _TabFixture('grandparent'), + _TabFixture('parent', parentId: 'grandparent'), + _TabFixture('child', parentId: 'parent'), + ]); + + await db.customStatement("DELETE FROM tab WHERE id = 'parent'"); + + final child = await db.tabDao.getTabDataById('child').getSingleOrNull(); + expect(child, isNotNull); + expect(child!.parentId, 'grandparent'); + }); } Future _insertTabs(TabDatabase db, List<_TabFixture> tabs) async { final orderKeys = _spacedOrderKeys(tabs.length); for (final (index, tab) in tabs.indexed) { - await db.tabDao.upsertTabTransactional( - () async => tab.id, + await db.tabDao.insertTab( + tab.id, + source: tab.source, parentId: Value(tab.parentId), orderKey: Value(orderKeys[index]), ); @@ -128,6 +332,15 @@ List _spacedOrderKeys(int count) { class _TabFixture { final String id; final String? parentId; + final TabSource source; - const _TabFixture(this.id, {this.parentId}); + const _TabFixture(this.id, {this.parentId, this.source = TabSource.manual}); +} + +TabState _tabState(String id, {String? parentId}) { + return TabState.$default(id).copyWith( + parentId: parentId, + url: Uri.parse('https://$id.example/'), + title: id, + ); }