From 5f521eea22b2fa52a264678b79ee29746f2e423c Mon Sep 17 00:00:00 2001 From: Devon Carew Date: Sat, 15 Jun 2019 20:40:34 -0700 Subject: [PATCH] add route information to Flutter.Navigation events (#34508) * add route information to Flutter.Navigation events * route can be null; re-work how we send route info to accomidate that --- .../bin/tasks/service_extensions_test.dart | 11 +++- .../flutter/lib/src/widgets/navigator.dart | 54 ++++++++++++++----- 2 files changed, 51 insertions(+), 14 deletions(-) diff --git a/dev/devicelab/bin/tasks/service_extensions_test.dart b/dev/devicelab/bin/tasks/service_extensions_test.dart index bd8262f05c4..b874ad61da4 100644 --- a/dev/devicelab/bin/tasks/service_extensions_test.dart +++ b/dev/devicelab/bin/tasks/service_extensions_test.dart @@ -94,9 +94,16 @@ void main() { final Future navigationFuture = navigationEvents.first; // This tap triggers a navigation event. device.tap(100, 200); + final VMExtensionEvent navigationEvent = await navigationFuture; - // Validate that there are not any fields. - expect(navigationEvent.data.isEmpty); + // validate the fields + expect(navigationEvent.data['route'] is Map); + final Map route = navigationEvent.data['route']; + expect(route['description'] is String); + expect(route['settings'] is Map); + final Map settings = route['settings']; + expect(settings.containsKey('name')); + expect(settings['isInitialRoute'] is bool); run.stdin.write('q'); final int result = await run.exitCode; diff --git a/packages/flutter/lib/src/widgets/navigator.dart b/packages/flutter/lib/src/widgets/navigator.dart index bdd4208d08f..b58fb9b37fc 100644 --- a/packages/flutter/lib/src/widgets/navigator.dart +++ b/packages/flutter/lib/src/widgets/navigator.dart @@ -3,6 +3,7 @@ // found in the LICENSE file. import 'dart:async'; +import 'dart:convert'; import 'dart:developer' as developer; import 'package:flutter/foundation.dart'; @@ -15,6 +16,7 @@ import 'focus_manager.dart'; import 'focus_scope.dart'; import 'framework.dart'; import 'overlay.dart'; +import 'routes.dart'; import 'ticker_provider.dart'; // Examples can assume: @@ -1760,18 +1762,46 @@ class NavigatorState extends State with TickerProviderStateMixin { for (NavigatorObserver observer in widget.observers) observer.didPush(route, oldRoute); assert(() { _debugLocked = false; return true; }()); - _afterNavigation(); + _afterNavigation(route); return route.popped; } - void _afterNavigation() { + void _afterNavigation(Route route) { if (!kReleaseMode) { - // This event is used by performance tools that show stats for the - // time interval since the last navigation event occurred ensuring that - // stats only reflect the current page. - // These tools do not need to know exactly what the new route is so no - // attempt is made to describe the current route as part of the event. - developer.postEvent('Flutter.Navigation', {}); + // Among other uses, performance tools use this event to ensure that perf + // stats reflect the time interval since the last navigation event + // occurred, ensuring that stats only reflect the current page. + + Map routeJsonable; + if (route != null) { + routeJsonable = {}; + + String description; + if (route is TransitionRoute) { + final TransitionRoute transitionRoute = route; + description = transitionRoute.debugLabel; + } else { + description = '$route'; + } + routeJsonable['description'] = description; + + final RouteSettings settings = route.settings; + final Map settingsJsonable = { + 'name': settings.name, + 'isInitialRoute': settings.isInitialRoute, + }; + if (settings.arguments != null) { + settingsJsonable['arguments'] = jsonEncode( + settings.arguments, + toEncodable: (Object object) => '$object', + ); + } + routeJsonable['settings'] = settingsJsonable; + } + + developer.postEvent('Flutter.Navigation', { + 'route': routeJsonable, + }); } _cancelActivePointers(); } @@ -1825,7 +1855,7 @@ class NavigatorState extends State with TickerProviderStateMixin { for (NavigatorObserver observer in widget.observers) observer.didReplace(newRoute: newRoute, oldRoute: oldRoute); assert(() { _debugLocked = false; return true; }()); - _afterNavigation(); + _afterNavigation(newRoute); return newRoute.popped; } @@ -1879,7 +1909,7 @@ class NavigatorState extends State with TickerProviderStateMixin { observer.didRemove(removedRoute, oldRoute); } assert(() { _debugLocked = false; return true; }()); - _afterNavigation(); + _afterNavigation(newRoute); return newRoute.popped; } @@ -2032,7 +2062,7 @@ class NavigatorState extends State with TickerProviderStateMixin { assert(!debugPredictedWouldPop); } assert(() { _debugLocked = false; return true; }()); - _afterNavigation(); + _afterNavigation(route); return true; } @@ -2074,7 +2104,7 @@ class NavigatorState extends State with TickerProviderStateMixin { observer.didRemove(route, previousRoute); route.dispose(); assert(() { _debugLocked = false; return true; }()); - _afterNavigation(); + _afterNavigation(nextRoute); } /// Immediately remove a route from the navigator, and [Route.dispose] it. The