From 9a8908153216fc825f01e327ac028242fb762fe8 Mon Sep 17 00:00:00 2001 From: Srujan Gaddam Date: Tue, 8 Jul 2025 17:25:06 -0700 Subject: [PATCH 01/12] Address flakes from using temp directory to run tests https://github.com/dart-lang/webdev/commit/2172ba7024d8885afc30f6634d98c00a003bb0cc added support to run tests in a temporary directory. This results in two flaky issues: 1. On Windows, build_daemon tests may fail to delete the temp directory because the process may have not been torn down yet, so it may still be accessing the file system. There was an initial retry after 1 second, but that appears to be not enough looking at a recent test run. 2. If a test times out, its tearDown may not be called. In this case, the ResidentWebRunner in frontend_server may not restore the current directory in the LocalFileSystem. This leads to cascading failures in subsequent tests due to no longer being in a path that contains 'webdev'. See https://github.com/dart-lang/webdev/actions/runs/15989286213/job/45099373212?pr=2641 for an example. See https://github.com/dart-lang/test/issues/897 as well for tracking work to call tearDown on timeouts. To address the above issues: 1. Increase the delay between the two tries and assert this only occurs on Windows. 2. Cache the current directory so that it can be used in utilities.dart with the same (correct) value every time. --- dwds/test/fixtures/project.dart | 5 +++-- test_common/lib/utilities.dart | 4 +++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/dwds/test/fixtures/project.dart b/dwds/test/fixtures/project.dart index 46bd9193b..9dc8e3def 100644 --- a/dwds/test/fixtures/project.dart +++ b/dwds/test/fixtures/project.dart @@ -240,9 +240,10 @@ class TestProject { try { _fixturesCopy.deleteSync(recursive: true); } on FileSystemException catch (_) { + assert(Platform.isWindows); // On Windows, the build daemon process might still be accessing the - // working directory, so wait a second and then try again. - await Future.delayed(const Duration(seconds: 1)); + // working directory, so wait a few seconds and then try again. + await Future.delayed(const Duration(seconds: 5)); _fixturesCopy.deleteSync(recursive: true); } } diff --git a/test_common/lib/utilities.dart b/test_common/lib/utilities.dart index d4d3619be..1a960a509 100644 --- a/test_common/lib/utilities.dart +++ b/test_common/lib/utilities.dart @@ -12,10 +12,12 @@ const fixturesDirName = 'fixtures'; const newDdcTypeSystemVersion = '3.3.0-242.0.dev'; +final _currentDirectory = p.current; + /// The path to the webdev directory in the local machine, e.g. /// '/workstation/webdev'. String get webdevPath { - final pathParts = p.split(p.current); + final pathParts = p.split(_currentDirectory); assert(pathParts.contains(webdevDirName)); return p.joinAll( pathParts.sublist(0, pathParts.lastIndexOf(webdevDirName) + 1), From 3bd14482062155a7544df1c28e3c341fcaa02347 Mon Sep 17 00:00:00 2001 From: Srujan Gaddam Date: Tue, 8 Jul 2025 17:37:01 -0700 Subject: [PATCH 02/12] Add artificial delay to test changes --- dwds/test/instances/common/patterns_inspection_common.dart | 1 + 1 file changed, 1 insertion(+) diff --git a/dwds/test/instances/common/patterns_inspection_common.dart b/dwds/test/instances/common/patterns_inspection_common.dart index b2f75b5fe..31a52c91a 100644 --- a/dwds/test/instances/common/patterns_inspection_common.dart +++ b/dwds/test/instances/common/patterns_inspection_common.dart @@ -83,6 +83,7 @@ void runTests({ tearDown(() => service.resume(isolateId)); test('pattern match case 1', () async { + await Future.delayed(Duration(minutes: 20)); await onBreakPoint('testPatternCase1', (event) async { final frame = event.topFrame!; From e3e63ecfc53599c2f0d2d541307e721254127d8a Mon Sep 17 00:00:00 2001 From: Srujan Gaddam Date: Tue, 8 Jul 2025 19:48:42 -0700 Subject: [PATCH 03/12] Revert "Add artificial delay to test changes" This reverts commit 3bd14482062155a7544df1c28e3c341fcaa02347. --- dwds/test/instances/common/patterns_inspection_common.dart | 1 - 1 file changed, 1 deletion(-) diff --git a/dwds/test/instances/common/patterns_inspection_common.dart b/dwds/test/instances/common/patterns_inspection_common.dart index 31a52c91a..b2f75b5fe 100644 --- a/dwds/test/instances/common/patterns_inspection_common.dart +++ b/dwds/test/instances/common/patterns_inspection_common.dart @@ -83,7 +83,6 @@ void runTests({ tearDown(() => service.resume(isolateId)); test('pattern match case 1', () async { - await Future.delayed(Duration(minutes: 20)); await onBreakPoint('testPatternCase1', (event) async { final frame = event.topFrame!; From 4426bc378d40b6ed4751621057e7a5c9428ea913 Mon Sep 17 00:00:00 2001 From: Srujan Gaddam Date: Fri, 11 Jul 2025 13:41:13 -0700 Subject: [PATCH 04/12] Avoid changing current directory in dart:io --- dwds/test/fixtures/project.dart | 22 +++++++++++++---- .../lib/src/asset_server.dart | 19 +++++++++------ frontend_server_common/lib/src/devfs.dart | 24 +++++++++---------- .../lib/src/frontend_server_client.dart | 3 +-- test_common/lib/utilities.dart | 4 +--- 5 files changed, 42 insertions(+), 30 deletions(-) diff --git a/dwds/test/fixtures/project.dart b/dwds/test/fixtures/project.dart index 9dc8e3def..82c6aba26 100644 --- a/dwds/test/fixtures/project.dart +++ b/dwds/test/fixtures/project.dart @@ -22,9 +22,12 @@ class TestProject { late Directory _fixturesCopy; /// The top level directory in which we run the test server, e.g. - /// "/tmp/_testSound". + /// "/tmp/_testSound/". String get absolutePackageDirectory => - p.join(_fixturesCopy.absolute.path, packageDirectory); + // Return it as a directory with a trailing slash. + Directory.fromUri( + _fixturesCopy.absolute.uri.resolve(packageDirectory), + ).uri.path; /// The directory to build and serve, e.g. "example". String get directoryToServe => p.split(webAssetsPath).first; @@ -242,9 +245,18 @@ class TestProject { } on FileSystemException catch (_) { assert(Platform.isWindows); // On Windows, the build daemon process might still be accessing the - // working directory, so wait a few seconds and then try again. - await Future.delayed(const Duration(seconds: 5)); - _fixturesCopy.deleteSync(recursive: true); + // working directory, so try again with an exponential backoff. + var seconds = 1; + final maxAttempts = 3; + for (var attempt = 0; attempt < maxAttempts; attempt++) { + try { + _fixturesCopy.deleteSync(recursive: true); + break; + } on FileSystemException catch (_) { + await Future.delayed(Duration(seconds: seconds)); + seconds *= 2; + } + } } } diff --git a/frontend_server_common/lib/src/asset_server.dart b/frontend_server_common/lib/src/asset_server.dart index 095320902..f8060b661 100644 --- a/frontend_server_common/lib/src/asset_server.dart +++ b/frontend_server_common/lib/src/asset_server.dart @@ -26,6 +26,7 @@ class TestAssetServer implements AssetReader { // Fallback to "application/octet-stream" on null which // makes no claims as to the structure of the data. static const String _defaultMimeType = 'application/octet-stream'; + final Uri _projectDirectory; final FileSystem _fileSystem; final HttpServer _httpServer; final Map _files = {}; @@ -41,6 +42,7 @@ class TestAssetServer implements AssetReader { this._httpServer, this._packageUriMapper, this.internetAddress, + this._projectDirectory, this._fileSystem, this._sdkLayout, ) { @@ -65,6 +67,7 @@ class TestAssetServer implements AssetReader { /// trace. static Future start( String sdkDirectory, + Uri projectDirectory, FileSystem fileSystem, String index, String hostname, @@ -75,8 +78,8 @@ class TestAssetServer implements AssetReader { final address = (await InternetAddress.lookup(hostname)).first; final httpServer = await HttpServer.bind(address, port); final sdkLayout = TestSdkLayout.createDefault(sdkDirectory); - final server = TestAssetServer( - index, httpServer, packageUriMapper, address, fileSystem, sdkLayout); + final server = TestAssetServer(index, httpServer, packageUriMapper, address, + projectDirectory, fileSystem, sdkLayout); return server; } @@ -94,7 +97,7 @@ class TestAssetServer implements AssetReader { final headers = {}; if (request.url.path.endsWith('.html')) { - final indexFile = _fileSystem.file(index); + final indexFile = _fileSystem.file(_projectDirectory.resolve(index)); if (indexFile.existsSync()) { headers[HttpHeaders.contentTypeHeader] = 'text/html'; headers[HttpHeaders.contentLengthHeader] = @@ -244,8 +247,7 @@ class TestAssetServer implements AssetReader { // If this is a dart file, it must be on the local file system and is // likely coming from a source map request. The tool doesn't currently // consider the case of Dart files as assets. - final dartFile = - _fileSystem.file(_fileSystem.currentDirectory.uri.resolve(path)); + final dartFile = _fileSystem.file(_projectDirectory.resolve(path)); if (dartFile.existsSync()) { return dartFile; } @@ -255,7 +257,10 @@ class TestAssetServer implements AssetReader { // The file might have been a package file which is signaled by a // `/packages//` request. if (segments.first == 'packages') { - final resolved = _packageUriMapper.serverPathToResolvedUri(path); + var resolved = _packageUriMapper.serverPathToResolvedUri(path); + if (resolved != null) { + resolved = _projectDirectory.resolveUri(resolved); + } final packageFile = _fileSystem.file(resolved); if (packageFile.existsSync()) { return packageFile; @@ -311,7 +316,7 @@ class TestAssetServer implements AssetReader { } String _parseBasePathFromIndexHtml(String index) { - final file = _fileSystem.file(index); + final file = _fileSystem.file(_projectDirectory.resolve(index)); if (!file.existsSync()) { throw StateError('Index file $index is not found'); } diff --git a/frontend_server_common/lib/src/devfs.dart b/frontend_server_common/lib/src/devfs.dart index ace83aa7c..a334b832a 100644 --- a/frontend_server_common/lib/src/devfs.dart +++ b/frontend_server_common/lib/src/devfs.dart @@ -51,15 +51,11 @@ class WebDevFS { final TestSdkLayout sdkLayout; final CompilerOptions compilerOptions; - late final Directory _savedCurrentDirectory; Future create() async { - _savedCurrentDirectory = fileSystem.currentDirectory; - - fileSystem.currentDirectory = projectDirectory.toFilePath(); - assetServer = await TestAssetServer.start( sdkLayout.sdkDirectory, + projectDirectory, fileSystem, index, hostname, @@ -71,7 +67,6 @@ class WebDevFS { } Future dispose() { - fileSystem.currentDirectory = _savedCurrentDirectory; return assetServer.close(); } @@ -84,7 +79,8 @@ class WebDevFS { required bool fullRestart, }) async { final mainPath = mainUri.toFilePath(); - final outputDirectoryPath = fileSystem.file(mainPath).parent.path; + final outputDirectory = fileSystem.directory( + fileSystem.file(projectDirectory.resolve(mainPath)).parent.path); final entryPoint = mainUri.toString(); var prefix = ''; @@ -103,7 +99,10 @@ class WebDevFS { final bootstrap = '${prefix}main_module.bootstrap.js'; assetServer.writeFile( - entryPoint, fileSystem.file(mainPath).readAsStringSync()); + entryPoint, + fileSystem + .file(projectDirectory.resolve(mainPath)) + .readAsStringSync()); assetServer.writeFile(stackMapper, stackTraceMapper.readAsStringSync()); switch (ddcModuleFormat) { @@ -199,14 +198,13 @@ class WebDevFS { File metadataFile; List modules; try { - final parentDirectory = fileSystem.directory(outputDirectoryPath); codeFile = - parentDirectory.childFile('${compilerOutput.outputFilename}.sources'); + outputDirectory.childFile('${compilerOutput.outputFilename}.sources'); manifestFile = - parentDirectory.childFile('${compilerOutput.outputFilename}.json'); + outputDirectory.childFile('${compilerOutput.outputFilename}.json'); sourcemapFile = - parentDirectory.childFile('${compilerOutput.outputFilename}.map'); - metadataFile = parentDirectory + outputDirectory.childFile('${compilerOutput.outputFilename}.map'); + metadataFile = outputDirectory .childFile('${compilerOutput.outputFilename}.metadata'); modules = assetServer.write( codeFile, manifestFile, sourcemapFile, metadataFile); diff --git a/frontend_server_common/lib/src/frontend_server_client.dart b/frontend_server_common/lib/src/frontend_server_client.dart index e500646b4..3c3ae3c1d 100644 --- a/frontend_server_common/lib/src/frontend_server_client.dart +++ b/frontend_server_common/lib/src/frontend_server_client.dart @@ -402,7 +402,6 @@ class ResidentCompiler { if (compilerOptions.moduleFormat == ModuleFormat.ddc) '--dartdevc-module-format=ddc' ]; - _logger.info(args.join(' ')); final workingDirectory = projectDirectory.toFilePath(); _server = await Process.start(sdkLayout.dartAotRuntimePath, args, @@ -657,7 +656,7 @@ String _toMultiRootPath( for (final fileSystemRoot in fileSystemRoots) { final rootPath = fileSystemRoot.toFilePath(windows: Platform.isWindows); if (filePath.startsWith(rootPath)) { - return '$scheme://${filePath.substring(rootPath.length)}'; + return '$scheme:///${filePath.substring(rootPath.length)}'; } } return fileUri.toString(); diff --git a/test_common/lib/utilities.dart b/test_common/lib/utilities.dart index 1a960a509..d4d3619be 100644 --- a/test_common/lib/utilities.dart +++ b/test_common/lib/utilities.dart @@ -12,12 +12,10 @@ const fixturesDirName = 'fixtures'; const newDdcTypeSystemVersion = '3.3.0-242.0.dev'; -final _currentDirectory = p.current; - /// The path to the webdev directory in the local machine, e.g. /// '/workstation/webdev'. String get webdevPath { - final pathParts = p.split(_currentDirectory); + final pathParts = p.split(p.current); assert(pathParts.contains(webdevDirName)); return p.joinAll( pathParts.sublist(0, pathParts.lastIndexOf(webdevDirName) + 1), From 8ac0cb9b3322be7a89685564ac294fb2f023c41d Mon Sep 17 00:00:00 2001 From: Srujan Gaddam Date: Sat, 12 Jul 2025 14:35:28 -0700 Subject: [PATCH 05/12] Fix up paths --- dwds/test/fixtures/context.dart | 6 ++++-- dwds/test/fixtures/project.dart | 8 ++------ 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/dwds/test/fixtures/context.dart b/dwds/test/fixtures/context.dart index a6a1ab373..68687d0f6 100644 --- a/dwds/test/fixtures/context.dart +++ b/dwds/test/fixtures/context.dart @@ -322,10 +322,12 @@ class TestContext { _webRunner = ResidentWebRunner( mainUri: entry, urlTunneler: debugSettings.urlEncoder, - projectDirectory: p.toUri(project.absolutePackageDirectory), + projectDirectory: Directory(project.absolutePackageDirectory).uri, packageConfigFile: project.packageConfigFile, packageUriMapper: packageUriMapper, - fileSystemRoots: [p.toUri(project.absolutePackageDirectory)], + fileSystemRoots: [ + Directory(project.absolutePackageDirectory).uri, + ], fileSystemScheme: 'org-dartlang-app', outputPath: outputDir.path, compilerOptions: compilerOptions, diff --git a/dwds/test/fixtures/project.dart b/dwds/test/fixtures/project.dart index 82c6aba26..3a828b35c 100644 --- a/dwds/test/fixtures/project.dart +++ b/dwds/test/fixtures/project.dart @@ -22,12 +22,9 @@ class TestProject { late Directory _fixturesCopy; /// The top level directory in which we run the test server, e.g. - /// "/tmp/_testSound/". + /// "/tmp/_testSound". String get absolutePackageDirectory => - // Return it as a directory with a trailing slash. - Directory.fromUri( - _fixturesCopy.absolute.uri.resolve(packageDirectory), - ).uri.path; + _fixturesCopy.absolute.uri.resolve(packageDirectory).path; /// The directory to build and serve, e.g. "example". String get directoryToServe => p.split(webAssetsPath).first; @@ -243,7 +240,6 @@ class TestProject { try { _fixturesCopy.deleteSync(recursive: true); } on FileSystemException catch (_) { - assert(Platform.isWindows); // On Windows, the build daemon process might still be accessing the // working directory, so try again with an exponential backoff. var seconds = 1; From 1c824259efbec6aff75ccfc8ccc66a55fdd8b851 Mon Sep 17 00:00:00 2001 From: Srujan Gaddam Date: Sat, 12 Jul 2025 15:27:38 -0700 Subject: [PATCH 06/12] Fix paths again --- dwds/test/fixtures/project.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dwds/test/fixtures/project.dart b/dwds/test/fixtures/project.dart index 3a828b35c..85c921f2d 100644 --- a/dwds/test/fixtures/project.dart +++ b/dwds/test/fixtures/project.dart @@ -24,7 +24,7 @@ class TestProject { /// The top level directory in which we run the test server, e.g. /// "/tmp/_testSound". String get absolutePackageDirectory => - _fixturesCopy.absolute.uri.resolve(packageDirectory).path; + p.join(_fixturesCopy.absolute.path, packageDirectory); /// The directory to build and serve, e.g. "example". String get directoryToServe => p.split(webAssetsPath).first; From 1b8586dd680292aeec33a0d9378c31e2a8ebebd3 Mon Sep 17 00:00:00 2001 From: Srujan Gaddam Date: Mon, 14 Jul 2025 10:56:53 -0700 Subject: [PATCH 07/12] Move to 24.4.1-wip --- dwds/CHANGELOG.md | 2 ++ dwds/lib/src/version.dart | 2 +- dwds/pubspec.yaml | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/dwds/CHANGELOG.md b/dwds/CHANGELOG.md index 17f414c11..8a6ba0c5a 100644 --- a/dwds/CHANGELOG.md +++ b/dwds/CHANGELOG.md @@ -1,3 +1,5 @@ +## 24.4.1-wip + ## 24.4.0 - Added support for breakpoint registering on a hot reload with the DDC library bundle format using PausePostRequests. diff --git a/dwds/lib/src/version.dart b/dwds/lib/src/version.dart index 2786a7b0a..c8bd103ab 100644 --- a/dwds/lib/src/version.dart +++ b/dwds/lib/src/version.dart @@ -1,2 +1,2 @@ // Generated code. Do not modify. -const packageVersion = '24.4.0'; +const packageVersion = '24.4.1-wip'; diff --git a/dwds/pubspec.yaml b/dwds/pubspec.yaml index f139974a6..da11ad7f1 100644 --- a/dwds/pubspec.yaml +++ b/dwds/pubspec.yaml @@ -1,6 +1,6 @@ name: dwds # Every time this changes you need to run `dart run build_runner build`. -version: 24.4.0 +version: 24.4.1-wip description: >- A service that proxies between the Chrome debug protocol and the Dart VM From 1525eb13f267bbac033a52fcf5f702d5fb35afb5 Mon Sep 17 00:00:00 2001 From: Srujan Gaddam Date: Wed, 16 Jul 2025 17:40:57 -0700 Subject: [PATCH 08/12] Wait until all scripts are parsed before recomputing metadata on a hot reload Fixes https://github.com/dart-lang/webdev/issues/2640 On a hot reload, we wait until all the scripts are downloaded, but we don't wait until all of them are parsed and a script ID is created for them to refer to. This then results in incorrect metadata being computed. Instead, return the srcs that are being loaded during a hot reload, use a controller to determine when scripts are parsed, and only compute metadata once we have parsed all the reloaded scripts. In order to compare the parsed scripts' URLs with the reloaded scripts' URLs, we now require full URLs in the hot reload sources metadata. This is already true in Flutter tools, so this just canonicalizes that and modifies the tests to do the same. To be consistent, hot restart also provides the full URL in the DDC library bundle format and the bootstrap is modified to reflect that (and to clean up some unused code around module tracking). --- dwds/lib/src/debugging/debugger.dart | 6 + dwds/lib/src/debugging/modules.dart | 3 - dwds/lib/src/injected/client.js | 24 ++- dwds/lib/src/loaders/ddc_library_bundle.dart | 4 +- .../src/services/chrome_proxy_service.dart | 31 +++- dwds/test/fixtures/context.dart | 17 +- dwds/test/hot_reload_breakpoints_test.dart | 165 +++++++++++++----- .../ddc_library_bundle_restarter.dart | 11 +- dwds/web/reloader/ddc_restarter.dart | 2 +- dwds/web/reloader/manager.dart | 11 +- dwds/web/reloader/require_restarter.dart | 2 +- dwds/web/reloader/restarter.dart | 11 +- frontend_server_common/lib/src/bootstrap.dart | 10 +- frontend_server_common/lib/src/devfs.dart | 39 ++--- .../lib/src/resident_runner.dart | 31 +++- 15 files changed, 239 insertions(+), 128 deletions(-) diff --git a/dwds/lib/src/debugging/debugger.dart b/dwds/lib/src/debugging/debugger.dart index c96c2bdbe..e68e5d315 100644 --- a/dwds/lib/src/debugging/debugger.dart +++ b/dwds/lib/src/debugging/debugger.dart @@ -56,6 +56,9 @@ class Debugger extends Domain { int _frameErrorCount = 0; + final StreamController parsedScriptsController = + StreamController.broadcast(); + Debugger._( this._remoteDebugger, this._streamNotify, @@ -489,6 +492,9 @@ class Debugger extends Domain { final scriptPath = _pathForChromeScript(e.script.url); if (scriptPath != null) { chromePathToRuntimeScriptId[scriptPath] = e.script.scriptId; + if (parsedScriptsController.hasListener) { + parsedScriptsController.sink.add(e.script.url); + } } } diff --git a/dwds/lib/src/debugging/modules.dart b/dwds/lib/src/debugging/modules.dart index 15f10b06a..a57bfb5b2 100644 --- a/dwds/lib/src/debugging/modules.dart +++ b/dwds/lib/src/debugging/modules.dart @@ -98,9 +98,6 @@ class Modules { ) async { final serverPath = await globalToolConfiguration.loadStrategy .serverPathForModule(entrypoint, module); - // TODO(srujzs): We should wait until all scripts are parsed before - // accessing after a hot reload. See - // https://github.com/dart-lang/webdev/issues/2640. return chromePathToRuntimeScriptId[serverPath]; } diff --git a/dwds/lib/src/injected/client.js b/dwds/lib/src/injected/client.js index e85ffd662..85c394005 100644 --- a/dwds/lib/src/injected/client.js +++ b/dwds/lib/src/injected/client.js @@ -26879,7 +26879,7 @@ }; A.main__closure.prototype = { call$0() { - return A.FutureOfJSAnyToJSPromise_get_toJS(this.manager._restarter.hotReloadStart$1(A.hotReloadSourcesPath()), type$.JSObject); + return A.FutureOfJSAnyToJSPromise_get_toJS(this.manager._restarter.hotReloadStart$1(A.hotReloadSourcesPath()), type$.JSArray_nullable_Object); }, $signature: 10 }; @@ -27296,8 +27296,8 @@ }, hotReloadStart$1(hotReloadSourcesPath) { var $async$goto = 0, - $async$completer = A._makeAsyncAwaitCompleter(type$.JSObject), - $async$returnValue, $async$self = this, t4, srcModuleLibraries, filesToLoad, _this, librariesToReload, t5, t6, srcModuleLibraryCast, module, libraries, t7, t1, t2, t3, xhr, $async$temp1, $async$temp2, $async$temp3; + $async$completer = A._makeAsyncAwaitCompleter(type$.JSArray_nullable_Object), + $async$returnValue, $async$self = this, t4, srcModuleLibraries, filesToLoad, librariesToReload, t5, t6, t7, srcModuleLibraryCast, src, libraries, t8, t1, t2, t3, xhr, $async$temp1, $async$temp2, $async$temp3; var $async$hotReloadStart$1 = A._wrapJsFunctionForAsync(function($async$errorCode, $async$result) { if ($async$errorCode === 1) return A._asyncRethrow($async$result, $async$completer); @@ -27324,23 +27324,21 @@ srcModuleLibraries = $async$temp1.cast$1$0$ax($async$temp2._as($async$temp3.decode$1($async$result)), type$.Map_dynamic_dynamic); t1 = type$.JSArray_nullable_Object; filesToLoad = t1._as(new t2.Array()); - _this = {}; librariesToReload = t1._as(new t2.Array()); - for (t1 = srcModuleLibraries.get$iterator(srcModuleLibraries), t5 = type$.String, t6 = type$.Object; t1.moveNext$0();) { - srcModuleLibraryCast = t1.get$current().cast$2$0(0, t5, t6); - filesToLoad.push(A._asString(srcModuleLibraryCast.$index(0, "src"))); - module = A._asString(srcModuleLibraryCast.$index(0, "module")); - libraries = J.cast$1$0$ax(t4._as(srcModuleLibraryCast.$index(0, "libraries")), t5); - _this[module] = A.jsify(libraries); - for (t7 = libraries.get$iterator(libraries); t7.moveNext$0();) - librariesToReload.push(t7.get$current()); + for (t5 = srcModuleLibraries.get$iterator(srcModuleLibraries), t6 = type$.String, t7 = type$.Object; t5.moveNext$0();) { + srcModuleLibraryCast = t5.get$current().cast$2$0(0, t6, t7); + src = A._asString(srcModuleLibraryCast.$index(0, "src")); + libraries = J.cast$1$0$ax(t4._as(srcModuleLibraryCast.$index(0, "libraries")), t6); + filesToLoad.push(src); + for (t8 = libraries.get$iterator(libraries); t8.moveNext$0();) + librariesToReload.push(t8.get$current()); } t3._as(t3._as(t2.dartDevEmbedder).config).capturedHotReloadEndHandler = A._functionToJS1(new A.DdcLibraryBundleRestarter_hotReloadStart_closure0($async$self)); $async$goto = 4; return A._asyncAwait(A.promiseToFuture(t3._as(t3._as(t2.dartDevEmbedder).hotReload(filesToLoad, librariesToReload)), type$.nullable_Object), $async$hotReloadStart$1); case 4: // returning from await. - $async$returnValue = _this; + $async$returnValue = t1._as(A.jsify(srcModuleLibraries)); // goto return $async$goto = 1; break; diff --git a/dwds/lib/src/loaders/ddc_library_bundle.dart b/dwds/lib/src/loaders/ddc_library_bundle.dart index 3fb592dde..a8f98b7ad 100644 --- a/dwds/lib/src/loaders/ddc_library_bundle.dart +++ b/dwds/lib/src/loaders/ddc_library_bundle.dart @@ -110,7 +110,8 @@ class DdcLibraryBundleStrategy extends LoadStrategy { /// ```json /// [ /// { - /// "src": "", + /// "src": "/", + /// "module": "", /// "libraries": ["", ""], /// }, /// ] @@ -118,6 +119,7 @@ class DdcLibraryBundleStrategy extends LoadStrategy { /// /// `src`: A string that corresponds to the file path containing a DDC library /// bundle. + /// `module`: The name of the library bundle in `src`. /// `libraries`: An array of strings containing the libraries that were /// compiled in `src`. /// diff --git a/dwds/lib/src/services/chrome_proxy_service.dart b/dwds/lib/src/services/chrome_proxy_service.dart index c98bbc8ab..37c50be82 100644 --- a/dwds/lib/src/services/chrome_proxy_service.dart +++ b/dwds/lib/src/services/chrome_proxy_service.dart @@ -1221,6 +1221,22 @@ class ChromeProxyService implements VmServiceInterface { Future _performClientSideHotReload(String isolateId) async { _logger.info('Attempting a hot reload'); + final debugger = await debuggerFuture; + final reloadedSrcs = {}; + final computedReloadedSrcs = Completer(); + final parsedAllReloadedSrcs = Completer(); + // Wait until all the reloaded scripts are parsed before we reinitialize + // metadata below. + final parsedScriptsSubscription = debugger.parsedScriptsController.stream + .listen((url) { + computedReloadedSrcs.future.then((_) { + reloadedSrcs.remove(Uri.parse(url).normalizePath().path); + if (reloadedSrcs.isEmpty) { + parsedAllReloadedSrcs.complete(); + } + }); + }); + // Initiate a hot reload. _logger.info('Issuing \$dartHotReloadStartDwds request'); final remoteObject = await inspector.jsEvaluate( @@ -1228,8 +1244,19 @@ class ChromeProxyService implements VmServiceInterface { awaitPromise: true, returnByValue: true, ); - final reloadedModulesToLibraries = - (remoteObject.value as Map).cast(); + final reloadedSrcModuleLibraries = (remoteObject.value as List).cast(); + final reloadedModulesToLibraries = >{}; + for (final srcModuleLibrary in reloadedSrcModuleLibraries) { + final srcModuleLibraryCast = srcModuleLibrary.cast(); + reloadedSrcs.add( + Uri.parse(srcModuleLibraryCast['src'] as String).normalizePath().path, + ); + reloadedModulesToLibraries[srcModuleLibraryCast['module'] as String] = + (srcModuleLibraryCast['libraries'] as List).cast(); + } + computedReloadedSrcs.complete(); + if (reloadedSrcs.isNotEmpty) await parsedAllReloadedSrcs.future; + await parsedScriptsSubscription.cancel(); if (!pauseIsolatesOnStart) { // Finish hot reload immediately. diff --git a/dwds/test/fixtures/context.dart b/dwds/test/fixtures/context.dart index 68687d0f6..9794fbebb 100644 --- a/dwds/test/fixtures/context.dart +++ b/dwds/test/fixtures/context.dart @@ -339,11 +339,12 @@ class TestContext { _hostname = appMetadata.hostname; await webRunner.run( frontendServerFileSystem, - _hostname, - assetServerPort, - filePathToServe, + hostname: _hostname, + port: assetServerPort, + index: filePathToServe, initialCompile: true, fullRestart: false, + fileServerUri: null, ); if (testSettings.enableExpressionEvaluation) { @@ -414,7 +415,7 @@ class TestContext { 'remote-debugging-port=$debugPort', if (enableDebugExtension) '--load-extension=debug_extension/prod_build', - if (headless) '--headless', + // if (headless) '--headless', ], }, }); @@ -433,6 +434,10 @@ class TestContext { final appConnectionCompleter = Completer(); final connection = ChromeConnection('localhost', debugPort); + // TODO(srujzs): In the case of the frontend server, it doesn't make sense + // that we initialize a new HTTP server instead of reusing the one in + // `TestAssetServer`. Why not just use that one? That seems to match with + // what Flutter tools is doing. _testServer = await TestServer.start( debugSettings: debugSettings.copyWith( expressionCompiler: expressionCompiler, @@ -592,11 +597,9 @@ class TestContext { Future recompile({required bool fullRestart}) async { await webRunner.run( frontendServerFileSystem, - _hostname, - await findUnusedPort(), - webCompatiblePath([project.directoryToServe, project.filePathToServe]), initialCompile: false, fullRestart: fullRestart, + fileServerUri: Uri.parse('http://${testServer.host}:${testServer.port}'), ); return; } diff --git a/dwds/test/hot_reload_breakpoints_test.dart b/dwds/test/hot_reload_breakpoints_test.dart index bd9ec0bff..5b120d845 100644 --- a/dwds/test/hot_reload_breakpoints_test.dart +++ b/dwds/test/hot_reload_breakpoints_test.dart @@ -8,7 +8,6 @@ library; import 'dart:async'; -import 'dart:io'; import 'package:dwds/expression_compiler.dart'; import 'package:test/test.dart'; @@ -207,6 +206,37 @@ void main() { Future waitForBreakpoint() => expectLater(stream, emitsThrough(_hasKind(EventKind.kPauseBreakpoint))); + test('empty hot reload keeps breakpoints', () async { + final genString = 'main gen0'; + + final bp = await addBreakpoint( + file: mainFile, + breakpointMarker: callLogMarker, + ); + + var breakpointFuture = waitForBreakpoint(); + + await callEvaluate(); + + // Should break at `callLog`. + await breakpointFuture; + await resumeAndExpectLog(genString); + + await context.recompile(fullRestart: false); + + await hotReloadAndHandlePausePost([ + (file: mainFile, breakpointMarker: callLogMarker, bp: bp), + ]); + + breakpointFuture = waitForBreakpoint(); + + await callEvaluate(); + + // Should break at `callLog`. + await breakpointFuture; + await resumeAndExpectLog(genString); + }); + test('after edit and hot reload, breakpoint is in new file', () async { final oldString = 'main gen0'; final newString = 'main gen1'; @@ -359,52 +389,109 @@ void main() { }, ); - test( - 'breakpoint in captured code is deleted', - () async { - var bp = await addBreakpoint( - file: mainFile, - breakpointMarker: capturedStringMarker, - ); + // Test that we wait for all scripts to be parsed first before computing + // location metadata. + test('after adding many files and putting breakpoint in the last one,' + 'breakpoint is correctly registered', () async { + final genLog = 'main gen0'; - final oldLog = "log('\$mainValue');"; - final newLog = "log('\${closure()}');"; - await makeEditAndRecompile(mainFile, oldLog, newLog); + final bp = await addBreakpoint( + file: mainFile, + breakpointMarker: callLogMarker, + ); - bp = - (await hotReloadAndHandlePausePost([ - (file: mainFile, breakpointMarker: capturedStringMarker, bp: bp), - ])).first; + var breakpointFuture = waitForBreakpoint(); - final breakpointFuture = waitForBreakpoint(); + await callEvaluate(); - await callEvaluate(); + // Should break at `callLog`. + await breakpointFuture; + await resumeAndExpectLog(genLog); - // Should break at `capturedString`. - await breakpointFuture; - final oldCapturedString = 'captured closure gen0'; - // Closure gets evaluated for the first time. - await resumeAndExpectLog(oldCapturedString); - - final newCapturedString = 'captured closure gen1'; - await makeEditAndRecompile( - mainFile, - oldCapturedString, - newCapturedString, + // Add library files, import them, but only refer to the last one in main. + final numFiles = 50; + for (var i = 1; i <= numFiles; i++) { + final libFile = 'library$i.dart'; + context.addLibraryFile( + libFileName: libFile, + contents: '''String get libraryValue$i { + return 'lib gen$i'; // Breakpoint: libValue$i + }''', ); + final oldImports = "import 'dart:js_interop';"; + final newImports = + '$oldImports\n' + "import 'package:_test_hot_reload_breakpoints/$libFile';"; + makeEdit(mainFile, oldImports, newImports); + } + final oldLog = "log('\$mainValue');"; + final newLog = "log('\$libraryValue$numFiles');"; + await makeEditAndRecompile(mainFile, oldLog, newLog); - await hotReloadAndHandlePausePost([ - (file: mainFile, breakpointMarker: capturedStringMarker, bp: bp), - ]); + await hotReloadAndHandlePausePost([ + (file: mainFile, breakpointMarker: callLogMarker, bp: bp), + ( + file: 'library$numFiles.dart', + breakpointMarker: 'libValue$numFiles', + bp: null, + ), + ]); - // Breakpoint should not be hit as it's now deleted. We should also see - // the old string still as the closure has not been reevaluated. - await callEvaluateAndExpectLog(oldCapturedString); - }, - // TODO(srujzs): Re-enable after - // https://github.com/dart-lang/webdev/issues/2640. - skip: Platform.isWindows, - ); + breakpointFuture = waitForBreakpoint(); + + await callEvaluate(); + + // Should break at `callLog`. + await breakpointFuture; + + breakpointFuture = waitForBreakpoint(); + + await resume(); + // Should break at the breakpoint in the last file. + await breakpointFuture; + await resumeAndExpectLog('lib gen$numFiles'); + }); + + test('breakpoint in captured code is deleted', () async { + var bp = await addBreakpoint( + file: mainFile, + breakpointMarker: capturedStringMarker, + ); + + final oldLog = "log('\$mainValue');"; + final newLog = "log('\${closure()}');"; + await makeEditAndRecompile(mainFile, oldLog, newLog); + + bp = + (await hotReloadAndHandlePausePost([ + (file: mainFile, breakpointMarker: capturedStringMarker, bp: bp), + ])).first; + + final breakpointFuture = waitForBreakpoint(); + + await callEvaluate(); + + // Should break at `capturedString`. + await breakpointFuture; + final oldCapturedString = 'captured closure gen0'; + // Closure gets evaluated for the first time. + await resumeAndExpectLog(oldCapturedString); + + final newCapturedString = 'captured closure gen1'; + await makeEditAndRecompile( + mainFile, + oldCapturedString, + newCapturedString, + ); + + await hotReloadAndHandlePausePost([ + (file: mainFile, breakpointMarker: capturedStringMarker, bp: bp), + ]); + + // Breakpoint should not be hit as it's now deleted. We should also see + // the old string still as the closure has not been reevaluated. + await callEvaluateAndExpectLog(oldCapturedString); + }); }, timeout: Timeout.factor(2)); group('when pause_isolates_on_start is false', () { diff --git a/dwds/web/reloader/ddc_library_bundle_restarter.dart b/dwds/web/reloader/ddc_library_bundle_restarter.dart index 6ce3faea2..5752cb983 100644 --- a/dwds/web/reloader/ddc_library_bundle_restarter.dart +++ b/dwds/web/reloader/ddc_library_bundle_restarter.dart @@ -5,7 +5,6 @@ import 'dart:async'; import 'dart:convert'; import 'dart:js_interop'; -import 'dart:js_interop_unsafe'; import 'package:dwds/src/utilities/shared.dart'; @@ -86,7 +85,7 @@ class DdcLibraryBundleRestarter implements Restarter { } @override - Future hotReloadStart(String hotReloadSourcesPath) async { + Future> hotReloadStart(String hotReloadSourcesPath) async { final completer = Completer(); final xhr = _XMLHttpRequest(); xhr.withCredentials = true; @@ -104,15 +103,13 @@ class DdcLibraryBundleRestarter implements Restarter { final srcModuleLibraries = (json.decode(responseText) as List).cast(); final filesToLoad = JSArray(); - final moduleMap = JSObject(); final librariesToReload = JSArray(); for (final srcModuleLibrary in srcModuleLibraries) { final srcModuleLibraryCast = srcModuleLibrary.cast(); - filesToLoad.push((srcModuleLibraryCast['src'] as String).toJS); - final module = srcModuleLibraryCast['module'] as String; + final src = srcModuleLibraryCast['src'] as String; final libraries = (srcModuleLibraryCast['libraries'] as List).cast(); - moduleMap[module] = libraries.jsify(); + filesToLoad.push(src.toJS); for (final library in libraries) { librariesToReload.push(library.toJS); } @@ -122,7 +119,7 @@ class DdcLibraryBundleRestarter implements Restarter { _capturedHotReloadEndCallback = hotReloadEndCallback; }.toJS; await _dartDevEmbedder.hotReload(filesToLoad, librariesToReload).toDart; - return moduleMap; + return srcModuleLibraries.jsify() as JSArray; } @override diff --git a/dwds/web/reloader/ddc_restarter.dart b/dwds/web/reloader/ddc_restarter.dart index 30399cfcb..ca324f540 100644 --- a/dwds/web/reloader/ddc_restarter.dart +++ b/dwds/web/reloader/ddc_restarter.dart @@ -47,7 +47,7 @@ class DdcRestarter implements Restarter { ); @override - Future hotReloadStart(String hotReloadSourcesPath) => + Future> hotReloadStart(String hotReloadSourcesPath) => throw UnimplementedError( 'Hot reload is not supported for the DDC module format.', ); diff --git a/dwds/web/reloader/manager.dart b/dwds/web/reloader/manager.dart index dc6c5a0b4..52ee179b5 100644 --- a/dwds/web/reloader/manager.dart +++ b/dwds/web/reloader/manager.dart @@ -42,27 +42,28 @@ class ReloadingManager { await _restarter.hotReloadEnd(); } - /// Computes the sources and libraries to reload, loads them into the page, - /// and returns a map of module names to libraries using - /// [hotReloadSourcesPath] as the path to a JSONified list of maps which + /// Using [hotReloadSourcesPath] as the path to a JSONified list of maps which /// follows the following format: /// /// ```json /// [ /// { - /// "src": "", + /// "src": "/", /// "module": "", /// "libraries": ["", ""], /// }, /// ] /// ``` /// + /// computes the sources and libraries to reload, loads them into the page, + /// and returns a JS version of the list of maps. + /// /// `src`: A string that corresponds to the file path containing a DDC library /// bundle. /// `module`: The name of the library bundle in `src`. /// `libraries`: An array of strings containing the libraries that were /// compiled in `src`. - Future hotReloadStart(String hotReloadSourcesPath) => + Future> hotReloadStart(String hotReloadSourcesPath) => _restarter.hotReloadStart(hotReloadSourcesPath); /// Does a hard reload of the application. diff --git a/dwds/web/reloader/require_restarter.dart b/dwds/web/reloader/require_restarter.dart index 01115fffb..88cc5b98e 100644 --- a/dwds/web/reloader/require_restarter.dart +++ b/dwds/web/reloader/require_restarter.dart @@ -168,7 +168,7 @@ class RequireRestarter implements Restarter { ); @override - Future hotReloadStart(String hotReloadSourcesPath) => + Future> hotReloadStart(String hotReloadSourcesPath) => throw UnimplementedError( 'Hot reload is not supported for the AMD module format.', ); diff --git a/dwds/web/reloader/restarter.dart b/dwds/web/reloader/restarter.dart index 31cb09484..aee6529c7 100644 --- a/dwds/web/reloader/restarter.dart +++ b/dwds/web/reloader/restarter.dart @@ -13,25 +13,26 @@ abstract class Restarter { /// reload by pushing the libraries into the Dart runtime. Future hotReloadEnd(); - /// Computes the sources and libraries to reload, loads them into the page, - /// and returns a map of module names to libraries using - /// [hotReloadSourcesPath] as the path to a JSONified list of maps which + /// Using [hotReloadSourcesPath] as the path to a JSONified list of maps which /// follows the following format: /// /// ```json /// [ /// { - /// "src": "", + /// "src": "/", /// "module": "", /// "libraries": ["", ""], /// }, /// ] /// ``` /// + /// computes the sources and libraries to reload, loads them into the page, + /// and returns a JS version of the list of maps. + /// /// `src`: A string that corresponds to the file path containing a DDC library /// bundle. /// `module`: The name of the library bundle in `src`. /// `libraries`: An array of strings containing the libraries that were /// compiled in `src`. - Future hotReloadStart(String hotReloadSourcesPath); + Future> hotReloadStart(String hotReloadSourcesPath); } diff --git a/frontend_server_common/lib/src/bootstrap.dart b/frontend_server_common/lib/src/bootstrap.dart index b45ff45c2..0854e44d0 100644 --- a/frontend_server_common/lib/src/bootstrap.dart +++ b/frontend_server_common/lib/src/bootstrap.dart @@ -503,15 +503,7 @@ $_simpleLoaderScript for (var i = 0; i < scripts.length; i++) { var script = scripts[i]; if (script.id == null) continue; - var src = _currentDirectory + '/' + script.src.toString(); - var oldSrc = window.\$dartLoader.moduleIdToUrl.get(script.id); - - // We might actually load from a different uri, delete the old one - // just to be sure. - window.\$dartLoader.urlToModuleId.delete(oldSrc); - - window.\$dartLoader.moduleIdToUrl.set(script.id, src); - window.\$dartLoader.urlToModuleId.set(src, script.id); + var src = script.src.toString(); numToLoad++; diff --git a/frontend_server_common/lib/src/devfs.dart b/frontend_server_common/lib/src/devfs.dart index a334b832a..caedddd73 100644 --- a/frontend_server_common/lib/src/devfs.dart +++ b/frontend_server_common/lib/src/devfs.dart @@ -77,6 +77,10 @@ class WebDevFS { required List invalidatedFiles, required bool initialCompile, required bool fullRestart, + // The uri of the `HttpServer` that handles file requests. + // TODO(srujzs): This should be the same as the uri of the AssetServer, but + // currently is not. Delete when that's fixed. + required Uri? fileServerUri, }) async { final mainPath = mainUri.toFilePath(); final outputDirectory = fileSystem.directory( @@ -215,9 +219,9 @@ class WebDevFS { compilerOptions.canaryFeatures && !initialCompile) { if (fullRestart) { - performRestart(modules); + performRestart(modules, fileServerUri!); } else { - performReload(modules, prefix); + performReload(modules, prefix, fileServerUri!); } } return UpdateFSReport( @@ -235,15 +239,15 @@ class WebDevFS { /// ```json /// [ /// { - /// "src": "", + /// "src": "/", /// "id": "", /// }, /// ] /// ``` - void performRestart(List modules) { + void performRestart(List modules, Uri fileServerUri) { final srcIdsList = >[]; for (final src in modules) { - srcIdsList.add({'src': src, 'id': src}); + srcIdsList.add({'src': '$fileServerUri/$src', 'id': src}); } assetServer.writeFile('restart_scripts.json', json.encode(srcIdsList)); } @@ -263,7 +267,7 @@ class WebDevFS { /// ```json /// [ /// { - /// "src": "", + /// "src": "/", /// "module": "", /// "libraries": ["", ""], /// }, @@ -275,7 +279,8 @@ class WebDevFS { /// /// [entrypointDirectory] is used to make the module paths relative to the /// entrypoint, which is needed in order to load `src`s correctly. - void performReload(List modules, String entrypointDirectory) { + void performReload( + List modules, String entrypointDirectory, Uri fileServerUri) { final moduleToLibrary = >[]; for (final module in modules) { final metadata = ModuleMetadata.fromJson( @@ -285,7 +290,7 @@ class WebDevFS { ); final libraries = metadata.libraries.keys.toList(); moduleToLibrary.add({ - 'src': _findModuleToLoad(module, entrypointDirectory), + 'src': '$fileServerUri/$module', 'module': metadata.name, 'libraries': libraries }); @@ -293,24 +298,6 @@ class WebDevFS { assetServer.writeFile(reloadScriptsFileName, json.encode(moduleToLibrary)); } - /// Given a [module] location from the [ModuleMetadata], return its path in - /// the server relative to the entrypoint in [entrypointDirectory]. - /// - /// This is needed in cases where the entrypoint is in a subdirectory in the - /// package. - String _findModuleToLoad(String module, String entrypointDirectory) { - if (entrypointDirectory.isEmpty) return module; - assert(entrypointDirectory.endsWith('/')); - if (module.startsWith(entrypointDirectory)) { - return module.substring(entrypointDirectory.length); - } - var numDirs = entrypointDirectory.split('/').length - 1; - while (numDirs-- > 0) { - module = '../$module'; - } - return module; - } - File get ddcModuleLoaderJS => fileSystem.file(sdkLayout.ddcModuleLoaderJsPath); File get requireJS => fileSystem.file(sdkLayout.requireJsPath); diff --git a/frontend_server_common/lib/src/resident_runner.dart b/frontend_server_common/lib/src/resident_runner.dart index 604b33035..c57a902e9 100644 --- a/frontend_server_common/lib/src/resident_runner.dart +++ b/frontend_server_common/lib/src/resident_runner.dart @@ -69,21 +69,25 @@ class ResidentWebRunner { late Iterable modules; Future run( - FileSystem fileSystem, + FileSystem fileSystem, { String? hostname, - int port, - String index, { + int? port, + String? index, required bool initialCompile, required bool fullRestart, + // The uri of the `HttpServer` that handles file requests. + // TODO(srujzs): This should be the same as the uri of the AssetServer, but + // currently is not. Delete when that's fixed. + required Uri? fileServerUri, }) async { _projectFileInvalidator ??= ProjectFileInvalidator(fileSystem: fileSystem); devFS ??= WebDevFS( fileSystem: fileSystem, hostname: hostname ?? 'localhost', - port: port, + port: port!, projectDirectory: projectDirectory, packageUriMapper: packageUriMapper, - index: index, + index: index!, urlTunneler: urlTunneler, sdkLayout: sdkLayout, compilerOptions: compilerOptions, @@ -91,7 +95,9 @@ class ResidentWebRunner { uri ??= await devFS!.create(); final report = await _updateDevFS( - initialCompile: initialCompile, fullRestart: fullRestart); + initialCompile: initialCompile, + fullRestart: fullRestart, + fileServerUri: fileServerUri); if (!report.success) { _logger.severe('Failed to compile application.'); return 1; @@ -103,8 +109,14 @@ class ResidentWebRunner { return 0; } - Future _updateDevFS( - {required bool initialCompile, required bool fullRestart}) async { + Future _updateDevFS({ + required bool initialCompile, + required bool fullRestart, + // The uri of the `TestServer` that handles file requests. + // TODO(srujzs): This should be the same as the uri of the AssetServer, but + // currently is not. Delete when that's fixed. + required Uri? fileServerUri, + }) async { final invalidationResult = await _projectFileInvalidator!.findInvalidated( lastCompiled: devFS!.lastCompiled, urisToMonitor: devFS!.sources, @@ -116,7 +128,8 @@ class ResidentWebRunner { generator: generator, invalidatedFiles: invalidationResult.uris!, initialCompile: initialCompile, - fullRestart: fullRestart); + fullRestart: fullRestart, + fileServerUri: fileServerUri); return report; } From 72d52751a8f6adbe47300b5ba200502a47cc2fa4 Mon Sep 17 00:00:00 2001 From: Srujan Gaddam Date: Wed, 16 Jul 2025 17:56:01 -0700 Subject: [PATCH 09/12] modify changelog --- dwds/CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/dwds/CHANGELOG.md b/dwds/CHANGELOG.md index 8a6ba0c5a..aa97c636b 100644 --- a/dwds/CHANGELOG.md +++ b/dwds/CHANGELOG.md @@ -1,5 +1,8 @@ ## 24.4.1-wip +- Fixed an issue where we didn't wait until all scripts were parsed before + recomputing metadata on a hot reload. + ## 24.4.0 - Added support for breakpoint registering on a hot reload with the DDC library bundle format using PausePostRequests. From 06f64d396afe531309db2a8cac86f384a3387059 Mon Sep 17 00:00:00 2001 From: Srujan Gaddam Date: Wed, 16 Jul 2025 18:00:26 -0700 Subject: [PATCH 10/12] Reenable headless --- dwds/test/fixtures/context.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dwds/test/fixtures/context.dart b/dwds/test/fixtures/context.dart index 9794fbebb..caa52bcb6 100644 --- a/dwds/test/fixtures/context.dart +++ b/dwds/test/fixtures/context.dart @@ -415,7 +415,7 @@ class TestContext { 'remote-debugging-port=$debugPort', if (enableDebugExtension) '--load-extension=debug_extension/prod_build', - // if (headless) '--headless', + if (headless) '--headless', ], }, }); From 32c4ad1a0943b3fc132f62396b4a2548985453e1 Mon Sep 17 00:00:00 2001 From: Srujan Gaddam Date: Mon, 21 Jul 2025 10:15:31 -0700 Subject: [PATCH 11/12] Revert module tracking cleanup, address review comments, and clean up a TODO to remove a test --- .../common/chrome_proxy_service_common.dart | 21 ---------- dwds/test/fixtures/context.dart | 11 ++--- frontend_server_common/lib/src/bootstrap.dart | 8 ++++ frontend_server_common/lib/src/devfs.dart | 4 +- .../lib/src/resident_runner.dart | 40 +++++++++++-------- 5 files changed, 36 insertions(+), 48 deletions(-) diff --git a/dwds/test/common/chrome_proxy_service_common.dart b/dwds/test/common/chrome_proxy_service_common.dart index 92b8a3e4e..17eb5a0a0 100644 --- a/dwds/test/common/chrome_proxy_service_common.dart +++ b/dwds/test/common/chrome_proxy_service_common.dart @@ -2225,27 +2225,6 @@ void runTests({ ); }); - test('reloadSources', () async { - final service = context.service; - final vm = await service.getVM(); - final isolateId = vm.isolates!.first.id!; - final report = await service.reloadSources(isolateId); - // TODO(srujzs): This naturally fails regardless of the module format - // because we didn't set up prerequisite state (the reload scripts). We - // should create new tests for hot reload within DWDS and remove this - // test. - expect(report.success, false); - // Make sure that a notice was provided in the expected format. - final notices = report.json?['notices']; - expect(notices, isNotNull); - expect(notices is List, isTrue); - final noticeList = (notices as List).cast(); - expect(noticeList, isNotEmpty); - final message = noticeList[0]['message']; - expect(message, isNotNull); - expect(message is String && message.isNotEmpty, isTrue); - }); - test('setIsolatePauseMode', () async { final service = context.service; final vm = await service.getVM(); diff --git a/dwds/test/fixtures/context.dart b/dwds/test/fixtures/context.dart index caa52bcb6..1f8686cbd 100644 --- a/dwds/test/fixtures/context.dart +++ b/dwds/test/fixtures/context.dart @@ -342,9 +342,6 @@ class TestContext { hostname: _hostname, port: assetServerPort, index: filePathToServe, - initialCompile: true, - fullRestart: false, - fileServerUri: null, ); if (testSettings.enableExpressionEvaluation) { @@ -436,8 +433,8 @@ class TestContext { // TODO(srujzs): In the case of the frontend server, it doesn't make sense // that we initialize a new HTTP server instead of reusing the one in - // `TestAssetServer`. Why not just use that one? That seems to match with - // what Flutter tools is doing. + // `TestAssetServer`. We should instead use that one to align with Flutter + // tools. _testServer = await TestServer.start( debugSettings: debugSettings.copyWith( expressionCompiler: expressionCompiler, @@ -595,9 +592,7 @@ class TestContext { } Future recompile({required bool fullRestart}) async { - await webRunner.run( - frontendServerFileSystem, - initialCompile: false, + await webRunner.rerun( fullRestart: fullRestart, fileServerUri: Uri.parse('http://${testServer.host}:${testServer.port}'), ); diff --git a/frontend_server_common/lib/src/bootstrap.dart b/frontend_server_common/lib/src/bootstrap.dart index 0854e44d0..8f1073278 100644 --- a/frontend_server_common/lib/src/bootstrap.dart +++ b/frontend_server_common/lib/src/bootstrap.dart @@ -504,6 +504,14 @@ $_simpleLoaderScript var script = scripts[i]; if (script.id == null) continue; var src = script.src.toString(); + var oldSrc = window.\$dartLoader.moduleIdToUrl.get(script.id); + + // We might actually load from a different uri, delete the old one + // just to be sure. + window.\$dartLoader.urlToModuleId.delete(oldSrc); + + window.\$dartLoader.moduleIdToUrl.set(script.id, src); + window.\$dartLoader.urlToModuleId.set(src, script.id); numToLoad++; diff --git a/frontend_server_common/lib/src/devfs.dart b/frontend_server_common/lib/src/devfs.dart index caedddd73..b6caf471d 100644 --- a/frontend_server_common/lib/src/devfs.dart +++ b/frontend_server_common/lib/src/devfs.dart @@ -78,8 +78,8 @@ class WebDevFS { required bool initialCompile, required bool fullRestart, // The uri of the `HttpServer` that handles file requests. - // TODO(srujzs): This should be the same as the uri of the AssetServer, but - // currently is not. Delete when that's fixed. + // TODO(srujzs): This should be the same as the uri of the AssetServer to + // align with Flutter tools, but currently is not. Delete when that's fixed. required Uri? fileServerUri, }) async { final mainPath = mainUri.toFilePath(); diff --git a/frontend_server_common/lib/src/resident_runner.dart b/frontend_server_common/lib/src/resident_runner.dart index c57a902e9..9909e2738 100644 --- a/frontend_server_common/lib/src/resident_runner.dart +++ b/frontend_server_common/lib/src/resident_runner.dart @@ -66,28 +66,21 @@ class ResidentWebRunner { ProjectFileInvalidator? _projectFileInvalidator; WebDevFS? devFS; Uri? uri; - late Iterable modules; Future run( FileSystem fileSystem, { String? hostname, - int? port, - String? index, - required bool initialCompile, - required bool fullRestart, - // The uri of the `HttpServer` that handles file requests. - // TODO(srujzs): This should be the same as the uri of the AssetServer, but - // currently is not. Delete when that's fixed. - required Uri? fileServerUri, + required int port, + required String index, }) async { _projectFileInvalidator ??= ProjectFileInvalidator(fileSystem: fileSystem); devFS ??= WebDevFS( fileSystem: fileSystem, hostname: hostname ?? 'localhost', - port: port!, + port: port, projectDirectory: projectDirectory, packageUriMapper: packageUriMapper, - index: index!, + index: index, urlTunneler: urlTunneler, sdkLayout: sdkLayout, compilerOptions: compilerOptions, @@ -95,15 +88,28 @@ class ResidentWebRunner { uri ??= await devFS!.create(); final report = await _updateDevFS( - initialCompile: initialCompile, - fullRestart: fullRestart, - fileServerUri: fileServerUri); + initialCompile: true, fullRestart: false, fileServerUri: null); if (!report.success) { _logger.severe('Failed to compile application.'); return 1; } - modules = report.invalidatedModules!; + generator.accept(); + return 0; + } + + Future rerun( + {required bool fullRestart, + // The uri of the `HttpServer` that handles file requests. + // TODO(srujzs): This should be the same as the uri of the AssetServer to + // align with Flutter tools, but currently is not. Delete when that's fixed. + required Uri fileServerUri}) async { + final report = await _updateDevFS( + initialCompile: false, fullRestart: fullRestart, fileServerUri: null); + if (!report.success) { + _logger.severe('Failed to compile application.'); + return 1; + } generator.accept(); return 0; @@ -113,8 +119,8 @@ class ResidentWebRunner { required bool initialCompile, required bool fullRestart, // The uri of the `TestServer` that handles file requests. - // TODO(srujzs): This should be the same as the uri of the AssetServer, but - // currently is not. Delete when that's fixed. + // TODO(srujzs): This should be the same as the uri of the AssetServer to + // align with Flutter tools, but currently is not. Delete when that's fixed. required Uri? fileServerUri, }) async { final invalidationResult = await _projectFileInvalidator!.findInvalidated( From 7046d532bec4f133ec9ec9be5779c1d760469980 Mon Sep 17 00:00:00 2001 From: Srujan Gaddam Date: Mon, 21 Jul 2025 11:22:39 -0700 Subject: [PATCH 12/12] Fix null uri passed --- frontend_server_common/lib/src/resident_runner.dart | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/frontend_server_common/lib/src/resident_runner.dart b/frontend_server_common/lib/src/resident_runner.dart index 9909e2738..6802a3689 100644 --- a/frontend_server_common/lib/src/resident_runner.dart +++ b/frontend_server_common/lib/src/resident_runner.dart @@ -105,7 +105,9 @@ class ResidentWebRunner { // align with Flutter tools, but currently is not. Delete when that's fixed. required Uri fileServerUri}) async { final report = await _updateDevFS( - initialCompile: false, fullRestart: fullRestart, fileServerUri: null); + initialCompile: false, + fullRestart: fullRestart, + fileServerUri: fileServerUri); if (!report.success) { _logger.severe('Failed to compile application.'); return 1;