From e666ea8de4da984e5503c445f181307f9062ba85 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Mon, 13 Jul 2020 15:12:46 -0700 Subject: [PATCH] [flutter_tools] cleanups to web runner functionality (#61178) Skip unnecessary parsing of chrome URI. Ensure stack traces are initialized in web server. Disclaimer on web server that it does not support debugging and remove help message. Fix generated entrypoint to check for main(List args) - Fixes #59643 - Fixes #55084 - Fixes #60417 --- .../src/build_runner/resident_web_runner.dart | 12 ++++- packages/flutter_tools/lib/src/doctor.dart | 2 +- .../lib/src/test/flutter_web_platform.dart | 4 +- .../flutter_tools/lib/src/web/bootstrap.dart | 26 +++++----- .../flutter_tools/lib/src/web/chrome.dart | 51 +++++-------------- .../flutter_tools/lib/src/web/web_device.dart | 8 ++- .../test/general.shard/web/chrome_test.dart | 18 ++----- .../general.shard/web/web_validator_test.dart | 2 +- .../test_data/basic_project.dart | 43 ++++++++++++++++ .../test/integration.shard/web_run_test.dart | 31 +++++++++++ 10 files changed, 122 insertions(+), 75 deletions(-) create mode 100644 packages/flutter_tools/test/integration.shard/web_run_test.dart diff --git a/packages/flutter_tools/lib/src/build_runner/resident_web_runner.dart b/packages/flutter_tools/lib/src/build_runner/resident_web_runner.dart index 6321228de00..2e93e2d36c5 100644 --- a/packages/flutter_tools/lib/src/build_runner/resident_web_runner.dart +++ b/packages/flutter_tools/lib/src/build_runner/resident_web_runner.dart @@ -189,7 +189,9 @@ abstract class ResidentWebRunner extends ResidentRunner { globals.printStatus(''); globals.printStatus(message); const String quitMessage = 'To quit, press "q".'; - globals.printStatus('For a more detailed help message, press "h". $quitMessage'); + if (device.device is! WebServerDevice) { + globals.printStatus('For a more detailed help message, press "h". $quitMessage'); + } } @override @@ -636,6 +638,7 @@ class _ResidentWebRunner extends ResidentWebRunner { '// Flutter web bootstrap script for $importedEntrypoint.', '', "import 'dart:ui' as ui;", + "import 'dart:async';", '', "import '$importedEntrypoint' as entrypoint;", if (hasWebPlugins) @@ -643,11 +646,16 @@ class _ResidentWebRunner extends ResidentWebRunner { if (hasWebPlugins) "import '$generatedImport';", '', + 'typedef _UnaryFunction = dynamic Function(List args);', + 'typedef _NullaryFunction = dynamic Function();', 'Future main() async {', if (hasWebPlugins) ' registerPlugins(webPluginRegistry);', ' await ui.webOnlyInitializePlatform();', - ' entrypoint.main();', + ' if (entrypoint.main is _UnaryFunction) {', + ' return (entrypoint.main as _UnaryFunction)([]);', + ' }', + ' return (entrypoint.main as _NullaryFunction)();', '}', '', ].join('\n'); diff --git a/packages/flutter_tools/lib/src/doctor.dart b/packages/flutter_tools/lib/src/doctor.dart index 372d0ee5fad..069bb7757c7 100644 --- a/packages/flutter_tools/lib/src/doctor.dart +++ b/packages/flutter_tools/lib/src/doctor.dart @@ -88,10 +88,10 @@ class _DefaultDoctorValidatorsProvider implements DoctorValidatorsProvider { chromiumLauncher: ChromiumLauncher( browserFinder: findChromeExecutable, fileSystem: globals.fs, - logger: globals.logger, operatingSystemUtils: globals.os, platform: globals.platform, processManager: globals.processManager, + logger: globals.logger, ), platform: globals.platform, ), diff --git a/packages/flutter_tools/lib/src/test/flutter_web_platform.dart b/packages/flutter_tools/lib/src/test/flutter_web_platform.dart index a62ed06673e..63f2addb0b5 100644 --- a/packages/flutter_tools/lib/src/test/flutter_web_platform.dart +++ b/packages/flutter_tools/lib/src/test/flutter_web_platform.dart @@ -628,9 +628,9 @@ class BrowserManager { browserFinder: findChromeExecutable, fileSystem: globals.fs, operatingSystemUtils: globals.os, - logger: globals.logger, platform: globals.platform, processManager: globals.processManager, + logger: globals.logger, ); final Chromium chrome = await chromiumLauncher.launch(url.toString(), headless: headless); @@ -668,7 +668,7 @@ class BrowserManager { /// Loads [_BrowserEnvironment]. Future<_BrowserEnvironment> _loadBrowserEnvironment() async { return _BrowserEnvironment( - this, null, _browser.remoteDebuggerUri, _onRestartController.stream); + this, null, _browser.chromeConnection.url, _onRestartController.stream); } /// Tells the browser to load a test suite from the URL [url]. diff --git a/packages/flutter_tools/lib/src/web/bootstrap.dart b/packages/flutter_tools/lib/src/web/bootstrap.dart index 76a8302a153..e0a076b132d 100644 --- a/packages/flutter_tools/lib/src/web/bootstrap.dart +++ b/packages/flutter_tools/lib/src/web/bootstrap.dart @@ -66,19 +66,19 @@ define("main_module.bootstrap", ["$entrypoint", "dart_sdk"], function(app, dart_ window.\$dartLoader.rootDirectories = []; if (window.\$requireLoader) { window.\$requireLoader.getModuleLibraries = dart_sdk.dart.getModuleLibraries; - if (window.\$dartStackTraceUtility && !window.\$dartStackTraceUtility.ready) { - window.\$dartStackTraceUtility.ready = true; - let dart = dart_sdk.dart; - window.\$dartStackTraceUtility.setSourceMapProvider(function(url) { - var baseUrl = window.location.protocol + '//' + window.location.host; - url = url.replace(baseUrl + '/', ''); - if (url == 'dart_sdk.js') { - return dart.getSourceMap('dart_sdk'); - } - url = url.replace(".lib.js", ""); - return dart.getSourceMap(url); - }); - } + } + if (window.\$dartStackTraceUtility && !window.\$dartStackTraceUtility.ready) { + window.\$dartStackTraceUtility.ready = true; + let dart = dart_sdk.dart; + window.\$dartStackTraceUtility.setSourceMapProvider(function(url) { + var baseUrl = window.location.protocol + '//' + window.location.host; + url = url.replace(baseUrl + '/', ''); + if (url == 'dart_sdk.js') { + return dart.getSourceMap('dart_sdk'); + } + url = url.replace(".lib.js", ""); + return dart.getSourceMap(url); + }); } }); '''; diff --git a/packages/flutter_tools/lib/src/web/chrome.dart b/packages/flutter_tools/lib/src/web/chrome.dart index 7601bfa7b89..b029a312b34 100644 --- a/packages/flutter_tools/lib/src/web/chrome.dart +++ b/packages/flutter_tools/lib/src/web/chrome.dart @@ -15,7 +15,6 @@ import '../base/logger.dart'; import '../base/os.dart'; import '../base/platform.dart'; import '../convert.dart'; -import '../globals.dart' as globals; /// An environment variable used to override the location of Google Chrome. const String kChromeEnvironment = 'CHROME_EXECUTABLE'; @@ -105,14 +104,14 @@ class ChromiumLauncher { @required Platform platform, @required ProcessManager processManager, @required OperatingSystemUtils operatingSystemUtils, - @required Logger logger, @required BrowserFinder browserFinder, + @required Logger logger, }) : _fileSystem = fileSystem, _platform = platform, _processManager = processManager, _operatingSystemUtils = operatingSystemUtils, - _logger = logger, _browserFinder = browserFinder, + _logger = logger, _fileSystemUtils = FileSystemUtils( fileSystem: fileSystem, platform: platform, @@ -122,9 +121,9 @@ class ChromiumLauncher { final Platform _platform; final ProcessManager _processManager; final OperatingSystemUtils _operatingSystemUtils; - Logger _logger; final BrowserFinder _browserFinder; final FileSystemUtils _fileSystemUtils; + final Logger _logger; bool get hasChromeInstance => _currentCompleter.isCompleted; @@ -163,7 +162,6 @@ class ChromiumLauncher { bool skipCheck = false, Directory cacheDir, }) async { - _logger ??= globals.logger; if (_currentCompleter.isCompleted) { throwToolExit('Only one instance of chrome can be started.'); } @@ -207,13 +205,6 @@ class ChromiumLauncher { final Process process = await _processManager.start(args); - // When the process exits, copy the user settings back to the provided data-dir. - if (cacheDir != null) { - unawaited(process.exitCode.whenComplete(() { - _cacheUserSessionInformation(userDataDir, cacheDir); - })); - } - process.stdout .transform(utf8.decoder) .transform(const LineSplitter()) @@ -221,7 +212,8 @@ class ChromiumLauncher { _logger.printTrace('[CHROME]: $line'); }); - // Wait until the DevTools are listening before trying to connect. + // Wait until the DevTools are listening before trying to connect. This is + // only required for flutter_test --platform=chrome and not flutter run. await process.stderr .transform(utf8.decoder) .transform(const LineSplitter()) @@ -232,13 +224,18 @@ class ChromiumLauncher { .firstWhere((String line) => line.startsWith('DevTools listening'), orElse: () { return 'Failed to spawn stderr'; }); - final Uri remoteDebuggerUri = await _getRemoteDebuggerUrl(Uri.parse('http://localhost:$port')); + + // When the process exits, copy the user settings back to the provided data-dir. + if (cacheDir != null) { + unawaited(process.exitCode.whenComplete(() { + _cacheUserSessionInformation(userDataDir, cacheDir); + })); + } return _connect(Chromium._( port, ChromeConnection('localhost', port), url: url, process: process, - remoteDebuggerUri: remoteDebuggerUri, chromiumLauncher: this, ), skipCheck); } @@ -311,28 +308,6 @@ class ChromiumLauncher { } Future get connectedInstance => _currentCompleter.future; - - /// Returns the full URL of the Chrome remote debugger for the main page. - /// - /// This takes the [base] remote debugger URL (which points to a browser-wide - /// page) and uses its JSON API to find the resolved URL for debugging the host - /// page. - Future _getRemoteDebuggerUrl(Uri base) async { - try { - final HttpClient client = HttpClient(); - final HttpClientRequest request = await client.getUrl(base.resolve('/json/list')); - final HttpClientResponse response = await request.close(); - final List jsonObject = await json.fuse(utf8).decoder.bind(response).single as List; - if (jsonObject == null || jsonObject.isEmpty) { - return base; - } - return base.resolve(jsonObject.first['devtoolsFrontendUrl'] as String); - } on Exception { - // If we fail to talk to the remote debugger protocol, give up and return - // the raw URL rather than crashing. - return base; - } - } } /// A class for managing an instance of a Chromium browser. @@ -342,7 +317,6 @@ class Chromium { this.chromeConnection, { this.url, Process process, - this.remoteDebuggerUri, @required ChromiumLauncher chromiumLauncher, }) : _process = process, _chromiumLauncher = chromiumLauncher; @@ -351,7 +325,6 @@ class Chromium { final int debugPort; final Process _process; final ChromeConnection chromeConnection; - final Uri remoteDebuggerUri; final ChromiumLauncher _chromiumLauncher; Future get onExit => _process.exitCode; diff --git a/packages/flutter_tools/lib/src/web/web_device.dart b/packages/flutter_tools/lib/src/web/web_device.dart index 8d72394130c..acf9cca3412 100644 --- a/packages/flutter_tools/lib/src/web/web_device.dart +++ b/packages/flutter_tools/lib/src/web/web_device.dart @@ -305,10 +305,10 @@ class WebDevices extends PollingDeviceDiscovery { chromiumLauncher: ChromiumLauncher( browserFinder: findChromeExecutable, fileSystem: fileSystem, - logger: logger, platform: platform, processManager: processManager, operatingSystemUtils: operatingSystemUtils, + logger: logger, ), ); if (platform.isWindows) { @@ -316,10 +316,10 @@ class WebDevices extends PollingDeviceDiscovery { chromiumLauncher: ChromiumLauncher( browserFinder: findEdgeExecutable, fileSystem: fileSystem, - logger: logger, platform: platform, processManager: processManager, operatingSystemUtils: operatingSystemUtils, + logger: logger, ), processManager: processManager, logger: logger, @@ -450,6 +450,10 @@ class WebServerDevice extends Device { } else { _logger.printStatus('$mainPath is being served at $url', emphasis: true); } + _logger.printStatus( + 'The web-server device does not support debugging. Consider using ' + 'the Chrome or Edge devices for an improved development workflow.' + ); _logger.sendEvent('app.webLaunchUrl', {'url': url, 'launched': false}); return LaunchResult.succeeded(observatoryUri: url != null ? Uri.parse(url): null); } diff --git a/packages/flutter_tools/test/general.shard/web/chrome_test.dart b/packages/flutter_tools/test/general.shard/web/chrome_test.dart index 15fb3de37f2..f32e8513a9b 100644 --- a/packages/flutter_tools/test/general.shard/web/chrome_test.dart +++ b/packages/flutter_tools/test/general.shard/web/chrome_test.dart @@ -35,10 +35,8 @@ void main() { Platform platform; FakeProcessManager processManager; OperatingSystemUtils operatingSystemUtils; - Logger logger; setUp(() { - logger = BufferLogger.test(); operatingSystemUtils = MockOperatingSystemUtils(); when(operatingSystemUtils.findFreePort()) .thenAnswer((Invocation invocation) async { @@ -54,8 +52,8 @@ void main() { platform: platform, processManager: processManager, operatingSystemUtils: operatingSystemUtils, - logger: logger, browserFinder: findChromeExecutable, + logger: BufferLogger.test(), ); }); @@ -162,7 +160,7 @@ void main() { .childFile('preferences'); preferencesFile ..createSync(recursive: true) - ..writeAsStringSync('example'); + ..writeAsStringSync('"exit_type":"Crashed"'); final Directory localStorageContentsDirectory = dataDir .childDirectory('Default') @@ -186,18 +184,8 @@ void main() { cacheDir: dataDir, ); - // validate preferences - final File tempFile = fileSystem - .directory('.tmp_rand0/flutter_tools_chrome_device.rand0') - .childDirectory('Default') - .childFile('preferences'); - - expect(tempFile.existsSync(), true); - expect(tempFile.readAsStringSync(), 'example'); - - // write crash to file: - tempFile.writeAsStringSync('"exit_type":"Crashed"'); exitCompleter.complete(); + await Future.delayed(const Duration(microseconds: 1)); // writes non-crash back to dart_tool expect(preferencesFile.readAsStringSync(), '"exit_type":"Normal"'); diff --git a/packages/flutter_tools/test/general.shard/web/web_validator_test.dart b/packages/flutter_tools/test/general.shard/web/web_validator_test.dart index ebf2dace486..876906daec7 100644 --- a/packages/flutter_tools/test/general.shard/web/web_validator_test.dart +++ b/packages/flutter_tools/test/general.shard/web/web_validator_test.dart @@ -34,8 +34,8 @@ void main() { platform: platform, processManager: processManager, operatingSystemUtils: null, - logger: BufferLogger.test(), browserFinder: findChromeExecutable, + logger: BufferLogger.test(), ); webValidator = webValidator = ChromeValidator( platform: platform, diff --git a/packages/flutter_tools/test/integration.shard/test_data/basic_project.dart b/packages/flutter_tools/test/integration.shard/test_data/basic_project.dart index a4318bf322e..a771ab51d25 100644 --- a/packages/flutter_tools/test/integration.shard/test_data/basic_project.dart +++ b/packages/flutter_tools/test/integration.shard/test_data/basic_project.dart @@ -52,3 +52,46 @@ class BasicProject extends Project { Uri get topLevelFunctionBreakpointUri => mainDart; int get topLevelFunctionBreakpointLine => lineContaining(main, '// TOP LEVEL BREAKPOINT'); } + +class BasicProjectWithUnaryMain extends Project { + + @override + final String pubspec = ''' + name: test + environment: + sdk: ">=2.0.0-dev.68.0 <3.0.0" + + dependencies: + flutter: + sdk: flutter + '''; + + @override + final String main = r''' + import 'dart:async'; + + import 'package:flutter/material.dart'; + + Future main(List args) async { + while (true) { + runApp(new MyApp()); + await Future.delayed(const Duration(milliseconds: 50)); + } + } + + class MyApp extends StatelessWidget { + @override + Widget build(BuildContext context) { + topLevelFunction(); + return new MaterialApp( // BUILD BREAKPOINT + title: 'Flutter Demo', + home: new Container(), + ); + } + } + + topLevelFunction() { + print("topLevelFunction"); // TOP LEVEL BREAKPOINT + } + '''; +} diff --git a/packages/flutter_tools/test/integration.shard/web_run_test.dart b/packages/flutter_tools/test/integration.shard/web_run_test.dart new file mode 100644 index 00000000000..78ce7d017ae --- /dev/null +++ b/packages/flutter_tools/test/integration.shard/web_run_test.dart @@ -0,0 +1,31 @@ +// Copyright 2014 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'package:file/file.dart'; +import 'package:flutter_tools/src/base/file_system.dart'; +import '../src/common.dart'; +import 'test_data/basic_project.dart'; +import 'test_driver.dart'; +import 'test_utils.dart'; + +void main() { + Directory tempDir; + final BasicProjectWithUnaryMain project = BasicProjectWithUnaryMain(); + FlutterRunTestDriver flutter; + + setUp(() async { + tempDir = createResolvedTempDirectorySync('run_test.'); + await project.setUpIn(tempDir); + flutter = FlutterRunTestDriver(tempDir); + }); + + tearDown(() async { + await flutter.stop(); + tryToDelete(tempDir); + }); + + test('flutter run works on web devices with a unary main function', () async { + await flutter.run(chrome: true); + }, skip: 'Web CI skipped'); +}