bug fix

Author: joshualitt

lib/web_ui/lib/src/engine/dom.dart

@@ -2260,13 +2260,10 @@ extension DomMediaQueryListExtension on DomMediaQueryList {
   bool get matches => _matches.toDart;
 
   @JS('addListener')
-  external JSVoid _addListener(JSFunction? listener);
-  void addListener(DomEventListener? listener) => _addListener(listener?.toJS);
+  external JSVoid addListener(JSFunction? listener);
 
   @JS('removeListener')
-  external JSVoid _removeListener(JSFunction? listener);
-  void removeListener(DomEventListener? listener) =>
-      _removeListener(listener?.toJS);
+  external JSVoid removeListener(JSFunction? listener);
 }
 
 @JS()

lib/web_ui/lib/src/engine/platform_dispatcher.dart

@@ -43,8 +43,8 @@ class HighContrastSupport {
 
   /// Reference to css media query that indicates whether high contrast is on.
   final DomMediaQueryList _highContrastMediaQuery = domWindow.matchMedia(_highContrastMediaQueryString);
-  late final DomEventListener _onHighContrastChangeListener =
-      allowInterop(_onHighContrastChange);
+  late final JSFunction _onHighContrastChangeListener =
+      _onHighContrastChange.toJS;
 
   bool get isHighContrastEnabled => _highContrastMediaQuery.matches;
 
@@ -64,7 +64,7 @@ class HighContrastSupport {
     }
   }
 
-  void _onHighContrastChange(DomEvent event) {
+  JSVoid _onHighContrastChange(DomEvent event) {
     final DomMediaQueryListEvent mqEvent = event as DomMediaQueryListEvent;
     final bool isHighContrastEnabled = mqEvent.matches!;
     for (final HighContrastListener listener in _listeners) {
@@ -1031,20 +1031,20 @@ class EnginePlatformDispatcher extends ui.PlatformDispatcher {
   /// A callback that is invoked whenever [_brightnessMediaQuery] changes value.
   ///
   /// Updates the [_platformBrightness] with the new user preference.
-  DomEventListener? _brightnessMediaQueryListener;
+  JSFunction? _brightnessMediaQueryListener;
 
   /// Set the callback function for listening changes in [_brightnessMediaQuery] value.
   void _addBrightnessMediaQueryListener() {
     _updatePlatformBrightness(_brightnessMediaQuery.matches
         ? ui.Brightness.dark
         : ui.Brightness.light);
 
-    _brightnessMediaQueryListener = allowInterop((DomEvent event) {
+    _brightnessMediaQueryListener = (DomEvent event) {
       final DomMediaQueryListEvent mqEvent =
           event as DomMediaQueryListEvent;
       _updatePlatformBrightness(
           mqEvent.matches! ? ui.Brightness.dark : ui.Brightness.light);
-    });
+    }.toJS;
     _brightnessMediaQuery.addListener(_brightnessMediaQueryListener);
   }
 

lib/web_ui/lib/src/engine/view_embedder/hot_restart_cache_handler.dart

@@ -9,18 +9,35 @@ import 'package:ui/src/engine.dart';
 
 import '../dom.dart';
 
+// This exists as an example of moving JS types into engine code. In the longer
+// term, [JSArrayStore] will become a regular [JSArray] and the interface here
+// will live someplace more discoverable.  However, we can't do that until we
+// reify JS types on JS backends.
+// TODO(joshualitt): Get rid of this after we reify types on JS backends.
+@JS()
+@staticInterop
+class JSArrayStore {}
+
+extension JSArrayMethods on JSArrayStore {
+  @JS('length')
+  external JSNumber get _length;
+  int get length => _length.toDart.toInt();
+
+  external JSVoid push(JSAny? o);
+  external JSAny? at(JSNumber i);
+  JSAny? operator[](int i) => at(i.toJS);
+}
+
+JSArrayStore newArray() => <JSAny>[].toJSAnyShallow as JSArrayStore;
+
 /// This is state persistent across hot restarts that indicates what
 /// to clear.  Delay removal of old visible state to make the
 /// transition appear smooth.
 @JS('window.__flutterState')
-external JSArray? get _hotRestartStore;
-List<Object?>? get hotRestartStore =>
-    _hotRestartStore?.toObjectShallow as List<Object>?;
+external JSArrayStore? get hotRestartStore;
 
 @JS('window.__flutterState')
-external set _hotRestartStore(JSArray? nodes);
-set hotRestartStore(List<Object?>? nodes) =>
-    _hotRestartStore = nodes?.toJSAnyShallow as JSArray?;
+external set hotRestartStore(JSArrayStore? nodes);
 
 /// Handles [DomElement]s that need to be removed after a hot-restart.
 ///
@@ -31,7 +48,7 @@ set hotRestartStore(List<Object?>? nodes) =>
 /// everything in [_elements] is removed from the DOM.
 class HotRestartCacheHandler {
   HotRestartCacheHandler() {
-    if (_elements.isNotEmpty) {
+    if (_elements.length > 0) {
       // We are in a post hot-restart world, clear the elements now.
       _clearAllElements();
     }
@@ -40,32 +57,35 @@ class HotRestartCacheHandler {
   /// The js-interop layer backing [_elements].
   ///
   /// Elements are stored in a JS global array named [defaultCacheName].
-  late List<Object?>? _jsElements;
+  late JSArrayStore? _jsElements;
 
   /// The elements that need to be cleaned up after hot-restart.
-  List<Object?> get _elements {
+  JSArrayStore get _elements {
     _jsElements = hotRestartStore;
     if (_jsElements == null) {
-      _jsElements = <Object>[];
+      _jsElements = newArray();
       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 JSArrayStore elements = _elements;
+    final int length = elements.length;
+    for (int i = 0; i < length; i++) {
+      final JSAny? element = _elements[i];
+      if (domInstanceOfString(element, 'Element')) {
+        (element! as DomElement).remove();
       }
     }
-    hotRestartStore = <Object>[];
+    hotRestartStore = newArray();
   }
 
   /// Registers a [DomElement] to be removed after hot-restart.
   void registerElement(DomElement element) {
-    final List<Object?> elements = _elements;
-    elements.add(element);
-    hotRestartStore = elements;
+    // TODO(joshualitt): remove when we reify types on JS backends.
+    // ignore: unnecessary_cast
+    _elements.push(element as JSAny);
   }
 }

lib/web_ui/test/engine/view_embedder/embedding_strategy/embedding_strategy_test.dart

@@ -47,11 +47,11 @@ void doTests() {
       strategy.registerElementForCleanup(other);
       strategy.registerElementForCleanup(another);
 
-      final List<Object?> cache = hotRestartStore!;
+      final JSArrayStore cache = hotRestartStore!;
 
-      expect(cache, hasLength(3));
-      expect(cache.first, toBeCached);
-      expect(cache.last, another);
+      expect(cache.length == 3, isTrue);
+      expect(cache[0], toBeCached);
+      expect(cache[2], another);
     });
   });
 }

lib/web_ui/test/engine/view_embedder/hot_restart_cache_handler_test.dart

@@ -20,10 +20,10 @@ void doTests() {
 
       expect(cache, isNotNull);
 
-      final List<Object?>? domCache = hotRestartStore;
+      final JSArrayStore? domCache = hotRestartStore;
 
       expect(domCache, isNotNull);
-      expect(domCache, isEmpty);
+      expect(domCache!.length == 0, isTrue);
     });
   });
 
@@ -38,20 +38,9 @@ void doTests() {
       final DomElement element = createDomElement('for-test');
       cache!.registerElement(element);
 
-      final List<Object?>? domCache = hotRestartStore;
-      expect(domCache, hasLength(1));
-      expect(domCache!.last, element);
-    });
-
-    test('Registers elements in the DOM cache', () async {
-      final DomElement element = createDomElement('for-test');
-      domDocument.body!.append(element);
-
-      cache!.registerElement(element);
-
-      final List<Object?>? domCache = hotRestartStore;
-      expect(domCache, hasLength(1));
-      expect(domCache!.last, element);
+      final JSArrayStore? domCache = hotRestartStore;
+      expect(domCache!.length == 1, isTrue);
+      expect(domCache[0], element);
     });
 
     test('Clears registered elements from the DOM and the cache upon restart',
@@ -69,8 +58,8 @@ void doTests() {
       // Simulate a hot restart...
       cache = HotRestartCacheHandler();
 
-      final List<Object?>? domCache = hotRestartStore;
-      expect(domCache, hasLength(0));
+      final JSArrayStore? domCache = hotRestartStore;
+      expect(domCache!.length == 0, isTrue);
       expect(element.isConnected, isFalse); // Removed
       expect(element2.isConnected, isTrue);
     });