[web] Build multiple CanvasKit variants (using toolchain_args)

[mdebbar]

I'm not sure if this is the right way to do it, but I wanted to start the conversation and iterate based on feedback from people who know more about the gn world than I do

---

The end goal here is to be able to build multiple CanvasKit variants for Flutter Web (e.g. one light, stripped down variant for Chromium; and one for other browsers).

I've found multiple ways to do this:

1. Python or shell script

The script would cd into //third_party/skia/modules/canvaskit and build CanvasKit in there (similar to how the CanvasKit team does it via compile.sh). The script has full control over the flags it passes to the CanvasKit build.

The script can then be called from web_sdk/BUILD.gn using an action(target_name). It can be called multiple times, one for each variant we need.

Pros/cons:

• ✅ Straightforward to implement and maintain.
• ❌ Requires doing ./bin/sync inside //third_party/skia to install all its deps (equivalent of gclient sync).
• ❌ Skia dependencies are 5GB+

2. Using toolchain_args

Today, we add CanvasKit as a dep in web_sdk/BUILD.gn and we rely on the global gn args set by tools/gn to configure the CanvasKit/Skia build. This means we can only have a single variant of CanvasKit.

In order to be able to have multiple variants of CanvasKit with different flags, we could use a different toolchain with different toolchain_args. To my (limited) knowledge, toolchain_args can override the global gn args.

This is the approach I'm taking in this PR.

Pros/cons:

• ✅ Can fit into our existing build and tools/gn.
• ❌ Leads to confusion because the resulting CanvasKit build won't match whatever is in args.gn.
• ❌ I don't understand the full ramifications of using a different toolchain to build CanvasKit.

3. Separate gn for building CanvasKit alone

Bypass tools/gn and create a canvaskit target that can be built alone without all the other ceremony that Flutter does. This new gn can be called multiple times to build multiple variants of CanvasKit with different flags. It can also be pointed to an out folder that makes sense (e.g. out/wasm_release/canvaskit1) so that it looks like part of the wasm_release build (and potentially easier for the flutter tool to deal with).

This is the approach I'm taking in this PR.

Pros/cons:

• ✅ Straightforward to implement and maintain.
• ✅ Might be the idiomatic way of doing things in gn?
• ❌ Another tool has to invoke CanvasKit's new gn along with the usual invocations of tools/gn (this tool would probably be felt for the flutter web engine).

---

A full flutter web engine build would like:

• Use tools/gn --web to build the Flutter Web Engine at out/wasm_release.
• Use third_party/canvaskit/gn to build a full variant of CanvasKit at out/wasm_release/canvaskit.
• Use third_party/canvaskit/gn --no-icu --no-decoders to build a light variant of CanvasKit at out/wasm_release/canvaskit_light.

---

Depends on flutter/buildroot#663
Depends on https://skia-review.googlesource.com/c/skia/+/616799

Fixes flutter/flutter#118746

[zanderso]

The straightforward, though more expensive, way to accomplish this in our current build system, as well as with the engine_v2 recipes, is to do two complete builds, for example one under out/wasm_release and one under out/wasm_release_lite, then pick and choose the artifacts needed from each at the packaging/testing step. This is the approach that is used for variants of the native engine.

We haven't done this in the Flutter engine build, yet, but the idiomatic way to do this in GN without doing two completely separate builds is (2). AFAIK it isn't a big concern, but we can take steps to alleviate confusion about the contents of args.gn by ensuring that it contains either nothing or meaningless defaults for the impacted args. Then, those args are overridden in the toolchain definition. Previously, a bigger concern about this approach was around introducing multiple wasm toolchain definitions: one (or a collection) for CanvasKit, and a different one for everything else. My opinion is that this isn't a huge problem as long as the toolchain definitions for CanvasKit don't override anything unrelated to Skia/CanvasKit, and we employ GN visibility declarations and asserts to ensure that the CanvasKit toolchains aren't used for anything else.

[mdebbar]

@zanderso thanks for the suggestion!

I updated the PR to use toolchain_args. Does it look like what you had in mind? (You probably need to also look at the corresponding PR in the buildroot repo).

I haven't looked at the visibility thing yet, and I'm not familiar with it. I'll read about it and see how it can be used in this PR.

[zanderso]

Yeah, this is roughly what I was thinking. Let's talk more next year =)

[zanderso]

I would need to investigate more to make specific suggestions about how to guard the canvaskit toolchain definitions with visibility annotations or asserts, but one thing that will probably help in any case is putting the targets you want to build with those toolchains right next to the toolchain definitions, with comments saying not to use those toolchains for anything else.

So here, you'd depend on a group target, where the group has a public_deps on //third_party/skia/..., and the group target is in the same file as the toolchain definitions.

[flutter-dashboard]

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

[zanderso]

Suggested change

	# Copyright 2022 The Flutter Authors. All rights reserved.
	# Copyright 2013 The Flutter Authors. All rights reserved.

[zanderso]

LGTM

[zanderso]

Double check that this won't revert someone else's buildroot roll =)

[mdebbar]

Thanks for the reminder! Yes, this needs to change to whatever SHA is produced once the buildroot PR lands.

[harryterkelsen]

LGTM