Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

[web] Better way to detect CanvasKit variant#40154

Merged
auto-submit[bot] merged 5 commits into
flutter-team-archive:mainfrom
mdebbar:ck_requires_client_icu
Mar 15, 2023
Merged

[web] Better way to detect CanvasKit variant#40154
auto-submit[bot] merged 5 commits into
flutter-team-archive:mainfrom
mdebbar:ck_requires_client_icu

Conversation

@mdebbar

@mdebbar mdebbar commented Mar 8, 2023

Copy link
Copy Markdown
Contributor

Partial revert of #40027

For more info, see: https://skia-review.googlesource.com/c/skia/+/653416

The above CL introduces a new CanvasKit API to tell us whether it needs client ICU data or not. This allows us to do a more robust detection of the CanvasKit variant we are using.

BEFORE MERGING:

  • Make a new CanvasKit release that contains the required changes, and roll to that version.
  • We are now checking for the existence of the method before invoking it.

Fixes flutter/flutter#121905

@flutter-dashboard flutter-dashboard Bot added the platform-web Code specifically for the web engine label Mar 8, 2023
@mdebbar mdebbar marked this pull request as draft March 8, 2023 22:25
@mdebbar mdebbar requested review from ditman and eyebrowsoffire and removed request for ditman March 9, 2023 21:06
@mdebbar mdebbar marked this pull request as ready for review March 9, 2023 21:07
@mdebbar mdebbar added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 9, 2023
@auto-submit auto-submit Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Mar 9, 2023
@auto-submit

auto-submit Bot commented Mar 9, 2023

Copy link
Copy Markdown
Contributor

auto label is removed for flutter/engine, pr: 40154, due to - The status or check suite Linux Web Engine has failed. Please fix the issues identified (or deflake) before re-applying this label.

@mdebbar mdebbar added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 9, 2023
const bool browserSupportsCanvaskitChromium = false;
// TODO(mdebbar): Uncomment this to enable real detection of browser support.
// final bool browserSupportsCanvaskitChromium = domIntl.v8BreakIterator != null;
final bool browserSupportsCanvaskitChromium = domIntl.v8BreakIterator != null;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If canvaskit_chromium also removes the wasm image codecs, then this should also add && browserSupportsImageDecoder. Although in practice, I'm not aware of any Chromium-based browser not supporting image decoders.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Agreed.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@auto-submit auto-submit Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Mar 9, 2023
@auto-submit

auto-submit Bot commented Mar 9, 2023

Copy link
Copy Markdown
Contributor

auto label is removed for flutter/engine, pr: 40154, due to - The status or check suite Linux Web Engine has failed. Please fix the issues identified (or deflake) before re-applying this label.

@mdebbar

mdebbar commented Mar 10, 2023

Copy link
Copy Markdown
Contributor Author

There's a timeout failure happening in wasm mode. This line is causing a hard wasm failure:
https://github.com/flutter/engine/blob/161f75931834b79ec9f700046c4d3fe748a7a60b/lib/web_ui/lib/src/engine/dom.dart#L2164
because the js_util.getProperty(...) patch in dart2wasm has the wrong signature (it shoud be Function(Object, Object) instead of Function(Object, String)).

This is being fixed in https://dart-review.googlesource.com/c/sdk/+/288161. (Thanks @joshualitt!). Once that Dart CL lands and rolls into the engine, I'll rebase and try again.

Thanks @eyebrowsoffire for looking into this with me!

@mdebbar mdebbar added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 15, 2023
@auto-submit auto-submit Bot merged commit e447f20 into flutter-team-archive:main Mar 15, 2023
gaaclarke added a commit that referenced this pull request Mar 15, 2023
gaaclarke added a commit that referenced this pull request Mar 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

autosubmit Merge PR when tree becomes green via auto submit App platform-web Code specifically for the web engine

Development

Successfully merging this pull request may close these issues.

[web] On hot restart _canvasKitVariant isn't initialized

3 participants