[web] Hook the new JS API to the FlutterViewManager

lib/web_ui/lib/src/engine/app_bootstrap.dart

@@ -2,15 +2,12 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-import 'dart:js_interop' show JSAny;
-
-import 'package:ui/src/engine/dom.dart';
-
 import 'configuration.dart';
 import 'js_interop/js_app.dart';
 import 'js_interop/js_loader.dart';
 
 import 'platform_dispatcher.dart';
+import 'view_embedder/flutter_view_manager.dart';
 
 /// The type of a function that initializes an engine (in Dart).
 typedef InitEngineFn = Future<void> Function([JsFlutterConfiguration? params]);
@@ -66,22 +63,18 @@ class AppBootstrap {
     });
   }
 
+  FlutterViewManager get viewManager => EnginePlatformDispatcher.instance.viewManager;
+
   /// Represents the App that was just started, and its JS API.
   FlutterApp _prepareFlutterApp() {
     return FlutterApp(
       addView: (JsFlutterViewOptions options) async {
         assert(configuration.multiViewEnabled, 'Cannot addView when multiView is not enabled');
-        // NEXT: Create a view from JS Options, then register it in
-        // the viewManager... (Or have a method that creates-and-registers a view)
-        // final EngineFlutterView view = View.createFromOptions(options);
-        // return EnginePlatformDispatcher.instance.viewManager.registerView(view).viewId;
-        domWindow.console.warn('Create view from JS is unimplemented!');
-        domWindow.console.warn((options as JSAny).toObjectDeep);
-        return -1;
+        return viewManager.createAndRegisterView(options).viewId;
       },
       removeView: (int viewId) async {
         assert(configuration.multiViewEnabled, 'Cannot removeView when multiView is not enabled');
-        return EnginePlatformDispatcher.instance.viewManager.unregisterView(viewId);
+        return viewManager.disposeAndUnregisterView(viewId);
       }
     );
   }

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

@@ -104,12 +104,7 @@ class EnginePlatformDispatcher extends ui.PlatformDispatcher {
     _disconnectFontSizeObserver();
     _removeLocaleChangedListener();
     HighContrastSupport.instance.removeListener(_updateHighContrast);
-
-    // We need to call `toList()` in order to avoid concurrent modification inside
-    // the loop (`view.dispose()` removes itself from the view map).
-    for (final EngineFlutterView view in views.toList()) {
-      view.dispose();
-    }
+    viewManager.dispose();
   }
 
   /// Receives all events related to platform configuration changes.
@@ -136,19 +131,7 @@ class EnginePlatformDispatcher extends ui.PlatformDispatcher {
     EngineFlutterDisplay.instance,
   ];
 
-  final FlutterViewManager viewManager = FlutterViewManager();
-
-  /// Adds [view] to the platform dispatcher's registry of [views].
-  void registerView(EngineFlutterView view) {
-    viewManager.registerView(view);
-  }
-
-  /// Removes [view] from the platform dispatcher's registry of [views].
-  ///
-  /// Nothing happens if the view is not already registered.
-  void unregisterView(EngineFlutterView view) {
-    viewManager.unregisterView(view.viewId);
-  }
+  late final FlutterViewManager viewManager = FlutterViewManager(this);
 
   /// The current list of windows.
   @override

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

@@ -6,6 +6,10 @@ import 'package:ui/src/engine.dart';
 
 /// Encapsulates view objects, and their optional metadata indexed by `viewId`.
 class FlutterViewManager {
+  FlutterViewManager(this._dispatcher);
+
+  final EnginePlatformDispatcher _dispatcher;
+
   // A map of EngineFlutterViews indexed by their viewId.
   final Map<int, EngineFlutterView> _viewData = <int, EngineFlutterView>{};
   // A map of (optional) JsFlutterViewOptions, indexed by their viewId.
@@ -26,6 +30,14 @@ class FlutterViewManager {
     return _viewData[viewId];
   }
 
+  EngineFlutterView createAndRegisterView(
+    JsFlutterViewOptions jsViewOptions,
+  ) {
+    final EngineFlutterView view = EngineFlutterView(_dispatcher, jsViewOptions.hostElement);
+    registerView(view, jsViewOptions: jsViewOptions);
+    return view;
+  }
+
   /// Stores a [view] and its (optional) [jsViewOptions], indexed by `viewId`.
   ///
   /// Returns the registered [view].
@@ -34,9 +46,7 @@ class FlutterViewManager {
     JsFlutterViewOptions? jsViewOptions,
   }) {
     final int viewId = view.viewId;
-    // This assert knows of kImplicitViewId, so some tests like test/engine/routing_test.dart
-    // can pass. The kImplicitViewId exception should be removed!
-    assert(viewId == kImplicitViewId || !_viewData.containsKey(viewId)); // Adding the same view twice?
+    assert(!_viewData.containsKey(viewId)); // Adding the same view twice?
 
     // Store the view, and the jsViewOptions, if any...
     _viewData[viewId] = view;
@@ -48,6 +58,16 @@ class FlutterViewManager {
     return view;
   }
 
+  JsFlutterViewOptions? disposeAndUnregisterView(int viewId) {
+    final EngineFlutterView? view = _viewData[viewId];
+    if (view == null) {
+      return null;
+    }
+    final JsFlutterViewOptions? options = unregisterView(viewId);
+    view.dispose();
+    return options;
+  }
+
   /// Un-registers [viewId].
   ///
   /// Returns its [JsFlutterViewOptions] (if any).
@@ -65,4 +85,13 @@ class FlutterViewManager {
   JsFlutterViewOptions? getOptions(int viewId) {
     return _jsViewOptions[viewId];
   }
+
+  void dispose() {
+    // We need to call `toList()` in order to avoid concurrent modification
+    // inside the loop.
+    _viewData.keys.toList().forEach(disposeAndUnregisterView);
+    // Let listeners receive the unregistration events from the loop above, then
+    // close the stream.
+    _onViewsChangedController.close();
+  }
 }

lib/web_ui/lib/src/engine/window.dart

@@ -31,6 +31,8 @@ const bool debugPrintPlatformMessages = false;
 /// The view ID for the implicit flutter view provided by the platform.
 const int kImplicitViewId = 0;
 
+int _nextViewId = kImplicitViewId + 1;
+
 /// Represents all views in the Flutter Web Engine.
 ///
 /// In addition to everything defined in [ui.FlutterView], this class adds
@@ -41,7 +43,6 @@ base class EngineFlutterView implements ui.FlutterView {
   /// The [hostElement] parameter specifies the container in the DOM into which
   /// the Flutter view will be rendered.
   factory EngineFlutterView(
-    int viewId,
     EnginePlatformDispatcher platformDispatcher,
     DomElement hostElement,
   ) = _EngineFlutterViewImpl;
@@ -55,13 +56,17 @@ base class EngineFlutterView implements ui.FlutterView {
     DomElement? hostElement,
   )   : embeddingStrategy = EmbeddingStrategy.create(hostElement: hostElement),
         dimensionsProvider = DimensionsProvider.create(hostElement: hostElement) {
-    platformDispatcher.registerView(this);
     // The embeddingStrategy will take care of cleaning up the rootElement on
     // hot restart.
     embeddingStrategy.attachGlassPane(dom.rootElement);
     registerHotRestartListener(dispose);
   }
 
+  static EngineFlutterWindow implicit(
+    EnginePlatformDispatcher platformDispatcher,
+    DomElement? hostElement,
+  ) => EngineFlutterWindow._(platformDispatcher, hostElement);
+
   @override
   final int viewId;
 
@@ -80,8 +85,10 @@ base class EngineFlutterView implements ui.FlutterView {
   /// tree and any event listeners.
   @mustCallSuper
   void dispose() {
+    if (isDisposed) {
+      return;
+    }
     isDisposed = true;
-    platformDispatcher.unregisterView(this);
     dimensionsProvider.close();
     dom.rootElement.remove();
     // TODO(harryterkelsen): What should we do about this in multi-view?
@@ -186,19 +193,17 @@ base class EngineFlutterView implements ui.FlutterView {
 
 final class _EngineFlutterViewImpl extends EngineFlutterView {
   _EngineFlutterViewImpl(
-    super.viewId,
-    super.platformDispatcher,
-    super.hostElement,
-  ) : super._();
+    EnginePlatformDispatcher platformDispatcher,
+    DomElement hostElement,
+  ) : super._(_nextViewId++, platformDispatcher, hostElement);
 }
 
 /// The Web implementation of [ui.SingletonFlutterWindow].
 final class EngineFlutterWindow extends EngineFlutterView implements ui.SingletonFlutterWindow {
-  EngineFlutterWindow(
-    super.viewId,
-    super.platformDispatcher,
-    super.hostElement,
-  ) : super._() {
+  EngineFlutterWindow._(
+    EnginePlatformDispatcher platformDispatcher,
+    DomElement? hostElement,
+  ) : super._(kImplicitViewId, platformDispatcher, hostElement) {
     if (ui_web.isCustomUrlStrategySet) {
       _browserHistory = createHistoryForExistingState(ui_web.urlStrategy);
     }
@@ -616,11 +621,14 @@ EngineFlutterWindow? _window;
 EngineFlutterWindow ensureImplicitViewInitialized({
   DomElement? hostElement,
 }) {
-  return _window ??= EngineFlutterWindow(
-    kImplicitViewId,
-    EnginePlatformDispatcher.instance,
-    hostElement,
-  );
+  if (_window == null) {
+    _window = EngineFlutterView.implicit(
+      EnginePlatformDispatcher.instance,
+      hostElement,
+    );
+    EnginePlatformDispatcher.instance.viewManager.registerView(_window!);
+  }
+  return _window!;
 }
 
 /// The Web implementation of [ui.ViewPadding].

lib/web_ui/test/engine/platform_dispatcher/platform_dispatcher_test.dart

@@ -203,27 +203,26 @@ void testMain() {
 
     test('disposes all its views', () {
       final EnginePlatformDispatcher dispatcher = EnginePlatformDispatcher();
-      final EngineFlutterView view20 =
-          EngineFlutterView(20, dispatcher, createDomHTMLDivElement());
-      final EngineFlutterView view21 =
-          EngineFlutterView(21, dispatcher, createDomHTMLDivElement());
-      final EngineFlutterView view22 =
-          EngineFlutterView(22, dispatcher, createDomHTMLDivElement());
-
-      // Add this again when views don't register themselves upon instantiation.
-      // dispatcher
-      //   ..registerView(view20)
-      //   ..registerView(view21)
-      //   ..registerView(view22);
-
-      expect(view20.isDisposed, isFalse);
-      expect(view21.isDisposed, isFalse);
-      expect(view22.isDisposed, isFalse);
+      final EngineFlutterView view1 =
+          EngineFlutterView(dispatcher, createDomHTMLDivElement());
+      final EngineFlutterView view2 =
+          EngineFlutterView(dispatcher, createDomHTMLDivElement());
+      final EngineFlutterView view3 =
+          EngineFlutterView(dispatcher, createDomHTMLDivElement());
+
+      dispatcher.viewManager
+        ..registerView(view1)
+        ..registerView(view2)
+        ..registerView(view3);
+
+      expect(view1.isDisposed, isFalse);
+      expect(view2.isDisposed, isFalse);
+      expect(view3.isDisposed, isFalse);
 
       dispatcher.dispose();
-      expect(view20.isDisposed, isTrue);
-      expect(view21.isDisposed, isTrue);
-      expect(view22.isDisposed, isTrue);
+      expect(view1.isDisposed, isTrue);
+      expect(view2.isDisposed, isTrue);
+      expect(view3.isDisposed, isTrue);
     });
   });
 }

lib/web_ui/test/engine/routing_test.dart

@@ -11,7 +11,6 @@ import 'package:ui/ui.dart' as ui;
 import 'package:ui/ui_web/src/ui_web.dart' as ui_web;
 
 import '../common/matchers.dart';
-import '../common/test_initialization.dart';
 import 'history_test.dart';
 
 const MethodCodec codec = JSONMethodCodec();
@@ -28,26 +27,19 @@ void main() {
 }
 
 void testMain() {
-  EngineFlutterWindow? savedWindow;
   late EngineFlutterWindow myWindow;
 
-  setUpAll(() async {
-    await bootstrapAndRunApp();
-  });
+  final EnginePlatformDispatcher dispatcher = EnginePlatformDispatcher.instance;
 
   setUp(() {
-    savedWindow = EnginePlatformDispatcher.instance.implicitView;
-    myWindow = EngineFlutterWindow(0, EnginePlatformDispatcher.instance, createDomHTMLDivElement());
+    myWindow = EngineFlutterView.implicit(dispatcher, createDomHTMLDivElement());
+    dispatcher.viewManager.registerView(myWindow);
   });
 
   tearDown(() async {
+    dispatcher.viewManager.unregisterView(myWindow.viewId);
     await myWindow.resetHistory();
-
-    // Restore the original implicit view.
-    EnginePlatformDispatcher.instance.unregisterView(myWindow);
-    if (savedWindow != null) {
-      EnginePlatformDispatcher.instance.registerView(savedWindow!);
-    }
+    myWindow.dispose();
   });
 
   // For now, web always has an implicit view provided by the web engine.

lib/web_ui/test/engine/surface/platform_view_test.dart

@@ -14,8 +14,7 @@ import '../../common/matchers.dart';
 import '../../common/test_initialization.dart';
 
 const MethodCodec codec = StandardMethodCodec();
-final EngineFlutterWindow window = EngineFlutterWindow(
-  0,
+final EngineFlutterWindow window = EngineFlutterView.implicit(
   EnginePlatformDispatcher.instance,
   createDomHTMLDivElement(),
 );

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

@@ -17,33 +17,30 @@ void main() {
 
 Future<void> doTests() async {
   group('FlutterViewManager', () {
-    int nextViewId = 1; // We keep track of this so we don't have to unregister from PlatformDispatcher.
-
     final EnginePlatformDispatcher platformDispatcher = EnginePlatformDispatcher.instance;
-    final FlutterViewManager viewManager = FlutterViewManager();
+    final FlutterViewManager viewManager = FlutterViewManager(platformDispatcher);
 
     group('registerView', () {
       test('can register view', () {
-        final int viewId = nextViewId++;
-        final EngineFlutterView view = EngineFlutterView(viewId, platformDispatcher, createDomElement('div'));
+        final EngineFlutterView view = EngineFlutterView(platformDispatcher, createDomElement('div'));
+        final int viewId = view.viewId;
 
         viewManager.registerView(view);
 
         expect(viewManager[viewId], view);
       });
 
       test('fails if the same viewId is already registered', () {
-        final int viewId = nextViewId++;
-        final EngineFlutterView view = EngineFlutterView(viewId, platformDispatcher, createDomElement('div'));
+        final EngineFlutterView view = EngineFlutterView(platformDispatcher, createDomElement('div'));
 
         viewManager.registerView(view);
 
         expect(() { viewManager.registerView(view); }, throwsAssertionError);
       });
 
       test('stores JSOptions that getOptions can retrieve', () {
-        final int viewId = nextViewId++;
-        final EngineFlutterView view = EngineFlutterView(viewId, platformDispatcher, createDomElement('div'));
+        final EngineFlutterView view = EngineFlutterView(platformDispatcher, createDomElement('div'));
+        final int viewId = view.viewId;
         final JsFlutterViewOptions expectedOptions = jsify(<String, Object?>{
           'hostElement': createDomElement('div'),
         }) as JsFlutterViewOptions;
@@ -57,8 +54,8 @@ Future<void> doTests() async {
 
     group('unregisterView', () {
       test('unregisters a view', () {
-        final int viewId = nextViewId++;
-        final EngineFlutterView view = EngineFlutterView(viewId, platformDispatcher, createDomElement('div'));
+        final EngineFlutterView view = EngineFlutterView(platformDispatcher, createDomElement('div'));
+        final int viewId = view.viewId;
 
         viewManager.registerView(view);
         expect(viewManager[viewId], isNotNull);
@@ -78,8 +75,8 @@ Future<void> doTests() async {
       });
 
       test('on view registered/unregistered - fires event', () async {
-        final int viewId = nextViewId++;
-        final EngineFlutterView view = EngineFlutterView(viewId, platformDispatcher, createDomElement('div'));
+        final EngineFlutterView view = EngineFlutterView(platformDispatcher, createDomElement('div'));
+        final int viewId = view.viewId;
 
         final Future<List<void>> viewChangeEvents = onViewsChanged.toList();
         viewManager.registerView(view);