Skip to content

Commit

Permalink
[flutter_tools] cleanups to web runner functionality (flutter#61178)
Browse files Browse the repository at this point in the history
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<String> args)

- Fixes flutter#59643
- Fixes flutter#55084
- Fixes flutter#60417
  • Loading branch information
jonahwilliams authored Jul 13, 2020
1 parent a1097ea commit e666ea8
Show file tree
Hide file tree
Showing 10 changed files with 122 additions and 75 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -636,18 +638,24 @@ class _ResidentWebRunner extends ResidentWebRunner {
'// Flutter web bootstrap script for $importedEntrypoint.',
'',
"import 'dart:ui' as ui;",
"import 'dart:async';",
'',
"import '$importedEntrypoint' as entrypoint;",
if (hasWebPlugins)
"import 'package:flutter_web_plugins/flutter_web_plugins.dart';",
if (hasWebPlugins)
"import '$generatedImport';",
'',
'typedef _UnaryFunction = dynamic Function(List<String> args);',
'typedef _NullaryFunction = dynamic Function();',
'Future<void> main() async {',
if (hasWebPlugins)
' registerPlugins(webPluginRegistry);',
' await ui.webOnlyInitializePlatform();',
' entrypoint.main();',
' if (entrypoint.main is _UnaryFunction) {',
' return (entrypoint.main as _UnaryFunction)(<String>[]);',
' }',
' return (entrypoint.main as _NullaryFunction)();',
'}',
'',
].join('\n');
Expand Down
2 changes: 1 addition & 1 deletion packages/flutter_tools/lib/src/doctor.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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,
),
Expand Down
4 changes: 2 additions & 2 deletions packages/flutter_tools/lib/src/test/flutter_web_platform.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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].
Expand Down
26 changes: 13 additions & 13 deletions packages/flutter_tools/lib/src/web/bootstrap.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
}
});
''';
Expand Down
51 changes: 12 additions & 39 deletions packages/flutter_tools/lib/src/web/chrome.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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,
Expand All @@ -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;

Expand Down Expand Up @@ -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.');
}
Expand Down Expand Up @@ -207,21 +205,15 @@ 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())
.listen((String line) {
_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())
Expand All @@ -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);
}
Expand Down Expand Up @@ -311,28 +308,6 @@ class ChromiumLauncher {
}

Future<Chromium> 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<Uri> _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<dynamic> jsonObject = await json.fuse(utf8).decoder.bind(response).single as List<dynamic>;
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.
Expand All @@ -342,7 +317,6 @@ class Chromium {
this.chromeConnection, {
this.url,
Process process,
this.remoteDebuggerUri,
@required ChromiumLauncher chromiumLauncher,
}) : _process = process,
_chromiumLauncher = chromiumLauncher;
Expand All @@ -351,7 +325,6 @@ class Chromium {
final int debugPort;
final Process _process;
final ChromeConnection chromeConnection;
final Uri remoteDebuggerUri;
final ChromiumLauncher _chromiumLauncher;

Future<int> get onExit => _process.exitCode;
Expand Down
8 changes: 6 additions & 2 deletions packages/flutter_tools/lib/src/web/web_device.dart
Original file line number Diff line number Diff line change
Expand Up @@ -305,21 +305,21 @@ class WebDevices extends PollingDeviceDiscovery {
chromiumLauncher: ChromiumLauncher(
browserFinder: findChromeExecutable,
fileSystem: fileSystem,
logger: logger,
platform: platform,
processManager: processManager,
operatingSystemUtils: operatingSystemUtils,
logger: logger,
),
);
if (platform.isWindows) {
_edgeDevice = MicrosoftEdgeDevice(
chromiumLauncher: ChromiumLauncher(
browserFinder: findEdgeExecutable,
fileSystem: fileSystem,
logger: logger,
platform: platform,
processManager: processManager,
operatingSystemUtils: operatingSystemUtils,
logger: logger,
),
processManager: processManager,
logger: logger,
Expand Down Expand Up @@ -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', <String, dynamic>{'url': url, 'launched': false});
return LaunchResult.succeeded(observatoryUri: url != null ? Uri.parse(url): null);
}
Expand Down
18 changes: 3 additions & 15 deletions packages/flutter_tools/test/general.shard/web/chrome_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -54,8 +52,8 @@ void main() {
platform: platform,
processManager: processManager,
operatingSystemUtils: operatingSystemUtils,
logger: logger,
browserFinder: findChromeExecutable,
logger: BufferLogger.test(),
);
});

Expand Down Expand Up @@ -162,7 +160,7 @@ void main() {
.childFile('preferences');
preferencesFile
..createSync(recursive: true)
..writeAsStringSync('example');
..writeAsStringSync('"exit_type":"Crashed"');

final Directory localStorageContentsDirectory = dataDir
.childDirectory('Default')
Expand All @@ -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<void>.delayed(const Duration(microseconds: 1));

// writes non-crash back to dart_tool
expect(preferencesFile.readAsStringSync(), '"exit_type":"Normal"');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<void> main(List<String> 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
}
''';
}
Loading

0 comments on commit e666ea8

Please sign in to comment.