From 74dcaafe7adf666f27dfd71dbdfa338ad2d0eee8 Mon Sep 17 00:00:00 2001 From: Yegor Date: Tue, 21 Sep 2021 15:57:35 -0700 Subject: [PATCH] [web] clear surfaces on hot restart (flutter/engine#28675) * [web] clear surfaces on hot restart --- .../flutter/lib/web_ui/lib/src/engine.dart | 11 ++++++ .../src/engine/canvaskit/surface_factory.dart | 25 ++++++++++--- .../test/canvaskit/surface_factory_test.dart | 35 +++++++++++++++++++ 3 files changed, 67 insertions(+), 4 deletions(-) diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine.dart b/engine/src/flutter/lib/web_ui/lib/src/engine.dart index 50cc107c91e..15c8d53889a 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine.dart @@ -361,6 +361,17 @@ void registerHotRestartListener(ui.VoidCallback listener) { _hotRestartListeners.add(listener); } +/// Pretends that hot restart is about to happen. +/// +/// Useful in tests to check that the engine performs appropriate clean-ups, +/// such as removing static DOM listeners, prior to allowing the Dart runtime +/// to re-initialize the program. +void debugEmulateHotRestart() { + for (final ui.VoidCallback listener in _hotRestartListeners) { + listener(); + } +} + /// This method performs one-time initialization of the Web environment that /// supports the Flutter framework. /// diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/canvaskit/surface_factory.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/canvaskit/surface_factory.dart index a741624572c..c7f7e73b33c 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/canvaskit/surface_factory.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/canvaskit/surface_factory.dart @@ -4,19 +4,33 @@ import 'package:meta/meta.dart'; +import '../../engine.dart'; import '../util.dart'; import 'embedded_views.dart'; import 'surface.dart'; /// Caches surfaces used to overlay platform views. class SurfaceFactory { - /// The cache singleton. - static final SurfaceFactory instance = - SurfaceFactory(HtmlViewEmbedder.maximumSurfaces); + /// The lazy-initialized singleton surface factory. + /// + /// [debugClear] causes this singleton to be reinitialized. + static SurfaceFactory get instance => + _instance ??= SurfaceFactory(HtmlViewEmbedder.maximumSurfaces); + + /// Returns the raw (potentially uninitialized) value of the singleton. + /// + /// Useful in tests for checking the lifecycle of this class. + static SurfaceFactory? get debugUninitializedInstance => _instance; + + static SurfaceFactory? _instance; SurfaceFactory(this.maximumSurfaces) : assert(maximumSurfaces >= 1, - 'The maximum number of surfaces must be at least 1'); + 'The maximum number of surfaces must be at least 1') { + if (assertionsEnabled) { + registerHotRestartListener(debugClear); + } + } /// The base surface to paint on. This is the default surface which will be /// painted to. If there are no platform views, then this surface will receive @@ -168,7 +182,10 @@ class SurfaceFactory { for (final Surface surface in _liveSurfaces) { surface.dispose(); } + baseSurface.dispose(); + backupSurface.dispose(); _liveSurfaces.clear(); _cache.clear(); + _instance = null; } } diff --git a/engine/src/flutter/lib/web_ui/test/canvaskit/surface_factory_test.dart b/engine/src/flutter/lib/web_ui/test/canvaskit/surface_factory_test.dart index f0be0203ffa..c312aca45a6 100644 --- a/engine/src/flutter/lib/web_ui/test/canvaskit/surface_factory_test.dart +++ b/engine/src/flutter/lib/web_ui/test/canvaskit/surface_factory_test.dart @@ -5,6 +5,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 '../matchers.dart'; import 'common.dart'; @@ -76,6 +77,40 @@ void testMain() { expect(factory.isLive(surface), isFalse); }); + test('hot restart', () { + void expectDisposed(Surface surface) { + expect(surface.htmlCanvas!.isConnected, isFalse); + } + + final SurfaceFactory originalFactory = SurfaceFactory.instance; + expect(SurfaceFactory.debugUninitializedInstance, isNotNull); + + // Cause the surface and its canvas to be attached to the page + originalFactory.baseSurface.acquireFrame(const ui.Size(10, 10)); + originalFactory.baseSurface.addToScene(); + expect(originalFactory.baseSurface.htmlCanvas!.isConnected, isTrue); + + // Create a few overlay surfaces + final List overlays = []; + for (int i = 0; i < 3; i++) { + overlays.add(originalFactory.getSurface() + ..acquireFrame(const ui.Size(10, 10)) + ..addToScene()); + } + expect(originalFactory.debugSurfaceCount, 5); + + originalFactory.backupSurface.acquireFrame(const ui.Size(10, 10)); + originalFactory.backupSurface.addToScene(); + expect(originalFactory.backupSurface.htmlCanvas!.isConnected, isTrue); + + // Trigger hot restart clean-up logic and check that we indeed clean up. + debugEmulateHotRestart(); + expect(SurfaceFactory.debugUninitializedInstance, isNull); + expectDisposed(originalFactory.baseSurface); + expectDisposed(originalFactory.backupSurface); + overlays.forEach(expectDisposed); + expect(originalFactory.debugSurfaceCount, 2); + }); // TODO(hterkelsen): https://github.com/flutter/flutter/issues/60040 }, skip: isIosSafari); }