From c94c54253a949c8cf6fc4b8bb085f3fc32d34d69 Mon Sep 17 00:00:00 2001 From: Harry Terkelsen <1961493+harryterkelsen@users.noreply.github.com> Date: Fri, 22 Mar 2024 14:58:39 -0700 Subject: [PATCH] [web] Add ability to customize font fallback download URL (flutter/engine#51569) Gives developers the ability to change the base URL to download fallback fonts from `fonts.gstatic.com` to a URL of their choosing. Fixes https://github.com/flutter/flutter/issues/132689 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide] and the [C++, Objective-C, Java style guides]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I added new tests to check the change I am making or feature I am adding, or the PR is [test-exempt]. See [testing the engine] for instructions on writing and running engine tests. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I signed the [CLA]. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. [Contributor Guide]: https://github.com/flutter/flutter/wiki/Tree-hygiene#overview [Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene [test-exempt]: https://github.com/flutter/flutter/wiki/Tree-hygiene#tests [Flutter Style Guide]: https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style [testing the engine]: https://github.com/flutter/flutter/wiki/Testing-the-engine [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/wiki/Chat --- .../lib/web_ui/flutter_js/src/types.d.ts | 1 + .../lib/src/engine/canvaskit/fonts.dart | 4 +- .../web_ui/lib/src/engine/configuration.dart | 59 ++++++++++------ .../web_ui/lib/src/engine/font_fallbacks.dart | 9 +-- .../skwasm/skwasm_impl/font_collection.dart | 4 +- .../test/canvaskit/canvas_golden_test.dart | 1 - .../lib/web_ui/test/canvaskit/common.dart | 8 +-- .../canvaskit/skia_font_collection_test.dart | 4 ++ .../test/common/test_initialization.dart | 5 +- .../test/ui/fallback_fonts_golden_test.dart | 69 ++++++++++++++----- 10 files changed, 109 insertions(+), 55 deletions(-) diff --git a/engine/src/flutter/lib/web_ui/flutter_js/src/types.d.ts b/engine/src/flutter/lib/web_ui/flutter_js/src/types.d.ts index 9cd1f7a7695..4013ddbcec2 100644 --- a/engine/src/flutter/lib/web_ui/flutter_js/src/types.d.ts +++ b/engine/src/flutter/lib/web_ui/flutter_js/src/types.d.ts @@ -53,6 +53,7 @@ export interface FlutterConfiguration { canvasKitVariant: CanvasKitVariant?; renderer: WebRenderer?; hostElement: HtmlElement?; + fontFallbackBaseUrl: string?; } export interface ServiceWorkerSettings { diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/canvaskit/fonts.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/canvaskit/fonts.dart index e984b1deb66..57b88acb878 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/canvaskit/fonts.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/canvaskit/fonts.dart @@ -13,8 +13,8 @@ import 'package:ui/ui_web/src/ui_web.dart' as ui_web; // this, list out all of the fonts and find the URL for the regular // Roboto font. The API reference is here: // https://developers.google.com/fonts/docs/developer_api -const String _robotoUrl = - 'https://fonts.gstatic.com/s/roboto/v20/KFOmCnqEu92Fr1Me5WZLCzYlKw.ttf'; +String _robotoUrl = + '${configuration.fontFallbackBaseUrl}roboto/v20/KFOmCnqEu92Fr1Me5WZLCzYlKw.ttf'; /// Manages the fonts used in the Skia-based backend. class SkiaFontCollection implements FlutterFontCollection { diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/configuration.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/configuration.dart index e47f8ed82f2..5591598795f 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/configuration.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/configuration.dart @@ -57,6 +57,7 @@ FlutterConfiguration get configuration { } return _configuration ??= FlutterConfiguration.legacy(_jsConfiguration); } + FlutterConfiguration? _configuration; FlutterConfiguration? _debugConfiguration; @@ -106,14 +107,15 @@ class FlutterConfiguration { // Warn the user of the deprecated behavior. assert(() { if (config != null) { - domWindow.console.warn('window.flutterConfiguration is now deprecated.\n' - 'Use engineInitializer.initializeEngine(config) instead.\n' - 'See: https://docs.flutter.dev/development/platform-integration/web/initialization'); + domWindow.console.warn( + 'window.flutterConfiguration is now deprecated.\n' + 'Use engineInitializer.initializeEngine(config) instead.\n' + 'See: https://docs.flutter.dev/development/platform-integration/web/initialization'); } if (_requestedRendererType != null) { domWindow.console.warn('window.flutterWebRenderer is now deprecated.\n' - 'Use engineInitializer.initializeEngine(config) instead.\n' - 'See: https://docs.flutter.dev/development/platform-integration/web/initialization'); + 'Use engineInitializer.initializeEngine(config) instead.\n' + 'See: https://docs.flutter.dev/development/platform-integration/web/initialization'); } return true; }()); @@ -143,14 +145,16 @@ class FlutterConfiguration { /// constructor. void setUserConfiguration(JsFlutterConfiguration? configuration) { if (configuration != null) { - assert(!_usedLegacyConfigStyle, - 'Use engineInitializer.initializeEngine(config) only. ' - 'Using the (deprecated) window.flutterConfiguration and initializeEngine ' - 'configuration simultaneously is not supported.'); - assert(_requestedRendererType == null || configuration.renderer == null, - 'Use engineInitializer.initializeEngine(config) only. ' - 'Using the (deprecated) window.flutterWebRenderer and initializeEngine ' - 'configuration simultaneously is not supported.'); + assert( + !_usedLegacyConfigStyle, + 'Use engineInitializer.initializeEngine(config) only. ' + 'Using the (deprecated) window.flutterConfiguration and initializeEngine ' + 'configuration simultaneously is not supported.'); + assert( + _requestedRendererType == null || configuration.renderer == null, + 'Use engineInitializer.initializeEngine(config) only. ' + 'Using the (deprecated) window.flutterWebRenderer and initializeEngine ' + 'configuration simultaneously is not supported.'); _configuration = configuration; } } @@ -177,9 +181,7 @@ class FlutterConfiguration { /// true. /// /// Using flutter tools option "--web-render=html" would set the value to false. - static const bool useSkia = - bool.fromEnvironment('FLUTTER_WEB_USE_SKIA'); - + static const bool useSkia = bool.fromEnvironment('FLUTTER_WEB_USE_SKIA'); // Runtime parameters. // @@ -235,7 +237,8 @@ class FlutterConfiguration { /// --web-renderer=canvaskit \ /// --dart-define=FLUTTER_WEB_CANVASKIT_URL=https://example.com/custom-canvaskit-build/ /// ``` - String get canvasKitBaseUrl => _configuration?.canvasKitBaseUrl ?? _defaultCanvasKitBaseUrl; + String get canvasKitBaseUrl => + _configuration?.canvasKitBaseUrl ?? _defaultCanvasKitBaseUrl; static const String _defaultCanvasKitBaseUrl = String.fromEnvironment( 'FLUTTER_WEB_CANVASKIT_URL', defaultValue: 'canvaskit/', @@ -262,7 +265,8 @@ class FlutterConfiguration { /// /// This is mainly used for testing or for apps that want to ensure they /// run on devices which don't support WebGL. - bool get canvasKitForceCpuOnly => _configuration?.canvasKitForceCpuOnly ?? _defaultCanvasKitForceCpuOnly; + bool get canvasKitForceCpuOnly => + _configuration?.canvasKitForceCpuOnly ?? _defaultCanvasKitForceCpuOnly; static const bool _defaultCanvasKitForceCpuOnly = bool.fromEnvironment( 'FLUTTER_WEB_CANVASKIT_FORCE_CPU_ONLY', ); @@ -278,7 +282,9 @@ class FlutterConfiguration { /// ``` /// flutter run -d chrome --profile --dart-define=FLUTTER_WEB_DEBUG_SHOW_SEMANTICS=true /// ``` - bool get debugShowSemanticsNodes => _configuration?.debugShowSemanticsNodes ?? _defaultDebugShowSemanticsNodes; + bool get debugShowSemanticsNodes => + _configuration?.debugShowSemanticsNodes ?? + _defaultDebugShowSemanticsNodes; static const bool _defaultDebugShowSemanticsNodes = bool.fromEnvironment( 'FLUTTER_WEB_DEBUG_SHOW_SEMANTICS', ); @@ -309,7 +315,16 @@ class FlutterConfiguration { /// `window.flutterWebRenderer`. /// /// This is used by the Renderer class to decide how to initialize the engine. - String? get requestedRendererType => _configuration?.renderer ?? _requestedRendererType; + String? get requestedRendererType => + _configuration?.renderer ?? _requestedRendererType; + + /// Returns the base URL to load fallback fonts from. Fallback fonts are + /// downloaded automatically when there is no font bundled with the app that + /// can show a glyph that is being rendered. + /// + /// Defaults to 'https://fonts.gstatic.com/s/'. + String get fontFallbackBaseUrl => + _configuration?.fontFallbackBaseUrl ?? 'https://fonts.gstatic.com/s/'; /// Whether to use color emojis or not. /// @@ -364,6 +379,10 @@ extension JsFlutterConfigurationExtension on JsFlutterConfiguration { external JSString? get _renderer; String? get renderer => _renderer?.toDart; + @JS('fontFallbackBaseUrl') + external JSString? get _fontFallbackBaseUrl; + String? get fontFallbackBaseUrl => _fontFallbackBaseUrl?.toDart; + @JS('useColorEmoji') external JSBoolean? get _useColorEmoji; bool? get useColorEmoji => _useColorEmoji?.toDart; diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/font_fallbacks.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/font_fallbacks.dart index e8ac39f5e7d..9773acb952e 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/font_fallbacks.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/font_fallbacks.dart @@ -449,11 +449,7 @@ class FallbackFontDownloadQueue { final FontFallbackManager fallbackManager; - static const String _defaultFallbackFontsUrlPrefix = - 'https://fonts.gstatic.com/s/'; - String? fallbackFontUrlPrefixOverride; - String get fallbackFontUrlPrefix => - fallbackFontUrlPrefixOverride ?? _defaultFallbackFontsUrlPrefix; + String get fallbackFontUrlPrefix => configuration.fontFallbackBaseUrl; final Set downloadedFonts = {}; final Map pendingFonts = {}; @@ -497,7 +493,8 @@ class FallbackFontDownloadQueue { downloadedFontFamilies.add(font.url); } catch (e) { pendingFonts.remove(font.url); - printWarning('Failed to load font ${font.name} at ${font.url}'); + printWarning('Failed to load font ${font.name} at ' + '$fallbackFontUrlPrefix${font.url}'); printWarning(e.toString()); return; } diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/skwasm/skwasm_impl/font_collection.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/skwasm/skwasm_impl/font_collection.dart index ed7a9dd85ce..4f561e41837 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/skwasm/skwasm_impl/font_collection.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/skwasm/skwasm_impl/font_collection.dart @@ -17,8 +17,8 @@ import 'package:ui/ui_web/src/ui_web.dart' as ui_web; // this, list out all of the fonts and find the URL for the regular // Roboto font. The API reference is here: // https://developers.google.com/fonts/docs/developer_api -const String _robotoUrl = - 'https://fonts.gstatic.com/s/roboto/v20/KFOmCnqEu92Fr1Me5WZLCzYlKw.ttf'; +String _robotoUrl = + '${configuration.fontFallbackBaseUrl}roboto/v20/KFOmCnqEu92Fr1Me5WZLCzYlKw.ttf'; class SkwasmTypeface extends SkwasmObjectWrapper { SkwasmTypeface(SkDataHandle data) : super(typefaceCreate(data), _registry); diff --git a/engine/src/flutter/lib/web_ui/test/canvaskit/canvas_golden_test.dart b/engine/src/flutter/lib/web_ui/test/canvaskit/canvas_golden_test.dart index 087817dc9a5..70570582e91 100644 --- a/engine/src/flutter/lib/web_ui/test/canvaskit/canvas_golden_test.dart +++ b/engine/src/flutter/lib/web_ui/test/canvaskit/canvas_golden_test.dart @@ -25,7 +25,6 @@ void testMain() { setUp(() { renderer.fontCollection.debugResetFallbackFonts(); - renderer.fontCollection.fontFallbackManager!.downloadQueue.fallbackFontUrlPrefixOverride = 'assets/fallback_fonts/'; }); test('renders using non-recording canvas if weak refs are supported', diff --git a/engine/src/flutter/lib/web_ui/test/canvaskit/common.dart b/engine/src/flutter/lib/web_ui/test/canvaskit/common.dart index 5fc177faf5a..cc69142d8b9 100644 --- a/engine/src/flutter/lib/web_ui/test/canvaskit/common.dart +++ b/engine/src/flutter/lib/web_ui/test/canvaskit/common.dart @@ -3,6 +3,7 @@ // found in the LICENSE file. import 'dart:async'; +import 'dart:js_interop'; import 'dart:typed_data'; import 'package:test/test.dart'; @@ -26,10 +27,9 @@ void setUpCanvasKitTest({bool withImplicitView = false}) { setUpTestViewDimensions: false, ); - setUp(() => renderer.fontCollection.fontFallbackManager!.downloadQueue - .fallbackFontUrlPrefixOverride = 'assets/fallback_fonts/'); - tearDown(() => renderer.fontCollection.fontFallbackManager!.downloadQueue - .fallbackFontUrlPrefixOverride = null); + setUp(() => debugOverrideJsConfiguration({ + 'fontFallbackBaseUrl': 'assets/fallback_fonts/', + }.jsify() as JsFlutterConfiguration?)); } /// Convenience getter for the implicit view. diff --git a/engine/src/flutter/lib/web_ui/test/canvaskit/skia_font_collection_test.dart b/engine/src/flutter/lib/web_ui/test/canvaskit/skia_font_collection_test.dart index 1527e707fab..6b364009e70 100644 --- a/engine/src/flutter/lib/web_ui/test/canvaskit/skia_font_collection_test.dart +++ b/engine/src/flutter/lib/web_ui/test/canvaskit/skia_font_collection_test.dart @@ -118,6 +118,10 @@ void testMain() { final SkiaFontCollection fontCollection = SkiaFontCollection(); testAssetScope.setAsset('FontManifest.json', stringAsUtf8Data(''' [ + { + "family":"Roboto", + "fonts":[{"asset":"/assets/fonts/Roboto-Regular.ttf"}] + }, { "family":"Ahem", "fonts":[{"asset":"/assets/fonts/Roboto-Regular.ttf"}] diff --git a/engine/src/flutter/lib/web_ui/test/common/test_initialization.dart b/engine/src/flutter/lib/web_ui/test/common/test_initialization.dart index e7a611648a9..2f0c9434621 100644 --- a/engine/src/flutter/lib/web_ui/test/common/test_initialization.dart +++ b/engine/src/flutter/lib/web_ui/test/common/test_initialization.dart @@ -3,6 +3,7 @@ // found in the LICENSE file. import 'dart:async'; +import 'dart:js_interop'; import 'package:test/test.dart'; import 'package:ui/src/engine.dart' as engine; @@ -27,7 +28,9 @@ void setUpUnitTests({ debugFontsScope = configureDebugFontsAssetScope(fakeAssetManager); debugOnlyAssetManager = fakeAssetManager; await bootstrapAndRunApp(withImplicitView: withImplicitView); - engine.renderer.fontCollection.fontFallbackManager?.downloadQueue.fallbackFontUrlPrefixOverride = 'assets/fallback_fonts/'; + engine.debugOverrideJsConfiguration({ + 'fontFallbackBaseUrl': 'assets/fallback_fonts/', + }.jsify() as engine.JsFlutterConfiguration?); if (setUpTestViewDimensions) { // The following parameters are hard-coded in Flutter's test embedder. Since diff --git a/engine/src/flutter/lib/web_ui/test/ui/fallback_fonts_golden_test.dart b/engine/src/flutter/lib/web_ui/test/ui/fallback_fonts_golden_test.dart index 621a9a3334d..f530d86c2c3 100644 --- a/engine/src/flutter/lib/web_ui/test/ui/fallback_fonts_golden_test.dart +++ b/engine/src/flutter/lib/web_ui/test/ui/fallback_fonts_golden_test.dart @@ -2,6 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +import 'dart:js_interop'; import 'dart:math' as math; import 'package:test/bootstrap/browser.dart'; @@ -39,9 +40,12 @@ void testMain() { setUp(() { renderer.fontCollection.debugResetFallbackFonts(); - renderer.fontCollection.fontFallbackManager!.downloadQueue.fallbackFontUrlPrefixOverride = 'assets/fallback_fonts/'; - renderer.fontCollection.fontFallbackManager!.downloadQueue.debugOnLoadFontFamily - = (String family) => downloadedFontFamilies.add(family); + debugOverrideJsConfiguration({ + 'fontFallbackBaseUrl': 'assets/fallback_fonts/', + }.jsify() as JsFlutterConfiguration?); + renderer.fontCollection.fontFallbackManager!.downloadQueue + .debugOnLoadFontFamily = + (String family) => downloadedFontFamilies.add(family); savedCallback = ui.PlatformDispatcher.instance.onPlatformMessage; }); @@ -51,11 +55,30 @@ void testMain() { }); test('Roboto is always a fallback font', () { - expect(renderer.fontCollection.fontFallbackManager!.globalFontFallbacks, contains('Roboto')); + expect(renderer.fontCollection.fontFallbackManager!.globalFontFallbacks, + contains('Roboto')); + }); + + test('can override font fallback base URL using JS', () { + expect( + renderer.fontCollection.fontFallbackManager!.downloadQueue + .fallbackFontUrlPrefix, + 'assets/fallback_fonts/', + ); + debugOverrideJsConfiguration({ + 'fontFallbackBaseUrl': 'http://my-special-fonts.com/', + }.jsify() as JsFlutterConfiguration?); + + expect( + renderer.fontCollection.fontFallbackManager!.downloadQueue + .fallbackFontUrlPrefix, + 'http://my-special-fonts.com/', + ); }); test('will download Noto Sans Arabic if Arabic text is added', () async { - expect(renderer.fontCollection.fontFallbackManager!.globalFontFallbacks, ['Roboto']); + expect(renderer.fontCollection.fontFallbackManager!.globalFontFallbacks, + ['Roboto']); // Creating this paragraph should cause us to start to download the // fallback font. @@ -92,9 +115,11 @@ void testMain() { // TODO(hterkelsen): https://github.com/flutter/flutter/issues/71520 }); - test('will put the Noto Color Emoji font before other fallback fonts in the list', + test( + 'will put the Noto Color Emoji font before other fallback fonts in the list', () async { - expect(renderer.fontCollection.fontFallbackManager!.globalFontFallbacks, ['Roboto']); + expect(renderer.fontCollection.fontFallbackManager!.globalFontFallbacks, + ['Roboto']); // Creating this paragraph should cause us to start to download the // Arabic fallback font. @@ -120,16 +145,20 @@ void testMain() { await renderer.fontCollection.fontFallbackManager!.debugWhenIdle(); - expect(renderer.fontCollection.fontFallbackManager!.globalFontFallbacks, [ - 'Roboto', - 'Noto Color Emoji', - 'Noto Sans Arabic', - ]); + expect( + renderer.fontCollection.fontFallbackManager!.globalFontFallbacks, + [ + 'Roboto', + 'Noto Color Emoji', + 'Noto Sans Arabic', + ]); }); - test('will download Noto Color Emojis and Noto Symbols if no matching Noto Font', + test( + 'will download Noto Color Emojis and Noto Symbols if no matching Noto Font', () async { - expect(renderer.fontCollection.fontFallbackManager!.globalFontFallbacks, ['Roboto']); + expect(renderer.fontCollection.fontFallbackManager!.globalFontFallbacks, + ['Roboto']); // Creating this paragraph should cause us to start to download the // fallback font. @@ -170,7 +199,8 @@ void testMain() { /// /// Then it does the same, but asserts that the families aren't downloaded again /// (because they already exist in memory). - Future checkDownloadedFamiliesForString(String text, List expectedFamilies) async { + Future checkDownloadedFamiliesForString( + String text, List expectedFamilies) async { // Try rendering text that requires fallback fonts, initially before the fonts are loaded. ui.ParagraphBuilder pb = ui.ParagraphBuilder(ui.ParagraphStyle()); pb.addText(text); @@ -217,7 +247,8 @@ void testMain() { ]); }); - test('findMinimumFontsForCodePoints for all supported code points', () async { + test('findMinimumFontsForCodePoints for all supported code points', + () async { // Collect all supported code points from all fallback fonts in the Noto // font tree. final Set testedFonts = {}; @@ -452,7 +483,7 @@ void testMain() { isNot(contains('Noto Color Emoji'))); }); }, - // HTML renderer doesn't use the fallback font manager. - skip: isHtml, - timeout: const Timeout.factor(4)); + // HTML renderer doesn't use the fallback font manager. + skip: isHtml, + timeout: const Timeout.factor(4)); }