Multiview: Add view ID to _render and remove render rule skipping (fl…

…utter#50220)

This PR adds a view ID parameter to the Dart FFI `_render` function, and
also remove the logic that skips illegal renders that violates the
render rule.

I decided to implement the change to add the view ID as a separate PR
because it's blocking us from performing benchmark testing internally,
which is required to verify before merging the full multiview pipeline.

We're also abolishing the render rule for now to allow presenting the
warmup frame, which affects the startup performance. It is planned to
implement the render rule once we can coordinate the warmup frame into
the pipeline.

### More on removing the render rule enforcement

The project needs to be divided into two phases:
* Phase 1: Dart:ui doesn't kip out-of-vsync frames. And the pipeline
presents these frames in a hacky way.
* Phase 2: Dart:ui skip out-of-vsync frames, but submit warmup frames
using a dedicated method. The pipeline contains no hacks.

The current logic only enforces the render rule in debug mode. It's not
desired to have the debug mode and the release mode behaving
differently. Moreover, I'd like to make only the necessary changes for
phase 1, so that there is as little "to be used in the future" code
between the two phases as possible.

## Pre-launch Checklist

- [ ] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [ ] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [ ] I read and followed the [Flutter Style Guide] and the [C++,
Objective-C, Java style guides].
- [ ] I listed at least one issue that this PR fixes in the description
above.
- [ ] 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.
- [ ] I updated/added relevant documentation (doc comments with `///`).
- [ ] I signed the [CLA].
- [ ] 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

lib/ui/dart_ui.cc

@@ -98,7 +98,7 @@ typedef CanvasPath Path;
   V(NativeStringAttribute::initSpellOutStringAttribute, 3)            \
   V(PlatformConfigurationNativeApi::DefaultRouteName, 0)              \
   V(PlatformConfigurationNativeApi::ScheduleFrame, 0)                 \
-  V(PlatformConfigurationNativeApi::Render, 1)                        \
+  V(PlatformConfigurationNativeApi::Render, 4)                        \
   V(PlatformConfigurationNativeApi::UpdateSemantics, 1)               \
   V(PlatformConfigurationNativeApi::SetNeedsReportTimings, 1)         \
   V(PlatformConfigurationNativeApi::SetIsolateDebugName, 1)           \

lib/ui/platform_dispatcher.dart

@@ -308,29 +308,6 @@ class PlatformDispatcher {
     _invoke(onMetricsChanged, _onMetricsChangedZone);
   }
 
-  // A debug-only variable that stores the [FlutterView]s for which
-  // [FlutterView.render] has already been called during the current
-  // [onBeginFrame]/[onDrawFrame] callback sequence.
-  //
-  // It is null outside the scope of those callbacks indicating that calls to
-  // [FlutterView.render] must be ignored. Furthermore, if a given [FlutterView]
-  // is already present in this set when its [FlutterView.render] is called
-  // again, that call must be ignored as a duplicate.
-  //
-  // Between [onBeginFrame] and [onDrawFrame] the properties value is
-  // temporarily stored in `_renderedViewsBetweenCallbacks` so that it survives
-  // the gap between the two callbacks.
-  //
-  // In release build, this variable is null, and therefore the calling rule is
-  // not enforced. This is because the check might hurt cold startup delay;
-  // see https://github.com/flutter/engine/pull/46919.
-  Set<FlutterView>? _debugRenderedViews;
-  // A debug-only variable that temporarily stores the `_renderedViews` value
-  // between `_beginFrame` and `_drawFrame`.
-  //
-  // In release build, this variable is null.
-  Set<FlutterView>? _debugRenderedViewsBetweenCallbacks;
-
   /// A callback invoked when any view begins a frame.
   ///
   /// A callback that is invoked to notify the application that it is an
@@ -351,26 +328,11 @@ class PlatformDispatcher {
 
   // Called from the engine, via hooks.dart
   void _beginFrame(int microseconds) {
-    assert(_debugRenderedViews == null);
-    assert(_debugRenderedViewsBetweenCallbacks == null);
-    assert(() {
-      _debugRenderedViews = <FlutterView>{};
-      return true;
-    }());
-
     _invoke1<Duration>(
       onBeginFrame,
       _onBeginFrameZone,
       Duration(microseconds: microseconds),
     );
-
-    assert(_debugRenderedViews != null);
-    assert(_debugRenderedViewsBetweenCallbacks == null);
-    assert(() {
-      _debugRenderedViewsBetweenCallbacks = _debugRenderedViews;
-      _debugRenderedViews = null;
-      return true;
-    }());
   }
 
   /// A callback that is invoked for each frame after [onBeginFrame] has
@@ -388,22 +350,7 @@ class PlatformDispatcher {
 
   // Called from the engine, via hooks.dart
   void _drawFrame() {
-    assert(_debugRenderedViews == null);
-    assert(_debugRenderedViewsBetweenCallbacks != null);
-    assert(() {
-      _debugRenderedViews = _debugRenderedViewsBetweenCallbacks;
-      _debugRenderedViewsBetweenCallbacks = null;
-      return true;
-    }());
-
     _invoke(onDrawFrame, _onDrawFrameZone);
-
-    assert(_debugRenderedViews != null);
-    assert(_debugRenderedViewsBetweenCallbacks == null);
-    assert(() {
-      _debugRenderedViews = null;
-      return true;
-    }());
   }
 
   /// A callback that is invoked when pointer data is available.

lib/ui/window.dart

@@ -372,22 +372,11 @@ class FlutterView {
   /// * [RendererBinding], the Flutter framework class which manages layout and
   ///   painting.
   void render(Scene scene, {Size? size}) {
-    // Duplicated calls or calls outside of onBeginFrame/onDrawFrame (indicated
-    // by _debugRenderedViews being null) are ignored. See _debugRenderedViews.
-    // TODO(dkwingsmt): We should change this skip into an assertion.
-    // https://github.com/flutter/flutter/issues/137073
-    bool validRender = true;
-    assert(() {
-      validRender = platformDispatcher._debugRenderedViews?.add(this) ?? false;
-      return true;
-    }());
-    if (validRender) {
-      _render(scene as _NativeScene, size?.width ?? physicalSize.width, size?.height ?? physicalSize.height);
-    }
+    _render(viewId, scene as _NativeScene, size?.width ?? physicalSize.width, size?.height ?? physicalSize.height);
   }
 
-  @Native<Void Function(Pointer<Void>, Double, Double)>(symbol: 'PlatformConfigurationNativeApi::Render')
-  external static void _render(_NativeScene scene, double width, double height);
+  @Native<Void Function(Int64, Pointer<Void>, Double, Double)>(symbol: 'PlatformConfigurationNativeApi::Render')
+  external static void _render(int viewId, _NativeScene scene, double width, double height);
 
   /// Change the retained semantics data about this [FlutterView].
   ///

lib/ui/window/platform_configuration.cc

@@ -449,9 +449,13 @@ void PlatformConfiguration::CompletePlatformMessageResponse(
   response->Complete(std::make_unique<fml::DataMapping>(std::move(data)));
 }
 
-void PlatformConfigurationNativeApi::Render(Scene* scene,
+void PlatformConfigurationNativeApi::Render(int64_t view_id,
+                                            Scene* scene,
                                             double width,
                                             double height) {
+  // TODO(dkwingsmt): Currently only supports a single window.
+  // See https://github.com/flutter/flutter/issues/135530, item 2.
+  FML_DCHECK(view_id == kFlutterImplicitViewId);
   UIDartState::ThrowIfUIOperationsProhibited();
   UIDartState::Current()->platform_configuration()->client()->Render(
       scene, width, height);

lib/ui/window/platform_configuration.h

@@ -557,7 +557,10 @@ class PlatformConfigurationNativeApi {
 
   static void ScheduleFrame();
 
-  static void Render(Scene* scene, double width, double height);
+  static void Render(int64_t view_id,
+                     Scene* scene,
+                     double width,
+                     double height);
 
   static void UpdateSemantics(SemanticsUpdate* update);