Allow multiple observers for the Navigator and MaterialApp (#7883)

* allow mulitple observers for Navigator and MaterialApp

* add test for the navigator observers

* fix style

* add test for adding/removing a navigator observer

packages/flutter/lib/src/material/app.dart

@@ -57,6 +57,7 @@ class MaterialApp extends StatefulWidget {
     this.initialRoute,
     this.onGenerateRoute,
     this.onLocaleChanged,
+    this.navigatorObservers: const <NavigatorObserver>[],
     this.debugShowMaterialGrid: false,
     this.showPerformanceOverlay: false,
     this.checkerboardRasterCacheImages: false,
@@ -154,6 +155,9 @@ class MaterialApp extends StatefulWidget {
   /// representative of what will happen in release mode.
   final bool debugShowCheckedModeBanner;
 
+  /// The list of observers for the [Navigator] created for this app.
+  final List<NavigatorObserver> navigatorObservers;
+
   /// Turns on a [GridPaper] overlay that paints a baseline grid
   /// Material apps:
   /// https://material.google.com/layout/metrics-keylines.html
@@ -278,7 +282,9 @@ class _MaterialAppState extends State<MaterialApp> {
         textStyle: _errorTextStyle,
         // blue[500] is the primary color of the default theme
         color: config.color ?? theme?.primaryColor ?? Colors.blue[500],
-        navigatorObserver: _heroController,
+        navigatorObservers:
+            new List<NavigatorObserver>.from(config.navigatorObservers)
+              ..add(_heroController),
         initialRoute: config.initialRoute,
         onGenerateRoute: _onGenerateRoute,
         onLocaleChanged: config.onLocaleChanged,

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

@@ -44,7 +44,7 @@ class WidgetsApp extends StatefulWidget {
     this.title,
     this.textStyle,
     @required this.color,
-    this.navigatorObserver,
+    this.navigatorObservers: const <NavigatorObserver>[],
     this.initialRoute,
     this.onLocaleChanged,
     this.showPerformanceOverlay: false,
@@ -111,8 +111,8 @@ class WidgetsApp extends StatefulWidget {
   /// representative of what will happen in release mode.
   final bool debugShowCheckedModeBanner;
 
-  /// The observer for the Navigator created for this app.
-  final NavigatorObserver navigatorObserver;
+  /// The list of observers for the [Navigator] created for this app.
+  final List<NavigatorObserver> navigatorObservers;
 
   /// If true, forces the performance overlay to be visible in all instances.
   ///
@@ -203,7 +203,7 @@ class _WidgetsAppState extends State<WidgetsApp> implements WidgetsBindingObserv
             key: _navigator,
             initialRoute: config.initialRoute ?? ui.window.defaultRouteName,
             onGenerateRoute: config.onGenerateRoute,
-            observer: config.navigatorObserver
+            observers: config.navigatorObservers
           )
         )
       )

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

@@ -445,7 +445,7 @@ class Navigator extends StatefulWidget {
     this.initialRoute,
     @required this.onGenerateRoute,
     this.onUnknownRoute,
-    this.observer
+    this.observers: const <NavigatorObserver>[]
   }) : super(key: key) {
     assert(onGenerateRoute != null);
   }
@@ -466,8 +466,8 @@ class Navigator extends StatefulWidget {
   /// requests to push routes, such as from Android intents.
   final RouteFactory onUnknownRoute;
 
-  /// An observer for this navigator.
-  final NavigatorObserver observer;
+  /// A list of observers for this navigator.
+  final List<NavigatorObserver> observers;
 
   /// The default name for the initial route.
   static const String defaultRouteName = '/';
@@ -668,8 +668,10 @@ class NavigatorState extends State<Navigator> with TickerProviderStateMixin {
   @override
   void initState() {
     super.initState();
-    assert(config.observer == null || config.observer.navigator == null);
-    config.observer?._navigator = this;
+    for (NavigatorObserver observer in config.observers) {
+      assert(observer.navigator == null);
+      observer._navigator = this;
+    }
     push(config.onGenerateRoute(new RouteSettings(
       name: config.initialRoute ?? Navigator.defaultRouteName,
       isInitialRoute: true
@@ -678,10 +680,13 @@ class NavigatorState extends State<Navigator> with TickerProviderStateMixin {
 
   @override
   void didUpdateConfig(Navigator oldConfig) {
-    if (oldConfig.observer != config.observer) {
-      oldConfig.observer?._navigator = null;
-      assert(config.observer == null || config.observer.navigator == null);
-      config.observer?._navigator = this;
+    if (oldConfig.observers != config.observers) {
+      for (NavigatorObserver observer in oldConfig.observers)
+        observer._navigator = null;
+      for (NavigatorObserver observer in config.observers) {
+        assert(observer.navigator == null);
+        observer._navigator = this;
+      }
     }
   }
 
@@ -689,7 +694,8 @@ class NavigatorState extends State<Navigator> with TickerProviderStateMixin {
   void dispose() {
     assert(!_debugLocked);
     assert(() { _debugLocked = true; return true; });
-    config.observer?._navigator = null;
+    for (NavigatorObserver observer in config.observers)
+      observer._navigator = null;
     final List<Route<dynamic>> doomed = _poppedRoutes.toList()..addAll(_history);
     for (Route<dynamic> route in doomed)
       route.dispose();
@@ -768,7 +774,8 @@ class NavigatorState extends State<Navigator> with TickerProviderStateMixin {
       route.didChangeNext(null);
       if (oldRoute != null)
         oldRoute.didChangeNext(route);
-      config.observer?.didPush(route, oldRoute);
+      for (NavigatorObserver observer in config.observers)
+        observer.didPush(route, oldRoute);
     });
     assert(() { _debugLocked = false; return true; });
     _cancelActivePointers();
@@ -850,7 +857,8 @@ class NavigatorState extends State<Navigator> with TickerProviderStateMixin {
       newRoute.didChangeNext(null);
       if (index > 0)
         _history[index - 1].didChangeNext(newRoute);
-      config.observer?.didPush(newRoute, oldRoute);
+      for (NavigatorObserver observer in config.observers)
+        observer.didPush(newRoute, oldRoute);
     });
     assert(() { _debugLocked = false; return true; });
     _cancelActivePointers();
@@ -957,7 +965,8 @@ class NavigatorState extends State<Navigator> with TickerProviderStateMixin {
           if (route._navigator != null)
             _poppedRoutes.add(route);
           _history.last.didPopNext(route);
-          config.observer?.didPop(route, _history.last);
+          for (NavigatorObserver observer in config.observers)
+            observer.didPop(route, _history.last);
         });
       } else {
         assert(() { _debugLocked = false; return true; });
@@ -1026,13 +1035,15 @@ class NavigatorState extends State<Navigator> with TickerProviderStateMixin {
   /// Used for the iOS back gesture.
   void didStartUserGesture() {
     _userGestureInProgress = true;
-    config.observer?.didStartUserGesture();
+    for (NavigatorObserver observer in config.observers)
+      observer.didStartUserGesture();
   }
 
   /// A user gesture is no longer controlling the navigator.
   void didStopUserGesture() {
     _userGestureInProgress = false;
-    config.observer?.didStopUserGesture();
+    for (NavigatorObserver observer in config.observers)
+      observer.didStopUserGesture();
   }
 
   final Set<int> _activePointers = new Set<int>();

packages/flutter/test/widgets/navigator_test.dart

@@ -89,6 +89,28 @@ class OnTapPage extends StatelessWidget {
   }
 }
 
+typedef void OnPushed(Route<dynamic> route, Route<dynamic> previousRoute);
+typedef void OnPopped(Route<dynamic> route, Route<dynamic> previousRoute);
+
+class TestObserver extends NavigatorObserver {
+  OnPushed onPushed;
+  OnPopped onPopped;
+
+  @override
+  void didPush(Route<dynamic> route, Route<dynamic> previousRoute) {
+    if (onPushed != null) {
+      onPushed(route, previousRoute);
+    }
+  }
+
+  @override
+  void didPop(Route<dynamic> route, Route<dynamic> previousRoute) {
+    if (onPopped != null) {
+      onPopped(route, previousRoute);
+    }
+  }
+}
+
 void main() {
   testWidgets('Can navigator navigate to and from a stateful widget', (WidgetTester tester) async {
     final Map<String, WidgetBuilder> routes = <String, WidgetBuilder>{
@@ -260,6 +282,114 @@ void main() {
     expect(find.text('B'), findsOneWidget);
   });
 
+  testWidgets('Push and pop should trigger the observers',
+      (WidgetTester tester) async {
+    final Map<String, WidgetBuilder> routes = <String, WidgetBuilder>{
+       '/': (BuildContext context) => new OnTapPage(id: '/', onTap: () { Navigator.pushNamed(context, '/A'); }),
+      '/A': (BuildContext context) => new OnTapPage(id: 'A', onTap: () { Navigator.pop(context); }),
+    };
+    bool isPushed = false;
+    bool isPopped = false;
+    TestObserver observer = new TestObserver()
+      ..onPushed = (Route<dynamic> route, Route<dynamic> previousRoute) {
+        // Pushes the initial route.
+        expect(route is PageRoute && route.settings.name == '/', isTrue);
+        expect(previousRoute, isNull);
+        isPushed = true;
+      }
+      ..onPopped = (Route<dynamic> route, Route<dynamic> previousRoute) {
+        isPopped = true;
+      };
+
+    await tester.pumpWidget(new MaterialApp(
+      routes: routes,
+      navigatorObservers: <NavigatorObserver>[observer],
+    ));
+    expect(find.text('/'), findsOneWidget);
+    expect(find.text('A'), findsNothing);
+    expect(isPushed, isTrue);
+    expect(isPopped, isFalse);
+
+    isPushed = false;
+    isPopped = false;
+    observer.onPushed = (Route<dynamic> route, Route<dynamic> previousRoute) {
+      expect(route is PageRoute && route.settings.name == '/A', isTrue);
+      expect(previousRoute is PageRoute && previousRoute.settings.name == '/', isTrue);
+      isPushed = true;
+    };
+
+    await tester.tap(find.text('/'));
+    await tester.pump();
+    await tester.pump(const Duration(seconds: 1));
+    expect(find.text('/'), findsNothing);
+    expect(find.text('A'), findsOneWidget);
+    expect(isPushed, isTrue);
+    expect(isPopped, isFalse);
+
+    isPushed = false;
+    isPopped = false;
+    observer.onPopped = (Route<dynamic> route, Route<dynamic> previousRoute) {
+      expect(route is PageRoute && route.settings.name == '/A', isTrue);
+      expect(previousRoute is PageRoute && previousRoute.settings.name == '/', isTrue);
+      isPopped = true;
+    };
+
+    await tester.tap(find.text('A'));
+    await tester.pump();
+    await tester.pump(const Duration(seconds: 1));
+    expect(find.text('/'), findsOneWidget);
+    expect(find.text('A'), findsNothing);
+    expect(isPushed, isFalse);
+    expect(isPopped, isTrue);
+  });
+
+  testWidgets("Add and remove an observer should work", (WidgetTester tester) async {
+    final Map<String, WidgetBuilder> routes = <String, WidgetBuilder>{
+       '/': (BuildContext context) => new OnTapPage(id: '/', onTap: () { Navigator.pushNamed(context, '/A'); }),
+      '/A': (BuildContext context) => new OnTapPage(id: 'A', onTap: () { Navigator.pop(context); }),
+    };
+    bool isPushed = false;
+    bool isPopped = false;
+    TestObserver observer1 = new TestObserver();
+    TestObserver observer2 = new TestObserver()
+      ..onPushed = (Route<dynamic> route, Route<dynamic> previousRoute) {
+        isPushed = true;
+      }
+      ..onPopped = (Route<dynamic> route, Route<dynamic> previousRoute) {
+        isPopped = true;
+      };
+
+    await tester.pumpWidget(new MaterialApp(
+      routes: routes,
+      navigatorObservers: <NavigatorObserver>[observer1],
+    ));
+    expect(isPushed, isFalse);
+    expect(isPopped, isFalse);
+
+    await tester.pumpWidget(new MaterialApp(
+      routes: routes,
+      navigatorObservers: <NavigatorObserver>[observer1, observer2],
+    ));
+    await tester.tap(find.text('/'));
+    await tester.pump();
+    await tester.pump(const Duration(seconds: 1));
+    expect(isPushed, isTrue);
+    expect(isPopped, isFalse);
+
+    isPushed = false;
+    isPopped = false;
+
+    await tester.pumpWidget(new MaterialApp(
+      routes: routes,
+      navigatorObservers: <NavigatorObserver>[observer1],
+    ));
+    await tester.tap(find.text('A'));
+    await tester.pump();
+    await tester.pump(const Duration(seconds: 1));
+    expect(isPushed, isFalse);
+    expect(isPopped, isFalse);
+  });
+
   testWidgets('replaceNamed', (WidgetTester tester) async {
     final Map<String, WidgetBuilder> routes = <String, WidgetBuilder>{
        '/': (BuildContext context) => new OnTapPage(id: '/', onTap: () { Navigator.pushReplacementNamed(context, '/A'); }),