[CP] Revert "Reland: Enforce the rule of calling FlutterView.Render (#4530… (#47075)
b/306089854
Co-authored-by: Tong Mu <dkwingsmt@users.noreply.github.com>
diff --git a/lib/ui/fixtures/ui_test.dart b/lib/ui/fixtures/ui_test.dart
index 3cc93a7..0aff177 100644
--- a/lib/ui/fixtures/ui_test.dart
+++ b/lib/ui/fixtures/ui_test.dart
@@ -1076,45 +1076,3 @@
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 5f42aa4..0bd3872 100644
--- a/lib/ui/platform_dispatcher.dart
+++ b/lib/ui/platform_dispatcher.dart
@@ -308,21 +308,6 @@
_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.
///
@@ -344,20 +329,11 @@
// 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
@@ -375,16 +351,7 @@
// 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 286e148..26a258c 100644
--- a/lib/ui/window.dart
+++ b/lib/ui/window.dart
@@ -327,21 +327,14 @@
/// Updates the view's rendering on the GPU with the newly provided [Scene].
///
- /// ## Requirement for calling this method
- ///
- /// This method must be called within the synchronous scope of the
+ /// This function must be called within the scope of the
/// [PlatformDispatcher.onBeginFrame] or [PlatformDispatcher.onDrawFrame]
- /// 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.
+ /// callbacks being invoked.
///
- /// 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.
- ///
- /// ## How to record a scene
+ /// 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.
///
/// To record graphical operations, first create a [PictureRecorder], then
/// construct a [Canvas], passing that [PictureRecorder] to its constructor.
@@ -360,14 +353,7 @@
/// scheduling of frames.
/// * [RendererBinding], the Flutter framework class which manages layout and
/// painting.
- 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);
- }
+ void render(Scene scene) => _render(scene as _NativeScene);
@Native<Void Function(Pointer<Void>)>(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 1fa8377..7410cae 100644
--- a/lib/ui/window/platform_configuration_unittests.cc
+++ b/lib/ui/window/platform_configuration_unittests.cc
@@ -15,171 +15,10 @@
#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) {
@@ -493,84 +332,5 @@
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 finish_latch = std::make_shared<fml::AutoResetWaitableEvent>();
- 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<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 finish_latch = std::make_shared<fml::AutoResetWaitableEvent>();
- 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 a6ba487..a7f7c9a 100644
--- a/shell/common/shell.cc
+++ b/shell/common/shell.cc
@@ -144,23 +144,6 @@
} // namespace
-std::pair<DartVMRef, fml::RefPtr<const DartSnapshot>>
-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);
-
- // 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> Shell::Create(
const PlatformData& platform_data,
const TaskRunners& task_runners,
@@ -173,7 +156,19 @@
TRACE_EVENT0("flutter", "Shell::Create");
- auto [vm, isolate_snapshot] = InferVmInitDataFromSettings(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();
+ }
auto resource_cache_limit_calculator =
std::make_shared<ResourceCacheLimitCalculator>(
settings.resource_cache_max_bytes_threshold);
diff --git a/shell/common/shell.h b/shell/common/shell.h
index 133fecc..039eb65 100644
--- a/shell/common/shell.h
+++ b/shell/common/shell.h
@@ -438,16 +438,6 @@
const std::shared_ptr<fml::ConcurrentTaskRunner>
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<DartVMRef, fml::RefPtr<const DartSnapshot>>
- InferVmInitDataFromSettings(Settings& settings);
-
private:
using ServiceProtocolHandler =
std::function<bool(const ServiceProtocol::Handler::ServiceProtocolMap&,
diff --git a/testing/dart/platform_view_test.dart b/testing/dart/platform_view_test.dart
index 7d467f8..146c865 100644
--- a/testing/dart/platform_view_test.dart
+++ b/testing/dart/platform_view_test.dart
@@ -10,13 +10,10 @@
test('PlatformView layers do not emit errors from tester', () async {
final SceneBuilder builder = SceneBuilder();
builder.addPlatformView(1);
+ final Scene scene = builder.build();
- PlatformDispatcher.instance.onBeginFrame = (Duration duration) {
- final Scene scene = builder.build();
- PlatformDispatcher.instance.implicitView!.render(scene);
- scene.dispose();
- };
- PlatformDispatcher.instance.scheduleFrame();
+ PlatformDispatcher.instance.implicitView!.render(scene);
+ scene.dispose();
// Test harness asserts that this does not emit an error from the shell logs.
});
}
