Fix future completion when navigator is disposed mid-task

.github/workflows/ci.yml

@@ -10,6 +10,7 @@ jobs:
   analyze:
     name: Analyze code
     runs-on: ubuntu-latest
+    timeout-minutes: 15
     steps:
       - uses: actions/checkout@v3
 
@@ -27,6 +28,7 @@ jobs:
   test:
     name: Run tests
     runs-on: ubuntu-latest
+    timeout-minutes: 15
     steps:
       - uses: actions/checkout@v3
 
@@ -44,15 +46,10 @@ jobs:
   test_web:
     name: Run tests on web
     runs-on: ubuntu-latest
+    timeout-minutes: 15
     steps:
       - uses: actions/checkout@v3
 
-      - name: Setup Chrome
-        id: setup-chrome
-        uses: browser-actions/setup-chrome@v1
-        with:
-          install-dependencies: true
-
       - name: Setup Flutter
         uses: subosito/flutter-action@v2
         with:
@@ -62,14 +59,16 @@ jobs:
         run: flutter pub get
 
       # Guards web support: a `dart:io` import in lib/ fails to compile here.
+      # Chrome ships preinstalled on ubuntu-latest runners, so no setup action is
+      # needed — installing one shells out to apt, which can block on a dpkg lock
+      # and hang the job with no output.
       - name: Run tests
         run: flutter test --platform chrome
-        env:
-          CHROME_EXECUTABLE: ${{ steps.setup-chrome.outputs.chrome-path }}
 
   dry_run:
     name: Publish dry run
     runs-on: ubuntu-latest
+    timeout-minutes: 15
     steps:
       - uses: actions/checkout@v3
 

CHANGELOG.md

@@ -12,6 +12,10 @@ exhaustive `switch` statements over it need a `Cancelled` case added.
   `map()` and `flatMap()` pass cancellation through
 * Note: the task is not interrupted on cancellation — Dart futures cannot be
   cancelled, so it runs to completion and its result is discarded
+* Fixed the returned future never completing when the hosting `Navigator` is
+  disposed while the task is in flight — for example a nested navigator removed
+  from the widget tree. The call now settles with `Cancelled` instead of leaving
+  the caller awaiting forever
 * Added Flutter Web support — `flutter_future_progress_dialog` no longer imports
   `dart:io`, so pub.dev now reports all six platforms
 * **Behaviour change:** `showAdaptiveProgressDialog` now selects its style from

README.md

@@ -322,7 +322,6 @@ Yes, if you use the `BuildContext` afterwards. `showProgressDialog` closes its o
 
 - **The task cannot be aborted.** Cancellation dismisses the dialog, not the work. See [Can the user cancel the progress dialog?](#can-the-user-cancel-the-progress-dialog)
 - **No determinate progress.** The dialog shows an indeterminate spinner; there is no percentage or step reporting.
-- **If the hosting `Navigator` is disposed while the task is in flight** — for example a nested navigator removed from the widget tree — the dialog is torn down without error, but the returned future never completes, so the `await` waits forever.
 
 ## Contributing
 

lib/src/exactly_once.dart

@@ -2,11 +2,23 @@ import 'dart:async';
 
 import 'package:flutter/widgets.dart';
 
+/// Runs [callback] exactly once, after [child]'s first frame, and reports the
+/// teardown of its own subtree through [onDispose].
+///
+/// [onDispose] is the only reliable signal that a route was torn down without
+/// being popped — for example when its [Navigator] is removed from the tree.
+/// The framework abandons `Route.popped` in that case, so nothing else fires.
 class ExactlyOnce extends StatefulWidget {
   final Future<void> Function() callback;
+  final VoidCallback? onDispose;
   final Widget child;
 
-  const ExactlyOnce({super.key, required this.callback, required this.child});
+  const ExactlyOnce({
+    super.key,
+    required this.callback,
+    required this.child,
+    this.onDispose,
+  });
 
   @override
   State<ExactlyOnce> createState() => _ExactlyOnceState();
@@ -21,6 +33,12 @@ class _ExactlyOnceState extends State<ExactlyOnce> {
     super.initState();
   }
 
+  @override
+  void dispose() {
+    widget.onDispose?.call();
+    super.dispose();
+  }
+
   @override
   Widget build(BuildContext context) {
     WidgetsBinding.instance.addPostFrameCallback((_) {

lib/src/show_progress_dialog.dart

@@ -1,3 +1,5 @@
+import 'dart:async';
+
 import 'package:flutter/cupertino.dart' as c;
 import 'package:flutter/material.dart' as m;
 import 'package:flutter/widgets.dart' as w;
@@ -12,14 +14,59 @@ Future<void> _callback<T>(
   w.NavigatorState navigator,
   Task<T> task,
   w.Route<ProgressDialogResult<T>> route,
+  _Settle<T> settle,
 ) async {
   final result = await task.result();
+  // Settle before removing the route: tearing the route down fires the dialog's
+  // onDispose, and the real result must win that race.
+  settle(result);
   if (!route.isActive) {
     return;
   }
   navigator.removeRoute(route, result);
 }
 
+typedef _Settle<T> = void Function(ProgressDialogResult<T> result);
+
+/// Drives the returned future from whichever of three things happens first: the
+/// task delivering a result, the route being popped, or the route's subtree
+/// being torn down.
+///
+/// The last case is why this exists. When a [w.Navigator] is disposed it force-
+/// disposes its routes without completing `Route.popped`, so a future derived
+/// from `Navigator.push` alone would never complete and the caller would await
+/// forever.
+class _Outcome<T> {
+  final _completer = Completer<ProgressDialogResult<T>>();
+
+  Future<ProgressDialogResult<T>> get future => _completer.future;
+
+  void settle(ProgressDialogResult<T> result) {
+    if (!_completer.isCompleted) {
+      _completer.complete(result);
+    }
+  }
+
+  /// Wires up the route-popped and never-installed paths, and returns the
+  /// future the caller should await.
+  Future<ProgressDialogResult<T>> track(
+    w.NavigatorState navigator,
+    w.Route<ProgressDialogResult<T>> route,
+  ) {
+    navigator.push<ProgressDialogResult<T>>(route).then((result) => settle(result ?? Cancelled<T>())).ignore();
+
+    // If the navigator is disposed before the route ever builds, the dialog's
+    // onDispose never fires because its state was never created.
+    w.WidgetsBinding.instance.addPostFrameCallback((_) {
+      if (route.navigator == null) {
+        settle(Cancelled<T>());
+      }
+    });
+
+    return future;
+  }
+}
+
 /// Shows a progress dialog while executing an asynchronous task.
 ///
 /// Displays a modal progress dialog that remains visible until [future] completes.
@@ -86,11 +133,13 @@ Future<ProgressDialogResult<T>> showProgressDialog<T>({
   );
 
   late final m.Route<ProgressDialogResult<T>> route;
+  final outcome = _Outcome<T>();
 
   route = m.DialogRoute<ProgressDialogResult<T>>(
     context: context,
     builder: (context) => ExactlyOnce(
-      callback: () => _callback(navigator, future, route),
+      callback: () => _callback(navigator, future, route, outcome.settle),
+      onDispose: () => outcome.settle(Cancelled<T>()),
       child: builder?.call(context) ?? const ProgressBarDialog(),
     ),
     barrierColor: barrierColor ??
@@ -109,9 +158,7 @@ Future<ProgressDialogResult<T>> showProgressDialog<T>({
     fullscreenDialog: fullscreenDialog,
   );
 
-  final result = await navigator.push<ProgressDialogResult<T>>(route);
-
-  return result ?? Cancelled<T>();
+  return outcome.track(navigator, route);
 }
 
 /// Shows a Cupertino-styled progress dialog while executing an asynchronous task.
@@ -167,10 +214,12 @@ Future<ProgressDialogResult<T>> showCupertinoProgressDialog<T>({
   final navigator = c.Navigator.of(context, rootNavigator: useRootNavigator);
 
   late final c.CupertinoDialogRoute<ProgressDialogResult<T>> route;
+  final outcome = _Outcome<T>();
 
   route = c.CupertinoDialogRoute<ProgressDialogResult<T>>(
     builder: (context) => ExactlyOnce(
-      callback: () => _callback(navigator, future, route),
+      callback: () => _callback(navigator, future, route, outcome.settle),
+      onDispose: () => outcome.settle(Cancelled<T>()),
       child: builder?.call(context) ?? const CupertinoProgressBarDialog(),
     ),
     context: context,
@@ -180,8 +229,7 @@ Future<ProgressDialogResult<T>> showCupertinoProgressDialog<T>({
     anchorPoint: anchorPoint,
     requestFocus: requestFocus,
   );
-  final result = await navigator.push<ProgressDialogResult<T>>(route);
-  return result ?? Cancelled<T>();
+  return outcome.track(navigator, route);
 }
 
 /// Shows a platform-adaptive progress dialog while executing an asynchronous task.

test/progress_dialog_test.dart

@@ -42,17 +42,22 @@ void main() {
     expect((result as Success<String>).value, 'ok');
   });
 
-  // Regression test: when the dialog is on a nested navigator and that
-  // navigator is removed from the tree while the task is in-flight,
-  // the dialog's context becomes unmounted. The original _callback would
-  // check context.mounted, return early, and never call removeRoute —
-  // causing push() to resolve with null and result! to throw.
+  // Regression test: when the dialog is on a nested navigator and that navigator
+  // is removed from the tree while the task is in-flight, the navigator force-
+  // disposes its routes without completing `Route.popped`. A result derived from
+  // `Navigator.push` alone would therefore never arrive and the caller would
+  // await forever. The dialog's own dispose signal settles it as Cancelled.
+  //
+  // Note this asserts on a captured value rather than awaiting the call: a
+  // regression here hangs rather than throws, so awaiting would stall the suite
+  // instead of failing it.
   testWidgets(
-    'showProgressDialog does not crash when context unmounts before task completes',
+    'showProgressDialog returns Cancelled when its navigator is disposed mid-task',
     (WidgetTester tester) async {
       final taskCompleter = Completer<void>();
       final showNestedNav = ValueNotifier(true);
       late BuildContext nestedContext;
+      ProgressDialogResult<String>? outcome;
 
       await tester.pumpWidget(
         MaterialApp(
@@ -82,7 +87,7 @@ void main() {
           return 'ok';
         },
         useRootNavigator: false,
-      ).ignore();
+      ).then((result) => outcome = result);
 
       // Let the dialog build and ExactlyOnce fire _callback.
       // Can't use pumpAndSettle here — the pending task keeps the dialog
@@ -92,17 +97,71 @@ void main() {
       await tester.pump();
 
       // Remove the nested navigator from the tree — this disposes all its
-      // routes and unmounts the dialog's context while the task is running.
+      // routes and unmounts the dialog's subtree while the task is running.
       showNestedNav.value = false;
       await tester.pumpAndSettle();
 
-      // Complete the task after context has been unmounted.
+      expect(
+        outcome,
+        isA<Cancelled<String>>(),
+        reason: 'the caller must be released when the navigator goes away',
+      );
+
+      // The task is not interrupted; completing it later changes nothing.
       taskCompleter.complete();
       await tester.pumpAndSettle();
+      expect(outcome, isA<Cancelled<String>>());
+      expect(tester.takeException(), isNull);
+    },
+  );
+
+  testWidgets(
+    'showCupertinoProgressDialog returns Cancelled when its navigator is disposed mid-task',
+    (WidgetTester tester) async {
+      final taskCompleter = Completer<void>();
+      final showNestedNav = ValueNotifier(true);
+      late BuildContext nestedContext;
+      ProgressDialogResult<String>? outcome;
 
-      // With the old code, _callback returns early (context.mounted == false),
-      // removeRoute is never called, and push() resolves with null causing
-      // result! to throw.
+      await tester.pumpWidget(
+        CupertinoApp(
+          home: ValueListenableBuilder<bool>(
+            valueListenable: showNestedNav,
+            builder: (_, show, __) {
+              if (!show) return const SizedBox();
+              return Navigator(
+                onGenerateRoute: (_) => CupertinoPageRoute(
+                  builder: (ctx) {
+                    nestedContext = ctx;
+                    return const SizedBox();
+                  },
+                ),
+              );
+            },
+          ),
+        ),
+      );
+      await tester.pumpAndSettle();
+
+      showCupertinoProgressDialog<String>(
+        context: nestedContext,
+        future: () async {
+          await taskCompleter.future;
+          return 'ok';
+        },
+        useRootNavigator: false,
+      ).then((result) => outcome = result);
+
+      await tester.pump();
+      await tester.pump();
+
+      showNestedNav.value = false;
+      await tester.pumpAndSettle();
+
+      expect(outcome, isA<Cancelled<String>>());
+
+      taskCompleter.complete();
+      await tester.pumpAndSettle();
       expect(tester.takeException(), isNull);
     },
   );