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
merged 5 commits into from
Mar 15, 2023

Conversation

mdebbar
Copy link
Contributor

@mdebbar mdebbar commented Mar 8, 2023

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
Copy link
Contributor

auto-submit bot commented Mar 9, 2023

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
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
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
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
Copy link
Contributor

auto-submit bot commented Mar 9, 2023

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
Copy link
Contributor Author

mdebbar commented Mar 10, 2023

There's a timeout failure happening in wasm mode. This line is causing a hard wasm failure:

final Object actualMethod = js_util.getProperty(o, method);

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: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
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 15, 2023
mdebbar added a commit that referenced this pull request Mar 16, 2023
mdebbar added a commit to mdebbar/engine that referenced this pull request Mar 16, 2023
auto-submit bot pushed a commit that referenced this pull request Mar 16, 2023
Reland "[web] Better way to detect CanvasKit variant (#40154)"
@mdebbar mdebbar deleted the ck_requires_client_icu branch June 22, 2023 21:37
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[web] On hot restart _canvasKitVariant isn't initialized
3 participants