diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/dom.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/dom.dart index 44cd59c4d02..733a43c82fd 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/dom.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/dom.dart @@ -3591,3 +3591,10 @@ extension DomFinalizationRegistryExtension on DomFinalizationRegistry { /// Whether the current browser supports `FinalizationRegistry`. bool browserSupportsFinalizationRegistry = _finalizationRegistryConstructor != null; + +@JS() +@staticInterop +extension JSArrayExtension on JSArray { + external void push(JSAny value); + external JSNumber get length; +} diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/view_embedder/embedding_strategy/custom_element_embedding_strategy.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/view_embedder/embedding_strategy/custom_element_embedding_strategy.dart index ecf92bb3d89..94a211b23a2 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/view_embedder/embedding_strategy/custom_element_embedding_strategy.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/view_embedder/embedding_strategy/custom_element_embedding_strategy.dart @@ -4,13 +4,14 @@ import 'package:ui/src/engine/dom.dart'; +import '../hot_restart_cache_handler.dart' show registerElementForCleanup; import 'embedding_strategy.dart'; /// An [EmbeddingStrategy] that renders flutter inside a target host element. /// /// This strategy attempts to minimize DOM modifications outside of the host /// element, so it plays "nice" with other web frameworks. -class CustomElementEmbeddingStrategy extends EmbeddingStrategy { +class CustomElementEmbeddingStrategy implements EmbeddingStrategy { /// Creates a [CustomElementEmbeddingStrategy] to embed a Flutter view into [_hostElement]. CustomElementEmbeddingStrategy(this._hostElement) { _hostElement.clearChildren(); diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/view_embedder/embedding_strategy/embedding_strategy.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/view_embedder/embedding_strategy/embedding_strategy.dart index bb1c9361ae2..f9f358be215 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/view_embedder/embedding_strategy/embedding_strategy.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/view_embedder/embedding_strategy/embedding_strategy.dart @@ -2,10 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -import 'package:meta/meta.dart'; - import 'package:ui/src/engine/dom.dart'; -import 'package:ui/src/engine/view_embedder/hot_restart_cache_handler.dart'; import 'custom_element_embedding_strategy.dart'; import 'full_page_embedding_strategy.dart'; @@ -21,14 +18,6 @@ import 'full_page_embedding_strategy.dart'; /// element, provided by the web app programmer through the engine /// initialization. abstract class EmbeddingStrategy { - EmbeddingStrategy() { - // Initialize code to handle hot-restart (debug only). - assert(() { - _hotRestartCache = HotRestartCacheHandler(); - return true; - }()); - } - factory EmbeddingStrategy.create({DomElement? hostElement}) { if (hostElement != null) { return CustomElementEmbeddingStrategy(hostElement); @@ -37,9 +26,6 @@ abstract class EmbeddingStrategy { } } - /// Keeps a list of elements to be cleaned up at hot-restart. - HotRestartCacheHandler? _hotRestartCache; - void initialize({ Map? hostElementAttributes, }); @@ -49,10 +35,4 @@ abstract class EmbeddingStrategy { /// Attaches the resourceHost element into the hostElement. void attachResourcesHost(DomElement resourceHost, {DomElement? nextTo}); - - /// Registers a [DomElement] to be cleaned up after hot restart. - @mustCallSuper - void registerElementForCleanup(DomElement element) { - _hotRestartCache?.registerElement(element); - } } diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/view_embedder/embedding_strategy/full_page_embedding_strategy.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/view_embedder/embedding_strategy/full_page_embedding_strategy.dart index b8abc6e2427..fdb7028fe99 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/view_embedder/embedding_strategy/full_page_embedding_strategy.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/view_embedder/embedding_strategy/full_page_embedding_strategy.dart @@ -5,13 +5,14 @@ import 'package:ui/src/engine/dom.dart'; import 'package:ui/src/engine/util.dart' show setElementStyle; +import '../hot_restart_cache_handler.dart' show registerElementForCleanup; import 'embedding_strategy.dart'; /// An [EmbeddingStrategy] that takes over the whole web page. /// /// This strategy takes over the element, modifies the viewport meta-tag, /// and ensures that the root Flutter view covers the whole screen. -class FullPageEmbeddingStrategy extends EmbeddingStrategy { +class FullPageEmbeddingStrategy implements EmbeddingStrategy { @override void initialize({ Map? hostElementAttributes, diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/view_embedder/hot_restart_cache_handler.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/view_embedder/hot_restart_cache_handler.dart index c37aff2504c..1751743280e 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/view_embedder/hot_restart_cache_handler.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/view_embedder/hot_restart_cache_handler.dart @@ -4,6 +4,7 @@ import 'dart:js_interop'; +import 'package:meta/meta.dart'; import 'package:ui/src/engine.dart'; import '../dom.dart'; @@ -12,59 +13,61 @@ import '../dom.dart'; /// to clear. Delay removal of old visible state to make the /// transition appear smooth. @JS('window.__flutterState') -external JSArray? get _hotRestartStore; -List? get hotRestartStore => - _hotRestartStore?.toObjectShallow as List?; +external JSArray? get _jsHotRestartStore; @JS('window.__flutterState') -external set _hotRestartStore(JSArray? nodes); -set hotRestartStore(List? nodes) => - _hotRestartStore = nodes?.toJSAnyShallow as JSArray?; +external set _jsHotRestartStore(JSArray? nodes); /// Handles [DomElement]s that need to be removed after a hot-restart. /// -/// Elements are stored in an [_elements] list, backed by a global JS variable, -/// named [defaultCacheName]. +/// This class shouldn't be used directly. It's only made public for testing +/// purposes. Instead, use [registerElementForCleanup]. +/// +/// Elements are stored in a [JSArray] stored globally at `window.__flutterState`. /// /// When the app hot-restarts (and a new instance of this class is created), -/// everything in [_elements] is removed from the DOM. +/// all elements in the global [JSArray] is removed from the DOM. class HotRestartCacheHandler { + @visibleForTesting HotRestartCacheHandler() { - if (_elements.isNotEmpty) { + _resetHotRestartStore(); + } + + /// Removes every element that was registered prior to the hot-restart from + /// the DOM. + void _resetHotRestartStore() { + final JSArray? jsStore = _jsHotRestartStore; + + if (jsStore != null) { // We are in a post hot-restart world, clear the elements now. - _clearAllElements(); - } - } - - /// The js-interop layer backing [_elements]. - /// - /// Elements are stored in a JS global array named [defaultCacheName]. - late List? _jsElements; - - /// The elements that need to be cleaned up after hot-restart. - List get _elements { - _jsElements = hotRestartStore; - if (_jsElements == null) { - _jsElements = []; - hotRestartStore = _jsElements; - } - return _jsElements!; - } - - /// Removes every element from [_elements] and empties the list. - void _clearAllElements() { - for (final Object? element in _elements) { - if (element is DomElement) { - element.remove(); + final List store = jsStore.toObjectShallow as List; + for (final Object? element in store) { + if (element != null) { + (element as DomElement).remove(); + } } } - hotRestartStore = []; + _jsHotRestartStore = JSArray(); } /// Registers a [DomElement] to be removed after hot-restart. + @visibleForTesting void registerElement(DomElement element) { - final List elements = _elements; - elements.add(element); - hotRestartStore = elements; + _jsHotRestartStore!.push(element); } } + +final HotRestartCacheHandler? _hotRestartCache = () { + // In release mode, we don't need a hot restart cache, so we leave it null. + HotRestartCacheHandler? cache; + assert(() { + cache = HotRestartCacheHandler(); + return true; + }()); + return cache; +}(); + +/// Registers a [DomElement] to be cleaned up after hot restart. +void registerElementForCleanup(DomElement element) { + _hotRestartCache?.registerElement(element); +} diff --git a/engine/src/flutter/lib/web_ui/test/engine/view_embedder/embedding_strategy/embedding_strategy_test.dart b/engine/src/flutter/lib/web_ui/test/engine/view_embedder/embedding_strategy/embedding_strategy_test.dart index f13cf67c4cb..f254455ecd5 100644 --- a/engine/src/flutter/lib/web_ui/test/engine/view_embedder/embedding_strategy/embedding_strategy_test.dart +++ b/engine/src/flutter/lib/web_ui/test/engine/view_embedder/embedding_strategy/embedding_strategy_test.dart @@ -11,7 +11,6 @@ import 'package:ui/src/engine/dom.dart'; import 'package:ui/src/engine/view_embedder/embedding_strategy/custom_element_embedding_strategy.dart'; import 'package:ui/src/engine/view_embedder/embedding_strategy/embedding_strategy.dart'; import 'package:ui/src/engine/view_embedder/embedding_strategy/full_page_embedding_strategy.dart'; -import 'package:ui/src/engine/view_embedder/hot_restart_cache_handler.dart'; void main() { internalBootstrapBrowserTest(() => doTests); @@ -35,24 +34,4 @@ void doTests() { expect(strategy, isA()); }); }); - - group('registerElementForCleanup', () { - test('stores elements in a global domCache', () async { - final EmbeddingStrategy strategy = EmbeddingStrategy.create(); - - final DomElement toBeCached = createDomElement('some-element-to-cache'); - final DomElement other = createDomElement('other-element-to-cache'); - final DomElement another = createDomElement('another-element-to-cache'); - - strategy.registerElementForCleanup(toBeCached); - strategy.registerElementForCleanup(other); - strategy.registerElementForCleanup(another); - - final List cache = hotRestartStore!; - - expect(cache, hasLength(3)); - expect(cache.first, toBeCached); - expect(cache.last, another); - }); - }); } diff --git a/engine/src/flutter/lib/web_ui/test/engine/view_embedder/hot_restart_cache_handler_test.dart b/engine/src/flutter/lib/web_ui/test/engine/view_embedder/hot_restart_cache_handler_test.dart index 7525b571d9a..934718ce75e 100644 --- a/engine/src/flutter/lib/web_ui/test/engine/view_embedder/hot_restart_cache_handler_test.dart +++ b/engine/src/flutter/lib/web_ui/test/engine/view_embedder/hot_restart_cache_handler_test.dart @@ -5,31 +5,58 @@ @TestOn('browser') library; +import 'dart:js_interop'; + import 'package:test/bootstrap/browser.dart'; import 'package:test/test.dart'; import 'package:ui/src/engine/dom.dart'; import 'package:ui/src/engine/view_embedder/hot_restart_cache_handler.dart'; +@JS('window.__flutterState') +external JSArray? get _jsHotRestartStore; + +@JS('window.__flutterState') +external set _jsHotRestartStore(JSArray? nodes); + void main() { internalBootstrapBrowserTest(() => doTests); } void doTests() { - group('Constructor', () { - test('Creates a cache in the JS environment', () async { - final HotRestartCacheHandler cache = HotRestartCacheHandler(); + tearDown(() { + _jsHotRestartStore = null; + }); - expect(cache, isNotNull); + group('registerElementForCleanup', () { + test('stores elements in a global cache', () async { + final DomElement toBeCached = createDomElement('some-element-to-cache'); + final DomElement other = createDomElement('other-element-to-cache'); + final DomElement another = createDomElement('another-element-to-cache'); - final List? domCache = hotRestartStore; + registerElementForCleanup(toBeCached); + registerElementForCleanup(other); + registerElementForCleanup(another); - expect(domCache, isNotNull); - expect(domCache, isEmpty); + expect(_jsHotRestartStore!.toDart, [ + toBeCached, + other, + another, + ]); }); }); - group('registerElement', () { - HotRestartCacheHandler? cache; + group('HotRestartCacheHandler Constructor', () { + test('Creates a cache in the JS environment', () async { + HotRestartCacheHandler(); + + expect(_jsHotRestartStore, isNotNull); + // For dart2wasm, we have to check the length this way. + expect(_jsHotRestartStore!.length, 0.toJS); + }); + }); + + group('HotRestartCacheHandler.registerElement', () { + late HotRestartCacheHandler cache; setUp(() { cache = HotRestartCacheHandler(); @@ -37,22 +64,18 @@ void doTests() { test('Registers an element in the DOM cache', () async { final DomElement element = createDomElement('for-test'); - cache!.registerElement(element); + cache.registerElement(element); - final List? domCache = hotRestartStore; - expect(domCache, hasLength(1)); - expect(domCache!.last, element); + expect(_jsHotRestartStore!.toDart, [element]); }); test('Registers elements in the DOM cache', () async { final DomElement element = createDomElement('for-test'); domDocument.body!.append(element); - cache!.registerElement(element); + cache.registerElement(element); - final List? domCache = hotRestartStore; - expect(domCache, hasLength(1)); - expect(domCache!.last, element); + expect(_jsHotRestartStore!.toDart, [element]); }); test('Clears registered elements from the DOM and the cache upon restart', @@ -62,7 +85,7 @@ void doTests() { domDocument.body!.append(element); domDocument.body!.append(element2); - cache!.registerElement(element); + cache.registerElement(element); expect(element.isConnected, isTrue); expect(element2.isConnected, isTrue); @@ -70,8 +93,8 @@ void doTests() { // Simulate a hot restart... cache = HotRestartCacheHandler(); - final List? domCache = hotRestartStore; - expect(domCache, hasLength(0)); + // For dart2wasm, we have to check the length this way. + expect(_jsHotRestartStore!.length, 0.toJS); expect(element.isConnected, isFalse); // Removed expect(element2.isConnected, isTrue); });