Skip to content

[flutter_tools] refactor drive launch into separate service, split by mobile+desktop and web #68451

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 14 commits into from
Oct 22, 2020
Merged

[flutter_tools] refactor drive launch into separate service, split by mobile+desktop and web #68451

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
11776ea
[flutter_tools] refactor drive service into separate class
jonahwilliams Oct 18, 2020
718aa70
[flutter_tools] refactor drive service into separate class
jonahwilliams Oct 18, 2020
1cfe926
fix tests
jonahwilliams Oct 18, 2020
17b6504
more cleanup
jonahwilliams Oct 18, 2020
56f5ab6
Update drive_service_test.dart
jonahwilliams Oct 19, 2020
ffa5e82
Update analyze_size_test.dart
jonahwilliams Oct 19, 2020
ca1c1db
fixes for web launch
Oct 19, 2020
889e9e2
fix web driver tests
Oct 19, 2020
865ed2c
Update drive.dart
jonahwilliams Oct 20, 2020
67397ed
Add comment for log reader change
Oct 20, 2020
cfb052e
Update drive_service.dart
jonahwilliams Oct 20, 2020
78c7868
Add test for dispose
Oct 20, 2020
4f7eddc
Merge branch 'master' of github.com:flutter/flutter into drive_service
Oct 20, 2020
41771f0
Update android_device.dart
jonahwilliams Oct 21, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion packages/flutter_tools/lib/executable.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,10 @@ Future<void> main(List<String> args) async {
DevicesCommand(),
DoctorCommand(verbose: verbose),
DowngradeCommand(),
DriveCommand(verboseHelp: verboseHelp),
DriveCommand(verboseHelp: verboseHelp,
fileSystem: globals.fs,
logger: globals.logger,
Comment on lines +99 to +100
Copy link
Member

@jmagman jmagman Oct 19, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aren't these null at this point?

Copy link
Contributor Author

@jonahwilliams jonahwilliams Oct 20, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, the only interfaces that can't be injected at this point are those that depend on Cache/Artifacts/Version since those are still constructed when the first FlutterCommand is executed

),
EmulatorsCommand(),
FormatCommand(),
GenerateCommand(),
Expand Down
12 changes: 7 additions & 5 deletions packages/flutter_tools/lib/src/android/android_device.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -590,13 +590,17 @@ class AndroidDevice extends Device {
}

final bool traceStartup = platformArgs['trace-startup'] as bool ?? false;
_logger.printTrace('$this startApp');

ProtocolDiscovery observatoryDiscovery;

if (debuggingOptions.debuggingEnabled) {
observatoryDiscovery = ProtocolDiscovery.observatory(
await getLogReader(),
// Avoid using getLogReader, which returns a singleton instance, because the
// observatory discovery will dipose at the end. creating a new logger here allows
// logs to be surfaced normally during `flutter drive`.
await AdbLogReader.createLogReader(
Copy link
Member

@jmagman jmagman Oct 19, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a comment why not to use getLogReader?

Copy link
Contributor Author

@jonahwilliams jonahwilliams Oct 20, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

this,
_processManager,
),
portForwarder: portForwarder,
hostPort: debuggingOptions.hostVmServicePort,
devicePort: debuggingOptions.deviceVmServicePort,
Expand Down Expand Up @@ -669,8 +673,6 @@ class AndroidDevice extends Device {
// Wait for the service protocol port here. This will complete once the
// device has printed "Observatory is listening on...".
_logger.printTrace('Waiting for observatory port to be available...');

// TODO(danrubel): Waiting for observatory services can be made common across all devices.
try {
Uri observatoryUri;
if (debuggingOptions.buildInfo.isDebug || debuggingOptions.buildInfo.isProfile) {
Expand Down
12 changes: 2 additions & 10 deletions packages/flutter_tools/lib/src/build_system/targets/web.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,6 @@ import 'assets.dart';
import 'common.dart';
import 'localizations.dart';

/// Whether web builds should call the platform initialization logic.
const String kInitializePlatform = 'InitializePlatform';

/// Whether the application has web plugins.
const String kHasWebPlugins = 'HasWebPlugins';

Expand Down Expand Up @@ -89,7 +86,6 @@ class WebEntrypointTarget extends Target {
@override
Future<void> build(Environment environment) async {
final String targetFile = environment.defines[kTargetFile];
final bool shouldInitializePlatform = environment.defines[kInitializePlatform] == 'true';
final bool hasPlugins = environment.defines[kHasWebPlugins] == 'true';
final Uri importUri = environment.fileSystem.file(targetFile).absolute.uri;
// TODO(jonahwilliams): support configuration of this file.
Expand Down Expand Up @@ -137,9 +133,7 @@ import '$mainImport' as entrypoint;

Future<void> main() async {
registerPlugins(webPluginRegistry);
if ($shouldInitializePlatform) {
await ui.webOnlyInitializePlatform();
}
await ui.webOnlyInitializePlatform();
entrypoint.main();
}
''';
Expand All @@ -152,9 +146,7 @@ import 'dart:ui' as ui;
import '$mainImport' as entrypoint;

Future<void> main() async {
if ($shouldInitializePlatform) {
await ui.webOnlyInitializePlatform();
}
await ui.webOnlyInitializePlatform();
entrypoint.main();
}
''';
Expand Down
1 change: 0 additions & 1 deletion packages/flutter_tools/lib/src/commands/attach.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -384,7 +384,6 @@ known, it can be explicitly provided to attach via the command-line, e.g.

final FlutterDevice flutterDevice = await FlutterDevice.create(
device,
flutterProject: flutterProject,
fileSystemRoots: stringsArg('filesystem-root'),
fileSystemScheme: stringArg('filesystem-scheme'),
target: stringArg('target'),
Expand Down
7 changes: 0 additions & 7 deletions packages/flutter_tools/lib/src/commands/build_web.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,6 @@ class BuildWebCommand extends BuildSubCommand {
usesDartDefineOption();
addEnableExperimentation(hide: !verboseHelp);
addNullSafetyModeOptions(hide: !verboseHelp);
argParser.addFlag('web-initialize-platform',
defaultsTo: true,
negatable: true,
hide: true,
help: 'Whether to automatically invoke webOnlyInitializePlatform.',
);
argParser.addFlag('csp',
defaultsTo: false,
negatable: false,
Expand Down Expand Up @@ -92,7 +86,6 @@ class BuildWebCommand extends BuildSubCommand {
flutterProject,
target,
buildInfo,
boolArg('web-initialize-platform'),
boolArg('csp'),
stringArg('pwa-strategy'),
boolArg('source-maps')
Expand Down
1 change: 0 additions & 1 deletion packages/flutter_tools/lib/src/commands/daemon.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -466,7 +466,6 @@ class AppDomain extends Domain {

final FlutterDevice flutterDevice = await FlutterDevice.create(
device,
flutterProject: flutterProject,
target: target,
buildInfo: options.buildInfo,
platform: globals.platform,
Expand Down
Loading