From 66d4a32e2214d23d020d464a495fedeaf3c17520 Mon Sep 17 00:00:00 2001 From: Mouad Debbar Date: Fri, 8 Sep 2023 13:58:57 -0400 Subject: [PATCH 1/2] [web] Stop using flutterViewEmbedder for platform views --- lib/web_ui/lib/src/engine/dom.dart | 1 + .../lib/src/engine/platform_dispatcher.dart | 10 +---- .../platform_views/message_handler.dart | 19 +++++---- lib/web_ui/lib/src/engine/window.dart | 9 +++- .../platform_views/message_handler_test.dart | 42 +++++++++++++------ 5 files changed, 49 insertions(+), 32 deletions(-) diff --git a/lib/web_ui/lib/src/engine/dom.dart b/lib/web_ui/lib/src/engine/dom.dart index b8be197e5301f..6a7547f250196 100644 --- a/lib/web_ui/lib/src/engine/dom.dart +++ b/lib/web_ui/lib/src/engine/dom.dart @@ -529,6 +529,7 @@ extension DomElementExtension on DomElement { createDomListWrapper(_children); external DomElement? get firstElementChild; + external DomElement? get lastElementChild; external DomElement? get nextElementSibling; diff --git a/lib/web_ui/lib/src/engine/platform_dispatcher.dart b/lib/web_ui/lib/src/engine/platform_dispatcher.dart index e2ddae4ff0e0a..5020981650830 100644 --- a/lib/web_ui/lib/src/engine/platform_dispatcher.dart +++ b/lib/web_ui/lib/src/engine/platform_dispatcher.dart @@ -432,8 +432,6 @@ class EnginePlatformDispatcher extends ui.PlatformDispatcher { }; } - PlatformViewMessageHandler? _platformViewMessageHandler; - void _sendPlatformMessage( String name, ByteData? data, @@ -595,13 +593,7 @@ class EnginePlatformDispatcher extends ui.PlatformDispatcher { return; case 'flutter/platform_views': - _platformViewMessageHandler ??= PlatformViewMessageHandler( - contentManager: platformViewManager, - contentHandler: (DomElement content) { - flutterViewEmbedder.glassPaneElement.append(content); - }, - ); - _platformViewMessageHandler!.handlePlatformViewCall(data, callback!); + implicitView!.platformViewMessageHandler.handlePlatformViewCall(data, callback!); return; case 'flutter/accessibility': diff --git a/lib/web_ui/lib/src/engine/platform_views/message_handler.dart b/lib/web_ui/lib/src/engine/platform_views/message_handler.dart index c55380ec6a325..68c0fe340cac8 100644 --- a/lib/web_ui/lib/src/engine/platform_views/message_handler.dart +++ b/lib/web_ui/lib/src/engine/platform_views/message_handler.dart @@ -5,9 +5,11 @@ import 'dart:typed_data'; import '../dom.dart'; +import '../platform_dispatcher.dart'; import '../services.dart'; import '../util.dart'; import 'content_manager.dart'; +import 'slots.dart'; /// The signature for a callback for a Platform Message. From the `ui` package. /// Copied here so there's no circular dependencies. @@ -40,24 +42,23 @@ typedef PlatformViewContentHandler = void Function(DomElement); /// [HtmlViewEmbedder.disposeViews] class PlatformViewMessageHandler { PlatformViewMessageHandler({ - required PlatformViewManager contentManager, - PlatformViewContentHandler? contentHandler, - }) : _contentManager = contentManager, - _contentHandler = contentHandler; + required DomElement platformViewsContainer, + PlatformViewManager? contentManager, + }) : _contentManager = contentManager ?? PlatformViewManager.instance, + _platformViewsContainer = platformViewsContainer; final MethodCodec _codec = const StandardMethodCodec(); final PlatformViewManager _contentManager; - final PlatformViewContentHandler? _contentHandler; + final DomElement _platformViewsContainer; /// Handle a `create` Platform View message. /// /// This will attempt to render the `contents` and of a Platform View, if its /// `viewType` has been registered previously. /// - /// (See [PlatformViewContentManager.registerFactory] for more details.) + /// (See [PlatformViewManager.registerFactory] for more details.) /// - /// The `contents` are delegated to a [_contentHandler] function, so the - /// active rendering backend can inject them in the right place of the DOM. + /// The `contents` are inserted into the [_platformViewsContainer]. /// /// If all goes well, this function will `callback` with an empty success envelope. /// In case of error, this will `callback` with an error envelope describing the error. @@ -98,7 +99,7 @@ class PlatformViewMessageHandler { // For now, we don't need anything fancier. If needed, this can be converted // to a PlatformViewStrategy class for each web-renderer backend? - _contentHandler?.call(content); + _platformViewsContainer.append(content); callback(_codec.encodeSuccessEnvelope(null)); } diff --git a/lib/web_ui/lib/src/engine/window.dart b/lib/web_ui/lib/src/engine/window.dart index 0dd502def1627..85e53367d3d1c 100644 --- a/lib/web_ui/lib/src/engine/window.dart +++ b/lib/web_ui/lib/src/engine/window.dart @@ -20,6 +20,7 @@ import 'embedder.dart'; import 'mouse/cursor.dart'; import 'navigation/history.dart'; import 'platform_dispatcher.dart'; +import 'platform_views/message_handler.dart'; import 'services.dart'; import 'util.dart'; @@ -36,8 +37,10 @@ const int kImplicitViewId = 0; /// In addition to everything defined in [ui.FlutterView], this class adds /// a few web-specific properties. abstract interface class EngineFlutterView extends ui.FlutterView { - MouseCursor get mouseCursor; DomElement get rootElement; + + MouseCursor get mouseCursor; + PlatformViewMessageHandler get platformViewMessageHandler; } /// The Web implementation of [ui.SingletonFlutterWindow]. @@ -70,6 +73,10 @@ class EngineFlutterWindow extends ui.SingletonFlutterWindow implements EngineFlu @override DomElement get rootElement => flutterViewEmbedder.flutterViewElement; + @override + late final PlatformViewMessageHandler platformViewMessageHandler = + PlatformViewMessageHandler(platformViewsContainer: flutterViewEmbedder.glassPaneElement); + /// Handles the browser history integration to allow users to use the back /// button, etc. BrowserHistory get browserHistory { diff --git a/lib/web_ui/test/engine/platform_views/message_handler_test.dart b/lib/web_ui/test/engine/platform_views/message_handler_test.dart index 2f30e7c5e13f4..9d911a297dd35 100644 --- a/lib/web_ui/test/engine/platform_views/message_handler_test.dart +++ b/lib/web_ui/test/engine/platform_views/message_handler_test.dart @@ -24,18 +24,17 @@ void testMain() { const int viewId = 6; late PlatformViewManager contentManager; late Completer completer; - late Completer contentCompleter; setUp(() { contentManager = PlatformViewManager(); completer = Completer(); - contentCompleter = Completer(); }); group('"create" message', () { test('unregistered viewType, fails with descriptive exception', () async { final PlatformViewMessageHandler messageHandler = PlatformViewMessageHandler( + platformViewsContainer: createDomElement('div'), contentManager: contentManager, ); final ByteData? message = _getCreateMessage(viewType, viewId); @@ -57,6 +56,7 @@ void testMain() { viewType, (int id) => createDomHTMLDivElement()); contentManager.renderContent(viewType, viewId, null); final PlatformViewMessageHandler messageHandler = PlatformViewMessageHandler( + platformViewsContainer: createDomElement('div'), contentManager: contentManager, ); final ByteData? message = _getCreateMessage(viewType, viewId); @@ -77,6 +77,7 @@ void testMain() { contentManager.registerFactory( viewType, (int id) => createDomHTMLDivElement()..id = 'success'); final PlatformViewMessageHandler messageHandler = PlatformViewMessageHandler( + platformViewsContainer: createDomElement('div'), contentManager: contentManager, ); final ByteData? message = _getCreateMessage(viewType, viewId); @@ -89,27 +90,37 @@ void testMain() { 'The response should be a success envelope, with null in it.'); }); - test('calls a contentHandler with the result of creating a view', + test('inserts the created view into the platformViewsContainer', () async { + final DomElement platformViewsContainer = createDomElement('pv-container'); contentManager.registerFactory( viewType, (int id) => createDomHTMLDivElement()..id = 'success'); final PlatformViewMessageHandler messageHandler = PlatformViewMessageHandler( + platformViewsContainer: platformViewsContainer, contentManager: contentManager, - contentHandler: contentCompleter.complete, ); final ByteData? message = _getCreateMessage(viewType, viewId); messageHandler.handlePlatformViewCall(message, completer.complete); - final DomElement contents = await contentCompleter.future; final ByteData? response = await completer.future; - expect(contents.querySelector('div#success'), isNotNull, - reason: - 'The element created by the factory should be present in the created view.'); - expect(codec.decodeEnvelope(response!), isNull, - reason: - 'The response should be a success envelope, with null in it.'); + expect( + platformViewsContainer.children.single, + isNotNull, + reason: 'The container has a single child, the created view.', + ); + final DomElement platformView = platformViewsContainer.lastElementChild!; + expect( + platformView.querySelector('div#success'), + isNotNull, + reason: 'The element created by the factory should be present in the created view.', + ); + expect( + codec.decodeEnvelope(response!), + isNull, + reason: 'The response should be a success envelope, with null in it.', + ); }); test('passes creation params to the factory', () async { @@ -119,6 +130,7 @@ void testMain() { return createDomHTMLDivElement(); }); final PlatformViewMessageHandler messageHandler = PlatformViewMessageHandler( + platformViewsContainer: createDomElement('div'), contentManager: contentManager, ); @@ -177,8 +189,10 @@ void testMain() { return Object(); }); - final PlatformViewMessageHandler messageHandler = - PlatformViewMessageHandler(contentManager: contentManager); + final PlatformViewMessageHandler messageHandler = PlatformViewMessageHandler( + platformViewsContainer: createDomElement('div'), + contentManager: contentManager, + ); final ByteData? message = _getCreateMessage(viewType, viewId); expect(() { @@ -196,6 +210,7 @@ void testMain() { test('never fails, even for unknown viewIds', () async { final PlatformViewMessageHandler messageHandler = PlatformViewMessageHandler( + platformViewsContainer: createDomElement('div'), contentManager: contentManager, ); final ByteData? message = _getDisposeMessage(viewId); @@ -210,6 +225,7 @@ void testMain() { test('never fails, even for unknown viewIds', () async { final PlatformViewMessageHandler messageHandler = PlatformViewMessageHandler( + platformViewsContainer: createDomElement('div'), contentManager: _FakePlatformViewManager(viewIdCompleter.complete), ); final ByteData? message = _getDisposeMessage(viewId); From 9953c85e184964b4657f2d00b018332e37e52c56 Mon Sep 17 00:00:00 2001 From: Mouad Debbar Date: Wed, 20 Sep 2023 14:33:36 -0400 Subject: [PATCH 2/2] some changes --- lib/web_ui/lib/src/engine/window.dart | 3 +-- .../test/engine/platform_views/message_handler_test.dart | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/web_ui/lib/src/engine/window.dart b/lib/web_ui/lib/src/engine/window.dart index 85e53367d3d1c..5e09148a21dc6 100644 --- a/lib/web_ui/lib/src/engine/window.dart +++ b/lib/web_ui/lib/src/engine/window.dart @@ -37,10 +37,9 @@ const int kImplicitViewId = 0; /// In addition to everything defined in [ui.FlutterView], this class adds /// a few web-specific properties. abstract interface class EngineFlutterView extends ui.FlutterView { - DomElement get rootElement; - MouseCursor get mouseCursor; PlatformViewMessageHandler get platformViewMessageHandler; + DomElement get rootElement; } /// The Web implementation of [ui.SingletonFlutterWindow]. diff --git a/lib/web_ui/test/engine/platform_views/message_handler_test.dart b/lib/web_ui/test/engine/platform_views/message_handler_test.dart index 9d911a297dd35..e1b28b68b954d 100644 --- a/lib/web_ui/test/engine/platform_views/message_handler_test.dart +++ b/lib/web_ui/test/engine/platform_views/message_handler_test.dart @@ -110,7 +110,7 @@ void testMain() { isNotNull, reason: 'The container has a single child, the created view.', ); - final DomElement platformView = platformViewsContainer.lastElementChild!; + final DomElement platformView = platformViewsContainer.children.single; expect( platformView.querySelector('div#success'), isNotNull,