From d3c96c65e582b793959d454c38bba8cd08e5b432 Mon Sep 17 00:00:00 2001 From: Srujan Gaddam <58529443+srujzs@users.noreply.github.com> Date: Tue, 4 Feb 2025 17:12:29 -0800 Subject: [PATCH] Wait until all scripts are loaded in the page before running main for the DDC library bundle format (#162707) https://github.com/flutter/flutter/issues/162567 - Uses the `bootstrapScript` field in `loadConfig` to run a script after all scripts have loaded. - This script just calls a callback that is set up beforehand and calls main. - Modifies the callback that calls `dartDevEmbedder.runMain` to wait until both DWDS called main and all scripts have loaded. - Unskips hot reload tests now that the race condition should no longer exist. ## Pre-launch Checklist - [ ] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [ ] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [ ] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [ ] I signed the [CLA]. - [ ] I listed at least one issue that this PR fixes in the description above. - [ ] I updated/added relevant documentation (doc comments with `///`). - [ ] I added new tests to check the change I am making, or this PR is [test-exempt]. - [ ] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [ ] All existing and new tests are passing. --- .../lib/src/isolated/devfs_web.dart | 5 ++ .../flutter_tools/lib/src/web/bootstrap.dart | 46 +++++++++++++++---- .../general.shard/web/bootstrap_test.dart | 3 ++ .../test_data/hot_reload_test_common.dart | 8 +--- .../hot_reload_with_asset_test_common.dart | 8 +--- ...eless_stateful_hot_reload_test_common.dart | 8 +--- .../web.shard/hot_reload_web_errors_test.dart | 4 -- .../test/web.shard/hot_reload_web_test.dart | 2 - .../hot_reload_with_asset_web_test.dart | 2 - ...tateless_stateful_hot_reload_web_test.dart | 2 - 10 files changed, 52 insertions(+), 36 deletions(-) diff --git a/packages/flutter_tools/lib/src/isolated/devfs_web.dart b/packages/flutter_tools/lib/src/isolated/devfs_web.dart index 6b294093fc..b34264c6c2 100644 --- a/packages/flutter_tools/lib/src/isolated/devfs_web.dart +++ b/packages/flutter_tools/lib/src/isolated/devfs_web.dart @@ -1087,6 +1087,10 @@ class WebDevFS implements DevFS { generateLoadingIndicator: enableDwds, ), ); + const String onLoadEndBootstrap = 'on_load_end_bootstrap.js'; + if (ddcModuleSystem) { + webAssetServer.writeFile(onLoadEndBootstrap, generateDDCLibraryBundleOnLoadEndBootstrap()); + } webAssetServer.writeFile( 'main_module.bootstrap.js', ddcModuleSystem @@ -1094,6 +1098,7 @@ class WebDevFS implements DevFS { entrypoint: entrypoint, nullAssertions: nullAssertions, nativeNullAssertions: nativeNullAssertions, + onLoadEndBootstrap: onLoadEndBootstrap, ) : generateMainModule( entrypoint: entrypoint, diff --git a/packages/flutter_tools/lib/src/web/bootstrap.dart b/packages/flutter_tools/lib/src/web/bootstrap.dart index 95383ff086..db09b97024 100644 --- a/packages/flutter_tools/lib/src/web/bootstrap.dart +++ b/packages/flutter_tools/lib/src/web/bootstrap.dart @@ -505,10 +505,13 @@ String generateDDCMainModule({ '''; } +const String _onLoadEndCallback = r'$onLoadEndCallback'; + String generateDDCLibraryBundleMainModule({ required String entrypoint, required bool nullAssertions, required bool nativeNullAssertions, + required String onLoadEndBootstrap, }) { // The typo below in "EXTENTION" is load-bearing, package:build depends on it. return ''' @@ -519,21 +522,48 @@ String generateDDCLibraryBundleMainModule({ dartDevEmbedder.debugger.registerDevtoolsFormatter(); - let child = {}; - child.main = function() { - let sdkOptions = { - nonNullAsserts: $nullAssertions, - nativeNonNullAsserts: $nativeNullAssertions, - }; - dartDevEmbedder.runMain(appName, sdkOptions); + // Set up a final script that lets us know when all scripts have been loaded. + let onLoadEndSrc = '$onLoadEndBootstrap'; + window.\$dartLoader.loadConfig.bootstrapScript = { + src: onLoadEndSrc, + id: onLoadEndSrc, + }; + window.\$dartLoader.loadConfig.tryLoadBootstrapScript = true; + let dwdsCalledMain = false; + let dartSrcsLoaded = false; + let runMainWhenBoth = function() { + // Only run once both all the scripts are loaded and DWDS triggers main. + if (dwdsCalledMain && dartSrcsLoaded) { + let sdkOptions = { + nonNullAsserts: $nullAssertions, + nativeNonNullAsserts: $nativeNullAssertions, + }; + dartDevEmbedder.runMain(appName, sdkOptions); + } + } + // DWDS expects the main function to be lowercase. + // TODO(srujzs): DWDS should be more robust to not have to require that. + dwdsmain = function() { + dwdsCalledMain = true; + runMainWhenBoth(); + } + // Should be called by $onLoadEndBootstrap once all the scripts have been + // loaded. + window.$_onLoadEndCallback = function() { + dartSrcsLoaded = true; + runMainWhenBoth(); } /* MAIN_EXTENSION_MARKER */ - child.main(); + dwdsmain(); })(); '''; } +String generateDDCLibraryBundleOnLoadEndBootstrap() { + return '''window.$_onLoadEndCallback();'''; +} + /// Generate a synthetic main module which captures the application's main /// method. /// diff --git a/packages/flutter_tools/test/general.shard/web/bootstrap_test.dart b/packages/flutter_tools/test/general.shard/web/bootstrap_test.dart index eb839738c8..b65401616b 100644 --- a/packages/flutter_tools/test/general.shard/web/bootstrap_test.dart +++ b/packages/flutter_tools/test/general.shard/web/bootstrap_test.dart @@ -273,6 +273,7 @@ void main() { entrypoint: 'main.js', nullAssertions: false, nativeNullAssertions: false, + onLoadEndBootstrap: 'on_load_end_bootstrap.js', ); // bootstrap main module has correct defined module. expect(result, contains('let appName = "org-dartlang-app:/main.js";')); @@ -284,6 +285,7 @@ void main() { entrypoint: 'main.js', nullAssertions: true, nativeNullAssertions: true, + onLoadEndBootstrap: 'on_load_end_bootstrap.js', ); expect(result, contains('nonNullAsserts: true')); @@ -295,6 +297,7 @@ void main() { entrypoint: 'main.js', nullAssertions: false, nativeNullAssertions: false, + onLoadEndBootstrap: 'on_load_end_bootstrap.js', ); expect(result, contains('nonNullAsserts: false')); diff --git a/packages/flutter_tools/test/integration.shard/test_data/hot_reload_test_common.dart b/packages/flutter_tools/test/integration.shard/test_data/hot_reload_test_common.dart index ef64309068..e1c73f67a7 100644 --- a/packages/flutter_tools/test/integration.shard/test_data/hot_reload_test_common.dart +++ b/packages/flutter_tools/test/integration.shard/test_data/hot_reload_test_common.dart @@ -12,11 +12,7 @@ import '../test_driver.dart'; import '../test_utils.dart'; import 'hot_reload_project.dart'; -void testAll({ - bool chrome = false, - List additionalCommandArgs = const [], - Object? skip = false, -}) { +void testAll({bool chrome = false, List additionalCommandArgs = const []}) { group('chrome: $chrome' '${additionalCommandArgs.isEmpty ? '' : ' with args: $additionalCommandArgs'}', () { late Directory tempDir; @@ -235,7 +231,7 @@ void testAll({ // isolates, so this test will wait forever. skip: chrome, ); - }, skip: skip); + }); } bool _isHotReloadCompletionEvent(Map? event) { diff --git a/packages/flutter_tools/test/integration.shard/test_data/hot_reload_with_asset_test_common.dart b/packages/flutter_tools/test/integration.shard/test_data/hot_reload_with_asset_test_common.dart index ba2006ec02..5ca27d5f2a 100644 --- a/packages/flutter_tools/test/integration.shard/test_data/hot_reload_with_asset_test_common.dart +++ b/packages/flutter_tools/test/integration.shard/test_data/hot_reload_with_asset_test_common.dart @@ -11,11 +11,7 @@ import '../test_driver.dart'; import '../test_utils.dart'; import 'hot_reload_with_asset.dart'; -void testAll({ - bool chrome = false, - List additionalCommandArgs = const [], - Object? skip = false, -}) { +void testAll({bool chrome = false, List additionalCommandArgs = const []}) { group('chrome: $chrome' '${additionalCommandArgs.isEmpty ? '' : ' with args: $additionalCommandArgs'}', () { late Directory tempDir; @@ -86,5 +82,5 @@ void testAll({ await flutter.hotRestart(); await onSecondLoad.future; }); - }, skip: skip); + }); } diff --git a/packages/flutter_tools/test/integration.shard/test_data/stateless_stateful_hot_reload_test_common.dart b/packages/flutter_tools/test/integration.shard/test_data/stateless_stateful_hot_reload_test_common.dart index 9acd6a86fc..be6b7a8e86 100644 --- a/packages/flutter_tools/test/integration.shard/test_data/stateless_stateful_hot_reload_test_common.dart +++ b/packages/flutter_tools/test/integration.shard/test_data/stateless_stateful_hot_reload_test_common.dart @@ -13,11 +13,7 @@ import '../test_utils.dart'; // This test verifies that we can hot reload a stateless widget into a // stateful one and back. -void testAll({ - bool chrome = false, - List additionalCommandArgs = const [], - Object? skip = false, -}) { +void testAll({bool chrome = false, List additionalCommandArgs = const []}) { group('chrome: $chrome' '${additionalCommandArgs.isEmpty ? '' : ' with args: $additionalCommandArgs'}', () { late Directory tempDir; @@ -60,5 +56,5 @@ void testAll({ expect(logs, contains('STATEFUL')); await subscription.cancel(); }); - }, skip: skip); + }); } diff --git a/packages/flutter_tools/test/web.shard/hot_reload_web_errors_test.dart b/packages/flutter_tools/test/web.shard/hot_reload_web_errors_test.dart index 41f7b1f937..cd26581c45 100644 --- a/packages/flutter_tools/test/web.shard/hot_reload_web_errors_test.dart +++ b/packages/flutter_tools/test/web.shard/hot_reload_web_errors_test.dart @@ -5,8 +5,6 @@ @Tags(['flutter-test-driver']) library; -import 'dart:io'; - import '../integration.shard/test_data/hot_reload_errors_common.dart'; import '../src/common.dart'; @@ -19,7 +17,5 @@ void main() { // TODO(srujzs): Remove this custom message once we have the delta inspector emitting the same // string as the VM. constClassFieldRemovalErrorMessage: 'Const class cannot remove fields', - // https://github.com/flutter/flutter/issues/162567 - skip: Platform.isWindows, ); } diff --git a/packages/flutter_tools/test/web.shard/hot_reload_web_test.dart b/packages/flutter_tools/test/web.shard/hot_reload_web_test.dart index 1ecce26566..685f6a88f9 100644 --- a/packages/flutter_tools/test/web.shard/hot_reload_web_test.dart +++ b/packages/flutter_tools/test/web.shard/hot_reload_web_test.dart @@ -14,7 +14,5 @@ void main() { additionalCommandArgs: [ '--extra-front-end-options=--dartdevc-canary,--dartdevc-module-format=ddc', ], - // https://github.com/flutter/flutter/issues/162567 - skip: true, ); } diff --git a/packages/flutter_tools/test/web.shard/hot_reload_with_asset_web_test.dart b/packages/flutter_tools/test/web.shard/hot_reload_with_asset_web_test.dart index 00f9fb7984..7704971185 100644 --- a/packages/flutter_tools/test/web.shard/hot_reload_with_asset_web_test.dart +++ b/packages/flutter_tools/test/web.shard/hot_reload_with_asset_web_test.dart @@ -14,7 +14,5 @@ void main() { additionalCommandArgs: [ '--extra-front-end-options=--dartdevc-canary,--dartdevc-module-format=ddc', ], - // https://github.com/flutter/flutter/issues/162567 - skip: true, ); } diff --git a/packages/flutter_tools/test/web.shard/stateless_stateful_hot_reload_web_test.dart b/packages/flutter_tools/test/web.shard/stateless_stateful_hot_reload_web_test.dart index d5e8c57da0..db44a80c59 100644 --- a/packages/flutter_tools/test/web.shard/stateless_stateful_hot_reload_web_test.dart +++ b/packages/flutter_tools/test/web.shard/stateless_stateful_hot_reload_web_test.dart @@ -14,7 +14,5 @@ void main() { additionalCommandArgs: [ '--extra-front-end-options=--dartdevc-canary,--dartdevc-module-format=ddc', ], - // https://github.com/flutter/flutter/issues/162567 - skip: true, ); }