improve parentid assignment

This commit is contained in:
Fabian Freund
2026-06-06 07:11:55 +02:00
parent beb9a27a9c
commit 213c1cd94b
4 changed files with 270 additions and 25 deletions
@@ -73,6 +73,10 @@ class TabStates extends _$TabStates {
_ => TabMode.regular,
};
// `current.parentId` still holds the last engine parent we applied, so
// capture whether the engine link changed before overwriting it below.
final engineParentChanged = contentState.parentId != current.parentId;
final newState = current.copyWith(
parentId: contentState.parentId,
contextId: contentState.contextId,
@@ -87,6 +91,21 @@ class TabStates extends _$TabStates {
state = {...state}..[contentState.id] = newState;
// Only reconcile DB hierarchy when the engine parent link actually changes.
// Content-state events also fire on every progress/title tick, and seeding
// opens a transaction, so re-running it on each tick would add needless DB
// I/O to this hot path. The debounced updateTabs pass is the backstop for
// anything not seeded here (e.g. a container assigned after the parent).
if (ref.mounted && engineParentChanged) {
await ref
.read(tabDataRepositoryProvider.notifier)
.seedParentFromEngineState(
childId: contentState.id,
parentId: contentState.parentId,
contextId: contentState.contextId,
);
}
if (newState.isFinishedLoading) {
ref
.read(geckoInferenceRepositoryProvider.notifier)
@@ -172,29 +172,36 @@ class TabDao extends DatabaseAccessor<TabDatabase> with $TabDaoMixin {
if (_pendingParentIds.isEmpty) return;
final pendingChildren = selectOnly(db.tab)
..addColumns([db.tab.id])
..addColumns([db.tab.id, db.tab.containerId])
..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();
final pendingChildContainerIds = {
for (final row in await pendingChildren.get())
row.read(db.tab.id)!: row.read(db.tab.containerId),
};
final pendingChildIds = pendingChildContainerIds.keys.toSet();
if (pendingChildIds.isEmpty) {
_pendingParentIds.clear();
return;
}
final resolvableParents = await getExistingTabIds(
pendingChildIds.map((childId) => _pendingParentIds[childId]!).toSet(),
).get().then((ids) => ids.toSet());
final parentIds = pendingChildIds
.map((childId) => _pendingParentIds[childId]!)
.toSet();
final resolvableParentContainerIds = await getTabsContainerId(
parentIds,
).get().then(Map.fromEntries);
await batch((batch) {
for (final childId in pendingChildIds) {
final parentId = _pendingParentIds[childId];
if (parentId == null || !resolvableParents.contains(parentId)) {
if (parentId == null ||
resolvableParentContainerIds[parentId] !=
pendingChildContainerIds[childId]) {
continue;
}
@@ -212,10 +219,65 @@ class TabDao extends DatabaseAccessor<TabDatabase> with $TabDaoMixin {
_pendingParentIds.removeWhere(
(childId, parentId) =>
!pendingChildIds.contains(childId) ||
resolvableParents.contains(parentId),
resolvableParentContainerIds.containsKey(parentId),
);
}
Future<bool> seedParentFromEngineState({
required String childId,
required String? parentId,
required String? contextId,
}) {
return db.transaction(() async {
if (parentId == null || parentId == childId) {
// A null or self-referential engine parent can never seed a hierarchy
// link (the latter would create a cycle), so drop any pending retry.
_pendingParentIds.remove(childId);
return false;
}
final child = await getTabDataById(childId).getSingleOrNull();
if (child == null) {
_pendingParentIds[childId] = parentId;
return false;
}
if (child.parentId != null || child.source == TabSource.manual) {
_pendingParentIds.remove(childId);
return false;
}
final parent = await getTabDataById(parentId).getSingleOrNull();
if (parent == null) {
_pendingParentIds[childId] = parentId;
return false;
}
final repairedContainerId = child.containerId == null && contextId != null
? (await _containerIdsByContextualIdentity({contextId}))[contextId]
: null;
final effectiveChildContainerId =
repairedContainerId ?? child.containerId;
if (effectiveChildContainerId != parent.containerId) {
// The parent row exists but lives in a different container. Pending is
// only for parents that don't exist yet; a retry can't fix a container
// mismatch (mirrors updateTabs), so drop it rather than spin on it.
_pendingParentIds.remove(childId);
return false;
}
await _updateByIdStatement(childId).write(
TabCompanion(
parentId: Value(parentId),
source: const Value(TabSource.manual),
containerId:
repairedContainerId.mapNotNull(Value.new) ?? const Value.absent(),
),
);
_pendingParentIds.remove(childId);
return true;
});
}
Future<String> _generateOrderKey({
required Value<String?> parentId,
required Value<String?> containerId,
@@ -1129,15 +1191,15 @@ class TabDao extends DatabaseAccessor<TabDatabase> with $TabDaoMixin {
// inserts can seed the parent later without needing another parentId
// change from Gecko.
final parentSyncEligibleIds = next.isEmpty
? const <String>{}
? const <String, String?>{}
: await (() async {
final query = selectOnly(db.tab)
..addColumns([db.tab.id, db.tab.source])
..addColumns([db.tab.id, db.tab.source, db.tab.containerId])
..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)!,
row.read(db.tab.id)!: row.read(db.tab.containerId),
};
})();
@@ -1149,7 +1211,7 @@ class TabDao extends DatabaseAccessor<TabDatabase> with $TabDaoMixin {
// and aborting the whole batch.
final parentIdsToValidate = <String>{
for (final state in next.values)
if (parentSyncEligibleIds.contains(state.id) &&
if (parentSyncEligibleIds.containsKey(state.id) &&
state.parentId != null)
state.parentId!,
};
@@ -1183,9 +1245,14 @@ class TabDao extends DatabaseAccessor<TabDatabase> with $TabDaoMixin {
if (containerIdsByContext[entry.value] case final containerId?)
entry.key: containerId,
};
final parentContainerIds = parentIdsToValidate.isEmpty
? const <String, String?>{}
: await getTabsContainerId(
parentIdsToValidate,
).get().then(Map.fromEntries);
for (final state in next.values) {
if (!parentSyncEligibleIds.contains(state.id)) {
if (!parentSyncEligibleIds.containsKey(state.id)) {
_pendingParentIds.remove(state.id);
continue;
}
@@ -1195,11 +1262,24 @@ class TabDao extends DatabaseAccessor<TabDatabase> with $TabDaoMixin {
continue;
}
if (existingParentIds.contains(state.parentId)) {
validatedParentIds[state.id] = state.parentId;
final parentId = state.parentId!;
if (parentId == state.id) {
// A self-referential engine parent would create a hierarchy cycle.
_pendingParentIds.remove(state.id);
continue;
}
final effectiveChildContainerId =
repairedContainerIds[state.id] ?? parentSyncEligibleIds[state.id];
final effectiveParentContainerId =
repairedContainerIds[parentId] ?? parentContainerIds[parentId];
if (existingParentIds.contains(parentId) &&
effectiveParentContainerId == effectiveChildContainerId) {
validatedParentIds[state.id] = parentId;
_pendingParentIds.remove(state.id);
} else if (!existingParentIds.contains(parentId)) {
_pendingParentIds[state.id] = parentId;
} else {
_pendingParentIds[state.id] = state.parentId!;
_pendingParentIds.remove(state.id);
}
}
@@ -150,17 +150,26 @@ class TabDataRepository extends _$TabDataRepository {
.setTabParent(tabId: tabId, newParentId: newParentId);
}
Future<bool> promoteChildToParent(String childId) {
Future<bool> seedParentFromEngineState({
required String childId,
required String? parentId,
required String? contextId,
}) {
return ref
.read(tabDatabaseProvider)
.tabDao
.promoteChildToParent(childId);
.seedParentFromEngineState(
childId: childId,
parentId: parentId,
contextId: contextId,
);
}
Future<bool> moveTabAmongSiblings(
String tabId, {
required bool down,
}) {
Future<bool> promoteChildToParent(String childId) {
return ref.read(tabDatabaseProvider).tabDao.promoteChildToParent(childId);
}
Future<bool> moveTabAmongSiblings(String tabId, {required bool down}) {
return ref
.read(tabDatabaseProvider)
.tabDao
@@ -1,5 +1,6 @@
import 'package:drift/drift.dart' show Value;
import 'package:drift/native.dart';
import 'package:flutter/material.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';
@@ -7,6 +8,7 @@ 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';
import 'package:weblibre/features/geckoview/features/tabs/data/models/container_data.dart';
void main() {
late TabDatabase db;
@@ -112,6 +114,107 @@ void main() {
expect(child.source, TabSource.manual);
});
test('engine parent seeding immediately claims an unclaimed row', () async {
await _insertTabs(db, const [
_TabFixture('gecko-parent', source: TabSource.addedEvent),
_TabFixture('child', source: TabSource.addedEvent),
]);
final seeded = await db.tabDao.seedParentFromEngineState(
childId: 'child',
parentId: 'gecko-parent',
contextId: null,
);
final child = await db.tabDao.getTabDataById('child').getSingleOrNull();
expect(seeded, isTrue);
expect(child, isNotNull);
expect(child!.parentId, 'gecko-parent');
expect(child.source, TabSource.manual);
});
test('engine parent seeding rejects a self-referential parent', () async {
await _insertTabs(db, const [
_TabFixture('tab', source: TabSource.addedEvent),
]);
final seeded = await db.tabDao.seedParentFromEngineState(
childId: 'tab',
parentId: 'tab',
contextId: null,
);
final tab = await db.tabDao.getTabDataById('tab').getSingleOrNull();
expect(seeded, isFalse);
expect(tab, isNotNull);
expect(tab!.parentId, isNull);
expect(tab.source, TabSource.addedEvent);
});
test('engine parent seeding rejects cross-container parents', () async {
await _insertContainers(db, const [
_ContainerFixture('parent-container', 'parent-context'),
_ContainerFixture('child-container', 'child-context'),
]);
await _insertTabs(db, const [
_TabFixture(
'gecko-parent',
source: TabSource.addedEvent,
containerId: 'parent-container',
),
_TabFixture(
'child',
source: TabSource.addedEvent,
containerId: 'child-container',
),
]);
final seeded = await db.tabDao.seedParentFromEngineState(
childId: 'child',
parentId: 'gecko-parent',
contextId: 'child-context',
);
final child = await db.tabDao.getTabDataById('child').getSingleOrNull();
expect(seeded, isFalse);
expect(child, isNotNull);
expect(child!.parentId, isNull);
expect(child.source, TabSource.addedEvent);
});
test(
'content-state sync validates parent against same-batch container repairs',
() async {
await _insertContainers(db, const [
_ContainerFixture('container', 'context'),
]);
await _insertTabs(db, const [
_TabFixture('gecko-parent', source: TabSource.addedEvent),
_TabFixture('child', source: TabSource.addedEvent),
]);
await db.tabDao.updateTabs(null, {
'gecko-parent': _tabState('gecko-parent', contextId: 'context'),
'child': _tabState(
'child',
parentId: 'gecko-parent',
contextId: 'context',
),
});
final parent = await db.tabDao
.getTabDataById('gecko-parent')
.getSingleOrNull();
final child = await db.tabDao.getTabDataById('child').getSingleOrNull();
expect(parent, isNotNull);
expect(parent!.containerId, 'container');
expect(child, isNotNull);
expect(child!.containerId, 'container');
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 {
@@ -335,11 +438,31 @@ Future<void> _insertTabs(TabDatabase db, List<_TabFixture> tabs) async {
tab.id,
source: tab.source,
parentId: Value(tab.parentId),
containerId: Value(tab.containerId),
orderKey: Value(orderKeys[index]),
);
}
}
Future<void> _insertContainers(
TabDatabase db,
List<_ContainerFixture> containers,
) async {
for (final container in containers) {
await db.containerDao.addContainer(
ContainerData(
id: container.id,
name: container.id,
color: Colors.blue,
orderKey: container.id,
metadata: ContainerMetadata.withDefaults(
contextualIdentity: container.contextId,
),
),
);
}
}
Future<List<String>> _orderedTabIds(TabDatabase db) {
return db.tabDao.getAllTabIds().get();
}
@@ -361,14 +484,28 @@ List<String> _spacedOrderKeys(int count) {
class _TabFixture {
final String id;
final String? parentId;
final String? containerId;
final TabSource source;
const _TabFixture(this.id, {this.parentId, this.source = TabSource.manual});
const _TabFixture(
this.id, {
this.parentId,
this.containerId,
this.source = TabSource.manual,
});
}
TabState _tabState(String id, {String? parentId}) {
class _ContainerFixture {
final String id;
final String contextId;
const _ContainerFixture(this.id, this.contextId);
}
TabState _tabState(String id, {String? parentId, String? contextId}) {
return TabState.$default(id).copyWith(
parentId: parentId,
contextId: contextId,
url: Uri.parse('https://$id.example/'),
title: id,
);