From d0b226c98deaa892b750b4d69760125a2cbc3205 Mon Sep 17 00:00:00 2001 From: David Iglesias Teixeira Date: Tue, 23 Jan 2024 20:26:37 -0800 Subject: [PATCH 1/4] [web] Do not dispose all platform view factories when disposing a view. --- .../src/engine/canvaskit/embedded_views.dart | 2 +- .../platform_views/content_manager.dart | 5 ++-- .../test/canvaskit/multi_view_test.dart | 24 +++++++++++++++++++ 3 files changed, 28 insertions(+), 3 deletions(-) diff --git a/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart b/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart index bb2b89b3ae0b9..d0c61332487dd 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart @@ -659,7 +659,7 @@ class HtmlViewEmbedder { /// Disposes the state of this view embedder. void dispose() { - final Set allViews = PlatformViewManager.instance.debugClear(); + final Set allViews = PlatformViewManager.instance.getKnownPlatformViewIds(); disposeViews(allViews); _context = EmbedderFrameContext(); _currentCompositionParams.clear(); diff --git a/lib/web_ui/lib/src/engine/platform_views/content_manager.dart b/lib/web_ui/lib/src/engine/platform_views/content_manager.dart index bfad69bf911b8..54f3eb94f9343 100644 --- a/lib/web_ui/lib/src/engine/platform_views/content_manager.dart +++ b/lib/web_ui/lib/src/engine/platform_views/content_manager.dart @@ -212,9 +212,10 @@ class PlatformViewManager { /// component. bool isVisible(int viewId) => !isInvisible(viewId); - /// Clears the state. Used in tests. - /// /// Returns the set of know view ids, so they can be cleaned up. + Set getKnownPlatformViewIds() => _contents.keys.toSet(); + + /// Clears the state. Used in tests. Set debugClear() { final Set result = _contents.keys.toSet(); result.forEach(clearPlatformView); diff --git a/lib/web_ui/test/canvaskit/multi_view_test.dart b/lib/web_ui/test/canvaskit/multi_view_test.dart index 3bc93153babf2..62b499226e75c 100644 --- a/lib/web_ui/test/canvaskit/multi_view_test.dart +++ b/lib/web_ui/test/canvaskit/multi_view_test.dart @@ -6,6 +6,7 @@ import 'package:test/bootstrap/browser.dart'; import 'package:test/test.dart'; import 'package:ui/src/engine.dart'; 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.dart'; @@ -68,5 +69,28 @@ void testMain() { isNull, ); }); + + // Issue https://github.com/flutter/flutter/issues/142094 + test('does not reset platform view factories when disposing a view', () async { + expect(PlatformViewManager.instance.knowsViewType('self-test'), isFalse); + + final EngineFlutterView view = EngineFlutterView( + EnginePlatformDispatcher.instance, createDomElement('multi-view')); + EnginePlatformDispatcher.instance.viewManager.registerView(view); + expect( + CanvasKitRenderer.instance.debugGetRasterizerForView(view), + isNotNull, + ); + + EnginePlatformDispatcher.instance.viewManager + .disposeAndUnregisterView(view.viewId); + expect( + CanvasKitRenderer.instance.debugGetRasterizerForView(view), + isNull, + ); + + expect(PlatformViewManager.instance.knowsViewType(ui_web.PlatformViewRegistry.defaultVisibleViewType), isTrue); + expect(PlatformViewManager.instance.knowsViewType(ui_web.PlatformViewRegistry.defaultInvisibleViewType), isTrue); + }); }); } From 4217b9c0aededd4c74effe130c8914f3936387dc Mon Sep 17 00:00:00 2001 From: David Iglesias Teixeira Date: Thu, 25 Jan 2024 19:32:56 -0800 Subject: [PATCH 2/4] Stop using PlatformViewManager from ck HtmlViewEmbedder. The HtmlViewEmbedder has enough information to dispose all the views it knows of (the keys of _viewClipChains), so there's no need to call the PlatformViewManager to ask for ALL the views in the app. --- lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart | 7 +++---- .../lib/src/engine/platform_views/content_manager.dart | 9 ++------- 2 files changed, 5 insertions(+), 11 deletions(-) diff --git a/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart b/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart index d0c61332487dd..6824e3a0e4232 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart @@ -410,8 +410,7 @@ class HtmlViewEmbedder { // are going to be added back. Moving rather than removing and re-adding // the view helps it maintain state. disposeViews(diffResult.viewsToRemove - .where((int view) => !diffResult.viewsToAdd.contains(view)) - .toSet()); + .where((int view) => !diffResult.viewsToAdd.contains(view))); _activeCompositionOrder.addAll(_compositionOrder); unusedViews.removeAll(_compositionOrder); @@ -510,7 +509,7 @@ class HtmlViewEmbedder { ); } - void disposeViews(Set viewsToDispose) { + void disposeViews(Iterable viewsToDispose) { for (final int viewId in viewsToDispose) { // Remove viewId from the _viewClipChains Map, and then from the DOM. final ViewClipChain? clipChain = _viewClipChains.remove(viewId); @@ -659,7 +658,7 @@ class HtmlViewEmbedder { /// Disposes the state of this view embedder. void dispose() { - final Set allViews = PlatformViewManager.instance.getKnownPlatformViewIds(); + final Iterable allViews = _viewClipChains.keys; disposeViews(allViews); _context = EmbedderFrameContext(); _currentCompositionParams.clear(); diff --git a/lib/web_ui/lib/src/engine/platform_views/content_manager.dart b/lib/web_ui/lib/src/engine/platform_views/content_manager.dart index 54f3eb94f9343..c09eefcc18972 100644 --- a/lib/web_ui/lib/src/engine/platform_views/content_manager.dart +++ b/lib/web_ui/lib/src/engine/platform_views/content_manager.dart @@ -212,18 +212,13 @@ class PlatformViewManager { /// component. bool isVisible(int viewId) => !isInvisible(viewId); - /// Returns the set of know view ids, so they can be cleaned up. - Set getKnownPlatformViewIds() => _contents.keys.toSet(); - /// Clears the state. Used in tests. - Set debugClear() { - final Set result = _contents.keys.toSet(); - result.forEach(clearPlatformView); + void debugClear() { + _contents.keys.toList().forEach(clearPlatformView); _factories.clear(); _contents.clear(); _invisibleViews.clear(); _viewIdToType.clear(); - return result; } } From 1d8e63ffe6cb594842176fef7346b5454a69468b Mon Sep 17 00:00:00 2001 From: David Iglesias Teixeira Date: Thu, 25 Jan 2024 19:36:36 -0800 Subject: [PATCH 3/4] Prevent mutation while iterating. --- lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart b/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart index 6824e3a0e4232..fb16f4939a2b9 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart @@ -658,8 +658,7 @@ class HtmlViewEmbedder { /// Disposes the state of this view embedder. void dispose() { - final Iterable allViews = _viewClipChains.keys; - disposeViews(allViews); + disposeViews(_viewClipChains.keys.toList()); _context = EmbedderFrameContext(); _currentCompositionParams.clear(); debugCleanupSvgClipPaths(); From 771d01b5b06798c3436231ed4e676edf758a84fe Mon Sep 17 00:00:00 2001 From: David Iglesias Teixeira Date: Fri, 26 Jan 2024 11:42:45 -0800 Subject: [PATCH 4/4] Remove unneeded toList --- lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart b/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart index fb16f4939a2b9..3b64dbb594721 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart @@ -658,7 +658,7 @@ class HtmlViewEmbedder { /// Disposes the state of this view embedder. void dispose() { - disposeViews(_viewClipChains.keys.toList()); + disposeViews(_viewClipChains.keys); _context = EmbedderFrameContext(); _currentCompositionParams.clear(); debugCleanupSvgClipPaths();