Improve backbuttondispatcher (#68735)

* Improve backbuttondispatcher

* update

* update

* fix analyzer

* addressing comments

* make takeprioirty recursive by default

* fix comment

* addressing comment

packages/flutter/lib/src/widgets/router.dart

@@ -800,10 +800,11 @@ class _CallbackHookProvider<T> {
 /// the callback should return a future that completes to true if it can handle
 /// the pop request, and a future that completes to false otherwise.
 abstract class BackButtonDispatcher extends _CallbackHookProvider<Future<bool>> {
-  LinkedHashSet<ChildBackButtonDispatcher>? _children;
+  late final LinkedHashSet<ChildBackButtonDispatcher> _children =
+    <ChildBackButtonDispatcher>{} as LinkedHashSet<ChildBackButtonDispatcher>;
 
   @override
-  bool get hasCallbacks => super.hasCallbacks || (_children != null && _children!.isNotEmpty);
+  bool get hasCallbacks => super.hasCallbacks || (_children.isNotEmpty);
 
   /// Handles a pop route request.
   ///
@@ -823,12 +824,12 @@ abstract class BackButtonDispatcher extends _CallbackHookProvider<Future<bool>>
   /// return a future of true or false.
   @override
   Future<bool> invokeCallback(Future<bool> defaultValue) {
-    if (_children != null && _children!.isNotEmpty) {
-      final List<ChildBackButtonDispatcher> children = _children!.toList();
+    if (_children.isNotEmpty) {
+      final List<ChildBackButtonDispatcher> children = _children.toList();
       int childIndex = children.length - 1;
 
       Future<bool> notifyNextChild(bool result) {
-        // If the previous child handles the callback, we returns the result.
+        // If the previous child handles the callback, we return the result.
         if (result)
           return SynchronousFuture<bool>(result);
         // If the previous child did not handle the callback, we ask the next
@@ -878,10 +879,7 @@ abstract class BackButtonDispatcher extends _CallbackHookProvider<Future<bool>>
   ///
   /// The [BackButtonDispatcher] must have a listener registered before it can
   /// be told to take priority.
-  void takePriority() {
-    if (_children != null)
-      _children!.clear();
-  }
+  void takePriority() => _children.clear();
 
   /// Mark the given child as taking priority over this object and the other
   /// children.
@@ -896,16 +894,12 @@ abstract class BackButtonDispatcher extends _CallbackHookProvider<Future<bool>>
   /// Calling this again without first calling [forget] moves the child back to
   /// the head of the list.
   ///
-  // (Actually it moves it to the end of the list and we treat the end of the
-  // list to be the priority end, but that's an implementation detail.)
-  //
   /// The [BackButtonDispatcher] must have a listener registered before it can
   /// be told to defer to a child.
   void deferTo(ChildBackButtonDispatcher child) {
     assert(hasCallbacks);
-    _children ??= <ChildBackButtonDispatcher>{} as LinkedHashSet<ChildBackButtonDispatcher>;
-    _children!.remove(child); // child may or may not be in the set already
-    _children!.add(child);
+    _children.remove(child); // child may or may not be in the set already
+    _children.add(child);
   }
 
   /// Causes the given child to be removed from the list of children to which
@@ -920,11 +914,7 @@ abstract class BackButtonDispatcher extends _CallbackHookProvider<Future<bool>>
   /// this object itself is a [ChildBackButtonDispatcher], [takePriority] would
   /// additionally attempt to claim priority from its parent, whereas removing
   /// the last child does not.)
-  void forget(ChildBackButtonDispatcher child) {
-    assert(_children != null);
-    assert(_children!.contains(child));
-    _children!.remove(child);
-  }
+  void forget(ChildBackButtonDispatcher child) => _children.remove(child);
 }
 
 /// The default implementation of back button dispatcher for the root router.
@@ -996,6 +986,7 @@ class ChildBackButtonDispatcher extends BackButtonDispatcher {
   @override
   void deferTo(ChildBackButtonDispatcher child) {
     assert(hasCallbacks);
+    parent.deferTo(this);
     super.deferTo(child);
   }
 

packages/flutter/test/widgets/router_test.dart

@@ -368,6 +368,116 @@ void main() {
     expect(find.text('popped inner2'), findsOneWidget);
   });
 
+  testWidgets('ChildBackButtonDispatcher can be replaced without calling the takePriority', (WidgetTester tester) async {
+
+    final BackButtonDispatcher outerDispatcher = RootBackButtonDispatcher();
+    BackButtonDispatcher innerDispatcher = ChildBackButtonDispatcher(outerDispatcher);
+    await tester.pumpWidget(buildBoilerPlate(
+      Router<RouteInformation>(
+        backButtonDispatcher: outerDispatcher,
+        routerDelegate: SimpleRouterDelegate(
+          builder: (BuildContext context, RouteInformation? information) {
+            // Creates the sub-router.
+            return Column(
+              children: <Widget>[
+                const Text('initial'),
+                Router<RouteInformation>(
+                  backButtonDispatcher: innerDispatcher,
+                  routerDelegate: SimpleRouterDelegate(
+                    builder: (BuildContext context, RouteInformation? innerInformation) {
+                      return Container();
+                    },
+                  ),
+                ),
+              ],
+            );
+          },
+        ),
+      )
+    ));
+
+    // Creates a new child back button dispatcher and rebuild, this will cause
+    // the old one to be replaced and discarded.
+    innerDispatcher = ChildBackButtonDispatcher(outerDispatcher);
+    await tester.pumpWidget(buildBoilerPlate(
+      Router<RouteInformation>(
+        backButtonDispatcher: outerDispatcher,
+        routerDelegate: SimpleRouterDelegate(
+          builder: (BuildContext context, RouteInformation? information) {
+            // Creates the sub-router.
+            return Column(
+              children: <Widget>[
+                const Text('initial'),
+                Router<RouteInformation>(
+                  backButtonDispatcher: innerDispatcher,
+                  routerDelegate: SimpleRouterDelegate(
+                    builder: (BuildContext context, RouteInformation? innerInformation) {
+                      return Container();
+                    },
+                  ),
+                ),
+              ],
+            );
+          },
+        ),
+      )
+    ));
+
+    expect(tester.takeException(), isNull);
+  });
+
+testWidgets('ChildBackButtonDispatcher take priority recursively', (WidgetTester tester) async {
+
+    final BackButtonDispatcher outerDispatcher = RootBackButtonDispatcher();
+    final BackButtonDispatcher innerDispatcher1 = ChildBackButtonDispatcher(outerDispatcher);
+    final BackButtonDispatcher innerDispatcher2 = ChildBackButtonDispatcher(innerDispatcher1);
+    final BackButtonDispatcher innerDispatcher3 = ChildBackButtonDispatcher(innerDispatcher2);
+    bool isPopped = false;
+    await tester.pumpWidget(buildBoilerPlate(
+      Router<RouteInformation>(
+        backButtonDispatcher: outerDispatcher,
+        routerDelegate: SimpleRouterDelegate(
+          builder: (BuildContext context, RouteInformation? information) {
+            // Creates the sub-router.
+            return Router<RouteInformation>(
+              backButtonDispatcher: innerDispatcher1,
+              routerDelegate: SimpleRouterDelegate(
+                builder: (BuildContext context, RouteInformation? innerInformation) {
+                  return Router<RouteInformation>(
+                    backButtonDispatcher: innerDispatcher2,
+                    routerDelegate: SimpleRouterDelegate(
+                      builder: (BuildContext context, RouteInformation? innerInformation) {
+                        return Router<RouteInformation>(
+                          backButtonDispatcher: innerDispatcher3,
+                          routerDelegate: SimpleRouterDelegate(
+                            onPopRoute: () {
+                              isPopped = true;
+                              return SynchronousFuture<bool>(true);
+                            },
+                            builder: (BuildContext context, RouteInformation? innerInformation) {
+                              return Container();
+                            },
+                          ),
+                        );
+                      },
+                    ),
+                  );
+                },
+              ),
+            );
+          },
+        ),
+      )
+    ));
+    // This should work without calling the takePrioirty on the innerDispatcher2
+    // and the innerDispatcher1.
+    innerDispatcher3.takePriority();
+    bool result = false;
+    result = await outerDispatcher.invokeCallback(SynchronousFuture<bool>(false));
+    expect(result, isTrue);
+    expect(isPopped, isTrue);
+  });
+
   testWidgets('router does report URL change correctly', (WidgetTester tester) async {
     RouteInformation? reportedRouteInformation;
     final SimpleRouteInformationProvider provider = SimpleRouteInformationProvider(