refactor and cleanup

apps/weblibre/lib/features/app_links/domain/services/app_links_coordinator.dart

@@ -17,6 +17,7 @@
  * 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' show immutable;
 import 'package:flutter_mozilla_components/flutter_mozilla_components.dart';
 import 'package:riverpod_annotation/riverpod_annotation.dart';
 import 'package:weblibre/core/logger.dart';
@@ -42,6 +43,28 @@ class _AppLinkEventsReceiver extends GeckoAppLinkEvents {
   }
 }
 
+/// A pending prompt paired with the absolute instant it lapses.
+///
+/// [AppLinkPromptRequest.expiresInMs] is a *snapshot* taken when native answered
+/// the query — it does not tick down. Comparing that raw field to zero on a later
+/// build would treat a long-lapsed request as live (switch tabs for three minutes
+/// and come back, and a 90 s banner still reports 90 s), so the remaining time is
+/// anchored to a wall-clock deadline the moment the answer arrives.
+@immutable
+class PendingAppLinkPrompt {
+  final AppLinkPromptRequest request;
+  final DateTime expiresAt;
+
+  const PendingAppLinkPrompt({required this.request, required this.expiresAt});
+
+  int get requestId => request.requestId;
+  String get tabId => request.tabId;
+  bool get isModal => request.isModal;
+
+  /// Whether native would still accept a resolution for this request.
+  bool isLive(DateTime now) => expiresAt.isAfter(now);
+}
+
 /// Orchestrates Flutter-owned app-link prompts (§2.6): registers the availability
 /// event handler, queries the native pending store on attach/resume/event, and
 /// exposes resolution (including the remember-then-resolve flow). The presented
@@ -52,7 +75,7 @@ class AppLinksCoordinator extends _$AppLinksCoordinator {
   final _service = GeckoAppLinksService();
 
   @override
-  List<AppLinkPromptRequest> build() {
+  List<PendingAppLinkPrompt> build() {
     final receiver = _AppLinkEventsReceiver((owner) {
       if (owner == AppLinkPromptOwner.flutterBrowser) {
         // ignore: discarded_futures
@@ -76,11 +99,22 @@ class AppLinksCoordinator extends _$AppLinksCoordinator {
       final prompts = await _service.getPendingAppLinkPrompts(
         AppLinkPromptOwner.flutterBrowser,
       );
+      // Anchor the reported TTL immediately: every millisecond spent between the
+      // native read and here has already been consumed.
+      final queriedAt = DateTime.now();
       logger.i(
         'app-link refresh -> ${prompts.length} prompt(s): '
         '${prompts.map((p) => '${p.requestId}@${p.tabId}(${p.isModal ? 'modal' : 'banner'})').toList()}',
       );
-      state = prompts;
+      state = [
+        for (final prompt in prompts)
+          PendingAppLinkPrompt(
+            request: prompt,
+            expiresAt: queriedAt.add(
+              Duration(milliseconds: prompt.expiresInMs),
+            ),
+          ),
+      ];
     } catch (error, stackTrace) {
       logger.w(
         'Failed to query pending app-link prompts',

apps/weblibre/lib/features/app_links/domain/services/app_links_coordinator.g.dart

@@ -23,7 +23,7 @@ final appLinksCoordinatorProvider = AppLinksCoordinatorProvider._();
 /// list is authoritative from the query and deduped by `requestId` — the event
 /// is only a nudge to re-query.
 final class AppLinksCoordinatorProvider
-    extends $NotifierProvider<AppLinksCoordinator, List<AppLinkPromptRequest>> {
+    extends $NotifierProvider<AppLinksCoordinator, List<PendingAppLinkPrompt>> {
   /// Orchestrates Flutter-owned app-link prompts (§2.6): registers the availability
   /// event handler, queries the native pending store on attach/resume/event, and
   /// exposes resolution (including the remember-then-resolve flow). The presented
@@ -48,16 +48,16 @@ final class AppLinksCoordinatorProvider
   AppLinksCoordinator create() => AppLinksCoordinator();
 
   /// {@macro riverpod.override_with_value}
-  Override overrideWithValue(List<AppLinkPromptRequest> value) {
+  Override overrideWithValue(List<PendingAppLinkPrompt> value) {
     return $ProviderOverride(
       origin: this,
-      providerOverride: $SyncValueProvider<List<AppLinkPromptRequest>>(value),
+      providerOverride: $SyncValueProvider<List<PendingAppLinkPrompt>>(value),
     );
   }
 }
 
 String _$appLinksCoordinatorHash() =>
-    r'183fc7ac1264a63c24b1d10f4a22cbfbf6046da7';
+    r'3dc91825b659add92d2f651306c30b9aec4e557b';
 
 /// Orchestrates Flutter-owned app-link prompts (§2.6): registers the availability
 /// event handler, queries the native pending store on attach/resume/event, and
@@ -66,22 +66,22 @@ String _$appLinksCoordinatorHash() =>
 /// is only a nudge to re-query.
 
 abstract class _$AppLinksCoordinator
-    extends $Notifier<List<AppLinkPromptRequest>> {
+    extends $Notifier<List<PendingAppLinkPrompt>> {
-  List<AppLinkPromptRequest> build();
+  List<PendingAppLinkPrompt> build();
   @$mustCallSuper
   @override
   WhenComplete runBuild() {
     final ref =
         this.ref
-            as $Ref<List<AppLinkPromptRequest>, List<AppLinkPromptRequest>>;
+            as $Ref<List<PendingAppLinkPrompt>, List<PendingAppLinkPrompt>>;
     final element =
         ref.element
             as $ClassProviderElement<
               AnyNotifier<
-                List<AppLinkPromptRequest>,
+                List<PendingAppLinkPrompt>,
-                List<AppLinkPromptRequest>
+                List<PendingAppLinkPrompt>
               >,
-              List<AppLinkPromptRequest>,
+              List<PendingAppLinkPrompt>,
               Object?,
               Object?
             >;

apps/weblibre/lib/features/app_links/presentation/widgets/app_link_prompt_host.dart

@@ -57,50 +57,76 @@ class AppLinkPromptHost extends HookConsumerWidget {
     // Native expiry is lazy — it only runs when the store is queried or consumed — and nothing
     // pushes an expiry event. A prompt that outlives its deadline would keep rendering live
     // buttons whose resolution is already a no-op, so drop anything already past due rather than
-    // offering an action that cannot happen.
+    // offering an action that cannot happen. The deadline is absolute
-    final activeRequests = prompts
+    // ([PendingAppLinkPrompt.expiresAt]); the raw `expiresInMs` is only valid at query time.
-        .where(
+    final now = DateTime.now();
-          (request) => request.tabId == activeTabId && request.expiresInMs > 0,
+    final livePrompts = prompts.where((prompt) => prompt.isLive(now)).toList();
-        )
+    final activeRequests = livePrompts
+        .where((prompt) => prompt.tabId == activeTabId)
         .toList();
 
     // ...and re-query when the soonest deadline passes, so a prompt retires itself on time
-    // instead of waiting for the next event or resume. The lower clamp matters: a request that
+    // instead of waiting for the next event or resume. Keyed on the absolute deadline so a
-    // reports 0 would otherwise reschedule instantly and spin.
+    // rebuild (tab switch, unrelated state change) never re-arms a full-length timer from a
-    final soonestExpiry = activeRequests.isEmpty
+    // stale TTL. The lower clamp matters: a deadline already reached would otherwise reschedule
+    // instantly and spin.
+    final soonestDeadline = livePrompts.isEmpty
         ? null
-        : activeRequests
+        : livePrompts
-              .map((request) => request.expiresInMs)
+              .map((prompt) => prompt.expiresAt)
-              .reduce((a, b) => a < b ? a : b);
+              .reduce((a, b) => a.isBefore(b) ? a : b);
     useEffect(() {
-      if (soonestExpiry == null) return null;
+      if (soonestDeadline == null) return null;
+      final delay = soonestDeadline.difference(DateTime.now());
       final timer = Timer(
-        Duration(milliseconds: soonestExpiry.clamp(250, 10 * 60 * 1000)),
+        Duration(milliseconds: delay.inMilliseconds.clamp(250, 10 * 60 * 1000)),
-        () => unawaited(ref.read(appLinksCoordinatorProvider.notifier).refresh()),
+        () =>
+            unawaited(ref.read(appLinksCoordinatorProvider.notifier).refresh()),
       );
       return timer.cancel;
-    }, [soonestExpiry]);
+    }, [soonestDeadline]);
 
     final modalRequest = activeRequests
-        .where((request) => request.isModal)
+        .where((prompt) => prompt.isModal)
         .lastOrNull;
     // At most one banner per tab; a newer banner-class request simply becomes the
     // one the UI renders.
     final bannerRequest = activeRequests
-        .where((request) => !request.isModal)
+        .where((prompt) => !prompt.isModal)
         .lastOrNull;
 
     // A modal is shown at most once per requestId. Rotation/teardown is not a
     // dismissal — the request stays pending and is re-presented on the next query
     // (a subsequent build re-runs this effect with the still-present id).
     final shownModalId = useRef<int?>(null);
+    // The route the modal lives on, so it can be retired without popping whatever
+    // else happens to sit on top of it.
+    final shownModalRoute = useRef<ModalRoute<void>?>(null);
+    // Ids we closed ourselves. Their pop must not be mistaken for a user
+    // dismissal, which would consume a request that is merely off-screen.
+    final retiredModalIds = useRef<Set<int>>(<int>{});
+
+    // Take down a dialog nobody can act on any more: its request expired, was
+    // invalidated (tab closed), or belongs to a tab the user has left. Without
+    // this the route just stays on screen with buttons that resolve to `stale`.
+    // Mirrors the native `NativeAppLinkPromptFeature.dismissStaleDialog`.
+    void retireShownModal() {
+      final shownId = shownModalId.value;
+      final route = shownModalRoute.value;
+      shownModalId.value = null;
+      shownModalRoute.value = null;
+      if (shownId == null || route == null || !route.isActive) return;
+      retiredModalIds.value.add(shownId);
+      route.navigator?.removeRoute(route);
+    }
+
     useEffect(() {
       final request = modalRequest;
-      if (request == null) {
+      final shownId = shownModalId.value;
-        shownModalId.value = null;
+      if (shownId != null && shownId != request?.requestId) {
-        return null;
+        retireShownModal();
       }
-      if (shownModalId.value == request.requestId) {
+      if (request == null || shownModalId.value == request.requestId) {
         return null;
       }
       shownModalId.value = request.requestId;
@@ -109,8 +135,17 @@ class AppLinkPromptHost extends HookConsumerWidget {
         unawaited(
           showDialog<void>(
             context: context,
-            builder: (_) => AppLinkPromptDialog(request: request),
+            builder: (dialogContext) {
+              shownModalRoute.value = ModalRoute.of<void>(dialogContext);
+              return AppLinkPromptDialog(request: request.request);
+            },
           ).then((_) {
+            if (retiredModalIds.value.remove(request.requestId)) {
+              // We closed it, not the user. The request is either already gone or
+              // still pending for a tab that is no longer in front — either way it
+              // must not be consumed here.
+              return;
+            }
             // Catch-all for a passive dismissal (Android back / touch-outside):
             // the dialog buttons resolve the request themselves, but a barrier
             // dismiss closes it without resolving, leaving the native request
@@ -134,7 +169,7 @@ class AppLinkPromptHost extends HookConsumerWidget {
 
     return AppLinkOpenBanner(
       key: ValueKey(bannerRequest.requestId),
-      request: bannerRequest,
+      request: bannerRequest.request,
     );
   }
 }

packages/flutter_mozilla_components/android/src/main/kotlin/eu/weblibre/flutter_mozilla_components/applinks/NativeAppLinkPromptFeature.kt

@@ -56,9 +56,25 @@ class NativeAppLinkPromptFeature(
     private val sessionUseCases: SessionUseCases,
 ) : LifecycleAwareFeature {
     private var dialog: AlertDialog? = null
-    private var shownRequestId: Long? = null
+    private var shownRequest: PendingAppLinkRequest? = null
     private val mainHandler = Handler(Looper.getMainLooper())
 
+    /**
+     * The lapse tick for the dialog currently on screen. Held as a single instance so it can be
+     * cancelled: the delay is up to [PendingAppLinkStore.REQUEST_EXPIRY_MS] (10 minutes) and the
+     * runnable retains this feature — and through it the Activity-derived [context] — for its whole
+     * duration, so it must never outlive [stop].
+     */
+    private val expiryTick = Runnable {
+        dismissStaleDialog()
+        // The store sweeps on a strict `>`, so a tick can land a millisecond before the request is
+        // actually droppable and dismiss nothing. Re-arm in that case rather than leave the dialog
+        // with no deadline at all; [MIN_EXPIRY_TICK_MS] keeps that from spinning.
+        shownRequest?.let(::scheduleExpiryTick)
+        // A dialog retired by its own deadline still has to make way for whatever else pends.
+        showNext()
+    }
+
     override fun start() {
         NativeAppLinkPromptNotifier.register(tabId, this)
         showNext()
@@ -66,12 +82,13 @@ class NativeAppLinkPromptFeature(
 
     override fun stop() {
         NativeAppLinkPromptNotifier.unregister(tabId, this)
+        mainHandler.removeCallbacksAndMessages(null)
         // Dismissing on stop is not a user dismissal: the request stays pending and
         // is re-presented on the next start().
         dialog?.setOnDismissListener(null)
         dialog?.dismiss()
         dialog = null
-        shownRequestId = null
+        shownRequest = null
     }
 
     /**
@@ -91,13 +108,26 @@ class NativeAppLinkPromptFeature(
      * dud whose Open button consumes nothing. Not a user dismissal: nothing is suppressed.
      */
     private fun dismissStaleDialog() {
-        val shown = shownRequestId ?: return
+        val shown = shownRequest ?: return
-        if (store.peek(shown) != null) return
+        if (store.peek(shown.requestId) != null) return
+        mainHandler.removeCallbacks(expiryTick)
         dialog?.setOnCancelListener(null)
         dialog?.setOnDismissListener(null)
         dialog?.dismiss()
         dialog = null
-        shownRequestId = null
+        shownRequest = null
+    }
+
+    /**
+     * Expiry in the store is lazy, so nothing would take a dialog down when its request lapses —
+     * its buttons would consume nothing. Retire it on its own deadline instead.
+     */
+    private fun scheduleExpiryTick(request: PendingAppLinkRequest) {
+        mainHandler.removeCallbacks(expiryTick)
+        mainHandler.postDelayed(
+            expiryTick,
+            store.expiresInMs(request).coerceAtLeast(MIN_EXPIRY_TICK_MS),
+        )
     }
 
     private fun showNext() {
@@ -126,14 +156,8 @@ class NativeAppLinkPromptFeature(
             }
             .setOnDismissListener { dialog = null }
             .show()
-        shownRequestId = request.requestId
+        shownRequest = request
-
+        scheduleExpiryTick(request)
-        // Expiry in the store is lazy, so nothing would take this dialog down when the request
-        // lapses — its buttons would consume nothing. Retire it on its own deadline.
-        mainHandler.postDelayed(
-            { dismissStaleDialog() },
-            store.expiresInMs(request).coerceAtLeast(MIN_EXPIRY_TICK_MS),
-        )
     }
 
     private fun resolveOpen(request: PendingAppLinkRequest) {
@@ -168,8 +192,9 @@ class NativeAppLinkPromptFeature(
     }
 
     private fun afterResolve() {
+        mainHandler.removeCallbacks(expiryTick)
         dialog = null
-        shownRequestId = null
+        shownRequest = null
         showNext()
     }
 

packages/flutter_mozilla_components/android/src/main/kotlin/eu/weblibre/flutter_mozilla_components/applinks/WebLibreAppLinksInterceptor.kt

@@ -65,17 +65,17 @@ class WebLibreAppLinksInterceptor(
         // round trip. Gated on the policy so turning the carve-out off restores the plain §2.4
         // eligibility rules rather than only skipping the launch below.
         val authExceptionsAllowed = policy.authExceptionsEnabled && isPossibleAuthentication(session)
+        val isSameDomainNavigation = isSameDomain(lastUri, uri)
 
         // Step 2 — navigation eligibility. Any hit lets the engine proceed normally.
         if (!isEligible(
-                uri,
-                lastUri,
                 uriScheme,
                 engineSupportsScheme,
                 hasUserGesture,
                 isRedirect,
                 isDirectNavigation,
                 isSubframeRequest,
+                isSameDomainNavigation,
                 authExceptionsAllowed,
             )
         ) {
@@ -109,15 +109,35 @@ class WebLibreAppLinksInterceptor(
         // to the app. AC declines here too — its package comes from the bound component, which is
         // only set for an unambiguous handler.
         val authTargetPackage = if (resolved.isAmbiguous) null else resolved.packageName
+        val isAuthCallback = isAuthenticationCallback(session, authTargetPackage)
+
+        // Re-apply the same-domain guard now that the target is known (AC parity: `AppLinksInterceptor`
+        // re-checks after resolution for exactly this reason). Eligibility waived it on the mere
+        // possibility of a sign-in round trip — the tab was opened by *some* app — which would
+        // otherwise re-classify every ordinary in-site navigation for the whole life of a Custom Tab
+        // and, under the default `ask` mode, prompt on each one. Only a navigation that really does
+        // target the calling app keeps the waiver.
+        if (engineSupportsScheme && isSameDomainNavigation && authExceptionsAllowed && !isAuthCallback) {
+            return null
+        }
+
+        val matchingRule = effectiveRules[resolved.scopeKey]
+        val fingerprint = targetFingerprint(uri, resolved)
+        val suppressionHit = session != null && pendingStore.isSuppressed(session.id, fingerprint)
 
         // §2.4 authentication carve-out (AC parity): a tab opened *by* the app the navigation
         // targets is a sign-in round trip rather than a general app link, so it returns to its
         // caller even under `never`. The forced-prompt contexts still win — a protected container,
         // a private tab or a wallet scheme must not leak out silently, so those fall through to the
-        // classifier, which prompts for them regardless of mode (§2.4 step 4).
+        // classifier, which prompts for them regardless of mode (§2.4 step 4). An explicit
+        // `neverOpen` rule for this scope and a live suppression are the user having answered this
+        // exact question already (classifier steps 5–6); the carve-out is about a mode the user set
+        // for links in general, not a licence to override a specific "no".
         if (authExceptionsAllowed &&
-            isAuthenticationCallback(session, authTargetPackage) &&
+            isAuthCallback &&
-            !isProtectedNavigation && !isPrivateNavigation && !isWalletNavigation
+            !isProtectedNavigation && !isPrivateNavigation && !isWalletNavigation &&
+            matchingRule?.decision != AppLinkRuleDecision.NEVER_OPEN &&
+            !suppressionHit
         ) {
             val result = runtime.launcher.launch(
                 uri,
@@ -141,9 +161,8 @@ class WebLibreAppLinksInterceptor(
             isPrivate = isPrivateNavigation,
             isWallet = isWalletNavigation,
             missingSession = session == null,
-            suppressionHit = session != null &&
+            suppressionHit = suppressionHit,
-                pendingStore.isSuppressed(session.id, targetFingerprint(uri, resolved)),
+            matchingRule = matchingRule,
-            matchingRule = effectiveRules[resolved.scopeKey],
             globalMode = effectiveMode,
             marketplaceFallbackEnabled = policy.marketplaceFallbackEnabled,
         )
@@ -346,14 +365,13 @@ class WebLibreAppLinksInterceptor(
     // ---- Eligibility (§2.4 step 2) ----
 
     private fun isEligible(
-        uri: String,
-        lastUri: String?,
         uriScheme: String?,
         engineSupportsScheme: Boolean,
         hasUserGesture: Boolean,
         isRedirect: Boolean,
         isDirectNavigation: Boolean,
         isSubframeRequest: Boolean,
+        isSameDomainNavigation: Boolean,
         authExceptionsAllowed: Boolean,
     ): Boolean {
         if (uriScheme == null) return false
@@ -366,8 +384,10 @@ class WebLibreAppLinksInterceptor(
         if (engineSupportsScheme && !isIntentionalNavigation) return false
         // Same-domain engine-supported navigation continues in the browser (AC subdomain stripping),
         // unless this tab could be hosting an authentication round trip whose callback is an http
-        // app link on the same site.
+        // app link on the same site. That "could be" is provisional — it only knows the tab was
-        if (engineSupportsScheme && isSameDomain(lastUri, uri) && !authExceptionsAllowed) return false
+        // opened by *some* app, not that this navigation targets it — so the guard is re-applied in
+        // [onLoadRequest] once resolution reveals the actual target package.
+        if (engineSupportsScheme && isSameDomainNavigation && !authExceptionsAllowed) return false
         // Always-denied schemes never resolve or launch externally.
         if (AppLinkSchemes.isAlwaysDenied(uriScheme)) return false
         return true