diff --git a/lib/ui/fixtures/ui_test.dart b/lib/ui/fixtures/ui_test.dart index 0aff1775688b7..3cc93a746921a 100644 --- a/lib/ui/fixtures/ui_test.dart +++ b/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. +} diff --git a/lib/ui/platform_dispatcher.dart b/lib/ui/platform_dispatcher.dart index 0bd387270ba13..5f42aa4af9f26 100644 --- a/lib/ui/platform_dispatcher.dart +++ b/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? _renderedViews; + // Temporary storage of the `_renderedViews` value between `_beginFrame` and + // `_drawFrame`. + Set? _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 = {}; _invoke1( 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. diff --git a/lib/ui/window.dart b/lib/ui/window.dart index 26a258cfa96c1..286e1485b3a9f 100644 --- a/lib/ui/window.dart +++ b/lib/ui/window.dart @@ -327,14 +327,21 @@ class FlutterView { /// Updates the view's rendering on the GPU with the newly provided [Scene]. /// - /// This function must be called within the scope of the + /// ## Requirement for calling this method + /// + /// This method must be called within the synchronous scope of the /// [PlatformDispatcher.onBeginFrame] or [PlatformDispatcher.onDrawFrame] - /// callbacks being invoked. + /// callbacks. Calls out of this scope will be ignored. To use this method, + /// create a callback that calls this method instead, and assign it to either + /// of the fields above; then schedule a frame, which is done typically with + /// [PlatformDispatcher.scheduleFrame]. Also, make sure the callback does not + /// have `await` before the `FlutterWindow.render` call. + /// + /// Additionally, this method can only be called once for each view during a + /// single [PlatformDispatcher.onBeginFrame]/[PlatformDispatcher.onDrawFrame] + /// callback sequence. Duplicate calls will be ignored in production. /// - /// If this function is called a second time during a single - /// [PlatformDispatcher.onBeginFrame]/[PlatformDispatcher.onDrawFrame] - /// callback sequence or called outside the scope of those callbacks, the call - /// will be ignored. + /// ## How to record a scene /// /// To record graphical operations, first create a [PictureRecorder], then /// construct a [Canvas], passing that [PictureRecorder] to its constructor. @@ -353,7 +360,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)>(symbol: 'PlatformConfigurationNativeApi::Render') external static void _render(_NativeScene scene); diff --git a/lib/ui/window/platform_configuration_unittests.cc b/lib/ui/window/platform_configuration_unittests.cc index 7410caeb66d6c..1fa8377d69a74 100644 --- a/lib/ui/window/platform_configuration_unittests.cc +++ b/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& 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, float), + (override)); + MOCK_METHOD(void, + UpdateSemantics, + (SemanticsNodeUpdates, CustomAccessibilityActionUpdates), + (override)); + MOCK_METHOD(void, + HandlePlatformMessage, + (std::unique_ptr), + (override)); + MOCK_METHOD(FontCollection&, GetFontCollection, (), (override)); + MOCK_METHOD(std::shared_ptr, 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>, + ComputePlatformResolvedLocale, + (const std::vector&), + (override)); + MOCK_METHOD(void, RequestDartDeferredLibrary, (intptr_t), (override)); + MOCK_METHOD(std::weak_ptr, + 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 message), + (override)); + MOCK_METHOD(bool, + DoesHandlePlatformMessageOnPlatformThread, + (), + (const, override)); + MOCK_METHOD(void, + InvokePlatformMessageResponseCallback, + (int response_id, std::unique_ptr 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; + + [[nodiscard]] static std::unique_ptr 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(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 isolate_snapshot) + : settings_(settings), + task_runners_(task_runners), + isolate_snapshot_(std::move(isolate_snapshot)), + vm_(std::move(vm)), + runtime_controller_(std::make_unique( + 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 isolate_snapshot_; + DartVMRef vm_; + std::unique_ptr 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(); + EXPECT_CALL(client, GetPlatformMessageHandler) + .WillOnce(Return(platform_message_handler)); + // Render should not be called. + EXPECT_CALL(client, Render).Times(0); + + auto finish_latch = std::make_shared(); + auto finish = [finish_latch](Dart_NativeArguments args) { + finish_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. + finish_latch->Wait(); +} + +TEST_F(PlatformConfigurationTest, DuplicateRenderCallsAreIgnored) { + Settings settings = CreateSettingsForFixture(); + TaskRunners task_runners = GetTaskRunnersForFixture(); + + MockRuntimeDelegate client; + auto platform_message_handler = + std::make_shared(); + 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 finish_latch = std::make_shared(); + auto finish = [finish_latch](Dart_NativeArguments args) { + finish_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. + finish_latch->Wait(); + + // This call synchronously calls PlatformDispatcher's handleBeginFrame and + // handleDrawFrame. Therefore it doesn't have to wait for latches. + runtime_controller_context->ControllerTaskSync( + [](RuntimeController& runtime_controller) { + runtime_controller.BeginFrame(fml::TimePoint::Now(), 0); + }); +} + } // namespace testing } // namespace flutter diff --git a/shell/common/shell.cc b/shell/common/shell.cc index 8c456dfd159cc..82db9dea4ff64 100644 --- a/shell/common/shell.cc +++ b/shell/common/shell.cc @@ -144,31 +144,36 @@ void PerformInitializationTasks(Settings& settings) { } // namespace -std::unique_ptr Shell::Create( - const PlatformData& platform_data, - const TaskRunners& task_runners, - Settings settings, - const Shell::CreateCallback& on_create_platform_view, - const Shell::CreateCallback& on_create_rasterizer, - bool is_gpu_disabled) { - // This must come first as it initializes tracing. - PerformInitializationTasks(settings); - - TRACE_EVENT0("flutter", "Shell::Create"); - +std::pair> +Shell::InferVmInitDataFromSettings(Settings& settings) { // Always use the `vm_snapshot` and `isolate_snapshot` provided by the // settings to launch the VM. If the VM is already running, the snapshot // arguments are ignored. auto vm_snapshot = DartSnapshot::VMSnapshotFromSettings(settings); auto isolate_snapshot = DartSnapshot::IsolateSnapshotFromSettings(settings); auto vm = DartVMRef::Create(settings, vm_snapshot, isolate_snapshot); - FML_CHECK(vm) << "Must be able to initialize the VM."; // If the settings did not specify an `isolate_snapshot`, fall back to the // one the VM was launched with. if (!isolate_snapshot) { isolate_snapshot = vm->GetVMData()->GetIsolateSnapshot(); } + return {std::move(vm), isolate_snapshot}; +} + +std::unique_ptr Shell::Create( + const PlatformData& platform_data, + const TaskRunners& task_runners, + Settings settings, + const Shell::CreateCallback& on_create_platform_view, + const Shell::CreateCallback& on_create_rasterizer, + bool is_gpu_disabled) { + // This must come first as it initializes tracing. + PerformInitializationTasks(settings); + + TRACE_EVENT0("flutter", "Shell::Create"); + + auto [vm, isolate_snapshot] = InferVmInitDataFromSettings(settings); auto resource_cache_limit_calculator = std::make_shared( settings.resource_cache_max_bytes_threshold); diff --git a/shell/common/shell.h b/shell/common/shell.h index b8c9c31052df6..d3bb7f0bd620c 100644 --- a/shell/common/shell.h +++ b/shell/common/shell.h @@ -438,6 +438,16 @@ class Shell final : public PlatformView::Delegate, const std::shared_ptr GetConcurrentWorkerTaskRunner() const; + // Infer the VM ref and the isolate snapshot based on the settings. + // + // If the VM is already running, the settings are ignored, but the returned + // isolate snapshot always prioritize what is specified by the settings, and + // falls back to the one VM was launched with. + // + // This function is what Shell::Create uses to infer snapshot settings. + static std::pair> + InferVmInitDataFromSettings(Settings& settings); + private: using ServiceProtocolHandler = std::function