diff --git a/lib/web_ui/lib/src/engine/navigation/history.dart b/lib/web_ui/lib/src/engine/navigation/history.dart index 9c6de07e95edf..f472c8a340643 100644 --- a/lib/web_ui/lib/src/engine/navigation/history.dart +++ b/lib/web_ui/lib/src/engine/navigation/history.dart @@ -4,6 +4,7 @@ import 'dart:html' as html; +import 'package:meta/meta.dart'; import 'package:ui/ui.dart' as ui; import '../platform_dispatcher.dart'; @@ -47,6 +48,7 @@ abstract class BrowserHistory { /// The strategy to interact with html browser history. UrlStrategy? get urlStrategy; + bool _isTornDown = false; bool _isDisposed = false; void _setupStrategy(UrlStrategy strategy) { @@ -55,6 +57,20 @@ abstract class BrowserHistory { ); } + /// Release any resources held by this [BrowserHistory] instance. + /// + /// This method has no effect on the browser history entries. Use [tearDown] + /// instead to revert this instance's modifications to browser history + /// entries. + @mustCallSuper + void dispose() { + if (_isDisposed || urlStrategy == null) { + return; + } + _isDisposed = true; + _unsubscribe(); + } + /// Exit this application and return to the previous page. Future exit() async { if (urlStrategy != null) { @@ -196,11 +212,12 @@ class MultiEntriesBrowserHistory extends BrowserHistory { @override Future tearDown() async { - if (_isDisposed || urlStrategy == null) { + dispose(); + + if (_isTornDown || urlStrategy == null) { return; } - _isDisposed = true; - _unsubscribe(); + _isTornDown = true; // Restores the html browser history. assert(_hasSerialCount(currentState)); @@ -367,11 +384,12 @@ class SingleEntryBrowserHistory extends BrowserHistory { @override Future tearDown() async { - if (_isDisposed || urlStrategy == null) { + dispose(); + + if (_isTornDown || urlStrategy == null) { return; } - _isDisposed = true; - _unsubscribe(); + _isTornDown = true; // We need to remove the flutter entry that we pushed in setup. await urlStrategy!.go(-1); diff --git a/lib/web_ui/lib/src/engine/window.dart b/lib/web_ui/lib/src/engine/window.dart index 17c1271513529..5ebb31825512f 100644 --- a/lib/web_ui/lib/src/engine/window.dart +++ b/lib/web_ui/lib/src/engine/window.dart @@ -13,6 +13,7 @@ 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'; @@ -51,6 +52,9 @@ class EngineFlutterWindow extends ui.SingletonFlutterWindow { if (_isUrlStrategySet) { _browserHistory = createHistoryForExistingState(_customUrlStrategy); } + registerHotRestartListener(() { + _browserHistory?.dispose(); + }); } final Object _windowId; diff --git a/lib/web_ui/test/engine/history_test.dart b/lib/web_ui/test/engine/history_test.dart index 7f87324b4091d..2593b42f5e542 100644 --- a/lib/web_ui/test/engine/history_test.dart +++ b/lib/web_ui/test/engine/history_test.dart @@ -8,6 +8,7 @@ import 'dart:async'; import 'dart:html' as html; +import 'package:quiver/testing/async.dart'; import 'package:test/bootstrap/browser.dart'; import 'package:test/test.dart'; import 'package:ui/src/engine.dart' show window; @@ -121,6 +122,68 @@ void testMain() { // TODO(mdebbar): https://github.com/flutter/flutter/issues/50836 skip: browserEngine == BrowserEngine.edge); + test('disposes of its listener without touching history', () async { + const String unwrappedOriginState = 'initial state'; + final Map wrappedOriginState = _wrapOriginState(unwrappedOriginState); + + final TestUrlStrategy strategy = TestUrlStrategy.fromEntry( + const TestHistoryEntry(unwrappedOriginState, null, '/initial'), + ); + expect(strategy.listeners, isEmpty); + + await window.debugInitializeHistory(strategy, useSingle: true); + + + // There should be one `popstate` listener and two history entries. + expect(strategy.listeners, hasLength(1)); + expect(strategy.history, hasLength(2)); + expect(strategy.history[0].state, wrappedOriginState); + expect(strategy.history[0].url, '/initial'); + expect(strategy.history[1].state, flutterState); + expect(strategy.history[1].url, '/initial'); + + FakeAsync().run((FakeAsync fakeAsync) { + window.browserHistory.dispose(); + // The `TestUrlStrategy` implementation uses microtasks to schedule the + // removal of event listeners. + fakeAsync.flushMicrotasks(); + }); + + // After disposing, there should no listeners, and the history entries + // remain unaffected. + expect(strategy.listeners, isEmpty); + expect(strategy.history, hasLength(2)); + expect(strategy.history[0].state, wrappedOriginState); + expect(strategy.history[0].url, '/initial'); + expect(strategy.history[1].state, flutterState); + expect(strategy.history[1].url, '/initial'); + + // An extra call to dispose should be safe. + FakeAsync().run((FakeAsync fakeAsync) { + expect(() => window.browserHistory.dispose(), returnsNormally); + fakeAsync.flushMicrotasks(); + }); + + // Same expectations should remain true after the second dispose. + expect(strategy.listeners, isEmpty); + expect(strategy.history, hasLength(2)); + expect(strategy.history[0].state, wrappedOriginState); + expect(strategy.history[0].url, '/initial'); + expect(strategy.history[1].state, flutterState); + expect(strategy.history[1].url, '/initial'); + + // Can still teardown after being disposed. + await window.browserHistory.tearDown(); + expect(strategy.history, hasLength(2)); + expect(strategy.currentEntry.state, unwrappedOriginState); + expect(strategy.currentEntry.url, '/initial'); + }); + + test('disposes gracefully when url strategy is null', () async { + await window.debugInitializeHistory(null, useSingle: true); + expect(() => window.browserHistory.dispose(), returnsNormally); + }); + test('browser back button pops routes correctly', () async { final TestUrlStrategy strategy = TestUrlStrategy.fromEntry( const TestHistoryEntry(null, null, '/home'), @@ -326,6 +389,62 @@ void testMain() { // TODO(mdebbar): https://github.com/flutter/flutter/issues/50836 skip: browserEngine == BrowserEngine.edge); + test('disposes of its listener without touching history', () async { + const String untaggedState = 'initial state'; + final Map taggedState = _tagStateWithSerialCount(untaggedState, 0); + + final TestUrlStrategy strategy = TestUrlStrategy.fromEntry( + const TestHistoryEntry(untaggedState, null, '/initial'), + ); + expect(strategy.listeners, isEmpty); + + await window.debugInitializeHistory(strategy, useSingle: false); + + + // There should be one `popstate` listener and one history entry. + expect(strategy.listeners, hasLength(1)); + expect(strategy.history, hasLength(1)); + expect(strategy.history.single.state, taggedState); + expect(strategy.history.single.url, '/initial'); + + FakeAsync().run((FakeAsync fakeAsync) { + window.browserHistory.dispose(); + // The `TestUrlStrategy` implementation uses microtasks to schedule the + // removal of event listeners. + fakeAsync.flushMicrotasks(); + }); + + // After disposing, there should no listeners, and the history entries + // remain unaffected. + expect(strategy.listeners, isEmpty); + expect(strategy.history, hasLength(1)); + expect(strategy.history.single.state, taggedState); + expect(strategy.history.single.url, '/initial'); + + // An extra call to dispose should be safe. + FakeAsync().run((FakeAsync fakeAsync) { + expect(() => window.browserHistory.dispose(), returnsNormally); + fakeAsync.flushMicrotasks(); + }); + + // Same expectations should remain true after the second dispose. + expect(strategy.listeners, isEmpty); + expect(strategy.history, hasLength(1)); + expect(strategy.history.single.state, taggedState); + expect(strategy.history.single.url, '/initial'); + + // Can still teardown after being disposed. + await window.browserHistory.tearDown(); + expect(strategy.history, hasLength(1)); + expect(strategy.history.single.state, untaggedState); + expect(strategy.history.single.url, '/initial'); + }); + + test('disposes gracefully when url strategy is null', () async { + await window.debugInitializeHistory(null, useSingle: false); + expect(() => window.browserHistory.dispose(), returnsNormally); + }); + test('browser back button push route information correctly', () async { final TestUrlStrategy strategy = TestUrlStrategy.fromEntry( const TestHistoryEntry('initial state', null, '/home'),