fix cleanup issues

apps/weblibre/lib/features/geckoview/features/browser/domain/services/browser_data.dart

@@ -17,8 +17,10 @@
  * You should have received a copy of the GNU Affero General Public License
  * along with this program. If not, see <http://www.gnu.org/licenses/>.
  */
+import 'package:flutter/foundation.dart';
 import 'package:flutter_mozilla_components/flutter_mozilla_components.dart';
 import 'package:riverpod_annotation/riverpod_annotation.dart';
+import 'package:weblibre/core/logger.dart';
 import 'package:weblibre/features/bangs/domain/repositories/data.dart';
 import 'package:weblibre/features/geckoview/features/tabs/data/providers.dart';
 import 'package:weblibre/features/user/data/models/general_settings.dart';
@@ -27,9 +29,17 @@ part 'browser_data.g.dart';
 
 @Riverpod(keepAlive: true)
 class BrowserDataService extends _$BrowserDataService {
-  final _service = GeckoDeleteBrowserDataService();
+  /// The [service] seam is test-only injection; production always constructs
+  /// the default. Kept `final` so the live singleton's backing service can't be
+  /// swapped at runtime.
+  @visibleForTesting
+  BrowserDataService({GeckoDeleteBrowserDataService? service})
+    : _service = service ?? GeckoDeleteBrowserDataService();
+
+  final GeckoDeleteBrowserDataService _service;
 
   var _onStartDeleted = false;
+  var _onStartContainerDataCleared = false;
 
   Future<void> deleteDataOnEngineStart(
     Set<DeleteBrowsingDataType>? types,
@@ -73,14 +83,32 @@ class BrowserDataService extends _$BrowserDataService {
     return _service.clearDataForContext(contextId);
   }
 
-  Future<void> clearContainerDataOnEngineStart(List<String> contextIds) async {
-    if (!_onStartDeleted && contextIds.isNotEmpty) {
-      for (final contextId in contextIds) {
+  /// Clears Gecko session-context data for every [contextId], best-effort: a
+  /// single failing context is logged and skipped so it never blocks the rest.
+  ///
+  /// Not gated by the one-shot startup guard — used both by the guarded
+  /// [clearContainerDataOnEngineStart] fallback and directly on explicit Quit.
+  Future<void> clearContainerData(List<String> contextIds) async {
+    for (final contextId in contextIds) {
+      try {
         await clearDataForContext(contextId);
+      } catch (e, st) {
+        logger.e(
+          'Failed to clear data for container context $contextId',
+          error: e,
+          stackTrace: st,
+        );
       }
     }
   }
 
+  Future<void> clearContainerDataOnEngineStart(List<String> contextIds) async {
+    if (!_onStartContainerDataCleared && contextIds.isNotEmpty) {
+      _onStartContainerDataCleared = true;
+      await clearContainerData(contextIds);
+    }
+  }
+
   @override
   void build() {}
 }

apps/weblibre/lib/features/geckoview/features/browser/domain/services/browser_data.g.dart

@@ -42,7 +42,7 @@ final class BrowserDataServiceProvider
 }
 
 String _$browserDataServiceHash() =>
-    r'a317f0d56d1bfc61dc99af1e60080aaa248d6eed';
+    r'2df2f652342efc3e16606b92fdef6062b02f72df';
 
 abstract class _$BrowserDataService extends $Notifier<void> {
   void build();

apps/weblibre/lib/features/geckoview/features/tabs/presentation/screens/container_edit.dart

@@ -122,31 +122,33 @@ class ContainerEditScreen extends HookConsumerWidget {
         name: name.isNotEmpty ? name : null,
         color: selectedColor.value,
         isPinned: isPinned.value,
-        metadata: initialContainer.metadata.copyWith(
-          contextualIdentity: contextualIdentity.value,
-          iconData: selectedIcon.value,
-          proxyConnectionId: contextualIdentity.value != null
-              ? proxyConnectionId.value
-              : null,
-          clearDataOnExit:
-              clearDataOnExit.value && contextualIdentity.value != null,
-          excludeFromIndex: excludeFromIndex.value,
-          // Requires a Gecko contextId: the native delegate can't hard-exclude
-          // a container's visits without one. sanitized() enforces the same
-          // invariant defensively on write.
-          excludeFromHistory:
-              excludeFromHistory.value && contextualIdentity.value != null,
-          bypassGlobalProxy:
-              contextualIdentity.value != null &&
-              proxyConnectionId.value == null &&
-              bypassGlobalProxy.value,
-          useCustomColor: useCustomColor.value,
-          assignedSites: assignedSites.value,
-          // Strict mode requires a Gecko contextId (the extension keys
-          // strictness on the tab's cookieStoreId). sanitized() enforces the
-          // same invariant defensively on write.
-          strictMode: strictMode.value && contextualIdentity.value != null,
-        ).sanitized(),
+        metadata: initialContainer.metadata
+            .copyWith(
+              contextualIdentity: contextualIdentity.value,
+              iconData: selectedIcon.value,
+              proxyConnectionId: contextualIdentity.value != null
+                  ? proxyConnectionId.value
+                  : null,
+              clearDataOnExit:
+                  clearDataOnExit.value && contextualIdentity.value != null,
+              excludeFromIndex: excludeFromIndex.value,
+              // Requires a Gecko contextId: the native delegate can't hard-exclude
+              // a container's visits without one. sanitized() enforces the same
+              // invariant defensively on write.
+              excludeFromHistory:
+                  excludeFromHistory.value && contextualIdentity.value != null,
+              bypassGlobalProxy:
+                  contextualIdentity.value != null &&
+                  proxyConnectionId.value == null &&
+                  bypassGlobalProxy.value,
+              useCustomColor: useCustomColor.value,
+              assignedSites: assignedSites.value,
+              // Strict mode requires a Gecko contextId (the extension keys
+              // strictness on the tab's cookieStoreId). sanitized() enforces the
+              // same invariant defensively on write.
+              strictMode: strictMode.value && contextualIdentity.value != null,
+            )
+            .sanitized(),
       );
     }
 
@@ -543,7 +545,9 @@ class ContainerEditScreen extends HookConsumerWidget {
                           value: clearDataOnExit.value,
                           title: const Text('Clear Data on Exit'),
                           subtitle: const Text(
-                            'Clear cookies and site data when app closes',
+                            "Clear cookies and site data for this container's "
+                            'regular tabs when the app closes. Isolated tabs '
+                            'keep separate data.',
                           ),
                           secondary: const Icon(MdiIcons.databaseRemove),
                           onChanged: (contextualIdentity.value != null)
@@ -572,7 +576,9 @@ class ContainerEditScreen extends HookConsumerWidget {
                           title: const Text('Exclude from History'),
                           subtitle: Text(
                             contextualIdentity.value != null
-                                ? "Don't record this container's browsing history"
+                                ? "Don't record new visits from this container's "
+                                      'regular tabs. Existing history is kept; '
+                                      'isolated tabs track separately.'
                                 : 'Requires cookie isolation to be enabled',
                           ),
                           secondary: const Icon(MdiIcons.incognito),

apps/weblibre/lib/utils/exit_app.dart

@@ -24,6 +24,8 @@ import 'package:flutter_mozilla_components/flutter_mozilla_components.dart';
 import 'package:riverpod/riverpod.dart';
 import 'package:weblibre/core/database_registry.dart';
 import 'package:weblibre/core/logger.dart';
+import 'package:weblibre/features/geckoview/features/browser/domain/services/browser_data.dart';
+import 'package:weblibre/features/geckoview/features/tabs/domain/repositories/container.dart';
 import 'package:weblibre/features/geckoview/features/tabs/domain/repositories/tab.dart';
 import 'package:weblibre/features/tor/domain/services/tor_proxy.dart';
 
@@ -41,6 +43,44 @@ Future<void> exitApp(ProviderContainer container) async {
     logger.e('Failed to close tabs', error: e, stackTrace: st);
   }
 
+  // 1b. Explicit-Quit cleanup for containers with "Clear Data on Exit" enabled.
+  //     Done here — while the databases and Gecko engine are still alive — so the
+  //     clear gets a chance to run on a deliberate Quit rather than only on the
+  //     next launch.
+  //
+  //     Caveat: GeckoView's session-context clear is fire-and-forget (see
+  //     GeckoDeleteBrowsingDataControllerImpl.clearDataForSessionContext) — it
+  //     has no completion signal, so awaiting it does NOT mean the clear
+  //     finished, only that it was dispatched. Since step 3 tears down the
+  //     GeckoRuntime, we give the dispatched clear a short best-effort window to
+  //     reach Gecko first. The startup fallback in browser_view.dart is retained
+  //     as the actual guarantee — it covers force-stop/process death and this
+  //     window elapsing before the clear lands.
+  try {
+    final containersToClear = await container
+        .read(containerRepositoryProvider.notifier)
+        .getContainersToClearOnExit();
+
+    if (containersToClear.isNotEmpty) {
+      await container
+          .read(browserDataServiceProvider.notifier)
+          .clearContainerData(containersToClear);
+      // Best-effort settle: yield before the engine shutdown in step 3 so the
+      // fire-and-forget native clear has a chance to be processed by Gecko.
+      await Future<void>.delayed(const Duration(milliseconds: 500));
+      logger.i(
+        'Dispatched data clear for ${containersToClear.length} '
+        'on-exit container(s)',
+      );
+    }
+  } catch (e, st) {
+    logger.e(
+      'Failed to clear container data on exit',
+      error: e,
+      stackTrace: st,
+    );
+  }
+
   // 2. Stop Tor proxy (only if it was initialized)
   if (container.exists(torProxyServiceProvider)) {
     try {

apps/weblibre/test/features/geckoview/features/browser/domain/services/browser_data_test.dart

@@ -0,0 +1,105 @@
+import 'package:flutter_mozilla_components/flutter_mozilla_components.dart';
+import 'package:flutter_test/flutter_test.dart';
+import 'package:weblibre/features/geckoview/features/browser/domain/services/browser_data.dart';
+
+class _FakeGeckoDeleteBrowserDataService extends GeckoDeleteBrowserDataService {
+  final clearedContexts = <String>[];
+
+  /// Contexts for which [clearDataForContext] should throw (simulating a native
+  /// failure), so we can assert one failure doesn't block the remaining ones.
+  final failingContexts = <String>{};
+
+  @override
+  Future<void> clearDataForContext(String contextId) async {
+    clearedContexts.add(contextId);
+    if (failingContexts.contains(contextId)) {
+      throw Exception('simulated native failure for $contextId');
+    }
+  }
+}
+
+void main() {
+  group('BrowserDataService.clearContainerDataOnEngineStart', () {
+    test(
+      'still clears container data after deleteDataOnEngineStart ran '
+      '(regression: shared one-shot flag skipped container clearing, #524)',
+      () async {
+        final gecko = _FakeGeckoDeleteBrowserDataService();
+        final service = BrowserDataService(service: gecko);
+
+        // Startup always runs the global on-start deletion first, even when
+        // no delete-on-quit types are configured (null).
+        await service.deleteDataOnEngineStart(null);
+        await service.clearContainerDataOnEngineStart(['ctx-1', 'ctx-2']);
+
+        expect(gecko.clearedContexts, ['ctx-1', 'ctx-2']);
+      },
+    );
+
+    test('clears only once per app start', () async {
+      final gecko = _FakeGeckoDeleteBrowserDataService();
+      final service = BrowserDataService(service: gecko);
+
+      await service.clearContainerDataOnEngineStart(['ctx-1']);
+      await service.clearContainerDataOnEngineStart(['ctx-2']);
+
+      expect(gecko.clearedContexts, ['ctx-1']);
+    });
+
+    test(
+      'empty context list neither clears nor consumes the one-shot run',
+      () async {
+        final gecko = _FakeGeckoDeleteBrowserDataService();
+        final service = BrowserDataService(service: gecko);
+
+        await service.clearContainerDataOnEngineStart(const []);
+        expect(gecko.clearedContexts, isEmpty);
+
+        await service.clearContainerDataOnEngineStart(['ctx-1']);
+        expect(gecko.clearedContexts, ['ctx-1']);
+      },
+    );
+
+    test(
+      'a single failing context does not block the remaining ones',
+      () async {
+        final gecko = _FakeGeckoDeleteBrowserDataService()
+          ..failingContexts.add('ctx-2');
+        final service = BrowserDataService(service: gecko);
+
+        await service.clearContainerDataOnEngineStart([
+          'ctx-1',
+          'ctx-2',
+          'ctx-3',
+        ]);
+
+        // ctx-2 threw, but ctx-1 and ctx-3 were still attempted.
+        expect(gecko.clearedContexts, ['ctx-1', 'ctx-2', 'ctx-3']);
+      },
+    );
+  });
+
+  group('BrowserDataService.clearContainerData (explicit Quit)', () {
+    test('is not gated by the one-shot startup guard', () async {
+      final gecko = _FakeGeckoDeleteBrowserDataService();
+      final service = BrowserDataService(service: gecko);
+
+      // Consume the startup one-shot guard first.
+      await service.clearContainerDataOnEngineStart(['startup']);
+      // Explicit Quit cleanup must still run afterwards.
+      await service.clearContainerData(['ctx-1', 'ctx-2']);
+
+      expect(gecko.clearedContexts, ['startup', 'ctx-1', 'ctx-2']);
+    });
+
+    test('logs and skips a failing context, clearing the rest', () async {
+      final gecko = _FakeGeckoDeleteBrowserDataService()
+        ..failingContexts.add('ctx-1');
+      final service = BrowserDataService(service: gecko);
+
+      await service.clearContainerData(['ctx-1', 'ctx-2']);
+
+      expect(gecko.clearedContexts, ['ctx-1', 'ctx-2']);
+    });
+  });
+}

packages/flutter_mozilla_components/android/src/main/kotlin/eu/weblibre/flutter_mozilla_components/api/GeckoDeleteBrowsingDataControllerImpl.kt

@@ -188,6 +188,7 @@ class GeckoDeleteBrowsingDataControllerImpl : GeckoDeleteBrowsingDataController
                         ) || dataTypes.contains(ClearDataType.ONLY_CACHES))
                     ) {
                         callback(Result.failure(Exception("Cookies/Cache must be exclusively!")))
+                        return@withContext
                     }
 
                     // Convert ClearDataType to Engine.BrowsingData flags
@@ -200,8 +201,10 @@ class GeckoDeleteBrowsingDataControllerImpl : GeckoDeleteBrowsingDataController
                         }
                     }.toIntArray()
 
-                    // Find tabs on this host (so we can detach their engine sessions and
-                    // also discover any container/contextId partitions that need clearing).
+                    // Find tabs on this host so we can detach their engine sessions
+                    // before clearing (across every container — the base-domain clear
+                    // below covers all partitions, so any open session on this host in
+                    // any container could otherwise re-accumulate cleared data).
                     val matchingTabs = components.core.store.state.allTabs.filter { tab ->
                         val tabHost = runCatching { tab.content.url.toUri().host }.getOrNull()
                             ?: return@filter false
@@ -215,22 +218,16 @@ class GeckoDeleteBrowsingDataControllerImpl : GeckoDeleteBrowsingDataController
                         components.core.store.dispatch(EngineAction.UnlinkEngineSessionAction(it.id))
                     }
 
-                    // GeckoView's clearDataFromBaseDomain (used by engine.clearData(host=))
-                    // only targets the default origin attributes partition. Tabs that live
-                    // inside a contextual identity ("container") store their cookies/storage
-                    // under a `geckoViewSessionContextId` origin attribute that the
-                    // base-domain clear does not touch. To make clearing actually effective
-                    // for container tabs, also fire clearDataForSessionContext for any
-                    // contextIds we found. This is broader than ideal (it clears the whole
-                    // container, not just this host) but there is no GeckoView API to
-                    // combine host + context.
-                    val contextIds = matchingTabs.mapNotNull { it.contextId }.toSet()
-                    contextIds.forEach { contextId ->
-                        components.core.runtime.storageController
-                            .clearDataForSessionContext(contextId)
-                    }
-
-                    // Clear data for the specific host (default partition).
+                    // Clear data for the specific host only. clearDataFromBaseDomain
+                    // (used by engine.clearData(host=)) deletes the site under an
+                    // empty OriginAttributes pattern, which already matches ALL
+                    // partitions — including every container's
+                    // `geckoViewSessionContextId`. So this clears this host across all
+                    // containers WITHOUT touching other sites in those containers.
+                    //
+                    // Do NOT fall back to clearDataForSessionContext here: that wipes a
+                    // container's entire storage (every unrelated site in it), which is
+                    // the container-wide data-loss reported in #524.
                     components.core.engine.clearData(
                         data = Engine.BrowsingData.select(*browsingDataTypes),
                         host = host,

packages/flutter_mozilla_components/android/src/main/kotlin/eu/weblibre/flutter_mozilla_components/middleware/HistoryVisitCorrelationMiddleware.kt

@@ -31,18 +31,22 @@ class HistoryVisitCorrelationMiddleware :
         next: (BrowserAction) -> Unit,
         action: BrowserAction,
     ) {
-        next(action)
-
+        // Record the correlation BEFORE forwarding the action. A tab's contextId
+        // is stable across a URL change, so it is already readable here — and Gecko
+        // can fire the delegate's onVisited for this navigation as soon as the store
+        // updates. Recording first closes the window where onVisited could resolve
+        // the visit before its correlation exists (and mis-attribute or leak it).
         if (action is ContentAction.UpdateUrlAction) {
             val normalTab = store.state.findNormalTab(action.sessionId)
             if (normalTab != null) {
                 HistoryVisitCorrelationCache.record(action.url, normalTab.contextId)
-                return
-            }
-
-            store.state.findCustomTab(action.sessionId)?.let { customTab ->
-                HistoryVisitCorrelationCache.record(action.url, customTab.contextId)
+            } else {
+                store.state.findCustomTab(action.sessionId)?.let { customTab ->
+                    HistoryVisitCorrelationCache.record(action.url, customTab.contextId)
+                }
             }
         }
+
+        next(action)
     }
 }