From b1d5bc7fbbdf73fb15424c84bd55fdebd624a963 Mon Sep 17 00:00:00 2001 From: Ian Hickson Date: Thu, 20 May 2021 12:29:01 -0700 Subject: [PATCH] Let the framework toggle between single- and multi-entry histories (flutter/engine#26164) --- engine/src/flutter/lib/web_ui/README.md | 1 + .../lib/web_ui/lib/src/engine/window.dart | 35 +++------ .../engine/surface/scene_builder_test.dart | 1 - .../flutter/lib/web_ui/test/window_test.dart | 73 +++++++++++++------ 4 files changed, 62 insertions(+), 48 deletions(-) create mode 100644 engine/src/flutter/lib/web_ui/README.md diff --git a/engine/src/flutter/lib/web_ui/README.md b/engine/src/flutter/lib/web_ui/README.md new file mode 100644 index 00000000000..c0ca9d45748 --- /dev/null +++ b/engine/src/flutter/lib/web_ui/README.md @@ -0,0 +1 @@ +To run tests, use `dev/felt test`. See dev/README.md for details. diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/window.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/window.dart index 846d6ab89f1..3d2c1d213a3 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/window.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/window.dart @@ -56,8 +56,8 @@ class EngineFlutterWindow extends ui.SingletonFlutterWindow { return urlStrategy; } - BrowserHistory? _browserHistory; - bool _usingRouter = false; + BrowserHistory? _browserHistory; // Must be either SingleEntryBrowserHistory or MultiEntriesBrowserHistory. + Future _useSingleEntryBrowserHistory() async { if (_browserHistory is SingleEntryBrowserHistory) { return; @@ -95,7 +95,6 @@ class EngineFlutterWindow extends ui.SingletonFlutterWindow { }) async { // Prevent any further customization of URL strategy. _isUrlStrategySet = true; - _usingRouter = false; await _browserHistory?.tearDown(); if (useSingle) { _browserHistory = SingleEntryBrowserHistory(urlStrategy: strategy); @@ -108,35 +107,25 @@ class EngineFlutterWindow extends ui.SingletonFlutterWindow { await _browserHistory?.tearDown(); _browserHistory = null; // Reset the globals too. - _usingRouter = false; _isUrlStrategySet = false; _customUrlStrategy = null; } - Future handleNavigationMessage( - ByteData? data, - ) async { + Future handleNavigationMessage(ByteData? data) async { final MethodCall decoded = JSONMethodCodec().decodeMethodCall(data); final Map arguments = decoded.arguments; - switch (decoded.method) { - case 'routeUpdated': - if (!_usingRouter) { - await _useSingleEntryBrowserHistory(); - browserHistory.setRouteName(arguments['routeName']); - } else { - assert( - false, - 'Receives old navigator update in a router application. ' - 'This can happen if you use non-router versions of MaterialApp/' - 'CupertinoApp/WidgetsApp together with the router versions of them.' - ); - return false; - } + case 'selectMultiEntryHistory': + await _useMultiEntryBrowserHistory(); + return true; + case 'selectSingleEntryHistory': + await _useSingleEntryBrowserHistory(); + return true; + case 'routeUpdated': // deprecated + await _useSingleEntryBrowserHistory(); + browserHistory.setRouteName(arguments['routeName']); return true; case 'routeInformationUpdated': - await _useMultiEntryBrowserHistory(); - _usingRouter = true; browserHistory.setRouteName( arguments['location'], state: arguments['state'], diff --git a/engine/src/flutter/lib/web_ui/test/engine/surface/scene_builder_test.dart b/engine/src/flutter/lib/web_ui/test/engine/surface/scene_builder_test.dart index c964d4b22d6..f6b0e4e1bda 100644 --- a/engine/src/flutter/lib/web_ui/test/engine/surface/scene_builder_test.dart +++ b/engine/src/flutter/lib/web_ui/test/engine/surface/scene_builder_test.dart @@ -495,7 +495,6 @@ void testMain() { // Renders a `string` by breaking it up into individual characters and // rendering each character into its own layer. Future testCase(String string, String description, { int deletions = 0, int additions = 0, int moves = 0 }) { - print('Testing "$string" - $description'); final Set actualDeletions = {}; final Set actualAdditions = {}; diff --git a/engine/src/flutter/lib/web_ui/test/window_test.dart b/engine/src/flutter/lib/web_ui/test/window_test.dart index b6230ed7a14..9dea67f8f60 100644 --- a/engine/src/flutter/lib/web_ui/test/window_test.dart +++ b/engine/src/flutter/lib/web_ui/test/window_test.dart @@ -44,10 +44,15 @@ void testMain() { expect(window.defaultRouteName, '/initial'); }); + // window.defaultRouteName is now permanently decoupled from the history, + // even in subsequent tests, because the PlatformDispatcher caches it. + test('window.defaultRouteName should reset after navigation platform message', () async { await window.debugInitializeHistory(TestUrlStrategy.fromEntry( - TestHistoryEntry('initial state', null, '/initial'), + // The URL here does not set the PlatformDispatcher's defaultRouteName, + // since it got cached as soon as we read it above. + TestHistoryEntry('initial state', null, '/not-really-inital/THIS_IS_IGNORED'), ), useSingle: true); // Reading it multiple times should return the same value. expect(window.defaultRouteName, '/initial'); @@ -62,17 +67,48 @@ void testMain() { )), (_) { callback.complete(); }, ); - // After a navigation platform message, [window.defaultRouteName] should - // reset to "/". + await callback.future; + // After a navigation platform message, the PlatformDispatcher's + // defaultRouteName resets to "/". expect(window.defaultRouteName, '/'); }); - test('should throw when using nav1 and nav2 together', + // window.defaultRouteName is now '/'. + + test('can switch history mode', () async { + Completer callback; + await window.debugInitializeHistory(TestUrlStrategy.fromEntry( + TestHistoryEntry('initial state', null, '/initial'), + ), useSingle: false); + expect(window.browserHistory, isA()); + + Future check(String method, Map arguments) async { + callback = Completer(); + window.sendPlatformMessage( + 'flutter/navigation', + JSONMethodCodec().encodeMethodCall(MethodCall(method, arguments)), + (_) { callback.complete(); }, + ); + await callback.future; + expect(window.browserHistory, isA()); + } + + await check('selectSingleEntryHistory', {}); // -> single + await check('selectMultiEntryHistory', {}); // -> multi + await check('routeUpdated', {'routeName': '/bar'}); // -> single + await check('routeInformationUpdated', {'location': '/bar'}); // does not change mode + await check('selectMultiEntryHistory', {}); // -> multi + await check('routeInformationUpdated', {'location': '/bar'}); // does not change mode + }); + + test('should not throw when using nav1 and nav2 together', () async { await window.debugInitializeHistory(TestUrlStrategy.fromEntry( TestHistoryEntry('initial state', null, '/initial'), ), useSingle: false); - // Receive nav1 update first. + expect(window.browserHistory, isA()); + + // routeUpdated resets the history type Completer callback = Completer(); window.sendPlatformMessage( 'flutter/navigation', @@ -83,10 +119,10 @@ void testMain() { (_) { callback.complete(); }, ); await callback.future; - expect(window.browserHistory is SingleEntryBrowserHistory, true); + expect(window.browserHistory, isA()); expect(window.browserHistory.urlStrategy!.getPath(), '/bar'); - // We can still receive nav2 update. + // routeInformationUpdated does not callback = Completer(); window.sendPlatformMessage( 'flutter/navigation', @@ -100,29 +136,18 @@ void testMain() { (_) { callback.complete(); }, ); await callback.future; - expect(window.browserHistory is MultiEntriesBrowserHistory, true); + expect(window.browserHistory, isA()); expect(window.browserHistory.urlStrategy!.getPath(), '/baz'); - // Throws assertion error if it receives nav1 update after nav2 update. - late AssertionError caughtAssertion; + // they can be interleaved safely await window.handleNavigationMessage( JSONMethodCodec().encodeMethodCall(MethodCall( 'routeUpdated', {'routeName': '/foo'}, )) - ).catchError((Object e) { - caughtAssertion = e as AssertionError; - }); - - expect( - caughtAssertion.message, - 'Receives old navigator update in a router application. This can ' - 'happen if you use non-router versions of ' - 'MaterialApp/CupertinoApp/WidgetsApp together with the router versions of them.' ); - // The history does not change. - expect(window.browserHistory is MultiEntriesBrowserHistory, true); - expect(window.browserHistory.urlStrategy!.getPath(), '/baz'); + expect(window.browserHistory, isA()); + expect(window.browserHistory.urlStrategy!.getPath(), '/foo'); }); test('initialize browser history with default url strategy (single)', () async { @@ -143,7 +168,7 @@ void testMain() { (_) { callback.complete(); }, ); await callback.future; - expect(window.browserHistory is SingleEntryBrowserHistory, true); + expect(window.browserHistory, isA()); // The url strategy should've been set to the default, and the path // should've been correctly set to "/bar". expect(window.browserHistory.urlStrategy, isNot(isNull)); @@ -171,7 +196,7 @@ void testMain() { (_) { callback.complete(); }, ); await callback.future; - expect(window.browserHistory is MultiEntriesBrowserHistory, true); + expect(window.browserHistory, isA()); // The url strategy should've been set to the default, and the path // should've been correctly set to "/baz". expect(window.browserHistory.urlStrategy, isNot(isNull));