diff --git a/apps/weblibre/lib/features/app_links/domain/services/app_links_coordinator.dart b/apps/weblibre/lib/features/app_links/domain/services/app_links_coordinator.dart index 570a046d..5b237ea2 100644 --- a/apps/weblibre/lib/features/app_links/domain/services/app_links_coordinator.dart +++ b/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 . */ +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 build() { + List 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', diff --git a/apps/weblibre/lib/features/app_links/domain/services/app_links_coordinator.g.dart b/apps/weblibre/lib/features/app_links/domain/services/app_links_coordinator.g.dart index 5cf2e47b..ce32234b 100644 --- a/apps/weblibre/lib/features/app_links/domain/services/app_links_coordinator.g.dart +++ b/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> { + extends $NotifierProvider> { /// 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 value) { + Override overrideWithValue(List value) { return $ProviderOverride( origin: this, - providerOverride: $SyncValueProvider>(value), + providerOverride: $SyncValueProvider>(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 build(); + extends $Notifier> { + List build(); @$mustCallSuper @override WhenComplete runBuild() { final ref = this.ref - as $Ref, List>; + as $Ref, List>; final element = ref.element as $ClassProviderElement< AnyNotifier< - List, - List + List, + List >, - List, + List, Object?, Object? >; diff --git a/apps/weblibre/lib/features/app_links/presentation/widgets/app_link_prompt_host.dart b/apps/weblibre/lib/features/app_links/presentation/widgets/app_link_prompt_host.dart index 08b99ebe..4a0277e0 100644 --- a/apps/weblibre/lib/features/app_links/presentation/widgets/app_link_prompt_host.dart +++ b/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. - final activeRequests = prompts - .where( - (request) => request.tabId == activeTabId && request.expiresInMs > 0, - ) + // offering an action that cannot happen. The deadline is absolute + // ([PendingAppLinkPrompt.expiresAt]); the raw `expiresInMs` is only valid at query time. + final now = DateTime.now(); + 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 - // reports 0 would otherwise reschedule instantly and spin. - final soonestExpiry = activeRequests.isEmpty + // instead of waiting for the next event or resume. Keyed on the absolute deadline so a + // rebuild (tab switch, unrelated state change) never re-arms a full-length timer from a + // stale TTL. The lower clamp matters: a deadline already reached would otherwise reschedule + // instantly and spin. + final soonestDeadline = livePrompts.isEmpty ? null - : activeRequests - .map((request) => request.expiresInMs) - .reduce((a, b) => a < b ? a : b); + : livePrompts + .map((prompt) => prompt.expiresAt) + .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)), - () => unawaited(ref.read(appLinksCoordinatorProvider.notifier).refresh()), + Duration(milliseconds: delay.inMilliseconds.clamp(250, 10 * 60 * 1000)), + () => + 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(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?>(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>({}); + + // 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) { - shownModalId.value = null; - return null; + final shownId = shownModalId.value; + if (shownId != null && shownId != request?.requestId) { + 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( context: context, - builder: (_) => AppLinkPromptDialog(request: request), + builder: (dialogContext) { + shownModalRoute.value = ModalRoute.of(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, ); } } diff --git a/packages/flutter_mozilla_components/android/src/main/kotlin/eu/weblibre/flutter_mozilla_components/applinks/NativeAppLinkPromptFeature.kt b/packages/flutter_mozilla_components/android/src/main/kotlin/eu/weblibre/flutter_mozilla_components/applinks/NativeAppLinkPromptFeature.kt index 30535cfc..40d5973f 100644 --- a/packages/flutter_mozilla_components/android/src/main/kotlin/eu/weblibre/flutter_mozilla_components/applinks/NativeAppLinkPromptFeature.kt +++ b/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 - if (store.peek(shown) != null) return + val shown = shownRequest ?: 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 - - // 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), - ) + shownRequest = request + scheduleExpiryTick(request) } private fun resolveOpen(request: PendingAppLinkRequest) { @@ -168,8 +192,9 @@ class NativeAppLinkPromptFeature( } private fun afterResolve() { + mainHandler.removeCallbacks(expiryTick) dialog = null - shownRequestId = null + shownRequest = null showNext() } diff --git a/packages/flutter_mozilla_components/android/src/main/kotlin/eu/weblibre/flutter_mozilla_components/applinks/WebLibreAppLinksInterceptor.kt b/packages/flutter_mozilla_components/android/src/main/kotlin/eu/weblibre/flutter_mozilla_components/applinks/WebLibreAppLinksInterceptor.kt index 006a0067..7eb7139a 100644 --- a/packages/flutter_mozilla_components/android/src/main/kotlin/eu/weblibre/flutter_mozilla_components/applinks/WebLibreAppLinksInterceptor.kt +++ b/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) && - !isProtectedNavigation && !isPrivateNavigation && !isWalletNavigation + isAuthCallback && + !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 && - pendingStore.isSuppressed(session.id, targetFingerprint(uri, resolved)), - matchingRule = effectiveRules[resolved.scopeKey], + suppressionHit = suppressionHit, + matchingRule = matchingRule, 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. - if (engineSupportsScheme && isSameDomain(lastUri, uri) && !authExceptionsAllowed) return false + // app link on the same site. That "could be" is provisional — it only knows the tab was + // 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