From aebf54843676fea793682da9a2f01370bbf32f1f Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Tue, 2 Feb 2021 09:05:44 -0800 Subject: [PATCH] [flutter_tools] skip web renderer defines unless web target is picked (#75160) --- .../flutter_tools/lib/src/commands/drive.dart | 2 +- .../flutter_tools/lib/src/commands/run.dart | 27 +++---- .../lib/src/runner/flutter_command.dart | 4 +- .../commands.shard/hermetic/run_test.dart | 74 +++++++++++++++++++ 4 files changed, 91 insertions(+), 16 deletions(-) diff --git a/packages/flutter_tools/lib/src/commands/drive.dart b/packages/flutter_tools/lib/src/commands/drive.dart index b7b3f079f96..6689b80b772 100644 --- a/packages/flutter_tools/lib/src/commands/drive.dart +++ b/packages/flutter_tools/lib/src/commands/drive.dart @@ -198,7 +198,7 @@ class DriveCommand extends RunCommandBase { ) ?? PackageConfig.empty; final DriverService driverService = _flutterDriverFactory.createDriverService(web); final BuildInfo buildInfo = await getBuildInfo(); - final DebuggingOptions debuggingOptions = await createDebuggingOptions(); + final DebuggingOptions debuggingOptions = await createDebuggingOptions(web); final File applicationBinary = stringArg('use-application-binary') == null ? null : _fileSystem.file(stringArg('use-application-binary')); diff --git a/packages/flutter_tools/lib/src/commands/run.dart b/packages/flutter_tools/lib/src/commands/run.dart index 071e667db48..1c045c71545 100644 --- a/packages/flutter_tools/lib/src/commands/run.dart +++ b/packages/flutter_tools/lib/src/commands/run.dart @@ -159,8 +159,8 @@ abstract class RunCommandBase extends FlutterCommand with DeviceBasedDevelopment String get traceAllowlist => stringArg('trace-allowlist'); /// Create a debugging options instance for the current `run` or `drive` invocation. - Future createDebuggingOptions() async { - final BuildInfo buildInfo = await getBuildInfo(); + Future createDebuggingOptions(bool webMode) async { + final BuildInfo buildInfo = await getBuildInfo(updateWebDefines: webMode); final int browserDebugPort = featureFlags.isWebEnabled && argResults.wasParsed('web-browser-debug-port') ? int.parse(stringArg('web-browser-debug-port')) : null; @@ -322,6 +322,7 @@ class RunCommand extends RunCommandBase { final String description = 'Run your Flutter app on an attached device.'; List devices; + bool webMode = false; String get userIdentifier => stringArg(FlutterOptions.kDeviceUser); @@ -393,7 +394,7 @@ class RunCommand extends RunCommandBase { } } - final BuildInfo buildInfo = await getBuildInfo(); + final BuildInfo buildInfo = await getBuildInfo(updateWebDefines: webMode); final String modeName = buildInfo.modeName; return { CustomDimensions.commandRunIsEmulator: '$isEmulator', @@ -448,13 +449,18 @@ class RunCommand extends RunCommandBase { '--${FlutterOptions.kDeviceUser} is only supported for Android. At least one Android device is required.' ); } + // Only support "web mode" with a single web device due to resident runner + // refactoring required otherwise. + webMode = featureFlags.isWebEnabled && + devices.length == 1 && + await devices.single.targetPlatform == TargetPlatform.web_javascript; } @override Future runCommand() async { // Enable hot mode by default if `--no-hot` was not passed and we are in // debug mode. - final BuildInfo buildInfo = await getBuildInfo(); + final BuildInfo buildInfo = await getBuildInfo(updateWebDefines: webMode); final bool hotMode = shouldUseHotMode(buildInfo); final String applicationBinaryPath = stringArg('use-application-binary'); @@ -476,7 +482,7 @@ class RunCommand extends RunCommandBase { try { app = await daemon.appDomain.startApp( devices.first, globals.fs.currentDirectory.path, targetFile, route, - await createDebuggingOptions(), hotMode, + await createDebuggingOptions(webMode), hotMode, applicationBinary: applicationBinaryPath == null ? null : globals.fs.file(applicationBinaryPath), @@ -550,18 +556,13 @@ class RunCommand extends RunCommandBase { platform: globals.platform, ), ]; - // Only support "web mode" with a single web device due to resident runner - // refactoring required otherwise. - final bool webMode = featureFlags.isWebEnabled && - devices.length == 1 && - await devices.single.targetPlatform == TargetPlatform.web_javascript; ResidentRunner runner; if (hotMode && !webMode) { runner = HotRunner( flutterDevices, target: targetFile, - debuggingOptions: await createDebuggingOptions(), + debuggingOptions: await createDebuggingOptions(webMode), benchmarkMode: boolArg('benchmark'), applicationBinary: applicationBinaryPath == null ? null @@ -577,7 +578,7 @@ class RunCommand extends RunCommandBase { target: targetFile, flutterProject: flutterProject, ipv6: ipv6, - debuggingOptions: await createDebuggingOptions(), + debuggingOptions: await createDebuggingOptions(webMode), stayResident: stayResident, urlTunneller: null, ); @@ -585,7 +586,7 @@ class RunCommand extends RunCommandBase { runner = ColdRunner( flutterDevices, target: targetFile, - debuggingOptions: await createDebuggingOptions(), + debuggingOptions: await createDebuggingOptions(webMode), traceStartup: traceStartup, awaitFirstFrameWhenTracing: awaitFirstFrameWhenTracing, applicationBinary: applicationBinaryPath == null diff --git a/packages/flutter_tools/lib/src/runner/flutter_command.dart b/packages/flutter_tools/lib/src/runner/flutter_command.dart index eee8df5a2de..eab1b11b7ed 100644 --- a/packages/flutter_tools/lib/src/runner/flutter_command.dart +++ b/packages/flutter_tools/lib/src/runner/flutter_command.dart @@ -820,7 +820,7 @@ abstract class FlutterCommand extends Command { /// /// Throws a [ToolExit] if the current set of options is not compatible with /// each other. - Future getBuildInfo({ BuildMode forcedBuildMode }) async { + Future getBuildInfo({ BuildMode forcedBuildMode, bool updateWebDefines = true }) async { final bool trackWidgetCreation = argParser.options.containsKey('track-widget-creation') && boolArg('track-widget-creation'); @@ -940,7 +940,7 @@ abstract class FlutterCommand extends Command { ? stringsArg(FlutterOptions.kDartDefinesOption) : []; - if (argParser.options.containsKey('web-renderer')) { + if (argParser.options.containsKey('web-renderer') && updateWebDefines) { dartDefines = updateDartDefines(dartDefines, stringArg('web-renderer')); } diff --git a/packages/flutter_tools/test/commands.shard/hermetic/run_test.dart b/packages/flutter_tools/test/commands.shard/hermetic/run_test.dart index ceb20b1b4be..f5b12ad9c1f 100644 --- a/packages/flutter_tools/test/commands.shard/hermetic/run_test.dart +++ b/packages/flutter_tools/test/commands.shard/hermetic/run_test.dart @@ -19,6 +19,7 @@ import 'package:flutter_tools/src/base/user_messages.dart'; import 'package:flutter_tools/src/build_info.dart'; import 'package:flutter_tools/src/cache.dart'; import 'package:flutter_tools/src/commands/run.dart'; +import 'package:flutter_tools/src/convert.dart'; import 'package:flutter_tools/src/device.dart'; import 'package:flutter_tools/src/globals.dart' as globals; import 'package:flutter_tools/src/reporting/reporting.dart'; @@ -388,6 +389,66 @@ void main() { ProcessManager: () => mockProcessManager, Usage: () => usage, }); + + testUsingContext('No web renderer options are added to non web device', () async { + final FakeApplicationPackageFactory applicationPackageFactory = ApplicationPackageFactory.instance as FakeApplicationPackageFactory; + final RunCommand command = RunCommand(); + final MockDevice mockDevice = MockDevice(TargetPlatform.ios); + when(mockDevice.supportsRuntimeMode(any)).thenAnswer((Invocation invocation) => true); + when(mockDevice.isLocalEmulator).thenAnswer((Invocation invocation) => Future.value(false)); + when(mockDevice.getLogReader(app: anyNamed('app'))).thenReturn(FakeDeviceLogReader()); + when(mockDevice.supportsFastStart).thenReturn(true); + when(mockDevice.sdkNameAndVersion).thenAnswer((Invocation invocation) => Future.value('iOS 13')); + applicationPackageFactory.package = PrebuiltIOSApp(projectBundleId: 'test'); + + DebuggingOptions debuggingOptions; + + when(mockDevice.startApp( + any, + mainPath: anyNamed('mainPath'), + debuggingOptions: anyNamed('debuggingOptions'), + platformArgs: anyNamed('platformArgs'), + route: anyNamed('route'), + prebuiltApplication: anyNamed('prebuiltApplication'), + ipv6: anyNamed('ipv6'), + userIdentifier: anyNamed('userIdentifier'), + )).thenAnswer((Invocation invocation) { + debuggingOptions = invocation.namedArguments[#debuggingOptions] as DebuggingOptions; + return Future.value(LaunchResult.failed()); + }); + + when(mockDeviceManager.getDevices()).thenAnswer( + (Invocation invocation) => Future>.value([mockDevice]) + ); + + when(mockDeviceManager.findTargetDevices(any, timeout: anyNamed('timeout'))).thenAnswer( + (Invocation invocation) => Future>.value([mockDevice]) + ); + + final Directory tempDir = globals.fs.systemTempDirectory.createTempSync('flutter_run_test.'); + tempDir.childDirectory('ios').childFile('AppDelegate.swift').createSync(recursive: true); + tempDir.childFile('.dart_tool/package_config') + ..createSync(recursive: true) + ..writeAsStringSync(json.encode({'configVersion': 2, 'packages': []})); + tempDir.childDirectory('lib').childFile('main.dart').createSync(recursive: true); + tempDir.childFile('pubspec.yaml').writeAsStringSync('name: test'); + globals.fs.currentDirectory = tempDir; + + await expectToolExitLater(createTestCommandRunner(command).run([ + 'run', + '--no-pub', + '--no-hot', + ]), isNull); + // No web renderer options are added. + expect(debuggingOptions.buildInfo.dartDefines, isEmpty); + }, overrides: { + Artifacts: () => artifacts, + Cache: () => mockCache, + DeviceManager: () => mockDeviceManager, + FileSystem: () => fs, + ProcessManager: () => mockProcessManager, + ApplicationPackageFactory: () => FakeApplicationPackageFactory(), + }); }); testUsingContext('should only request artifacts corresponding to connected devices', () async { @@ -579,3 +640,16 @@ class FakeDevice extends Fake implements Device { return null; } } + +class FakeApplicationPackageFactory extends Fake implements ApplicationPackageFactory { + ApplicationPackage package; + + @override + Future getPackageForPlatform( + TargetPlatform platform, { + BuildInfo buildInfo, + File applicationBinary, + }) async { + return package; + } +}