From f2c3cd7a2624db25c184fb7f5caa7fdf5cd28965 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Fri, 25 Aug 2023 10:30:59 -0700 Subject: [PATCH 01/19] Changes from Michael --- lib/ui/platform_dispatcher.dart | 33 +++++++++++++++++++++++++++++++++ lib/ui/window.dart | 9 ++++++++- 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/lib/ui/platform_dispatcher.dart b/lib/ui/platform_dispatcher.dart index 6d86cc6270315..7432c96a74c51 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 d86781ef2567f..4c5df1874fed4 100644 --- a/lib/ui/window.dart +++ b/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)>(symbol: 'PlatformConfigurationNativeApi::Render') external static void _render(_NativeScene scene); From f783ef3f9fdac8a65bdd484e88f511d9b5d19d77 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Mon, 28 Aug 2023 14:56:47 -0700 Subject: [PATCH 02/19] Remove RunOnPlatformTaskRunner --- shell/common/shell_unittests.cc | 24 +++++++----------------- 1 file changed, 7 insertions(+), 17 deletions(-) diff --git a/shell/common/shell_unittests.cc b/shell/common/shell_unittests.cc index 43a14bb13b75f..24dd0dfe5f881 100644 --- a/shell/common/shell_unittests.cc +++ b/shell/common/shell_unittests.cc @@ -4391,17 +4391,6 @@ static void ParseViewIdsCallback(const Dart_NativeArguments& args, ASSERT_EQ(exception, nullptr); } -// Run the given task in the platform runner, and blocks until its finished. -static void RunOnPlatformTaskRunner(Shell& shell, const fml::closure& task) { - fml::AutoResetWaitableEvent latch; - fml::TaskRunner::RunNowOrPostTask( - shell.GetTaskRunners().GetPlatformTaskRunner(), [&task, &latch]() { - task(); - latch.Signal(); - }); - latch.Wait(); -} - TEST_F(ShellTest, ShellStartsWithImplicitView) { ASSERT_FALSE(DartVMRef::IsInstanceRunning()); Settings settings = CreateSettingsForFixture(); @@ -4472,21 +4461,22 @@ TEST_F(ShellTest, ShellCanAddViewOrRemoveView) { ASSERT_EQ(viewIds.size(), 1u); ASSERT_EQ(viewIds[0], 0ll); - RunOnPlatformTaskRunner(*shell, - [&shell] { shell->AddView(2, ViewportMetrics{}); }); + PostSync(shell->GetTaskRunners().GetPlatformTaskRunner(), + [&shell] { shell->AddView(2, ViewportMetrics{}); }); reportLatch.Wait(); ASSERT_TRUE(hasImplicitView); ASSERT_EQ(viewIds.size(), 2u); ASSERT_EQ(viewIds[1], 2ll); - RunOnPlatformTaskRunner(*shell, [&shell] { shell->RemoveView(2); }); + PostSync(shell->GetTaskRunners().GetPlatformTaskRunner(), + [&shell] { shell->RemoveView(2); }); reportLatch.Wait(); ASSERT_TRUE(hasImplicitView); ASSERT_EQ(viewIds.size(), 1u); ASSERT_EQ(viewIds[0], 0ll); - RunOnPlatformTaskRunner(*shell, - [&shell] { shell->AddView(4, ViewportMetrics{}); }); + PostSync(shell->GetTaskRunners().GetPlatformTaskRunner(), + [&shell] { shell->AddView(4, ViewportMetrics{}); }); reportLatch.Wait(); ASSERT_TRUE(hasImplicitView); ASSERT_EQ(viewIds.size(), 2u); @@ -4531,7 +4521,7 @@ TEST_F(ShellTest, ShellFlushesPlatformStatesByMain) { std::unique_ptr shell = CreateShell(settings, task_runners); ASSERT_TRUE(shell); - RunOnPlatformTaskRunner(*shell, [&shell] { + PostSync(shell->GetTaskRunners().GetPlatformTaskRunner(), [&shell] { auto platform_view = shell->GetPlatformView(); // The construtor for ViewportMetrics{_, width, _, _, _} (only the 2nd // argument matters in this test). From 3c3ddbe806a350aab9c573870a4d252845556af5 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Mon, 28 Aug 2023 14:59:31 -0700 Subject: [PATCH 03/19] Impl --- shell/common/shell_unittests.cc | 24 +++++++----------------- 1 file changed, 7 insertions(+), 17 deletions(-) diff --git a/shell/common/shell_unittests.cc b/shell/common/shell_unittests.cc index 43a14bb13b75f..24dd0dfe5f881 100644 --- a/shell/common/shell_unittests.cc +++ b/shell/common/shell_unittests.cc @@ -4391,17 +4391,6 @@ static void ParseViewIdsCallback(const Dart_NativeArguments& args, ASSERT_EQ(exception, nullptr); } -// Run the given task in the platform runner, and blocks until its finished. -static void RunOnPlatformTaskRunner(Shell& shell, const fml::closure& task) { - fml::AutoResetWaitableEvent latch; - fml::TaskRunner::RunNowOrPostTask( - shell.GetTaskRunners().GetPlatformTaskRunner(), [&task, &latch]() { - task(); - latch.Signal(); - }); - latch.Wait(); -} - TEST_F(ShellTest, ShellStartsWithImplicitView) { ASSERT_FALSE(DartVMRef::IsInstanceRunning()); Settings settings = CreateSettingsForFixture(); @@ -4472,21 +4461,22 @@ TEST_F(ShellTest, ShellCanAddViewOrRemoveView) { ASSERT_EQ(viewIds.size(), 1u); ASSERT_EQ(viewIds[0], 0ll); - RunOnPlatformTaskRunner(*shell, - [&shell] { shell->AddView(2, ViewportMetrics{}); }); + PostSync(shell->GetTaskRunners().GetPlatformTaskRunner(), + [&shell] { shell->AddView(2, ViewportMetrics{}); }); reportLatch.Wait(); ASSERT_TRUE(hasImplicitView); ASSERT_EQ(viewIds.size(), 2u); ASSERT_EQ(viewIds[1], 2ll); - RunOnPlatformTaskRunner(*shell, [&shell] { shell->RemoveView(2); }); + PostSync(shell->GetTaskRunners().GetPlatformTaskRunner(), + [&shell] { shell->RemoveView(2); }); reportLatch.Wait(); ASSERT_TRUE(hasImplicitView); ASSERT_EQ(viewIds.size(), 1u); ASSERT_EQ(viewIds[0], 0ll); - RunOnPlatformTaskRunner(*shell, - [&shell] { shell->AddView(4, ViewportMetrics{}); }); + PostSync(shell->GetTaskRunners().GetPlatformTaskRunner(), + [&shell] { shell->AddView(4, ViewportMetrics{}); }); reportLatch.Wait(); ASSERT_TRUE(hasImplicitView); ASSERT_EQ(viewIds.size(), 2u); @@ -4531,7 +4521,7 @@ TEST_F(ShellTest, ShellFlushesPlatformStatesByMain) { std::unique_ptr shell = CreateShell(settings, task_runners); ASSERT_TRUE(shell); - RunOnPlatformTaskRunner(*shell, [&shell] { + PostSync(shell->GetTaskRunners().GetPlatformTaskRunner(), [&shell] { auto platform_view = shell->GetPlatformView(); // The construtor for ViewportMetrics{_, width, _, _, _} (only the 2nd // argument matters in this test). From 9ac34e2e71b03fb37b9847af9c7397f9aad15458 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Tue, 29 Aug 2023 19:07:09 -0700 Subject: [PATCH 04/19] First running runtime controller --- lib/ui/fixtures/ui_test.dart | 10 ++ .../platform_configuration_unittests.cc | 116 +++++++++++++++++- 2 files changed, 125 insertions(+), 1 deletion(-) diff --git a/lib/ui/fixtures/ui_test.dart b/lib/ui/fixtures/ui_test.dart index 0aff1775688b7..c37cf2da6a631 100644 --- a/lib/ui/fixtures/ui_test.dart +++ b/lib/ui/fixtures/ui_test.dart @@ -1076,3 +1076,13 @@ external void _callHook( Object? arg20, Object? arg21, ]); + +@pragma('vm:entry-point') +@pragma('vm:external-name', 'NotifyNative') +external void notifyNative(); + +@pragma('vm:entry-point') +void renderRule() { + print('Done!!'); + notifyNative(); +} diff --git a/lib/ui/window/platform_configuration_unittests.cc b/lib/ui/window/platform_configuration_unittests.cc index 7410caeb66d6c..a20e7a761b452 100644 --- a/lib/ui/window/platform_configuration_unittests.cc +++ b/lib/ui/window/platform_configuration_unittests.cc @@ -11,15 +11,72 @@ #include "flutter/common/task_runners.h" #include "flutter/fml/synchronization/waitable_event.h" #include "flutter/lib/ui/painting/vertices.h" +#include "gmock/gmock.h" #include "flutter/runtime/dart_vm.h" #include "flutter/shell/common/shell_test.h" #include "flutter/shell/common/thread_host.h" #include "flutter/testing/testing.h" +///\note Deprecated MOCK_METHOD macros used until this issue is resolved: +// https://github.com/google/googletest/issues/2490 + namespace flutter { + +namespace { + +class MockRuntimeDelegate : public RuntimeDelegate { + public: + MOCK_METHOD0(ImplicitViewEnabled, bool()); + MOCK_METHOD0(DefaultRouteName, std::string()); + MOCK_METHOD1(ScheduleFrame, void(bool)); + MOCK_METHOD2(Render, void(std::unique_ptr, float)); + MOCK_METHOD2(UpdateSemantics, + void(SemanticsNodeUpdates, CustomAccessibilityActionUpdates)); + MOCK_METHOD1(HandlePlatformMessage, void(std::unique_ptr)); + MOCK_METHOD0(GetFontCollection, FontCollection&()); + MOCK_METHOD0(GetAssetManager, std::shared_ptr()); + MOCK_METHOD0(OnRootIsolateCreated, void()); + MOCK_METHOD2(UpdateIsolateDescription, void(const std::string, int64_t)); + MOCK_METHOD1(SetNeedsReportTimings, void(bool)); + MOCK_METHOD1(ComputePlatformResolvedLocale, + std::unique_ptr>( + const std::vector&)); + MOCK_METHOD1(RequestDartDeferredLibrary, void(intptr_t)); + MOCK_CONST_METHOD0(GetPlatformMessageHandler, + std::weak_ptr()); + MOCK_CONST_METHOD2(GetScaledFontSize, + double(double font_size, int configuration_id)); +}; + +class MockPlatformMessageHandler : public PlatformMessageHandler { + public: + MOCK_METHOD1(HandlePlatformMessage, + void(std::unique_ptr message)); + MOCK_CONST_METHOD0(DoesHandlePlatformMessageOnPlatformThread, bool()); + MOCK_METHOD2(InvokePlatformMessageResponseCallback, + void(int response_id, std::unique_ptr mapping)); + MOCK_METHOD1(InvokePlatformMessageEmptyResponseCallback, + void(int response_id)); +}; +} // namespace + namespace testing { -class PlatformConfigurationTest : public ShellTest {}; +using ::testing::_; +using ::testing::Return; + +class PlatformConfigurationTest : public ShellTest { + public: + 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(); + } +}; TEST_F(PlatformConfigurationTest, Initialization) { auto message_latch = std::make_shared(); @@ -332,5 +389,62 @@ TEST_F(PlatformConfigurationTest, SetDartPerformanceMode) { DestroyShell(std::move(shell), task_runners); } +TEST_F(PlatformConfigurationTest, RenderRules) { + Settings settings = CreateSettingsForFixture(); + TaskRunners task_runners = GetTaskRunnersForFixture(); + + // 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 (!isolate_snapshot) { + isolate_snapshot = vm->GetVMData()->GetIsolateSnapshot(); + } + + MockRuntimeDelegate client; + auto platform_message_handler = + std::make_shared(); + EXPECT_CALL(client, GetPlatformMessageHandler()) + .WillOnce(Return(platform_message_handler)); + + auto 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}); + + fml::AutoResetWaitableEvent latch; + AddNativeCallback("NotifyNative", CREATE_NATIVE_ENTRY([&latch](auto args) { + latch.Signal(); + })); + + auto configuration = RunConfiguration::InferFromSettings(settings); + configuration.SetEntrypoint("renderRule"); + PostSync(task_runners.GetUITaskRunner(), [&]() { + bool launch_success = runtime_controller->LaunchRootIsolate( + settings, // + []() {}, // + configuration.GetEntrypoint(), // + configuration.GetEntrypointLibrary(), // + configuration.GetEntrypointArgs(), // + configuration.TakeIsolateConfiguration()); // + ASSERT_TRUE(launch_success); + }); + + latch.Wait(); + + PostSync(task_runners.GetUITaskRunner(), [&]() { + runtime_controller.reset(); + }); +} + } // namespace testing } // namespace flutter From 5e15f5d29ae114a3004ae5c1a1da19eb976fb8e8 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Tue, 29 Aug 2023 22:23:08 -0700 Subject: [PATCH 05/19] CallingRenderOutOfScopeIsIgnored --- lib/ui/fixtures/ui_test.dart | 19 +++- .../platform_configuration_unittests.cc | 94 ++++++++++++------- 2 files changed, 75 insertions(+), 38 deletions(-) diff --git a/lib/ui/fixtures/ui_test.dart b/lib/ui/fixtures/ui_test.dart index c37cf2da6a631..e6192d3d8dc7c 100644 --- a/lib/ui/fixtures/ui_test.dart +++ b/lib/ui/fixtures/ui_test.dart @@ -1077,12 +1077,27 @@ external void _callHook( Object? arg21, ]); +Picture CreateRedBox(Size size) { + Paint paint = Paint() + ..color = Color.fromARGB(255, 255, 0, 0) + ..style = PaintingStyle.fill; + PictureRecorder baseRecorder = PictureRecorder(); + Canvas canvas = Canvas(baseRecorder); + canvas.drawRect(Rect.fromLTRB(0.0, 0.0, size.width, size.height), paint); + return baseRecorder.endRecording(); +} + @pragma('vm:entry-point') @pragma('vm:external-name', 'NotifyNative') external void notifyNative(); @pragma('vm:entry-point') -void renderRule() { - print('Done!!'); +void incorrectImmediateRender() { + SceneBuilder builder = SceneBuilder(); + builder.pushOffset(0.0, 0.0); + builder.addPicture(Offset(0.0, 0.0), CreateRedBox(Size(2.0, 2.0))); + builder.pop(); + PlatformDispatcher.instance.views.first.render(builder.build()); + PlatformDispatcher.instance.scheduleFrame(); notifyNative(); } diff --git a/lib/ui/window/platform_configuration_unittests.cc b/lib/ui/window/platform_configuration_unittests.cc index a20e7a761b452..963ec1bba8965 100644 --- a/lib/ui/window/platform_configuration_unittests.cc +++ b/lib/ui/window/platform_configuration_unittests.cc @@ -11,11 +11,11 @@ #include "flutter/common/task_runners.h" #include "flutter/fml/synchronization/waitable_event.h" #include "flutter/lib/ui/painting/vertices.h" -#include "gmock/gmock.h" #include "flutter/runtime/dart_vm.h" #include "flutter/shell/common/shell_test.h" #include "flutter/shell/common/thread_host.h" #include "flutter/testing/testing.h" +#include "gmock/gmock.h" ///\note Deprecated MOCK_METHOD macros used until this issue is resolved: // https://github.com/google/googletest/issues/2490 @@ -24,6 +24,8 @@ namespace flutter { namespace { +static constexpr int64_t kImplicitViewId = 0; + class MockRuntimeDelegate : public RuntimeDelegate { public: MOCK_METHOD0(ImplicitViewEnabled, bool()); @@ -58,17 +60,52 @@ class MockPlatformMessageHandler : public PlatformMessageHandler { MOCK_METHOD1(InvokePlatformMessageEmptyResponseCallback, void(int response_id)); }; -} // namespace + +class MockRuntimeControllerContext { + public: + MockRuntimeControllerContext(Settings settings, + TaskRunners task_runners, + RuntimeDelegate& client) + : vm_snapshot_(DartSnapshot::VMSnapshotFromSettings(settings)), + isolate_snapshot_(DartSnapshot::IsolateSnapshotFromSettings(settings)), + vm_(DartVMRef::Create(settings, vm_snapshot_, isolate_snapshot_)) { + // 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. + FML_CHECK(vm_) << "Must be able to initialize the VM."; + if (!isolate_snapshot_) { + isolate_snapshot_ = vm_->GetVMData()->GetIsolateSnapshot(); + } + 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}); + } + + RuntimeController& GetController() { return *runtime_controller_; } + + private: + fml::RefPtr vm_snapshot_; + fml::RefPtr isolate_snapshot_; + DartVMRef vm_; + std::unique_ptr runtime_controller_; +}; +} // namespace namespace testing { using ::testing::_; +using ::testing::Exactly; using ::testing::Return; class PlatformConfigurationTest : public ShellTest { public: void PostSync(const fml::RefPtr& task_runner, - const fml::closure& task) { + const fml::closure& task) { fml::AutoResetWaitableEvent latch; fml::TaskRunner::RunNowOrPostTask(task_runner, [&latch, &task] { task(); @@ -389,37 +426,19 @@ TEST_F(PlatformConfigurationTest, SetDartPerformanceMode) { DestroyShell(std::move(shell), task_runners); } -TEST_F(PlatformConfigurationTest, RenderRules) { +TEST_F(PlatformConfigurationTest, CallingRenderOutOfScopeIsIgnored) { Settings settings = CreateSettingsForFixture(); TaskRunners task_runners = GetTaskRunnersForFixture(); - // 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 (!isolate_snapshot) { - isolate_snapshot = vm->GetVMData()->GetIsolateSnapshot(); - } - MockRuntimeDelegate client; auto platform_message_handler = std::make_shared(); EXPECT_CALL(client, GetPlatformMessageHandler()) .WillOnce(Return(platform_message_handler)); + EXPECT_CALL(client, Render(_, _)).Times(Exactly(0)); - auto 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}); + auto context = std::make_unique( + settings, task_runners, client); fml::AutoResetWaitableEvent latch; AddNativeCallback("NotifyNative", CREATE_NATIVE_ENTRY([&latch](auto args) { @@ -427,23 +446,26 @@ TEST_F(PlatformConfigurationTest, RenderRules) { })); auto configuration = RunConfiguration::InferFromSettings(settings); - configuration.SetEntrypoint("renderRule"); + configuration.SetEntrypoint("incorrectImmediateRender"); + PostSync(task_runners.GetUITaskRunner(), [&]() { - bool launch_success = runtime_controller->LaunchRootIsolate( - settings, // - []() {}, // - configuration.GetEntrypoint(), // - configuration.GetEntrypointLibrary(), // - configuration.GetEntrypointArgs(), // - configuration.TakeIsolateConfiguration()); // + bool launch_success = context->GetController().LaunchRootIsolate( + settings, // + []() {}, // + configuration.GetEntrypoint(), // + configuration.GetEntrypointLibrary(), // + configuration.GetEntrypointArgs(), // + configuration.TakeIsolateConfiguration()); // ASSERT_TRUE(launch_success); + context->GetController().AddView( + kImplicitViewId, + ViewportMetrics(/*device_pixel_ratio=*/1.0, /*width=*/20, /*height=*/20, + /*physical_touch_slop=*/2, /*display_id=*/0)); }); latch.Wait(); - PostSync(task_runners.GetUITaskRunner(), [&]() { - runtime_controller.reset(); - }); + PostSync(task_runners.GetUITaskRunner(), [&]() { context.reset(); }); } } // namespace testing From 8c7b5d63b72d9cc65111e072347a7d5f0053cc31 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Tue, 29 Aug 2023 22:32:16 -0700 Subject: [PATCH 06/19] Extract methods --- .../platform_configuration_unittests.cc | 63 ++++++++++--------- 1 file changed, 35 insertions(+), 28 deletions(-) diff --git a/lib/ui/window/platform_configuration_unittests.cc b/lib/ui/window/platform_configuration_unittests.cc index 963ec1bba8965..662de17c9eca3 100644 --- a/lib/ui/window/platform_configuration_unittests.cc +++ b/lib/ui/window/platform_configuration_unittests.cc @@ -26,6 +26,16 @@ 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_METHOD0(ImplicitViewEnabled, bool()); @@ -66,7 +76,9 @@ class MockRuntimeControllerContext { MockRuntimeControllerContext(Settings settings, TaskRunners task_runners, RuntimeDelegate& client) - : vm_snapshot_(DartSnapshot::VMSnapshotFromSettings(settings)), + : settings_(settings), + task_runners_(task_runners), + vm_snapshot_(DartSnapshot::VMSnapshotFromSettings(settings)), isolate_snapshot_(DartSnapshot::IsolateSnapshotFromSettings(settings)), vm_(DartVMRef::Create(settings, vm_snapshot_, isolate_snapshot_)) { // Always use the `vm_snapshot` and `isolate_snapshot` provided by the @@ -86,9 +98,26 @@ class MockRuntimeControllerContext { UIDartState::Context{task_runners}); } + void LaunchRootIsolate(RunConfiguration& configuration, + ViewportMetrics implicit_view_metrics) { + PostSync(task_runners_.GetUITaskRunner(), [&]() { + bool launch_success = runtime_controller_->LaunchRootIsolate( + settings_, // + []() {}, // + configuration.GetEntrypoint(), // + configuration.GetEntrypointLibrary(), // + configuration.GetEntrypointArgs(), // + configuration.TakeIsolateConfiguration()); // + ASSERT_TRUE(launch_success); + runtime_controller_->AddView(kImplicitViewId, implicit_view_metrics); + }); + } + RuntimeController& GetController() { return *runtime_controller_; } private: + Settings settings_; + TaskRunners task_runners_; fml::RefPtr vm_snapshot_; fml::RefPtr isolate_snapshot_; DartVMRef vm_; @@ -102,18 +131,7 @@ using ::testing::_; using ::testing::Exactly; using ::testing::Return; -class PlatformConfigurationTest : public ShellTest { - public: - 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 PlatformConfigurationTest : public ShellTest {}; TEST_F(PlatformConfigurationTest, Initialization) { auto message_latch = std::make_shared(); @@ -447,21 +465,10 @@ TEST_F(PlatformConfigurationTest, CallingRenderOutOfScopeIsIgnored) { auto configuration = RunConfiguration::InferFromSettings(settings); configuration.SetEntrypoint("incorrectImmediateRender"); - - PostSync(task_runners.GetUITaskRunner(), [&]() { - bool launch_success = context->GetController().LaunchRootIsolate( - settings, // - []() {}, // - configuration.GetEntrypoint(), // - configuration.GetEntrypointLibrary(), // - configuration.GetEntrypointArgs(), // - configuration.TakeIsolateConfiguration()); // - ASSERT_TRUE(launch_success); - context->GetController().AddView( - kImplicitViewId, - ViewportMetrics(/*device_pixel_ratio=*/1.0, /*width=*/20, /*height=*/20, - /*physical_touch_slop=*/2, /*display_id=*/0)); - }); + ViewportMetrics implicit_view_metrics( + /*device_pixel_ratio=*/1.0, /*width=*/20, /*height=*/20, + /*physical_touch_slop=*/2, /*display_id=*/0); + context->LaunchRootIsolate(configuration, implicit_view_metrics); latch.Wait(); From 0d563e8c96a5e8e57bda0440a511bbc2627f7935 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Tue, 29 Aug 2023 23:08:55 -0700 Subject: [PATCH 07/19] DuplicateRenderCallsAreIgnored --- lib/ui/fixtures/ui_test.dart | 35 ++++++++++------ .../platform_configuration_unittests.cc | 42 ++++++++++++++++--- 2 files changed, 60 insertions(+), 17 deletions(-) diff --git a/lib/ui/fixtures/ui_test.dart b/lib/ui/fixtures/ui_test.dart index e6192d3d8dc7c..ae361c733b8ac 100644 --- a/lib/ui/fixtures/ui_test.dart +++ b/lib/ui/fixtures/ui_test.dart @@ -1077,14 +1077,19 @@ external void _callHook( Object? arg21, ]); -Picture CreateRedBox(Size size) { - Paint paint = Paint() +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; - PictureRecorder baseRecorder = PictureRecorder(); - Canvas canvas = Canvas(baseRecorder); + final PictureRecorder baseRecorder = PictureRecorder(); + final Canvas canvas = Canvas(baseRecorder); canvas.drawRect(Rect.fromLTRB(0.0, 0.0, size.width, size.height), paint); - return baseRecorder.endRecording(); + final Picture picture = baseRecorder.endRecording(); + builder.addPicture(Offset(0.0, 0.0), picture); + builder.pop(); + return builder.build(); } @pragma('vm:entry-point') @@ -1093,11 +1098,17 @@ external void notifyNative(); @pragma('vm:entry-point') void incorrectImmediateRender() { - SceneBuilder builder = SceneBuilder(); - builder.pushOffset(0.0, 0.0); - builder.addPicture(Offset(0.0, 0.0), CreateRedBox(Size(2.0, 2.0))); - builder.pop(); - PlatformDispatcher.instance.views.first.render(builder.build()); - PlatformDispatcher.instance.scheduleFrame(); - notifyNative(); + PlatformDispatcher.instance.views.first.render(CreateRedBoxScene(Size(2, 2))); +} + +@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))); + }; } diff --git a/lib/ui/window/platform_configuration_unittests.cc b/lib/ui/window/platform_configuration_unittests.cc index 662de17c9eca3..98917bf278098 100644 --- a/lib/ui/window/platform_configuration_unittests.cc +++ b/lib/ui/window/platform_configuration_unittests.cc @@ -444,7 +444,7 @@ TEST_F(PlatformConfigurationTest, SetDartPerformanceMode) { DestroyShell(std::move(shell), task_runners); } -TEST_F(PlatformConfigurationTest, CallingRenderOutOfScopeIsIgnored) { +TEST_F(PlatformConfigurationTest, OutOfScopeRenderCallsAreIgnored) { Settings settings = CreateSettingsForFixture(); TaskRunners task_runners = GetTaskRunnersForFixture(); @@ -453,25 +453,57 @@ TEST_F(PlatformConfigurationTest, CallingRenderOutOfScopeIsIgnored) { std::make_shared(); EXPECT_CALL(client, GetPlatformMessageHandler()) .WillOnce(Return(platform_message_handler)); + // Render should not be called. EXPECT_CALL(client, Render(_, _)).Times(Exactly(0)); auto context = std::make_unique( settings, task_runners, client); + auto configuration = RunConfiguration::InferFromSettings(settings); + configuration.SetEntrypoint("incorrectImmediateRender"); + ViewportMetrics implicit_view_metrics( + /*device_pixel_ratio=*/1.0, /*width=*/20, /*height=*/20, + /*physical_touch_slop=*/2, /*display_id=*/0); + context->LaunchRootIsolate(configuration, implicit_view_metrics); + + // Wait for the Dart main function to end. fml::AutoResetWaitableEvent latch; - AddNativeCallback("NotifyNative", CREATE_NATIVE_ENTRY([&latch](auto args) { - latch.Signal(); - })); + PostSync(task_runners.GetUITaskRunner(), [&]() { latch.Signal(); }); + latch.Wait(); + + PostSync(task_runners.GetUITaskRunner(), [&]() { context.reset(); }); +} + +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(Exactly(1)); + + auto context = std::make_unique( + settings, task_runners, client); auto configuration = RunConfiguration::InferFromSettings(settings); - configuration.SetEntrypoint("incorrectImmediateRender"); + configuration.SetEntrypoint("incorrectDoubleRender"); ViewportMetrics implicit_view_metrics( /*device_pixel_ratio=*/1.0, /*width=*/20, /*height=*/20, /*physical_touch_slop=*/2, /*display_id=*/0); context->LaunchRootIsolate(configuration, implicit_view_metrics); + // Wait for the Dart main function to end, which means the callbacks have been + // registered. + fml::AutoResetWaitableEvent latch; + PostSync(task_runners.GetUITaskRunner(), [&]() { latch.Signal(); }); latch.Wait(); + context->GetController().BeginFrame(fml::TimePoint::Now(), 0); + PostSync(task_runners.GetUITaskRunner(), [&]() { context.reset(); }); } From 49a3560a63362a5940867a8c9fc12a8bb15c3e78 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Wed, 30 Aug 2023 13:30:15 -0700 Subject: [PATCH 08/19] Docs and callback --- .../platform_configuration_unittests.cc | 55 +++++++++++++------ 1 file changed, 37 insertions(+), 18 deletions(-) diff --git a/lib/ui/window/platform_configuration_unittests.cc b/lib/ui/window/platform_configuration_unittests.cc index 98917bf278098..26f566926bea0 100644 --- a/lib/ui/window/platform_configuration_unittests.cc +++ b/lib/ui/window/platform_configuration_unittests.cc @@ -71,8 +71,16 @@ class MockPlatformMessageHandler : public PlatformMessageHandler { void(int response_id)); }; +// A class that can launch a RuntimeController with the specified +// RuntimeDelegate. +// +// To use this class, contruct this class, call LaunchRootIsolate, and get the +// runtime controller with Controller(). By the end of the test, destruct this +// class in the UI thread. class MockRuntimeControllerContext { public: + using VoidCallback = std::function; + MockRuntimeControllerContext(Settings settings, TaskRunners task_runners, RuntimeDelegate& client) @@ -98,8 +106,10 @@ class MockRuntimeControllerContext { UIDartState::Context{task_runners}); } + // 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, - ViewportMetrics implicit_view_metrics) { + VoidCallback post_launch) { PostSync(task_runners_.GetUITaskRunner(), [&]() { bool launch_success = runtime_controller_->LaunchRootIsolate( settings_, // @@ -109,11 +119,12 @@ class MockRuntimeControllerContext { configuration.GetEntrypointArgs(), // configuration.TakeIsolateConfiguration()); // ASSERT_TRUE(launch_success); - runtime_controller_->AddView(kImplicitViewId, implicit_view_metrics); + post_launch(); }); } - RuntimeController& GetController() { return *runtime_controller_; } + // Get the runtime controller. + RuntimeController& Controller() { return *runtime_controller_; } private: Settings settings_; @@ -456,22 +467,26 @@ TEST_F(PlatformConfigurationTest, OutOfScopeRenderCallsAreIgnored) { // Render should not be called. EXPECT_CALL(client, Render(_, _)).Times(Exactly(0)); - auto context = std::make_unique( - settings, task_runners, client); + auto runtime_controller_context = + std::make_unique(settings, task_runners, + client); auto configuration = RunConfiguration::InferFromSettings(settings); configuration.SetEntrypoint("incorrectImmediateRender"); - ViewportMetrics implicit_view_metrics( - /*device_pixel_ratio=*/1.0, /*width=*/20, /*height=*/20, - /*physical_touch_slop=*/2, /*display_id=*/0); - context->LaunchRootIsolate(configuration, implicit_view_metrics); + runtime_controller_context->LaunchRootIsolate(configuration, [&] { + runtime_controller_context->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. fml::AutoResetWaitableEvent latch; PostSync(task_runners.GetUITaskRunner(), [&]() { latch.Signal(); }); latch.Wait(); - PostSync(task_runners.GetUITaskRunner(), [&]() { context.reset(); }); + PostSync(task_runners.GetUITaskRunner(), + [&]() { runtime_controller_context.reset(); }); } TEST_F(PlatformConfigurationTest, DuplicateRenderCallsAreIgnored) { @@ -486,15 +501,18 @@ TEST_F(PlatformConfigurationTest, DuplicateRenderCallsAreIgnored) { // Render should only be called once, because the second call is ignored. EXPECT_CALL(client, Render(_, _)).Times(Exactly(1)); - auto context = std::make_unique( - settings, task_runners, client); + auto runtime_controller_context = + std::make_unique(settings, task_runners, + client); auto configuration = RunConfiguration::InferFromSettings(settings); configuration.SetEntrypoint("incorrectDoubleRender"); - ViewportMetrics implicit_view_metrics( - /*device_pixel_ratio=*/1.0, /*width=*/20, /*height=*/20, - /*physical_touch_slop=*/2, /*display_id=*/0); - context->LaunchRootIsolate(configuration, implicit_view_metrics); + runtime_controller_context->LaunchRootIsolate(configuration, [&] { + runtime_controller_context->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, which means the callbacks have been // registered. @@ -502,9 +520,10 @@ TEST_F(PlatformConfigurationTest, DuplicateRenderCallsAreIgnored) { PostSync(task_runners.GetUITaskRunner(), [&]() { latch.Signal(); }); latch.Wait(); - context->GetController().BeginFrame(fml::TimePoint::Now(), 0); + runtime_controller_context->Controller().BeginFrame(fml::TimePoint::Now(), 0); - PostSync(task_runners.GetUITaskRunner(), [&]() { context.reset(); }); + PostSync(task_runners.GetUITaskRunner(), + [&]() { runtime_controller_context.reset(); }); } } // namespace testing From 713648e86f5c79db019b09c5f979672436d2296c Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Wed, 30 Aug 2023 13:33:12 -0700 Subject: [PATCH 09/19] Move destruction to destructor --- lib/ui/window/platform_configuration_unittests.cc | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/lib/ui/window/platform_configuration_unittests.cc b/lib/ui/window/platform_configuration_unittests.cc index 26f566926bea0..4d0012a423456 100644 --- a/lib/ui/window/platform_configuration_unittests.cc +++ b/lib/ui/window/platform_configuration_unittests.cc @@ -75,8 +75,7 @@ class MockPlatformMessageHandler : public PlatformMessageHandler { // RuntimeDelegate. // // To use this class, contruct this class, call LaunchRootIsolate, and get the -// runtime controller with Controller(). By the end of the test, destruct this -// class in the UI thread. +// runtime controller with Controller(). class MockRuntimeControllerContext { public: using VoidCallback = std::function; @@ -106,6 +105,11 @@ class MockRuntimeControllerContext { UIDartState::Context{task_runners}); } + ~MockRuntimeControllerContext() { + 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, @@ -484,9 +488,6 @@ TEST_F(PlatformConfigurationTest, OutOfScopeRenderCallsAreIgnored) { fml::AutoResetWaitableEvent latch; PostSync(task_runners.GetUITaskRunner(), [&]() { latch.Signal(); }); latch.Wait(); - - PostSync(task_runners.GetUITaskRunner(), - [&]() { runtime_controller_context.reset(); }); } TEST_F(PlatformConfigurationTest, DuplicateRenderCallsAreIgnored) { @@ -521,9 +522,6 @@ TEST_F(PlatformConfigurationTest, DuplicateRenderCallsAreIgnored) { latch.Wait(); runtime_controller_context->Controller().BeginFrame(fml::TimePoint::Now(), 0); - - PostSync(task_runners.GetUITaskRunner(), - [&]() { runtime_controller_context.reset(); }); } } // namespace testing From 48135a7efb2248dabe3d3c9a9ed9bbe5788a3398 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Wed, 30 Aug 2023 13:40:39 -0700 Subject: [PATCH 10/19] Format --- lib/ui/window/platform_configuration_unittests.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ui/window/platform_configuration_unittests.cc b/lib/ui/window/platform_configuration_unittests.cc index 4d0012a423456..a9a94a5bc543a 100644 --- a/lib/ui/window/platform_configuration_unittests.cc +++ b/lib/ui/window/platform_configuration_unittests.cc @@ -107,7 +107,7 @@ class MockRuntimeControllerContext { ~MockRuntimeControllerContext() { PostSync(task_runners_.GetUITaskRunner(), - [&]() { runtime_controller_.reset(); }); + [&]() { runtime_controller_.reset(); }); } // Launch the root isolate. The post_launch callback will be executed in the From d0b1cf73f1bdcc31225e93287df30e1ef0f0ae37 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Wed, 30 Aug 2023 14:26:52 -0700 Subject: [PATCH 11/19] Extract Shell InferVmInitDataFromSettings --- .../platform_configuration_unittests.cc | 69 ++++++++++--------- shell/common/shell.cc | 31 +++++---- shell/common/shell.h | 10 +++ 3 files changed, 65 insertions(+), 45 deletions(-) diff --git a/lib/ui/window/platform_configuration_unittests.cc b/lib/ui/window/platform_configuration_unittests.cc index a9a94a5bc543a..f4d18bbc3431e 100644 --- a/lib/ui/window/platform_configuration_unittests.cc +++ b/lib/ui/window/platform_configuration_unittests.cc @@ -74,38 +74,26 @@ class MockPlatformMessageHandler : public PlatformMessageHandler { // A class that can launch a RuntimeController with the specified // RuntimeDelegate. // -// To use this class, contruct this class, call LaunchRootIsolate, and get the -// runtime controller with Controller(). -class MockRuntimeControllerContext { +// To use this class, contruct this class with Create, call LaunchRootIsolate, +// and get the runtime controller with Controller(). +class RuntimeControllerContext { public: using VoidCallback = std::function; - MockRuntimeControllerContext(Settings settings, - TaskRunners task_runners, - RuntimeDelegate& client) - : settings_(settings), - task_runners_(task_runners), - vm_snapshot_(DartSnapshot::VMSnapshotFromSettings(settings)), - isolate_snapshot_(DartSnapshot::IsolateSnapshotFromSettings(settings)), - vm_(DartVMRef::Create(settings, vm_snapshot_, isolate_snapshot_)) { - // 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. - FML_CHECK(vm_) << "Must be able to initialize the VM."; - if (!isolate_snapshot_) { - isolate_snapshot_ = vm_->GetVMData()->GetIsolateSnapshot(); - } - 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}); + [[nodiscard]] static std::unique_ptr Create( + Settings settings, // + 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); } - ~MockRuntimeControllerContext() { + ~RuntimeControllerContext() { PostSync(task_runners_.GetUITaskRunner(), [&]() { runtime_controller_.reset(); }); } @@ -131,9 +119,28 @@ class MockRuntimeControllerContext { RuntimeController& Controller() { return *runtime_controller_; } private: + RuntimeControllerContext(Settings settings, + TaskRunners task_runners, + RuntimeDelegate& client, + DartVMRef vm, + fml::RefPtr isolate_snapshot) + : settings_(settings), + task_runners_(task_runners), + isolate_snapshot_(isolate_snapshot), + vm_(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 vm_snapshot_; fml::RefPtr isolate_snapshot_; DartVMRef vm_; std::unique_ptr runtime_controller_; @@ -472,8 +479,7 @@ TEST_F(PlatformConfigurationTest, OutOfScopeRenderCallsAreIgnored) { EXPECT_CALL(client, Render(_, _)).Times(Exactly(0)); auto runtime_controller_context = - std::make_unique(settings, task_runners, - client); + RuntimeControllerContext::Create(settings, task_runners, client); auto configuration = RunConfiguration::InferFromSettings(settings); configuration.SetEntrypoint("incorrectImmediateRender"); @@ -503,8 +509,7 @@ TEST_F(PlatformConfigurationTest, DuplicateRenderCallsAreIgnored) { EXPECT_CALL(client, Render(_, _)).Times(Exactly(1)); auto runtime_controller_context = - std::make_unique(settings, task_runners, - client); + RuntimeControllerContext::Create(settings, task_runners, client); auto configuration = RunConfiguration::InferFromSettings(settings); configuration.SetEntrypoint("incorrectDoubleRender"); diff --git a/shell/common/shell.cc b/shell/common/shell.cc index 128607232d7e9..836797b352654 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 e808caec0ef9e..a5d71867751e5 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 Date: Wed, 30 Aug 2023 14:52:10 -0700 Subject: [PATCH 12/19] lint --- lib/ui/window/platform_configuration_unittests.cc | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/ui/window/platform_configuration_unittests.cc b/lib/ui/window/platform_configuration_unittests.cc index f4d18bbc3431e..e2c36709edf1b 100644 --- a/lib/ui/window/platform_configuration_unittests.cc +++ b/lib/ui/window/platform_configuration_unittests.cc @@ -81,8 +81,8 @@ class RuntimeControllerContext { using VoidCallback = std::function; [[nodiscard]] static std::unique_ptr Create( - Settings settings, // - TaskRunners task_runners, // + 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."; @@ -120,14 +120,14 @@ class RuntimeControllerContext { private: RuntimeControllerContext(Settings settings, - TaskRunners task_runners, + const TaskRunners& task_runners, RuntimeDelegate& client, DartVMRef vm, fml::RefPtr isolate_snapshot) : settings_(settings), task_runners_(task_runners), - isolate_snapshot_(isolate_snapshot), - vm_(vm), + isolate_snapshot_(std::move(isolate_snapshot)), + vm_(std::move(vm)), runtime_controller_(std::make_unique( client, &vm_, From c38c2024b07e819adb961d15cffb8085077687bd Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Wed, 30 Aug 2023 15:03:55 -0700 Subject: [PATCH 13/19] Revert contents of #45190 --- shell/common/shell_unittests.cc | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/shell/common/shell_unittests.cc b/shell/common/shell_unittests.cc index 24dd0dfe5f881..43a14bb13b75f 100644 --- a/shell/common/shell_unittests.cc +++ b/shell/common/shell_unittests.cc @@ -4391,6 +4391,17 @@ static void ParseViewIdsCallback(const Dart_NativeArguments& args, ASSERT_EQ(exception, nullptr); } +// Run the given task in the platform runner, and blocks until its finished. +static void RunOnPlatformTaskRunner(Shell& shell, const fml::closure& task) { + fml::AutoResetWaitableEvent latch; + fml::TaskRunner::RunNowOrPostTask( + shell.GetTaskRunners().GetPlatformTaskRunner(), [&task, &latch]() { + task(); + latch.Signal(); + }); + latch.Wait(); +} + TEST_F(ShellTest, ShellStartsWithImplicitView) { ASSERT_FALSE(DartVMRef::IsInstanceRunning()); Settings settings = CreateSettingsForFixture(); @@ -4461,22 +4472,21 @@ TEST_F(ShellTest, ShellCanAddViewOrRemoveView) { ASSERT_EQ(viewIds.size(), 1u); ASSERT_EQ(viewIds[0], 0ll); - PostSync(shell->GetTaskRunners().GetPlatformTaskRunner(), - [&shell] { shell->AddView(2, ViewportMetrics{}); }); + RunOnPlatformTaskRunner(*shell, + [&shell] { shell->AddView(2, ViewportMetrics{}); }); reportLatch.Wait(); ASSERT_TRUE(hasImplicitView); ASSERT_EQ(viewIds.size(), 2u); ASSERT_EQ(viewIds[1], 2ll); - PostSync(shell->GetTaskRunners().GetPlatformTaskRunner(), - [&shell] { shell->RemoveView(2); }); + RunOnPlatformTaskRunner(*shell, [&shell] { shell->RemoveView(2); }); reportLatch.Wait(); ASSERT_TRUE(hasImplicitView); ASSERT_EQ(viewIds.size(), 1u); ASSERT_EQ(viewIds[0], 0ll); - PostSync(shell->GetTaskRunners().GetPlatformTaskRunner(), - [&shell] { shell->AddView(4, ViewportMetrics{}); }); + RunOnPlatformTaskRunner(*shell, + [&shell] { shell->AddView(4, ViewportMetrics{}); }); reportLatch.Wait(); ASSERT_TRUE(hasImplicitView); ASSERT_EQ(viewIds.size(), 2u); @@ -4521,7 +4531,7 @@ TEST_F(ShellTest, ShellFlushesPlatformStatesByMain) { std::unique_ptr shell = CreateShell(settings, task_runners); ASSERT_TRUE(shell); - PostSync(shell->GetTaskRunners().GetPlatformTaskRunner(), [&shell] { + RunOnPlatformTaskRunner(*shell, [&shell] { auto platform_view = shell->GetPlatformView(); // The construtor for ViewportMetrics{_, width, _, _, _} (only the 2nd // argument matters in this test). From c4d273c414df7e678f0916476b35d720e2d3e617 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Wed, 30 Aug 2023 15:30:06 -0700 Subject: [PATCH 14/19] Use new mocks --- .../platform_configuration_unittests.cc | 81 ++++++++++++------- 1 file changed, 50 insertions(+), 31 deletions(-) diff --git a/lib/ui/window/platform_configuration_unittests.cc b/lib/ui/window/platform_configuration_unittests.cc index e2c36709edf1b..13c469e07295c 100644 --- a/lib/ui/window/platform_configuration_unittests.cc +++ b/lib/ui/window/platform_configuration_unittests.cc @@ -17,9 +17,6 @@ #include "flutter/testing/testing.h" #include "gmock/gmock.h" -///\note Deprecated MOCK_METHOD macros used until this issue is resolved: -// https://github.com/google/googletest/issues/2490 - namespace flutter { namespace { @@ -38,37 +35,59 @@ static void PostSync(const fml::RefPtr& task_runner, class MockRuntimeDelegate : public RuntimeDelegate { public: - MOCK_METHOD0(ImplicitViewEnabled, bool()); - MOCK_METHOD0(DefaultRouteName, std::string()); - MOCK_METHOD1(ScheduleFrame, void(bool)); - MOCK_METHOD2(Render, void(std::unique_ptr, float)); - MOCK_METHOD2(UpdateSemantics, - void(SemanticsNodeUpdates, CustomAccessibilityActionUpdates)); - MOCK_METHOD1(HandlePlatformMessage, void(std::unique_ptr)); - MOCK_METHOD0(GetFontCollection, FontCollection&()); - MOCK_METHOD0(GetAssetManager, std::shared_ptr()); - MOCK_METHOD0(OnRootIsolateCreated, void()); - MOCK_METHOD2(UpdateIsolateDescription, void(const std::string, int64_t)); - MOCK_METHOD1(SetNeedsReportTimings, void(bool)); - MOCK_METHOD1(ComputePlatformResolvedLocale, - std::unique_ptr>( - const std::vector&)); - MOCK_METHOD1(RequestDartDeferredLibrary, void(intptr_t)); - MOCK_CONST_METHOD0(GetPlatformMessageHandler, - std::weak_ptr()); - MOCK_CONST_METHOD2(GetScaledFontSize, - double(double font_size, int configuration_id)); + 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_METHOD1(HandlePlatformMessage, - void(std::unique_ptr message)); - MOCK_CONST_METHOD0(DoesHandlePlatformMessageOnPlatformThread, bool()); - MOCK_METHOD2(InvokePlatformMessageResponseCallback, - void(int response_id, std::unique_ptr mapping)); - MOCK_METHOD1(InvokePlatformMessageEmptyResponseCallback, - void(int response_id)); + 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 @@ -119,7 +138,7 @@ class RuntimeControllerContext { RuntimeController& Controller() { return *runtime_controller_; } private: - RuntimeControllerContext(Settings settings, + RuntimeControllerContext(const Settings& settings, const TaskRunners& task_runners, RuntimeDelegate& client, DartVMRef vm, From 45f014993be744d4651fb4ed4357ad8baf6bd817 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Wed, 30 Aug 2023 15:36:36 -0700 Subject: [PATCH 15/19] Lint --- lib/ui/window/platform_configuration_unittests.cc | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/lib/ui/window/platform_configuration_unittests.cc b/lib/ui/window/platform_configuration_unittests.cc index 13c469e07295c..595d3503f0ea4 100644 --- a/lib/ui/window/platform_configuration_unittests.cc +++ b/lib/ui/window/platform_configuration_unittests.cc @@ -79,13 +79,16 @@ class MockPlatformMessageHandler : public PlatformMessageHandler { HandlePlatformMessage, (std::unique_ptr message), (override)); - MOCK_METHOD(bool DoesHandlePlatformMessageOnPlatformThread, + MOCK_METHOD(bool, + DoesHandlePlatformMessageOnPlatformThread, (), - (const override)); - MOCK_METHOD(void InvokePlatformMessageResponseCallback, + (const, override)); + MOCK_METHOD(void, + InvokePlatformMessageResponseCallback, (int response_id, std::unique_ptr mapping), (override)); - MOCK_METHOD(void InvokePlatformMessageEmptyResponseCallback, + MOCK_METHOD(void, + InvokePlatformMessageEmptyResponseCallback, (int response_id), (override)); }; From bc112154497e55f767702d4fad39072d0ab7267c Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Thu, 31 Aug 2023 13:40:58 -0700 Subject: [PATCH 16/19] correctly post sync for begin frame --- lib/ui/fixtures/ui_test.dart | 6 ++-- .../platform_configuration_unittests.cc | 36 ++++++++++++------- 2 files changed, 25 insertions(+), 17 deletions(-) diff --git a/lib/ui/fixtures/ui_test.dart b/lib/ui/fixtures/ui_test.dart index ae361c733b8ac..85343961fbcf6 100644 --- a/lib/ui/fixtures/ui_test.dart +++ b/lib/ui/fixtures/ui_test.dart @@ -1092,13 +1092,10 @@ Scene CreateRedBoxScene(Size size) { return builder.build(); } -@pragma('vm:entry-point') -@pragma('vm:external-name', 'NotifyNative') -external void notifyNative(); - @pragma('vm:entry-point') void incorrectImmediateRender() { PlatformDispatcher.instance.views.first.render(CreateRedBoxScene(Size(2, 2))); + _finish(); } @pragma('vm:entry-point') @@ -1111,4 +1108,5 @@ void incorrectDoubleRender() { PlatformDispatcher.instance.views.first.render(CreateRedBoxScene(Size(4, 4))); PlatformDispatcher.instance.views.first.render(CreateRedBoxScene(Size(5, 5))); }; + _finish(); } diff --git a/lib/ui/window/platform_configuration_unittests.cc b/lib/ui/window/platform_configuration_unittests.cc index 595d3503f0ea4..baa9cce293437 100644 --- a/lib/ui/window/platform_configuration_unittests.cc +++ b/lib/ui/window/platform_configuration_unittests.cc @@ -495,10 +495,16 @@ TEST_F(PlatformConfigurationTest, OutOfScopeRenderCallsAreIgnored) { MockRuntimeDelegate client; auto platform_message_handler = std::make_shared(); - EXPECT_CALL(client, GetPlatformMessageHandler()) + EXPECT_CALL(client, GetPlatformMessageHandler) .WillOnce(Return(platform_message_handler)); // Render should not be called. - EXPECT_CALL(client, Render(_, _)).Times(Exactly(0)); + EXPECT_CALL(client, Render).Times(Exactly(0)); + + auto message_latch = std::make_shared(); + 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); @@ -513,9 +519,7 @@ TEST_F(PlatformConfigurationTest, OutOfScopeRenderCallsAreIgnored) { }); // Wait for the Dart main function to end. - fml::AutoResetWaitableEvent latch; - PostSync(task_runners.GetUITaskRunner(), [&]() { latch.Signal(); }); - latch.Wait(); + message_latch->Wait(); } TEST_F(PlatformConfigurationTest, DuplicateRenderCallsAreIgnored) { @@ -525,10 +529,16 @@ TEST_F(PlatformConfigurationTest, DuplicateRenderCallsAreIgnored) { MockRuntimeDelegate client; auto platform_message_handler = std::make_shared(); - EXPECT_CALL(client, GetPlatformMessageHandler()) + 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(Exactly(1)); + EXPECT_CALL(client, Render).Times(Exactly(1)); + + auto message_latch = std::make_shared(); + 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); @@ -542,13 +552,13 @@ TEST_F(PlatformConfigurationTest, DuplicateRenderCallsAreIgnored) { /*touch_slop=*/2, /*display_id=*/0)); }); - // Wait for the Dart main function to end, which means the callbacks have been - // registered. - fml::AutoResetWaitableEvent latch; - PostSync(task_runners.GetUITaskRunner(), [&]() { latch.Signal(); }); - latch.Wait(); + // Wait for the Dart main function to end. + message_latch->Wait(); - runtime_controller_context->Controller().BeginFrame(fml::TimePoint::Now(), 0); + PostSync(task_runners.GetUITaskRunner(), [&]() { + runtime_controller_context->Controller().BeginFrame(fml::TimePoint::Now(), + 0); + }); } } // namespace testing From 151ed777ca244f187588945f9b2f29012ca8869d Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Thu, 31 Aug 2023 13:48:44 -0700 Subject: [PATCH 17/19] Extract into methods --- .../platform_configuration_unittests.cc | 50 +++++++++++-------- 1 file changed, 29 insertions(+), 21 deletions(-) diff --git a/lib/ui/window/platform_configuration_unittests.cc b/lib/ui/window/platform_configuration_unittests.cc index baa9cce293437..915ccea330628 100644 --- a/lib/ui/window/platform_configuration_unittests.cc +++ b/lib/ui/window/platform_configuration_unittests.cc @@ -100,7 +100,7 @@ class MockPlatformMessageHandler : public PlatformMessageHandler { // and get the runtime controller with Controller(). class RuntimeControllerContext { public: - using VoidCallback = std::function; + using ControllerCallback = std::function; [[nodiscard]] static std::unique_ptr Create( Settings settings, // @@ -123,7 +123,7 @@ class RuntimeControllerContext { // 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, - VoidCallback post_launch) { + ControllerCallback post_launch) { PostSync(task_runners_.GetUITaskRunner(), [&]() { bool launch_success = runtime_controller_->LaunchRootIsolate( settings_, // @@ -133,12 +133,16 @@ class RuntimeControllerContext { configuration.GetEntrypointArgs(), // configuration.TakeIsolateConfiguration()); // ASSERT_TRUE(launch_success); - post_launch(); + post_launch(*runtime_controller_); }); } - // Get the runtime controller. - RuntimeController& Controller() { return *runtime_controller_; } + void ControllerTaskSync(ControllerCallback task) { + ASSERT_TRUE(runtime_controller_); + ASSERT_TRUE(task); + PostSync(task_runners_.GetUITaskRunner(), + [&]() { task(*runtime_controller_); }); + } private: RuntimeControllerContext(const Settings& settings, @@ -511,12 +515,14 @@ TEST_F(PlatformConfigurationTest, OutOfScopeRenderCallsAreIgnored) { auto configuration = RunConfiguration::InferFromSettings(settings); configuration.SetEntrypoint("incorrectImmediateRender"); - runtime_controller_context->LaunchRootIsolate(configuration, [&] { - runtime_controller_context->Controller().AddView( - kImplicitViewId, ViewportMetrics( - /*pixel_ratio=*/1.0, /*width=*/20, /*height=*/20, - /*touch_slop=*/2, /*display_id=*/0)); - }); + 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(); @@ -545,20 +551,22 @@ TEST_F(PlatformConfigurationTest, DuplicateRenderCallsAreIgnored) { auto configuration = RunConfiguration::InferFromSettings(settings); configuration.SetEntrypoint("incorrectDoubleRender"); - runtime_controller_context->LaunchRootIsolate(configuration, [&] { - runtime_controller_context->Controller().AddView( - kImplicitViewId, ViewportMetrics( - /*pixel_ratio=*/1.0, /*width=*/20, /*height=*/20, - /*touch_slop=*/2, /*display_id=*/0)); - }); + 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(); - PostSync(task_runners.GetUITaskRunner(), [&]() { - runtime_controller_context->Controller().BeginFrame(fml::TimePoint::Now(), - 0); - }); + runtime_controller_context->ControllerTaskSync( + [](RuntimeController& runtime_controller) { + runtime_controller.BeginFrame(fml::TimePoint::Now(), 0); + }); } } // namespace testing From 3370e8c0b35c4426cb962fce1be3c35bb5ea9566 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Thu, 31 Aug 2023 18:13:25 -0700 Subject: [PATCH 18/19] Address comments --- lib/ui/fixtures/ui_test.dart | 20 +++++++++++++------ .../platform_configuration_unittests.cc | 6 +++++- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/lib/ui/fixtures/ui_test.dart b/lib/ui/fixtures/ui_test.dart index 85343961fbcf6..3cc93a746921a 100644 --- a/lib/ui/fixtures/ui_test.dart +++ b/lib/ui/fixtures/ui_test.dart @@ -1077,7 +1077,7 @@ external void _callHook( Object? arg21, ]); -Scene CreateRedBoxScene(Size size) { +Scene _createRedBoxScene(Size size) { final SceneBuilder builder = SceneBuilder(); builder.pushOffset(0.0, 0.0); final Paint paint = Paint() @@ -1094,19 +1094,27 @@ Scene CreateRedBoxScene(Size size) { @pragma('vm:entry-point') void incorrectImmediateRender() { - PlatformDispatcher.instance.views.first.render(CreateRedBoxScene(Size(2, 2))); + 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.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))); + 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/window/platform_configuration_unittests.cc b/lib/ui/window/platform_configuration_unittests.cc index 915ccea330628..9128dfeffe5a5 100644 --- a/lib/ui/window/platform_configuration_unittests.cc +++ b/lib/ui/window/platform_configuration_unittests.cc @@ -97,7 +97,7 @@ class MockPlatformMessageHandler : public PlatformMessageHandler { // RuntimeDelegate. // // To use this class, contruct this class with Create, call LaunchRootIsolate, -// and get the runtime controller with Controller(). +// and use the controller with ControllerTaskSync(). class RuntimeControllerContext { public: using ControllerCallback = std::function; @@ -137,6 +137,8 @@ class RuntimeControllerContext { }); } + // 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); @@ -565,6 +567,8 @@ TEST_F(PlatformConfigurationTest, DuplicateRenderCallsAreIgnored) { 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); }); } From d768cc44fa4a2f2ff8a0ce3d76b03f348d0d6563 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Fri, 1 Sep 2023 17:34:03 -0700 Subject: [PATCH 19/19] Remove exact --- lib/ui/window/platform_configuration_unittests.cc | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/ui/window/platform_configuration_unittests.cc b/lib/ui/window/platform_configuration_unittests.cc index 595d3503f0ea4..80903ea87cb8e 100644 --- a/lib/ui/window/platform_configuration_unittests.cc +++ b/lib/ui/window/platform_configuration_unittests.cc @@ -172,7 +172,6 @@ class RuntimeControllerContext { namespace testing { using ::testing::_; -using ::testing::Exactly; using ::testing::Return; class PlatformConfigurationTest : public ShellTest {}; @@ -498,7 +497,7 @@ TEST_F(PlatformConfigurationTest, OutOfScopeRenderCallsAreIgnored) { EXPECT_CALL(client, GetPlatformMessageHandler()) .WillOnce(Return(platform_message_handler)); // Render should not be called. - EXPECT_CALL(client, Render(_, _)).Times(Exactly(0)); + EXPECT_CALL(client, Render(_, _)).Times(0); auto runtime_controller_context = RuntimeControllerContext::Create(settings, task_runners, client); @@ -528,7 +527,7 @@ TEST_F(PlatformConfigurationTest, DuplicateRenderCallsAreIgnored) { 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(Exactly(1)); + EXPECT_CALL(client, Render(_, _)).Times(1); auto runtime_controller_context = RuntimeControllerContext::Create(settings, task_runners, client);