From 92281aae46980dc584c5ba989668de50409e72f1 Mon Sep 17 00:00:00 2001 From: Harry Terkelsen <1961493+harryterkelsen@users.noreply.github.com> Date: Fri, 14 Feb 2025 15:36:26 -0800 Subject: [PATCH] [canvaskit] Handle MakeGrContext returning null (#163332) Mark up the CanvasKit binding API to acknowledge that `MakeGrContext` can return null. Towards https://github.com/flutter/flutter/issues/162868 ## 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], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [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/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md --- .../lib/src/engine/canvaskit/canvaskit_api.dart | 4 ++-- .../web_ui/lib/src/engine/canvaskit/surface.dart | 2 ++ .../test/canvaskit/canvaskit_api_test.dart | 8 ++++---- .../lib/web_ui/test/canvaskit/surface_test.dart | 16 ++++++++++++++++ 4 files changed, 24 insertions(+), 6 deletions(-) diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/canvaskit/canvaskit_api.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/canvaskit/canvaskit_api.dart index a1c69c2816..c994d295c3 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/canvaskit/canvaskit_api.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/canvaskit/canvaskit_api.dart @@ -158,8 +158,8 @@ extension CanvasKitExtension on CanvasKit { _GetOffscreenWebGLContext(canvas, options).toDartDouble; @JS('MakeGrContext') - external SkGrContext _MakeGrContext(JSNumber glContext); - SkGrContext MakeGrContext(double glContext) => _MakeGrContext(glContext.toJS); + external SkGrContext? _MakeGrContext(JSNumber glContext); + SkGrContext? MakeGrContext(double glContext) => _MakeGrContext(glContext.toJS); @JS('MakeOnScreenGLSurface') external SkSurface? _MakeOnScreenGLSurface( diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/canvaskit/surface.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/canvaskit/surface.dart index f7819238be..a19541cd66 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/canvaskit/surface.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/canvaskit/surface.dart @@ -403,6 +403,8 @@ class Surface extends DisplayCanvas { if (_glContext != 0) { _grContext = canvasKit.MakeGrContext(glContext.toDouble()); if (_grContext == null) { + // TODO(harryterkelsen): Make this error message more descriptive by + // reporting the number of currently live Surfaces, https://github.com/flutter/flutter/issues/162868. throw CanvasKitError( 'Failed to initialize CanvasKit. ' 'CanvasKit.MakeGrContext returned null.', diff --git a/engine/src/flutter/lib/web_ui/test/canvaskit/canvaskit_api_test.dart b/engine/src/flutter/lib/web_ui/test/canvaskit/canvaskit_api_test.dart index 3bd46f4051..011af1451a 100644 --- a/engine/src/flutter/lib/web_ui/test/canvaskit/canvaskit_api_test.dart +++ b/engine/src/flutter/lib/web_ui/test/canvaskit/canvaskit_api_test.dart @@ -1730,9 +1730,9 @@ void _paragraphTests() { canvas, SkWebGLContextOptions(antialias: 0, majorVersion: webGLVersion.toDouble()), ); - final SkGrContext grContext = canvasKit.MakeGrContext(glContext); + final SkGrContext? grContext = canvasKit.MakeGrContext(glContext); final SkSurface? skSurface = canvasKit.MakeOnScreenGLSurface( - grContext, + grContext!, 100, 100, SkColorSpaceSRGB, @@ -1755,8 +1755,8 @@ void _paragraphTests() { canvas, SkWebGLContextOptions(antialias: 0, majorVersion: webGLVersion.toDouble()), ).toInt(); - final SkGrContext grContext = canvasKit.MakeGrContext(glContext.toDouble()); - final SkSurface? surface = canvasKit.MakeRenderTarget(grContext, 1, 1); + final SkGrContext? grContext = canvasKit.MakeGrContext(glContext.toDouble()); + final SkSurface? surface = canvasKit.MakeRenderTarget(grContext!, 1, 1); expect(surface, isNotNull); }, diff --git a/engine/src/flutter/lib/web_ui/test/canvaskit/surface_test.dart b/engine/src/flutter/lib/web_ui/test/canvaskit/surface_test.dart index 50b28f27c9..bd49152faa 100644 --- a/engine/src/flutter/lib/web_ui/test/canvaskit/surface_test.dart +++ b/engine/src/flutter/lib/web_ui/test/canvaskit/surface_test.dart @@ -322,6 +322,22 @@ void testMain() { expect(transferToImageBitmapCalls, 1); }, skip: !Surface.offscreenCanvasSupported); + test('throws error if CanvasKit.MakeGrContext returns null', () async { + final Object originalMakeGrContext = js_util.getProperty(canvasKit, 'MakeGrContext'); + js_util.setProperty(canvasKit, 'originalMakeGrContext', originalMakeGrContext); + js_util.setProperty( + canvasKit, + 'MakeGrContext', + js_util.allowInterop((int glContext) { + return null; + }), + ); + final Surface surface = Surface(); + expect(() => surface.ensureSurface(const BitmapSize(10, 10)), throwsA(isA())); + js_util.setProperty(canvasKit, 'MakeGrContext', originalMakeGrContext); + // Skipping on Firefox for now since Firefox headless doesn't support WebGL + }, skip: isFirefox); + test('can recover from MakeSWCanvasSurface failure', () async { debugOverrideJsConfiguration( {'canvasKitForceCpuOnly': true}.jsify() as JsFlutterConfiguration?,