From 016c1d49db9f963a071a8a821a6da43c5b9af24e Mon Sep 17 00:00:00 2001 From: Mouad Debbar Date: Tue, 27 Jul 2021 14:00:48 -0700 Subject: [PATCH 1/3] [web] Don't reset history on hot restart --- lib/web_ui/lib/src/engine/window.dart | 64 +++++++++++++++++++++++++-- 1 file changed, 60 insertions(+), 4 deletions(-) diff --git a/lib/web_ui/lib/src/engine/window.dart b/lib/web_ui/lib/src/engine/window.dart index 0e5d6f9aa207a..6f94be086117f 100644 --- a/lib/web_ui/lib/src/engine/window.dart +++ b/lib/web_ui/lib/src/engine/window.dart @@ -41,6 +41,34 @@ set customUrlStrategy(UrlStrategy? strategy) { _customUrlStrategy = strategy; } +@JS('window.flutterHistoryMode') +external set _windowFlutterHistoryMode(_HistoryMode? mode); + +@JS('window.flutterHistoryMode') +external _HistoryMode? get _windowFlutterHistoryMode; + +/// Indicates the mode that the browser history is being managed in. +enum _HistoryMode { + /// The browser history is managed by [MultiEntriesBrowserHistory] which + /// pushes multiple history entries in the browser history list. + multiEntry, + + /// The browser history is managed by [SingleEntryBrowserHistory] which keeps + /// updating a single history entry instead of pushing new ones. + singleEntry, +} + +BrowserHistory _createDefaultBrowserHistory(UrlStrategy? urlStrategy) { + switch (_windowFlutterHistoryMode) { + case _HistoryMode.singleEntry: + return SingleEntryBrowserHistory(urlStrategy: urlStrategy); + + case _HistoryMode.multiEntry: + default: + return MultiEntriesBrowserHistory(urlStrategy: urlStrategy); + } +} + /// The Web implementation of [ui.SingletonFlutterWindow]. class EngineFlutterWindow extends ui.SingletonFlutterWindow { EngineFlutterWindow(this._windowId, this.platformDispatcher) { @@ -49,11 +77,15 @@ class EngineFlutterWindow extends ui.SingletonFlutterWindow { engineDispatcher.windows[_windowId] = this; engineDispatcher.windowConfigurations[_windowId] = const ui.ViewConfiguration(); if (_isUrlStrategySet) { - _browserHistory = - MultiEntriesBrowserHistory(urlStrategy: _customUrlStrategy); + _browserHistory = _createDefaultBrowserHistory(_customUrlStrategy); } registerHotRestartListener(() { - window.resetHistory(); + // Remember which history mode was being used when hot restart happened. + if (_browserHistory is MultiEntriesBrowserHistory) { + _windowFlutterHistoryMode = _HistoryMode.multiEntry; + } else if (_browserHistory is SingleEntryBrowserHistory) { + _windowFlutterHistoryMode = _HistoryMode.singleEntry; + } }); } @@ -66,7 +98,7 @@ class EngineFlutterWindow extends ui.SingletonFlutterWindow { /// button, etc. BrowserHistory get browserHistory { return _browserHistory ??= - MultiEntriesBrowserHistory(urlStrategy: _urlStrategyForInitialization); + _createDefaultBrowserHistory(_urlStrategyForInitialization); } UrlStrategy? get _urlStrategyForInitialization { @@ -85,6 +117,18 @@ class EngineFlutterWindow extends ui.SingletonFlutterWindow { return; } + // If the app was running in the other mode (multi entry), let's re-create + // it, then ask it to tear down. + // + // This happens on hot restart. We don't want to tear down the browser + // history on hot restart. Instead, we let it be, and if the app starts with + // the same history mode, we start from there without tearing down anything. + // + // See: https://github.com/flutter/flutter/issues/79241 + if (_windowFlutterHistoryMode == _HistoryMode.multiEntry) { + _browserHistory = _createDefaultBrowserHistory(_urlStrategyForInitialization); + } + final UrlStrategy? strategy; if (_browserHistory == null) { strategy = _urlStrategyForInitialization; @@ -100,6 +144,18 @@ class EngineFlutterWindow extends ui.SingletonFlutterWindow { return; } + // If the app was running in the other mode (single entry), let's re-create + // it, then ask it to tear down. + // + // This happens on hot restart. We don't want to tear down the browser + // history on hot restart. Instead, we let it be, and if the app starts with + // the same history mode, we start from there without tearing down anything. + // + // See: https://github.com/flutter/flutter/issues/79241 + if (_windowFlutterHistoryMode == _HistoryMode.singleEntry) { + _browserHistory = _createDefaultBrowserHistory(_urlStrategyForInitialization); + } + final UrlStrategy? strategy; if (_browserHistory == null) { strategy = _urlStrategyForInitialization; From 3624eac2d7b9947586952f5dd0fe60e6c533ace4 Mon Sep 17 00:00:00 2001 From: Mouad Debbar Date: Mon, 9 Aug 2021 11:07:27 -0700 Subject: [PATCH 2/3] infer mode from existing history state --- .../lib/src/engine/navigation/history.dart | 18 ++- lib/web_ui/lib/src/engine/window.dart | 107 ++++++------------ 2 files changed, 48 insertions(+), 77 deletions(-) diff --git a/lib/web_ui/lib/src/engine/navigation/history.dart b/lib/web_ui/lib/src/engine/navigation/history.dart index c5eec94e78f02..9c6de07e95edf 100644 --- a/lib/web_ui/lib/src/engine/navigation/history.dart +++ b/lib/web_ui/lib/src/engine/navigation/history.dart @@ -11,6 +11,20 @@ import '../services/message_codec.dart'; import '../services/message_codecs.dart'; import 'url_strategy.dart'; +/// Infers the history mode from the existing browser history state, then +/// creates the appropriate instance of [BrowserHistory] for it. +/// +/// If it can't infer, it creates a [MultiEntriesBrowserHistory] by default. +BrowserHistory createHistoryForExistingState(UrlStrategy? urlStrategy) { + if (urlStrategy != null) { + final Object? state = urlStrategy.getState(); + if (SingleEntryBrowserHistory._isOriginEntry(state) || SingleEntryBrowserHistory._isFlutterEntry(state)) { + return SingleEntryBrowserHistory(urlStrategy: urlStrategy); + } + } + return MultiEntriesBrowserHistory(urlStrategy: urlStrategy); +} + /// An abstract class that provides the API for [EngineWindow] to delegate its /// navigating events. /// @@ -263,14 +277,14 @@ class SingleEntryBrowserHistory extends BrowserHistory { /// The origin entry is the history entry that the Flutter app landed on. It's /// created by the browser when the user navigates to the url of the app. - bool _isOriginEntry(Object? state) { + static bool _isOriginEntry(Object? state) { return state is Map && state[_kOriginTag] == true; } /// The flutter entry is a history entry that we maintain on top of the origin /// entry. It allows us to catch popstate events when the user hits the back /// button. - bool _isFlutterEntry(Object? state) { + static bool _isFlutterEntry(Object? state) { return state is Map && state[_kFlutterTag] == true; } diff --git a/lib/web_ui/lib/src/engine/window.dart b/lib/web_ui/lib/src/engine/window.dart index 1e550ce8e7453..434c568d62f4f 100644 --- a/lib/web_ui/lib/src/engine/window.dart +++ b/lib/web_ui/lib/src/engine/window.dart @@ -13,7 +13,6 @@ import 'package:js/js.dart'; import 'package:meta/meta.dart'; import 'package:ui/ui.dart' as ui; -import '../engine.dart' show registerHotRestartListener; import 'browser_detection.dart'; import 'navigation/history.dart'; import 'navigation/js_url_strategy.dart'; @@ -42,34 +41,6 @@ set customUrlStrategy(UrlStrategy? strategy) { _customUrlStrategy = strategy; } -@JS('window.flutterHistoryMode') -external set _windowFlutterHistoryMode(_HistoryMode? mode); - -@JS('window.flutterHistoryMode') -external _HistoryMode? get _windowFlutterHistoryMode; - -/// Indicates the mode that the browser history is being managed in. -enum _HistoryMode { - /// The browser history is managed by [MultiEntriesBrowserHistory] which - /// pushes multiple history entries in the browser history list. - multiEntry, - - /// The browser history is managed by [SingleEntryBrowserHistory] which keeps - /// updating a single history entry instead of pushing new ones. - singleEntry, -} - -BrowserHistory _createDefaultBrowserHistory(UrlStrategy? urlStrategy) { - switch (_windowFlutterHistoryMode) { - case _HistoryMode.singleEntry: - return SingleEntryBrowserHistory(urlStrategy: urlStrategy); - - case _HistoryMode.multiEntry: - default: - return MultiEntriesBrowserHistory(urlStrategy: urlStrategy); - } -} - /// The Web implementation of [ui.SingletonFlutterWindow]. class EngineFlutterWindow extends ui.SingletonFlutterWindow { EngineFlutterWindow(this._windowId, this.platformDispatcher) { @@ -78,16 +49,8 @@ class EngineFlutterWindow extends ui.SingletonFlutterWindow { engineDispatcher.windows[_windowId] = this; engineDispatcher.windowConfigurations[_windowId] = const ui.ViewConfiguration(); if (_isUrlStrategySet) { - _browserHistory = _createDefaultBrowserHistory(_customUrlStrategy); + _browserHistory = createHistoryForExistingState(_customUrlStrategy); } - registerHotRestartListener(() { - // Remember which history mode was being used when hot restart happened. - if (_browserHistory is MultiEntriesBrowserHistory) { - _windowFlutterHistoryMode = _HistoryMode.multiEntry; - } else if (_browserHistory is SingleEntryBrowserHistory) { - _windowFlutterHistoryMode = _HistoryMode.singleEntry; - } - }); } final Object _windowId; @@ -99,7 +62,7 @@ class EngineFlutterWindow extends ui.SingletonFlutterWindow { /// button, etc. BrowserHistory get browserHistory { return _browserHistory ??= - _createDefaultBrowserHistory(_urlStrategyForInitialization); + createHistoryForExistingState(_urlStrategyForInitialization); } UrlStrategy? get _urlStrategyForInitialization { @@ -114,56 +77,50 @@ class EngineFlutterWindow extends ui.SingletonFlutterWindow { _browserHistory; // Must be either SingleEntryBrowserHistory or MultiEntriesBrowserHistory. Future _useSingleEntryBrowserHistory() async { - if (_browserHistory is SingleEntryBrowserHistory) { - return; - } - - // If the app was running in the other mode (multi entry), let's re-create - // it, then ask it to tear down. + // Recreate the browser history mode that's appropriate for the existing + // history state. // - // This happens on hot restart. We don't want to tear down the browser - // history on hot restart. Instead, we let it be, and if the app starts with - // the same history mode, we start from there without tearing down anything. + // If it happens to be a single-entry one, then there's nothing further to do. + // + // But if it's a multi-entry one, it will be torn down below and replaced + // with a single-entry history. // // See: https://github.com/flutter/flutter/issues/79241 - if (_windowFlutterHistoryMode == _HistoryMode.multiEntry) { - _browserHistory = _createDefaultBrowserHistory(_urlStrategyForInitialization); - } + _browserHistory ??= + createHistoryForExistingState(_urlStrategyForInitialization); - final UrlStrategy? strategy; - if (_browserHistory == null) { - strategy = _urlStrategyForInitialization; - } else { - strategy = _browserHistory?.urlStrategy; - await _browserHistory?.tearDown(); + if (_browserHistory is SingleEntryBrowserHistory) { + return; } + + // At this point, we know that `_browserHistory` is a non-null + // `MultiEntriesBrowserHistory` instance. + final UrlStrategy? strategy = _browserHistory?.urlStrategy; + await _browserHistory?.tearDown(); _browserHistory = SingleEntryBrowserHistory(urlStrategy: strategy); } Future _useMultiEntryBrowserHistory() async { - if (_browserHistory is MultiEntriesBrowserHistory) { - return; - } - - // If the app was running in the other mode (single entry), let's re-create - // it, then ask it to tear down. + // Recreate the browser history mode that's appropriate for the existing + // history state. + // + // If it happens to be a multi-entry one, then there's nothing further to do. // - // This happens on hot restart. We don't want to tear down the browser - // history on hot restart. Instead, we let it be, and if the app starts with - // the same history mode, we start from there without tearing down anything. + // But if it's a single-entry one, it will be torn down below and replaced + // with a multi-entry history. // // See: https://github.com/flutter/flutter/issues/79241 - if (_windowFlutterHistoryMode == _HistoryMode.singleEntry) { - _browserHistory = _createDefaultBrowserHistory(_urlStrategyForInitialization); - } + _browserHistory ??= + createHistoryForExistingState(_urlStrategyForInitialization); - final UrlStrategy? strategy; - if (_browserHistory == null) { - strategy = _urlStrategyForInitialization; - } else { - strategy = _browserHistory?.urlStrategy; - await _browserHistory?.tearDown(); + if (_browserHistory is MultiEntriesBrowserHistory) { + return; } + + // At this point, we know that `_browserHistory` is a non-null + // `SingleEntryBrowserHistory` instance. + final UrlStrategy? strategy = _browserHistory?.urlStrategy; + await _browserHistory?.tearDown(); _browserHistory = MultiEntriesBrowserHistory(urlStrategy: strategy); } From 3ea7c9889c875247f3272ab70880af19f36299b7 Mon Sep 17 00:00:00 2001 From: Mouad Debbar Date: Thu, 12 Aug 2021 12:49:23 -0700 Subject: [PATCH 3/3] add test --- lib/web_ui/test/engine/history_test.dart | 44 ++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/lib/web_ui/test/engine/history_test.dart b/lib/web_ui/test/engine/history_test.dart index 08b2aed6ad856..7f87324b4091d 100644 --- a/lib/web_ui/test/engine/history_test.dart +++ b/lib/web_ui/test/engine/history_test.dart @@ -39,6 +39,50 @@ void main() { } void testMain() { + test('createHistoryForExistingState', () { + TestUrlStrategy strategy; + BrowserHistory history; + + // No url strategy. + history = createHistoryForExistingState(null); + expect(history, isA()); + expect(history.urlStrategy, isNull); + + // Random history state. + strategy = TestUrlStrategy.fromEntry( + const TestHistoryEntry({'foo': 123}, null, '/'), + ); + history = createHistoryForExistingState(strategy); + expect(history, isA()); + expect(history.urlStrategy, strategy); + + // Multi-entry history state. + final Map state = { + 'serialCount': 1, + 'state': {'foo': 123}, + }; + strategy = TestUrlStrategy.fromEntry(TestHistoryEntry(state, null, '/')); + history = createHistoryForExistingState(strategy); + expect(history, isA()); + expect(history.urlStrategy, strategy); + + // Single-entry history "origin" state. + strategy = TestUrlStrategy.fromEntry( + const TestHistoryEntry({'origin': true}, null, '/'), + ); + history = createHistoryForExistingState(strategy); + expect(history, isA()); + expect(history.urlStrategy, strategy); + + // Single-entry history "flutter" state. + strategy = TestUrlStrategy.fromEntry( + const TestHistoryEntry({'flutter': true}, null, '/'), + ); + history = createHistoryForExistingState(strategy); + expect(history, isA()); + expect(history.urlStrategy, strategy); + }); + group('$SingleEntryBrowserHistory', () { final PlatformMessagesSpy spy = PlatformMessagesSpy();