Revert "Revert "Enforce the rule of calling FlutterView.Render" (fl…

…utter#45525)"

This reverts commit 75437a3.

lib/ui/fixtures/ui_test.dart

@@ -1076,3 +1076,45 @@ external void _callHook(
   Object? arg20,
   Object? arg21,
 ]);
+
+Scene _createRedBoxScene(Size size) {
+  final SceneBuilder builder = SceneBuilder();
+  builder.pushOffset(0.0, 0.0);
+  final Paint paint = Paint()
+    ..color = Color.fromARGB(255, 255, 0, 0)
+    ..style = PaintingStyle.fill;
+  final PictureRecorder baseRecorder = PictureRecorder();
+  final Canvas canvas = Canvas(baseRecorder);
+  canvas.drawRect(Rect.fromLTRB(0.0, 0.0, size.width, size.height), paint);
+  final Picture picture = baseRecorder.endRecording();
+  builder.addPicture(Offset(0.0, 0.0), picture);
+  builder.pop();
+  return builder.build();
+}
+
+@pragma('vm:entry-point')
+void incorrectImmediateRender() {
+  PlatformDispatcher.instance.views.first.render(_createRedBoxScene(Size(2, 2)));
+  _finish();
+  // Don't schedule a frame here. This test only checks if the
+  // [FlutterView.render] call is propagated to PlatformConfiguration.render
+  // and thus doesn't need anything from `Animator` or `Engine`, which,
+  // besides, are not even created in the native side at all.
+}
+
+@pragma('vm:entry-point')
+void incorrectDoubleRender() {
+  PlatformDispatcher.instance.onBeginFrame = (Duration value) {
+    PlatformDispatcher.instance.views.first.render(_createRedBoxScene(Size(2, 2)));
+    PlatformDispatcher.instance.views.first.render(_createRedBoxScene(Size(3, 3)));
+  };
+  PlatformDispatcher.instance.onDrawFrame = () {
+    PlatformDispatcher.instance.views.first.render(_createRedBoxScene(Size(4, 4)));
+    PlatformDispatcher.instance.views.first.render(_createRedBoxScene(Size(5, 5)));
+  };
+  _finish();
+  // Don't schedule a frame here. This test only checks if the
+  // [FlutterView.render] call is propagated to PlatformConfiguration.render
+  // and thus doesn't need anything from `Animator` or `Engine`, which,
+  // besides, are not even created in the native side at all.
+}

lib/ui/platform_dispatcher.dart

@@ -308,6 +308,21 @@ class PlatformDispatcher {
     _invoke(onMetricsChanged, _onMetricsChangedZone);
   }
 
+  // [FlutterView]s for which [FlutterView.render] has already been called
+  // during the current [onBeginFrame]/[onDrawFrame] callback sequence.
+  //
+  // The field 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.
+  Set<FlutterView>? _renderedViews;
+  // Temporary storage of the `_renderedViews` value between `_beginFrame` and
+  // `_drawFrame`.
+  Set<FlutterView>? _renderedViewsBetweenCallbacks;
 
   /// A callback invoked when any view begins a frame.
   ///
@@ -329,11 +344,20 @@ class PlatformDispatcher {
 
   // Called from the engine, via hooks.dart
   void _beginFrame(int microseconds) {
+    assert(_renderedViews == null);
+    assert(_renderedViewsBetweenCallbacks == null);
+
+    _renderedViews = <FlutterView>{};
     _invoke1<Duration>(
       onBeginFrame,
       _onBeginFrameZone,
       Duration(microseconds: microseconds),
     );
+    _renderedViewsBetweenCallbacks = _renderedViews;
+    _renderedViews = null;
+
+    assert(_renderedViews == null);
+    assert(_renderedViewsBetweenCallbacks != null);
   }
 
   /// A callback that is invoked for each frame after [onBeginFrame] has
@@ -351,7 +375,16 @@ class PlatformDispatcher {
 
   // Called from the engine, via hooks.dart
   void _drawFrame() {
+    assert(_renderedViews == null);
+    assert(_renderedViewsBetweenCallbacks != null);
+
+    _renderedViews = _renderedViewsBetweenCallbacks;
+    _renderedViewsBetweenCallbacks = null;
     _invoke(onDrawFrame, _onDrawFrameZone);
+    _renderedViews = null;
+
+    assert(_renderedViews == null);
+    assert(_renderedViewsBetweenCallbacks == null);
   }
 
   /// A callback that is invoked when pointer data is available.

lib/ui/window.dart

@@ -353,7 +353,14 @@ class FlutterView {
   ///   scheduling of frames.
   /// * [RendererBinding], the Flutter framework class which manages layout and
   ///   painting.
-  void render(Scene scene) => _render(scene as _NativeScene);
+  void render(Scene scene) {
+    if (platformDispatcher._renderedViews?.add(this) != true) {
+      // Duplicated calls or calls outside of onBeginFrame/onDrawFrame
+      // (indicated by _renderedViews being null) are ignored, as documented.
+      return;
+    }
+    _render(scene as _NativeScene);
+  }
 
   @Native<Void Function(Pointer<Void>)>(symbol: 'PlatformConfigurationNativeApi::Render')
   external static void _render(_NativeScene scene);

lib/ui/window/platform_configuration_unittests.cc

@@ -15,10 +15,171 @@
 #include "flutter/shell/common/shell_test.h"
 #include "flutter/shell/common/thread_host.h"
 #include "flutter/testing/testing.h"
+#include "gmock/gmock.h"
 
 namespace flutter {
+
+namespace {
+
+static constexpr int64_t kImplicitViewId = 0;
+
+static void PostSync(const fml::RefPtr<fml::TaskRunner>& task_runner,
+                     const fml::closure& task) {
+  fml::AutoResetWaitableEvent latch;
+  fml::TaskRunner::RunNowOrPostTask(task_runner, [&latch, &task] {
+    task();
+    latch.Signal();
+  });
+  latch.Wait();
+}
+
+class MockRuntimeDelegate : public RuntimeDelegate {
+ public:
+  MOCK_METHOD(std::string, DefaultRouteName, (), (override));
+  MOCK_METHOD(void, ScheduleFrame, (bool), (override));
+  MOCK_METHOD(void,
+              Render,
+              (std::unique_ptr<flutter::LayerTree>, float),
+              (override));
+  MOCK_METHOD(void,
+              UpdateSemantics,
+              (SemanticsNodeUpdates, CustomAccessibilityActionUpdates),
+              (override));
+  MOCK_METHOD(void,
+              HandlePlatformMessage,
+              (std::unique_ptr<PlatformMessage>),
+              (override));
+  MOCK_METHOD(FontCollection&, GetFontCollection, (), (override));
+  MOCK_METHOD(std::shared_ptr<AssetManager>, GetAssetManager, (), (override));
+  MOCK_METHOD(void, OnRootIsolateCreated, (), (override));
+  MOCK_METHOD(void,
+              UpdateIsolateDescription,
+              (const std::string, int64_t),
+              (override));
+  MOCK_METHOD(void, SetNeedsReportTimings, (bool), (override));
+  MOCK_METHOD(std::unique_ptr<std::vector<std::string>>,
+              ComputePlatformResolvedLocale,
+              (const std::vector<std::string>&),
+              (override));
+  MOCK_METHOD(void, RequestDartDeferredLibrary, (intptr_t), (override));
+  MOCK_METHOD(std::weak_ptr<PlatformMessageHandler>,
+              GetPlatformMessageHandler,
+              (),
+              (const, override));
+  MOCK_METHOD(void, SendChannelUpdate, (std::string, bool), (override));
+  MOCK_METHOD(double,
+              GetScaledFontSize,
+              (double font_size, int configuration_id),
+              (const, override));
+};
+
+class MockPlatformMessageHandler : public PlatformMessageHandler {
+ public:
+  MOCK_METHOD(void,
+              HandlePlatformMessage,
+              (std::unique_ptr<PlatformMessage> message),
+              (override));
+  MOCK_METHOD(bool,
+              DoesHandlePlatformMessageOnPlatformThread,
+              (),
+              (const, override));
+  MOCK_METHOD(void,
+              InvokePlatformMessageResponseCallback,
+              (int response_id, std::unique_ptr<fml::Mapping> mapping),
+              (override));
+  MOCK_METHOD(void,
+              InvokePlatformMessageEmptyResponseCallback,
+              (int response_id),
+              (override));
+};
+
+// A class that can launch a RuntimeController with the specified
+// RuntimeDelegate.
+//
+// To use this class, contruct this class with Create, call LaunchRootIsolate,
+// and use the controller with ControllerTaskSync().
+class RuntimeControllerContext {
+ public:
+  using ControllerCallback = std::function<void(RuntimeController&)>;
+
+  [[nodiscard]] static std::unique_ptr<RuntimeControllerContext> Create(
+      Settings settings,                //
+      const TaskRunners& task_runners,  //
+      RuntimeDelegate& client) {
+    auto [vm, isolate_snapshot] = Shell::InferVmInitDataFromSettings(settings);
+    FML_CHECK(vm) << "Must be able to initialize the VM.";
+    // Construct the class with `new` because `make_unique` has no access to the
+    // private constructor.
+    RuntimeControllerContext* raw_pointer = new RuntimeControllerContext(
+        settings, task_runners, client, std::move(vm), isolate_snapshot);
+    return std::unique_ptr<RuntimeControllerContext>(raw_pointer);
+  }
+
+  ~RuntimeControllerContext() {
+    PostSync(task_runners_.GetUITaskRunner(),
+             [&]() { runtime_controller_.reset(); });
+  }
+
+  // Launch the root isolate. The post_launch callback will be executed in the
+  // same UI task, which can be used to create initial views.
+  void LaunchRootIsolate(RunConfiguration& configuration,
+                         ControllerCallback post_launch) {
+    PostSync(task_runners_.GetUITaskRunner(), [&]() {
+      bool launch_success = runtime_controller_->LaunchRootIsolate(
+          settings_,                                  //
+          []() {},                                    //
+          configuration.GetEntrypoint(),              //
+          configuration.GetEntrypointLibrary(),       //
+          configuration.GetEntrypointArgs(),          //
+          configuration.TakeIsolateConfiguration());  //
+      ASSERT_TRUE(launch_success);
+      post_launch(*runtime_controller_);
+    });
+  }
+
+  // Run a task that operates the RuntimeController on the UI thread, and wait
+  // for the task to end.
+  void ControllerTaskSync(ControllerCallback task) {
+    ASSERT_TRUE(runtime_controller_);
+    ASSERT_TRUE(task);
+    PostSync(task_runners_.GetUITaskRunner(),
+             [&]() { task(*runtime_controller_); });
+  }
+
+ private:
+  RuntimeControllerContext(const Settings& settings,
+                           const TaskRunners& task_runners,
+                           RuntimeDelegate& client,
+                           DartVMRef vm,
+                           fml::RefPtr<const DartSnapshot> isolate_snapshot)
+      : settings_(settings),
+        task_runners_(task_runners),
+        isolate_snapshot_(std::move(isolate_snapshot)),
+        vm_(std::move(vm)),
+        runtime_controller_(std::make_unique<RuntimeController>(
+            client,
+            &vm_,
+            std::move(isolate_snapshot_),
+            settings.idle_notification_callback,  // idle notification callback
+            flutter::PlatformData(),              // platform data
+            settings.isolate_create_callback,     // isolate create callback
+            settings.isolate_shutdown_callback,   // isolate shutdown callback
+            settings.persistent_isolate_data,     // persistent isolate data
+            UIDartState::Context{task_runners})) {}
+
+  Settings settings_;
+  TaskRunners task_runners_;
+  fml::RefPtr<const DartSnapshot> isolate_snapshot_;
+  DartVMRef vm_;
+  std::unique_ptr<RuntimeController> runtime_controller_;
+};
+}  // namespace
+
 namespace testing {
 
+using ::testing::_;
+using ::testing::Return;
+
 class PlatformConfigurationTest : public ShellTest {};
 
 TEST_F(PlatformConfigurationTest, Initialization) {
@@ -332,5 +493,84 @@ TEST_F(PlatformConfigurationTest, SetDartPerformanceMode) {
   DestroyShell(std::move(shell), task_runners);
 }
 
+TEST_F(PlatformConfigurationTest, OutOfScopeRenderCallsAreIgnored) {
+  Settings settings = CreateSettingsForFixture();
+  TaskRunners task_runners = GetTaskRunnersForFixture();
+
+  MockRuntimeDelegate client;
+  auto platform_message_handler =
+      std::make_shared<MockPlatformMessageHandler>();
+  EXPECT_CALL(client, GetPlatformMessageHandler)
+      .WillOnce(Return(platform_message_handler));
+  // Render should not be called.
+  EXPECT_CALL(client, Render).Times(0);
+
+  auto message_latch = std::make_shared<fml::AutoResetWaitableEvent>();
+  auto finish = [message_latch](Dart_NativeArguments args) {
+    message_latch->Signal();
+  };
+  AddNativeCallback("Finish", CREATE_NATIVE_ENTRY(finish));
+
+  auto runtime_controller_context =
+      RuntimeControllerContext::Create(settings, task_runners, client);
+
+  auto configuration = RunConfiguration::InferFromSettings(settings);
+  configuration.SetEntrypoint("incorrectImmediateRender");
+  runtime_controller_context->LaunchRootIsolate(
+      configuration, [](RuntimeController& runtime_controller) {
+        runtime_controller.AddView(
+            kImplicitViewId,
+            ViewportMetrics(
+                /*pixel_ratio=*/1.0, /*width=*/20, /*height=*/20,
+                /*touch_slop=*/2, /*display_id=*/0));
+      });
+
+  // Wait for the Dart main function to end.
+  message_latch->Wait();
+}
+
+TEST_F(PlatformConfigurationTest, DuplicateRenderCallsAreIgnored) {
+  Settings settings = CreateSettingsForFixture();
+  TaskRunners task_runners = GetTaskRunnersForFixture();
+
+  MockRuntimeDelegate client;
+  auto platform_message_handler =
+      std::make_shared<MockPlatformMessageHandler>();
+  EXPECT_CALL(client, GetPlatformMessageHandler)
+      .WillOnce(Return(platform_message_handler));
+  // Render should only be called once, because the second call is ignored.
+  EXPECT_CALL(client, Render).Times(1);
+
+  auto message_latch = std::make_shared<fml::AutoResetWaitableEvent>();
+  auto finish = [message_latch](Dart_NativeArguments args) {
+    message_latch->Signal();
+  };
+  AddNativeCallback("Finish", CREATE_NATIVE_ENTRY(finish));
+
+  auto runtime_controller_context =
+      RuntimeControllerContext::Create(settings, task_runners, client);
+
+  auto configuration = RunConfiguration::InferFromSettings(settings);
+  configuration.SetEntrypoint("incorrectDoubleRender");
+  runtime_controller_context->LaunchRootIsolate(
+      configuration, [](RuntimeController& runtime_controller) {
+        runtime_controller.AddView(
+            kImplicitViewId,
+            ViewportMetrics(
+                /*pixel_ratio=*/1.0, /*width=*/20, /*height=*/20,
+                /*touch_slop=*/2, /*display_id=*/0));
+      });
+
+  // Wait for the Dart main function to end.
+  message_latch->Wait();
+
+  runtime_controller_context->ControllerTaskSync(
+      [](RuntimeController& runtime_controller) {
+        // This BeginFrame calls PlatformDispatcher's handleBeginFrame and
+        // handleDrawFrame synchronously. Therefore don't wait after it.
+        runtime_controller.BeginFrame(fml::TimePoint::Now(), 0);
+      });
+}
+
 }  // namespace testing
 }  // namespace flutter