Reland "[web] Enforce onDrawFrame/onBeginFrame render rule" (#49336)

Enforces the render rule on the web except in the HTML renderer, which
must still render to the DOM even outside of `onDrawFrame` or
`onBegineFrame` scopes in order for golden tests to continue to work in
the framework.

This is a reland of #49214 with
one change (checking for `renderer.rendererTag == 'html'` in the
`render` method).

## 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].

<!-- Links -->
[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

Author: harryterkelsen

lib/web_ui/lib/platform_dispatcher.dart

@@ -80,7 +80,7 @@ abstract class PlatformDispatcher {
 
   void scheduleFrame();
 
-  void render(Scene scene, [FlutterView view]);
+  Future<void> render(Scene scene, [FlutterView view]);
 
   AccessibilityFeatures get accessibilityFeatures;
 

lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart

@@ -364,7 +364,7 @@ class HtmlViewEmbedder {
     sceneHost.append(_svgPathDefs!);
   }
 
-  void submitFrame() {
+  Future<void> submitFrame() async {
     final ViewListDiffResult? diffResult =
         (_activeCompositionOrder.isEmpty || _compositionOrder.isEmpty)
             ? null
@@ -388,7 +388,7 @@ class HtmlViewEmbedder {
             _context.pictureRecorders[pictureRecorderIndex].endRecording());
         pictureRecorderIndex++;
       }
-      rasterizer.rasterizeToCanvas(overlay, pictures);
+      await rasterizer.rasterizeToCanvas(overlay, pictures);
     }
     for (final CkPictureRecorder recorder
         in _context.pictureRecordersCreatedDuringPreroll) {

lib/web_ui/lib/src/engine/canvaskit/rasterizer.dart

@@ -30,7 +30,7 @@ class Rasterizer {
 
   /// Creates a new frame from this rasterizer's surface, draws the given
   /// [LayerTree] into it, and then submits the frame.
-  void draw(LayerTree layerTree) {
+  Future<void> draw(LayerTree layerTree) async {
     final ui.Size frameSize = view.physicalSize;
     if (frameSize.isEmpty) {
       // Available drawing area is empty. Skip drawing.
@@ -49,10 +49,10 @@ class Rasterizer {
     compositorFrame.raster(layerTree, ignoreRasterCache: true);
 
     sceneHost.prepend(renderCanvasFactory.baseCanvas.htmlElement);
-    rasterizeToCanvas(renderCanvasFactory.baseCanvas,
+    await rasterizeToCanvas(renderCanvasFactory.baseCanvas,
         <CkPicture>[pictureRecorder.endRecording()]);
 
-    viewEmbedder.submitFrame();
+    await viewEmbedder.submitFrame();
   }
 
   /// Disposes of this rasterizer.

lib/web_ui/lib/src/engine/canvaskit/renderer.dart

@@ -402,7 +402,7 @@ class CanvasKitRenderer implements Renderer {
     CkParagraphBuilder(style);
 
   @override
-  void renderScene(ui.Scene scene, ui.FlutterView view) {
+  Future<void> renderScene(ui.Scene scene, ui.FlutterView view) async {
     // "Build finish" and "raster start" happen back-to-back because we
     // render on the same thread, so there's no overhead from hopping to
     // another thread.
@@ -417,7 +417,7 @@ class CanvasKitRenderer implements Renderer {
         "Unable to render to a view which hasn't been registered");
     final Rasterizer rasterizer = _rasterizers[view.viewId]!;
 
-    rasterizer.draw((scene as LayerScene).layerTree);
+    await rasterizer.draw((scene as LayerScene).layerTree);
     frameTimingsOnRasterFinish();
   }
 

lib/web_ui/lib/src/engine/canvaskit/surface.dart

@@ -115,22 +115,18 @@ class Surface {
     _surface!.flush();
 
     if (browserSupportsCreateImageBitmap) {
-      DomImageBitmap bitmap;
+      JSObject bitmapSource;
       if (Surface.offscreenCanvasSupported) {
-        bitmap = (await createImageBitmap(_offscreenCanvas! as JSObject, (
-          x: 0,
-          y: _pixelHeight - frameSize.height.toInt(),
-          width: frameSize.width.toInt(),
-          height: frameSize.height.toInt(),
-        )).toDart)! as DomImageBitmap;
+        bitmapSource = _offscreenCanvas! as JSObject;
       } else {
-        bitmap = (await createImageBitmap(_canvasElement! as JSObject, (
-          x: 0,
-          y: _pixelHeight - frameSize.height.toInt(),
-          width: frameSize.width.toInt(),
-          height: frameSize.height.toInt()
-        )).toDart)! as DomImageBitmap;
+        bitmapSource = _canvasElement! as JSObject;
       }
+      final DomImageBitmap bitmap = await createImageBitmap(bitmapSource, (
+        x: 0,
+        y: _pixelHeight - frameSize.height.toInt(),
+        width: frameSize.width.toInt(),
+        height: frameSize.height.toInt(),
+      ));
       canvas.render(bitmap);
     } else {
       // If the browser doesn't support `createImageBitmap` (e.g. Safari 14)

lib/web_ui/lib/src/engine/dom.dart

@@ -210,14 +210,16 @@ external JSPromise<JSAny?> _createImageBitmap2(
   JSNumber width,
   JSNumber height,
 );
-JSPromise<JSAny?> createImageBitmap(JSAny source,
+Future<DomImageBitmap> createImageBitmap(JSAny source,
     [({int x, int y, int width, int height})? bounds]) {
+  JSPromise<JSAny?> jsPromise;
   if (bounds != null) {
-    return _createImageBitmap2(source, bounds.x.toJS, bounds.y.toJS,
+    jsPromise = _createImageBitmap2(source, bounds.x.toJS, bounds.y.toJS,
         bounds.width.toJS, bounds.height.toJS);
   } else {
-    return _createImageBitmap1(source);
+    jsPromise = _createImageBitmap1(source);
   }
+  return js_util.promiseToFuture<DomImageBitmap>(jsPromise);
 }
 
 @JS()

lib/web_ui/lib/src/engine/html/renderer.dart

@@ -323,7 +323,7 @@ class HtmlRenderer implements Renderer {
     CanvasParagraphBuilder(style as EngineParagraphStyle);
 
   @override
-  void renderScene(ui.Scene scene, ui.FlutterView view) {
+  Future<void> renderScene(ui.Scene scene, ui.FlutterView view) async {
     final EngineFlutterView implicitView = EnginePlatformDispatcher.instance.implicitView!;
     implicitView.dom.setScene((scene as SurfaceScene).webOnlyRootElement!);
     frameTimingsOnRasterFinish();

lib/web_ui/lib/src/engine/platform_dispatcher.dart

@@ -213,6 +213,10 @@ class EnginePlatformDispatcher extends ui.PlatformDispatcher {
     }
   }
 
+  /// A set of views which have rendered in the current `onBeginFrame` or
+  /// `onDrawFrame` scope.
+  Set<ui.FlutterView>? _viewsRenderedInCurrentFrame;
+
   /// A callback invoked when any window begins a frame.
   ///
   /// A callback that is invoked to notify the application that it is an
@@ -235,7 +239,9 @@ class EnginePlatformDispatcher extends ui.PlatformDispatcher {
   /// Engine code should use this method instead of the callback directly.
   /// Otherwise zones won't work properly.
   void invokeOnBeginFrame(Duration duration) {
+    _viewsRenderedInCurrentFrame = <ui.FlutterView>{};
     invoke1<Duration>(_onBeginFrame, _onBeginFrameZone, duration);
+    _viewsRenderedInCurrentFrame = null;
   }
 
   /// A callback that is invoked for each frame after [onBeginFrame] has
@@ -256,7 +262,9 @@ class EnginePlatformDispatcher extends ui.PlatformDispatcher {
   /// Engine code should use this method instead of the callback directly.
   /// Otherwise zones won't work properly.
   void invokeOnDrawFrame() {
+    _viewsRenderedInCurrentFrame = <ui.FlutterView>{};
     invoke(_onDrawFrame, _onDrawFrameZone);
+    _viewsRenderedInCurrentFrame = null;
   }
 
   /// A callback that is invoked when pointer data is available.
@@ -753,14 +761,27 @@ class EnginePlatformDispatcher extends ui.PlatformDispatcher {
   ///  * [RendererBinding], the Flutter framework class which manages layout and
   ///    painting.
   @override
-  void render(ui.Scene scene, [ui.FlutterView? view]) {
+  Future<void> render(ui.Scene scene, [ui.FlutterView? view]) async {
     assert(view != null || implicitView != null,
         'Calling render without a FlutterView');
     if (view == null && implicitView == null) {
       // If there is no view to render into, then this is a no-op.
       return;
     }
-    renderer.renderScene(scene, view ?? implicitView!);
+    final ui.FlutterView viewToRender = view ?? implicitView!;
+
+    // Only render in an `onDrawFrame` or `onBeginFrame` scope. This is checked
+    // by checking if the `_viewsRenderedInCurrentFrame` is non-null and this
+    // view hasn't been rendered already in this scope.
+    final bool shouldRender =
+        _viewsRenderedInCurrentFrame?.add(viewToRender) ?? false;
+    // TODO(harryterkelsen): HTML renderer needs to violate the render rule in
+    // order to perform golden tests in Flutter framework because on the HTML
+    // renderer, golden tests render to DOM and then take a browser screenshot,
+    // https://github.com/flutter/flutter/issues/137073.
+    if (shouldRender || renderer.rendererTag == 'html') {
+      await renderer.renderScene(scene, viewToRender);
+    }
   }
 
   /// Additional accessibility features that may be enabled by the platform.
@@ -1275,7 +1296,8 @@ class EnginePlatformDispatcher extends ui.PlatformDispatcher {
   String get defaultRouteName {
     // TODO(mdebbar): What should we do in multi-view mode?
     //                https://github.com/flutter/flutter/issues/139174
-    return _defaultRouteName ??= implicitView?.browserHistory.currentPath ?? '/';
+    return _defaultRouteName ??=
+        implicitView?.browserHistory.currentPath ?? '/';
   }
 
   /// Lazily initialized when the `defaultRouteName` getter is invoked.

lib/web_ui/lib/src/engine/renderer.dart

@@ -222,5 +222,5 @@ abstract class Renderer {
 
   ui.ParagraphBuilder createParagraphBuilder(ui.ParagraphStyle style);
 
-  FutureOr<void> renderScene(ui.Scene scene, ui.FlutterView view);
+  Future<void> renderScene(ui.Scene scene, ui.FlutterView view);
 }

lib/web_ui/lib/src/engine/skwasm/skwasm_stub/renderer.dart

@@ -150,7 +150,7 @@ class SkwasmRenderer implements Renderer {
   }
 
   @override
-  void renderScene(ui.Scene scene, ui.FlutterView view) {
+  Future<void> renderScene(ui.Scene scene, ui.FlutterView view) {
     throw UnimplementedError('Skwasm not implemented on this platform.');
   }