[flutter_tools] Reland: fix multiple dart defines (#54973)

dev/bots/test.dart

@@ -590,6 +590,18 @@ Future<void> _runWebIntegrationTests() async {
   await _runWebDebugTest('lib/stack_trace.dart');
   await _runWebDebugTest('lib/web_directory_loading.dart');
   await _runWebDebugTest('test/test.dart');
+  await _runWebDebugTest('lib/web_define_loading.dart',
+    additionalArguments: <String>[
+      '--dart-define=test.valueA=Example',
+      '--dart-define=test.valueB=Value',
+    ]
+  );
+  await _runWebReleaseTest('lib/web_define_loading.dart',
+    additionalArguments: <String>[
+      '--dart-define=test.valueA=Example',
+      '--dart-define=test.valueB=Value',
+    ]
+  );
 }
 
 Future<void> _runWebStackTraceTest(String buildMode) async {
@@ -632,10 +644,56 @@ Future<void> _runWebStackTraceTest(String buildMode) async {
   }
 }
 
+/// Run a web integration test in release mode.
+Future<void> _runWebReleaseTest(String target, {
+  List<String> additionalArguments = const<String>[],
+}) async {
+  final String testAppDirectory = path.join(flutterRoot, 'dev', 'integration_tests', 'web');
+  final String appBuildDirectory = path.join(testAppDirectory, 'build', 'web');
+
+  // Build the app.
+  await runCommand(
+    flutter,
+    <String>[ 'clean' ],
+    workingDirectory: testAppDirectory,
+  );
+  await runCommand(
+    flutter,
+    <String>[
+      'build',
+      'web',
+      '--release',
+      ...additionalArguments,
+      '-t',
+      target,
+    ],
+    workingDirectory: testAppDirectory,
+    environment: <String, String>{
+      'FLUTTER_WEB': 'true',
+    },
+  );
+
+  // Run the app.
+  final String result = await evalTestAppInChrome(
+    appUrl: 'http://localhost:8080/index.html',
+    appDirectory: appBuildDirectory,
+  );
+
+  if (result.contains('--- TEST SUCCEEDED ---')) {
+    print('${green}Web release mode test passed.$reset');
+  } else {
+    print(result);
+    print('${red}Web release mode test failed.$reset');
+    exit(1);
+  }
+}
+
 /// Debug mode is special because `flutter build web` doesn't build in debug mode.
 ///
 /// Instead, we use `flutter run --debug` and sniff out the standard output.
-Future<void> _runWebDebugTest(String target) async {
+Future<void> _runWebDebugTest(String target, {
+  List<String> additionalArguments = const<String>[],
+}) async {
   final String testAppDirectory = path.join(flutterRoot, 'dev', 'integration_tests', 'web');
   final CapturedOutput output = CapturedOutput();
   bool success = false;
@@ -647,6 +705,7 @@ Future<void> _runWebDebugTest(String target) async {
       '-d',
       'chrome',
       '--web-run-headless',
+      ...additionalArguments,
       '-t',
       target,
     ],

dev/devicelab/lib/tasks/defines_task.dart

@@ -21,7 +21,8 @@ Future<TaskResult> runDartDefinesTask() async {
       '--verbose',
       '-d',
       deviceId,
-      '--dart-define=test.value=ExampleValue',
+      '--dart-define=test.valueA=Example',
+      '--dart-define=test.valueB=Value',
       'lib/defines.dart',
     ]);
   });

dev/integration_tests/ui/lib/defines.dart

@@ -11,7 +11,7 @@ void main() {
   runApp(
     const Center(
       child: Text(
-        String.fromEnvironment('test.value'),
+        String.fromEnvironment('test.valueA') + String.fromEnvironment('test.valueB'),
         textDirection: TextDirection.ltr,
       ),
     ),

dev/integration_tests/ui/test_driver/defines_test.dart

@@ -16,7 +16,7 @@ void main() {
     await driver.close();
   });
 
-  test('Can run with --dart-deinfe', () async {
+  test('Can run with --dart-define', () async {
     await driver.waitFor(find.text('ExampleValue'));
   });
 }

dev/integration_tests/web/lib/web_define_loading.dart

@@ -0,0 +1,24 @@
+// 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 'dart:html' as html;
+
+Future<void> main() async {
+  final StringBuffer output = StringBuffer();
+  const String combined = String.fromEnvironment('test.valueA') +
+    String.fromEnvironment('test.valueB');
+  if (combined == 'ExampleValue') {
+    output.write('--- TEST SUCCEEDED ---');
+    print('--- TEST SUCCEEDED ---');
+  } else {
+    output.write('--- TEST FAILED ---');
+    print('--- TEST FAILED ---');
+  }
+
+  html.HttpRequest.request(
+    '/test-result',
+    method: 'POST',
+    sendData: '$output',
+  );
+}

packages/flutter_tools/bin/macos_assemble.sh

@@ -83,7 +83,7 @@ RunCommand "${FLUTTER_ROOT}/bin/flutter"                                    \
     -dTreeShakeIcons="${icon_tree_shaker_flag}"                             \
     -dDartObfuscation="${dart_obfuscation_flag}"                            \
     -dSplitDebugInfo="${SPLIT_DEBUG_INFO}"                                  \
-    -dDartDefines="${DART_DEFINES}"                                         \
+    --DartDefines="${DART_DEFINES}"                                         \
     -dExtraFrontEndOptions="${EXTRA_FRONT_END_OPTIONS}"                     \
     --build-inputs="${build_inputs_path}"                                   \
     --build-outputs="${build_outputs_path}"                                 \

packages/flutter_tools/bin/xcode_backend.sh

@@ -186,7 +186,7 @@ BuildApp() {
     -dTrackWidgetCreation="${track_widget_creation_flag}"                 \
     -dDartObfuscation="${dart_obfuscation_flag}"                          \
     -dEnableBitcode="${bitcode_flag}"                                     \
-    -dDartDefines="${DART_DEFINES}"                                       \
+    --DartDefines="${DART_DEFINES}"                                       \
     -dExtraFrontEndOptions="${EXTRA_FRONT_END_OPTIONS}"                   \
     "${build_mode}_ios_bundle_flutter_assets"
 

packages/flutter_tools/gradle/flutter.gradle

@@ -916,7 +916,7 @@ abstract class BaseFlutterTask extends DefaultTask {
                 args "-dDartObfuscation=true"
             }
             if (dartDefines != null) {
-                args "-dDartDefines=${dartDefines}"
+                args "--DartDefines=${dartDefines}"
             }
             if (extraGenSnapshotOptions != null) {
                 args "--ExtraGenSnapshotOptions=${extraGenSnapshotOptions}"

packages/flutter_tools/lib/src/android/gradle.dart

@@ -19,7 +19,6 @@ import '../base/terminal.dart';
 import '../base/utils.dart';
 import '../build_info.dart';
 import '../cache.dart';
-import '../convert.dart';
 import '../flutter_manifest.dart';
 import '../globals.dart' as globals;
 import '../project.dart';
@@ -331,7 +330,7 @@ Future<void> buildGradleApp({
     command.add('-Pshrink=true');
   }
   if (androidBuildInfo.buildInfo.dartDefines?.isNotEmpty ?? false) {
-    command.add('-Pdart-defines=${jsonEncode(androidBuildInfo.buildInfo.dartDefines)}');
+    command.add('-Pdart-defines=${androidBuildInfo.buildInfo.dartDefines.join(',')}');
   }
   if (shouldBuildPluginAsAar) {
     // Pass a system flag instead of a project flag, so this flag can be

packages/flutter_tools/lib/src/aot.dart

@@ -14,7 +14,6 @@ import 'build_system/targets/dart.dart';
 import 'build_system/targets/icon_tree_shaker.dart';
 import 'build_system/targets/ios.dart';
 import 'cache.dart';
-import 'convert.dart';
 import 'globals.dart' as globals;
 import 'ios/bitcode.dart';
 import 'project.dart';
@@ -87,7 +86,7 @@ class AotBuilder {
         kBuildMode: getNameForBuildMode(buildInfo.mode),
         kTargetPlatform: getNameForTargetPlatform(platform),
         kIconTreeShakerFlag: buildInfo.treeShakeIcons.toString(),
-        kDartDefines: jsonEncode(buildInfo.dartDefines),
+        kDartDefines: buildInfo.dartDefines.join(','),
         kBitcodeFlag: bitcode.toString(),
         if (buildInfo?.extraGenSnapshotOptions?.isNotEmpty ?? false)
           kExtraGenSnapshotOptions: buildInfo.extraGenSnapshotOptions.join(','),

packages/flutter_tools/lib/src/build_system/targets/dart.dart

@@ -7,7 +7,6 @@ import '../../base/build.dart';
 import '../../base/file_system.dart';
 import '../../build_info.dart';
 import '../../compile.dart';
-import '../../convert.dart';
 import '../../globals.dart' as globals;
 import '../../project.dart';
 import '../build_system.dart';
@@ -390,21 +389,10 @@ abstract class CopyFlutterAotBundle extends Target {
   }
 }
 
-/// Dart defines are encoded inside [Environment] as a JSON array.
+/// Dart defines are encoded inside [Environment] as a comma-separated list.
 List<String> parseDartDefines(Environment environment) {
   if (!environment.defines.containsKey(kDartDefines) || environment.defines[kDartDefines].isEmpty) {
     return const <String>[];
   }
-
+  return environment.defines[kDartDefines].split(',');
-  final String dartDefinesJson = environment.defines[kDartDefines];
-  try {
-    final List<Object> parsedDefines = jsonDecode(dartDefinesJson) as List<Object>;
-    return parsedDefines.cast<String>();
-  } on FormatException {
-    throw Exception(
-      'The value of -D$kDartDefines is not formatted correctly.\n'
-      'The value must be a JSON-encoded list of strings but was:\n'
-      '$dartDefinesJson'
-    );
-  }
 }

packages/flutter_tools/lib/src/build_system/targets/web.dart

@@ -168,6 +168,8 @@ class Dart2JSTarget extends Target {
       '--libraries-spec=$specPath',
       '-o',
       outputKernel.path,
+      for (final String dartDefine in dartDefines)
+        '-D$dartDefine',
       '--packages=$packageFile',
       '--cfe-only',
       environment.buildDir.childFile('main.dart').path,

packages/flutter_tools/lib/src/bundle.dart

@@ -17,7 +17,6 @@ import 'build_system/depfile.dart';
 import 'build_system/targets/dart.dart';
 import 'build_system/targets/icon_tree_shaker.dart';
 import 'cache.dart';
-import 'convert.dart';
 import 'dart/package_map.dart';
 import 'devfs.dart';
 import 'globals.dart' as globals;
@@ -130,7 +129,7 @@ Future<void> buildWithAssemble({
       kTrackWidgetCreation: trackWidgetCreation?.toString(),
       kIconTreeShakerFlag: treeShakeIcons ? 'true' : null,
       if (dartDefines != null && dartDefines.isNotEmpty)
-        kDartDefines: jsonEncode(dartDefines),
+        kDartDefines: dartDefines.join(','),
     },
     artifacts: globals.artifacts,
     fileSystem: globals.fs,

packages/flutter_tools/lib/src/commands/assemble.dart

@@ -87,6 +87,7 @@ class AssembleCommand extends FlutterCommand {
         'root of the current Flutter project.',
     );
     argParser.addOption(kExtraGenSnapshotOptions);
+    argParser.addOption(kDartDefines);
     argParser.addOption(
       'resource-pool-size',
       help: 'The maximum number of concurrent tasks the build system will run.',
@@ -182,6 +183,10 @@ class AssembleCommand extends FlutterCommand {
     if (argResults.wasParsed(kExtraGenSnapshotOptions)) {
       results[kExtraGenSnapshotOptions] = argResults[kExtraGenSnapshotOptions] as String;
     }
+    // Workaround for dart-define formatting
+    if (argResults.wasParsed(kDartDefines)) {
+      results[kDartDefines] = argResults[kDartDefines] as String;
+    }
     return results;
   }
 

packages/flutter_tools/lib/src/ios/xcodeproj.dart

@@ -18,7 +18,6 @@ import '../base/terminal.dart';
 import '../base/utils.dart';
 import '../build_info.dart';
 import '../cache.dart';
-import '../convert.dart';
 import '../flutter_manifest.dart';
 import '../globals.dart' as globals;
 import '../project.dart';
@@ -236,7 +235,7 @@ List<String> _xcodeBuildSettingsLines({
   }
 
   if (buildInfo.dartDefines?.isNotEmpty ?? false) {
-    xcodeBuildSettings.add('DART_DEFINES=${jsonEncode(buildInfo.dartDefines)}');
+    xcodeBuildSettings.add('DART_DEFINES=${buildInfo.dartDefines.join(',')}');
   }
 
   if (buildInfo.extraFrontEndOptions?.isNotEmpty ?? false) {

packages/flutter_tools/lib/src/web/compile.dart

@@ -13,7 +13,6 @@ import '../build_system/build_system.dart';
 import '../build_system/targets/dart.dart';
 import '../build_system/targets/icon_tree_shaker.dart';
 import '../build_system/targets/web.dart';
-import '../convert.dart';
 import '../globals.dart' as globals;
 import '../platform_plugins.dart';
 import '../plugins.dart';
@@ -49,7 +48,7 @@ Future<void> buildWeb(
         kTargetFile: target,
         kInitializePlatform: initializePlatform.toString(),
         kHasWebPlugins: hasWebPlugins.toString(),
-        kDartDefines: jsonEncode(buildInfo.dartDefines),
+        kDartDefines: buildInfo.dartDefines.join(','),
         kCspMode: csp.toString(),
         kIconTreeShakerFlag: buildInfo.treeShakeIcons.toString(),
       },

packages/flutter_tools/test/general.shard/build_system/targets/web_test.dart

@@ -360,12 +360,14 @@ void main() {
 
   test('Dart2JSTarget calls dart2js with Dart defines in release mode', () => testbed.run(() async {
     environment.defines[kBuildMode] = 'release';
-    environment.defines[kDartDefines] = '["FOO=bar","BAZ=qux"]';
+    environment.defines[kDartDefines] = 'FOO=bar,BAZ=qux';
     processManager.addCommand(FakeCommand(
       command: <String>[
         ...kDart2jsLinuxArgs,
         '-o',
         environment.buildDir.childFile('app.dill').absolute.path,
+        '-DFOO=bar',
+        '-DBAZ=qux',
          '--packages=${globals.fs.path.join('foo', '.packages')}',
         '--cfe-only',
         environment.buildDir.childFile('main.dart').absolute.path,
@@ -391,12 +393,14 @@ void main() {
 
   test('Dart2JSTarget calls dart2js with Dart defines in profile mode', () => testbed.run(() async {
     environment.defines[kBuildMode] = 'profile';
-    environment.defines[kDartDefines] = '["FOO=bar","BAZ=qux"]';
+    environment.defines[kDartDefines] = 'FOO=bar,BAZ=qux';
     processManager.addCommand(FakeCommand(
       command: <String>[
         ...kDart2jsLinuxArgs,
         '-o',
         environment.buildDir.childFile('app.dill').absolute.path,
+        '-DFOO=bar',
+        '-DBAZ=qux',
          '--packages=${globals.fs.path.join('foo', '.packages')}',
         '--cfe-only',
         environment.buildDir.childFile('main.dart').absolute.path,
@@ -421,27 +425,6 @@ void main() {
     ProcessManager: () => processManager,
   }));
 
-  test('Dart2JSTarget throws developer-friendly exception on misformatted DartDefines', () => testbed.run(() async {
-    environment.defines[kBuildMode] = 'profile';
-    environment.defines[kDartDefines] = '[misformatted json';
-    try {
-      await const Dart2JSTarget().build(environment);
-      fail('Call to build() must not have succeeded.');
-    } on Exception catch(exception) {
-      expect(
-        '$exception',
-        'Exception: The value of -D$kDartDefines is not formatted correctly.\n'
-        'The value must be a JSON-encoded list of strings but was:\n'
-        '[misformatted json',
-      );
-    }
-
-    // Should not attempt to run any processes.
-    verifyNever(globals.processManager.run(any));
-  }, overrides: <Type, Generator>{
-    ProcessManager: () => MockProcessManager(),
-  }));
-
   test('Generated service worker correctly inlines file hashes', () {
     final String result = generateServiceWorker(<String, String>{'/foo': 'abcd'});
 

packages/flutter_tools/test/general.shard/commands/build_aot_test.dart

@@ -109,7 +109,8 @@ void main() {
   });
 
   testUsingContext('build aot outputs timing info', () async {
-    globals.fs.file('.dart_tool/flutter_build/cce09742720db17ffec62331bd7e42d5/app.so')
+    globals.fs
+      .file('.dart_tool/flutter_build/0c21fd4ab3b8bde8b385ff01d08e0093/app.so')
       .createSync(recursive: true);
     when(globals.buildSystem.build(any, any))
       .thenAnswer((Invocation invocation) async {