[web] Change --web-renderer default from auto to canvaskit (#14…

…9773) - When `--web-renderer` is omitted, keep the value `null` until it later materializes to either `canvaskit` or `skwasm`. - No more hardcoded defaults anywhere. We use `WebRendererMode.defaultForJs/defaultForWasm` instead. - When in `--wasm` mode, the JS fallback is now `canvaskit` instead of `auto`. - Add test for defaulting to `skwasm` when `--wasm` is enabled. Fixes flutter/flutter#149826
engine-flutter-autoroll · Jun 10, 2024 · ee10d2f · ee10d2f
1 parent 4e6e61d
commit ee10d2f
Show file tree

Hide file tree

Showing 9 changed files with 111 additions and 62 deletions.
diff --git a/packages/flutter_tools/lib/src/commands/build_web.dart b/packages/flutter_tools/lib/src/commands/build_web.dart
@@ -142,9 +142,14 @@ class BuildWebCommand extends BuildSubCommand {
         ? int.parse(dart2jsOptimizationLevelValue.substring(1))
         : optimizationLevel;
 
+    final String? webRendererString = stringArg(FlutterOptions.kWebRendererFlag);
+    final WebRendererMode? webRenderer = webRendererString == null
+        ? null
+        : WebRendererMode.values.byName(webRendererString);
+
     final List<WebCompilerConfig> compilerConfigs;
-    if (boolArg('wasm')) {
-      if (stringArg(FlutterOptions.kWebRendererFlag) != argParser.defaultFor(FlutterOptions.kWebRendererFlag)) {
+    if (boolArg(FlutterOptions.kWebWasmFlag)) {
+      if (webRenderer != null) {
         throwToolExit('"--${FlutterOptions.kWebRendererFlag}" cannot be combined with "--${FlutterOptions.kWebWasmFlag}"');
       }
       globals.logger.printBox(
@@ -158,7 +163,6 @@ class BuildWebCommand extends BuildSubCommand {
         WasmCompilerConfig(
           optimizationLevel: optimizationLevel,
           stripWasm: boolArg('strip-wasm'),
-          renderer: WebRendererMode.skwasm,
         ),
         JsCompilerConfig(
           csp: boolArg('csp'),
@@ -167,21 +171,16 @@ class BuildWebCommand extends BuildSubCommand {
           nativeNullAssertions: boolArg('native-null-assertions'),
           noFrequencyBasedMinification: boolArg('no-frequency-based-minification'),
           sourceMaps: boolArg('source-maps'),
-          renderer: WebRendererMode.canvaskit,
         )];
     } else {
-      WebRendererMode webRenderer = WebRendererMode.auto;
-      if (argParser.options.containsKey(FlutterOptions.kWebRendererFlag)) {
-        webRenderer = WebRendererMode.values.byName(stringArg(FlutterOptions.kWebRendererFlag)!);
-      }
       compilerConfigs = <WebCompilerConfig>[JsCompilerConfig(
         csp: boolArg('csp'),
         optimizationLevel: jsOptimizationLevel,
         dumpInfo: boolArg('dump-info'),
         nativeNullAssertions: boolArg('native-null-assertions'),
         noFrequencyBasedMinification: boolArg('no-frequency-based-minification'),
         sourceMaps: boolArg('source-maps'),
-        renderer: webRenderer,
+        renderer: webRenderer ?? WebRendererMode.defaultForJs,
       )];
     }
 

diff --git a/packages/flutter_tools/lib/src/device.dart b/packages/flutter_tools/lib/src/device.dart
@@ -992,7 +992,7 @@ class DebuggingOptions {
     this.webEnableExpressionEvaluation = false,
     this.webHeaders = const <String, String>{},
     this.webLaunchUrl,
-    this.webRenderer = WebRendererMode.auto,
+    WebRendererMode? webRenderer,
     this.webUseWasm = false,
     this.vmserviceOutFile,
     this.fastStart = false,
@@ -1006,7 +1006,8 @@ class DebuggingOptions {
     this.enableEmbedderApi = false,
     this.usingCISystem = false,
     this.debugLogsDirectoryPath,
-   }) : debuggingEnabled = true;
+   })  : debuggingEnabled = true,
+        webRenderer = webRenderer ?? WebRendererMode.getDefault(useWasm: webUseWasm);
 
   DebuggingOptions.disabled(this.buildInfo, {
       this.dartEntrypointArgs = const <String>[],
@@ -1023,7 +1024,7 @@ class DebuggingOptions {
       this.webBrowserFlags = const <String>[],
       this.webLaunchUrl,
       this.webHeaders = const <String, String>{},
-      this.webRenderer = WebRendererMode.auto,
+      WebRendererMode? webRenderer,
       this.webUseWasm = false,
       this.cacheSkSL = false,
       this.traceAllowlist,
@@ -1061,7 +1062,8 @@ class DebuggingOptions {
       webEnableExpressionEvaluation = false,
       nullAssertions = false,
       nativeNullAssertions = false,
-      serveObservatory = false;
+      serveObservatory = false,
+      webRenderer = webRenderer ?? WebRendererMode.getDefault(useWasm: webUseWasm);
 
   DebuggingOptions._({
     required this.buildInfo,

diff --git a/packages/flutter_tools/lib/src/runner/flutter_command.dart b/packages/flutter_tools/lib/src/runner/flutter_command.dart
@@ -711,7 +711,6 @@ abstract class FlutterCommand extends Command<void> {
   void usesWebRendererOption() {
     argParser.addOption(
       FlutterOptions.kWebRendererFlag,
-      defaultsTo: WebRendererMode.auto.name,
       allowed: WebRendererMode.values.map((WebRendererMode e) => e.name),
       help: 'The renderer implementation to use when building for the web.',
       allowedHelp: CliEnum.allowedHelp(WebRendererMode.values)

diff --git a/packages/flutter_tools/lib/src/web/compile.dart b/packages/flutter_tools/lib/src/web/compile.dart
@@ -185,16 +185,19 @@ enum WebRendererMode implements CliEnum {
   skwasm;
 
   factory WebRendererMode.fromCliOption(String? webRendererString, {required bool useWasm}) {
-    final WebRendererMode mode = webRendererString != null
-      ? WebRendererMode.values.byName(webRendererString)
-      : WebRendererMode.auto;
-    if (mode == WebRendererMode.auto && useWasm) {
-      // Wasm defaults to skwasm
-      return WebRendererMode.skwasm;
+    if (webRendererString == null) {
+      return getDefault(useWasm: useWasm);
     }
-    return mode;
+    return WebRendererMode.values.byName(webRendererString);
   }
 
+  static WebRendererMode getDefault({required bool useWasm}) {
+    return useWasm ? defaultForWasm : defaultForJs;
+  }
+
+  static const WebRendererMode defaultForJs = WebRendererMode.canvaskit;
+  static const WebRendererMode defaultForWasm = WebRendererMode.skwasm;
+
   @override
   String get cliName => kebabCase(name);
 
@@ -210,22 +213,22 @@ enum WebRendererMode implements CliEnum {
       };
 
   Iterable<String> get dartDefines => switch (this) {
-        WebRendererMode.auto => <String>[
+        auto => <String>[
             'FLUTTER_WEB_AUTO_DETECT=true',
           ],
-        WebRendererMode.canvaskit => <String>[
+        canvaskit => <String>[
             'FLUTTER_WEB_AUTO_DETECT=false',
             'FLUTTER_WEB_USE_SKIA=true',
           ],
-        WebRendererMode.html => <String>[
+        html => <String>[
             'FLUTTER_WEB_AUTO_DETECT=false',
             'FLUTTER_WEB_USE_SKIA=false',
           ],
-        WebRendererMode.skwasm => <String>[
+        skwasm => <String>[
             'FLUTTER_WEB_AUTO_DETECT=false',
             'FLUTTER_WEB_USE_SKIA=false',
             'FLUTTER_WEB_USE_SKWASM=true',
-          ]
+          ],
       };
 
   List<String> updateDartDefines(List<String> inputDefines) {

diff --git a/packages/flutter_tools/lib/src/web/compiler_config.dart b/packages/flutter_tools/lib/src/web/compiler_config.dart
@@ -51,7 +51,7 @@ class JsCompilerConfig extends WebCompilerConfig {
     super.optimizationLevel = WebCompilerConfig.kDefaultOptimizationLevel,
     this.noFrequencyBasedMinification = false,
     this.sourceMaps = true,
-    super.renderer = WebRendererMode.auto,
+    super.renderer = WebRendererMode.defaultForJs,
   });
 
   /// Instantiates [JsCompilerConfig] suitable for the `flutter run` command.
@@ -136,7 +136,7 @@ class WasmCompilerConfig extends WebCompilerConfig {
   const WasmCompilerConfig({
     super.optimizationLevel = WebCompilerConfig.kDefaultOptimizationLevel,
     this.stripWasm = true,
-    super.renderer = WebRendererMode.auto,
+    super.renderer = WebRendererMode.defaultForWasm,
   });
 
   /// Build environment for [stripWasm].

diff --git a/packages/flutter_tools/test/commands.shard/hermetic/build_web_test.dart b/packages/flutter_tools/test/commands.shard/hermetic/build_web_test.dart
@@ -274,7 +274,7 @@ void main() {
     ProcessManager: () => processManager,
   });
 
-  testUsingContext('Defaults to web renderer auto mode when no option is specified', () async {
+  testUsingContext('Defaults to web renderer canvaskit mode when no option is specified', () async {
     final TestWebBuildCommand buildCommand = TestWebBuildCommand(fileSystem: fileSystem);
     final CommandRunner<void> runner = createTestCommandRunner(buildCommand);
     setupFileSystemForEndToEndTest(fileSystem);
@@ -288,7 +288,28 @@ void main() {
       expect(target, isA<WebServiceWorker>());
       final List<WebCompilerConfig> configs = (target as WebServiceWorker).compileConfigs;
       expect(configs.length, 1);
-      expect(configs.first.renderer, WebRendererMode.auto);
+      expect(configs.first.renderer, WebRendererMode.canvaskit);
+    }),
+  });
+
+  testUsingContext('Defaults to web renderer skwasm mode for wasm when no option is specified', () async {
+    final TestWebBuildCommand buildCommand = TestWebBuildCommand(fileSystem: fileSystem);
+    final CommandRunner<void> runner = createTestCommandRunner(buildCommand);
+    setupFileSystemForEndToEndTest(fileSystem);
+    await runner.run(<String>['build', 'web', '--no-pub', '--wasm']);
+  }, overrides: <Type, Generator>{
+    Platform: () => fakePlatform,
+    FileSystem: () => fileSystem,
+    FeatureFlags: () => TestFeatureFlags(isWebEnabled: true),
+    ProcessManager: () => processManager,
+    BuildSystem: () => TestBuildSystem.all(BuildResult(success: true), (Target target, Environment environment) {
+      expect(target, isA<WebServiceWorker>());
+      final List<WebCompilerConfig> configs = (target as WebServiceWorker).compileConfigs;
+      expect(configs.length, 2);
+      expect(configs[0].renderer, WebRendererMode.skwasm);
+      expect(configs[0].compileTarget, CompileTarget.wasm);
+      expect(configs[1].renderer, WebRendererMode.canvaskit);
+      expect(configs[1].compileTarget, CompileTarget.js);
     }),
   });
 

diff --git a/packages/flutter_tools/test/general.shard/build_system/targets/web_defines_test.dart b/packages/flutter_tools/test/general.shard/build_system/targets/web_defines_test.dart
@@ -35,13 +35,13 @@ void main() {
       expect(dartDefines, <String>['FLUTTER_WEB_AUTO_DETECT=true']);
     });
 
-    test('canvaskit web-renderer with no dart-defines', () {
+    test('canvaskit web-renderer with existing dart-defines', () {
       dartDefines = <String>['FLUTTER_WEB_USE_SKIA=false'];
       dartDefines = WebRendererMode.canvaskit.updateDartDefines(dartDefines);
       expect(dartDefines, <String>['FLUTTER_WEB_AUTO_DETECT=false','FLUTTER_WEB_USE_SKIA=true']);
     });
 
-    test('html web-renderer with no dart-defines', () {
+    test('html web-renderer with existing dart-defines', () {
       dartDefines = <String>['FLUTTER_WEB_USE_SKIA=true'];
       dartDefines = WebRendererMode.html.updateDartDefines(dartDefines);
       expect(dartDefines, <String>['FLUTTER_WEB_AUTO_DETECT=false','FLUTTER_WEB_USE_SKIA=false']);