improve tab sync

2026-05-27 06:53:53 +02:00
parent 6f59d048bf
commit c3d3f726b7
2 changed files with 274 additions and 34 deletions
@@ -489,11 +489,13 @@ class TabDao extends DatabaseAccessor<TabDatabase> with $TabDaoMixin {
      // hierarchical views.
      Value<String?> parentValue = const Value.absent();
      Value<String?> containerValue = const Value.absent();
+      Value<TabSource> 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<TabDatabase> 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<TabDatabase> 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<TabDatabase> 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<TabDatabase> 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<TabDatabase> 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<TabDatabase> 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<TabDatabase> with $TabDaoMixin {
    Map<String, TabState> 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 <String>{}
+          : {
+              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 = <String>{};
      final validatedParentIds = <String, String?>{};

      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<TabDatabase> 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(),
              ),
@@ -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<void> _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<String> _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,
+  );
 }