Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit 77b8ebf

Browse files
dkwingsmtdkwingsmt
authored
Revert "Revert "Enforce the rule of calling FlutterView.Render" (#45525)"
This reverts commit 75437a3.
1 parent a828c26 commit 77b8ebf

File tree

6 files changed

+351
-14
lines changed

6 files changed

+351
-14
lines changed

‎lib/ui/fixtures/ui_test.dart

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1076,3 +1076,45 @@ external void _callHook(
10761076
Object? arg20,
10771077
Object? arg21,
10781078
]);
1079+
1080+
Scene _createRedBoxScene(Size size) {
1081+
final SceneBuilder builder = SceneBuilder();
1082+
builder.pushOffset(0.0, 0.0);
1083+
final Paint paint = Paint()
1084+
..color = Color.fromARGB(255, 255, 0, 0)
1085+
..style = PaintingStyle.fill;
1086+
final PictureRecorder baseRecorder = PictureRecorder();
1087+
final Canvas canvas = Canvas(baseRecorder);
1088+
canvas.drawRect(Rect.fromLTRB(0.0, 0.0, size.width, size.height), paint);
1089+
final Picture picture = baseRecorder.endRecording();
1090+
builder.addPicture(Offset(0.0, 0.0), picture);
1091+
builder.pop();
1092+
return builder.build();
1093+
}
1094+
1095+
@pragma('vm:entry-point')
1096+
void incorrectImmediateRender() {
1097+
PlatformDispatcher.instance.views.first.render(_createRedBoxScene(Size(2, 2)));
1098+
_finish();
1099+
// Don't schedule a frame here. This test only checks if the
1100+
// [FlutterView.render] call is propagated to PlatformConfiguration.render
1101+
// and thus doesn't need anything from `Animator` or `Engine`, which,
1102+
// besides, are not even created in the native side at all.
1103+
}
1104+
1105+
@pragma('vm:entry-point')
1106+
void incorrectDoubleRender() {
1107+
PlatformDispatcher.instance.onBeginFrame = (Duration value) {
1108+
PlatformDispatcher.instance.views.first.render(_createRedBoxScene(Size(2, 2)));
1109+
PlatformDispatcher.instance.views.first.render(_createRedBoxScene(Size(3, 3)));
1110+
};
1111+
PlatformDispatcher.instance.onDrawFrame = () {
1112+
PlatformDispatcher.instance.views.first.render(_createRedBoxScene(Size(4, 4)));
1113+
PlatformDispatcher.instance.views.first.render(_createRedBoxScene(Size(5, 5)));
1114+
};
1115+
_finish();
1116+
// Don't schedule a frame here. This test only checks if the
1117+
// [FlutterView.render] call is propagated to PlatformConfiguration.render
1118+
// and thus doesn't need anything from `Animator` or `Engine`, which,
1119+
// besides, are not even created in the native side at all.
1120+
}

‎lib/ui/platform_dispatcher.dart

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -308,6 +308,21 @@ class PlatformDispatcher {
308308
_invoke(onMetricsChanged, _onMetricsChangedZone);
309309
}
310310

311+
// [FlutterView]s for which [FlutterView.render] has already been called
312+
// during the current [onBeginFrame]/[onDrawFrame] callback sequence.
313+
//
314+
// The field is null outside the scope of those callbacks indicating that
315+
// calls to [FlutterView.render] must be ignored. Furthermore, if a given
316+
// [FlutterView] is already present in this set when its [FlutterView.render]
317+
// is called again, that call must be ignored as a duplicate.
318+
//
319+
// Between [onBeginFrame] and [onDrawFrame] the properties value is
320+
// temporarily stored in `_renderedViewsBetweenCallbacks` so that it survives
321+
// the gap between the two callbacks.
322+
Set<FlutterView>? _renderedViews;
323+
// Temporary storage of the `_renderedViews` value between `_beginFrame` and
324+
// `_drawFrame`.
325+
Set<FlutterView>? _renderedViewsBetweenCallbacks;
311326

312327
/// A callback invoked when any view begins a frame.
313328
///
@@ -329,11 +344,20 @@ class PlatformDispatcher {
329344

330345
// Called from the engine, via hooks.dart
331346
void _beginFrame(int microseconds) {
347+
assert(_renderedViews == null);
348+
assert(_renderedViewsBetweenCallbacks == null);
349+
350+
_renderedViews = <FlutterView>{};
332351
_invoke1<Duration>(
333352
onBeginFrame,
334353
_onBeginFrameZone,
335354
Duration(microseconds: microseconds),
336355
);
356+
_renderedViewsBetweenCallbacks = _renderedViews;
357+
_renderedViews = null;
358+
359+
assert(_renderedViews == null);
360+
assert(_renderedViewsBetweenCallbacks != null);
337361
}
338362

339363
/// A callback that is invoked for each frame after [onBeginFrame] has
@@ -351,7 +375,16 @@ class PlatformDispatcher {
351375

352376
// Called from the engine, via hooks.dart
353377
void _drawFrame() {
378+
assert(_renderedViews == null);
379+
assert(_renderedViewsBetweenCallbacks != null);
380+
381+
_renderedViews = _renderedViewsBetweenCallbacks;
382+
_renderedViewsBetweenCallbacks = null;
354383
_invoke(onDrawFrame, _onDrawFrameZone);
384+
_renderedViews = null;
385+
386+
assert(_renderedViews == null);
387+
assert(_renderedViewsBetweenCallbacks == null);
355388
}
356389

357390
/// A callback that is invoked when pointer data is available.

‎lib/ui/window.dart

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -353,7 +353,14 @@ class FlutterView {
353353
/// scheduling of frames.
354354
/// * [RendererBinding], the Flutter framework class which manages layout and
355355
/// painting.
356-
void render(Scene scene) => _render(scene as _NativeScene);
356+
void render(Scene scene) {
357+
if (platformDispatcher._renderedViews?.add(this) != true) {
358+
// Duplicated calls or calls outside of onBeginFrame/onDrawFrame
359+
// (indicated by _renderedViews being null) are ignored, as documented.
360+
return;
361+
}
362+
_render(scene as _NativeScene);
363+
}
357364

358365
@Native<Void Function(Pointer<Void>)>(symbol: 'PlatformConfigurationNativeApi::Render')
359366
external static void _render(_NativeScene scene);

‎lib/ui/window/platform_configuration_unittests.cc

Lines changed: 240 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,171 @@
1515
#include "flutter/shell/common/shell_test.h"
1616
#include "flutter/shell/common/thread_host.h"
1717
#include "flutter/testing/testing.h"
18+
#include "gmock/gmock.h"
1819

1920
namespace flutter {
21+
22+
namespace {
23+
24+
static constexpr int64_t kImplicitViewId = 0;
25+
26+
static void PostSync(const fml::RefPtr<fml::TaskRunner>& task_runner,
27+
const fml::closure& task) {
28+
fml::AutoResetWaitableEvent latch;
29+
fml::TaskRunner::RunNowOrPostTask(task_runner, [&latch, &task] {
30+
task();
31+
latch.Signal();
32+
});
33+
latch.Wait();
34+
}
35+
36+
class MockRuntimeDelegate : public RuntimeDelegate {
37+
public:
38+
MOCK_METHOD(std::string, DefaultRouteName, (), (override));
39+
MOCK_METHOD(void, ScheduleFrame, (bool), (override));
40+
MOCK_METHOD(void,
41+
Render,
42+
(std::unique_ptr<flutter::LayerTree>, float),
43+
(override));
44+
MOCK_METHOD(void,
45+
UpdateSemantics,
46+
(SemanticsNodeUpdates, CustomAccessibilityActionUpdates),
47+
(override));
48+
MOCK_METHOD(void,
49+
HandlePlatformMessage,
50+
(std::unique_ptr<PlatformMessage>),
51+
(override));
52+
MOCK_METHOD(FontCollection&, GetFontCollection, (), (override));
53+
MOCK_METHOD(std::shared_ptr<AssetManager>, GetAssetManager, (), (override));
54+
MOCK_METHOD(void, OnRootIsolateCreated, (), (override));
55+
MOCK_METHOD(void,
56+
UpdateIsolateDescription,
57+
(const std::string, int64_t),
58+
(override));
59+
MOCK_METHOD(void, SetNeedsReportTimings, (bool), (override));
60+
MOCK_METHOD(std::unique_ptr<std::vector<std::string>>,
61+
ComputePlatformResolvedLocale,
62+
(const std::vector<std::string>&),
63+
(override));
64+
MOCK_METHOD(void, RequestDartDeferredLibrary, (intptr_t), (override));
65+
MOCK_METHOD(std::weak_ptr<PlatformMessageHandler>,
66+
GetPlatformMessageHandler,
67+
(),
68+
(const, override));
69+
MOCK_METHOD(void, SendChannelUpdate, (std::string, bool), (override));
70+
MOCK_METHOD(double,
71+
GetScaledFontSize,
72+
(double font_size, int configuration_id),
73+
(const, override));
74+
};
75+
76+
class MockPlatformMessageHandler : public PlatformMessageHandler {
77+
public:
78+
MOCK_METHOD(void,
79+
HandlePlatformMessage,
80+
(std::unique_ptr<PlatformMessage> message),
81+
(override));
82+
MOCK_METHOD(bool,
83+
DoesHandlePlatformMessageOnPlatformThread,
84+
(),
85+
(const, override));
86+
MOCK_METHOD(void,
87+
InvokePlatformMessageResponseCallback,
88+
(int response_id, std::unique_ptr<fml::Mapping> mapping),
89+
(override));
90+
MOCK_METHOD(void,
91+
InvokePlatformMessageEmptyResponseCallback,
92+
(int response_id),
93+
(override));
94+
};
95+
96+
// A class that can launch a RuntimeController with the specified
97+
// RuntimeDelegate.
98+
//
99+
// To use this class, contruct this class with Create, call LaunchRootIsolate,
100+
// and use the controller with ControllerTaskSync().
101+
class RuntimeControllerContext {
102+
public:
103+
using ControllerCallback = std::function<void(RuntimeController&)>;
104+
105+
[[nodiscard]] static std::unique_ptr<RuntimeControllerContext> Create(
106+
Settings settings, //
107+
const TaskRunners& task_runners, //
108+
RuntimeDelegate& client) {
109+
auto [vm, isolate_snapshot] = Shell::InferVmInitDataFromSettings(settings);
110+
FML_CHECK(vm) << "Must be able to initialize the VM.";
111+
// Construct the class with `new` because `make_unique` has no access to the
112+
// private constructor.
113+
RuntimeControllerContext* raw_pointer = new RuntimeControllerContext(
114+
settings, task_runners, client, std::move(vm), isolate_snapshot);
115+
return std::unique_ptr<RuntimeControllerContext>(raw_pointer);
116+
}
117+
118+
~RuntimeControllerContext() {
119+
PostSync(task_runners_.GetUITaskRunner(),
120+
[&]() { runtime_controller_.reset(); });
121+
}
122+
123+
// Launch the root isolate. The post_launch callback will be executed in the
124+
// same UI task, which can be used to create initial views.
125+
void LaunchRootIsolate(RunConfiguration& configuration,
126+
ControllerCallback post_launch) {
127+
PostSync(task_runners_.GetUITaskRunner(), [&]() {
128+
bool launch_success = runtime_controller_->LaunchRootIsolate(
129+
settings_, //
130+
[]() {}, //
131+
configuration.GetEntrypoint(), //
132+
configuration.GetEntrypointLibrary(), //
133+
configuration.GetEntrypointArgs(), //
134+
configuration.TakeIsolateConfiguration()); //
135+
ASSERT_TRUE(launch_success);
136+
post_launch(*runtime_controller_);
137+
});
138+
}
139+
140+
// Run a task that operates the RuntimeController on the UI thread, and wait
141+
// for the task to end.
142+
void ControllerTaskSync(ControllerCallback task) {
143+
ASSERT_TRUE(runtime_controller_);
144+
ASSERT_TRUE(task);
145+
PostSync(task_runners_.GetUITaskRunner(),
146+
[&]() { task(*runtime_controller_); });
147+
}
148+
149+
private:
150+
RuntimeControllerContext(const Settings& settings,
151+
const TaskRunners& task_runners,
152+
RuntimeDelegate& client,
153+
DartVMRef vm,
154+
fml::RefPtr<const DartSnapshot> isolate_snapshot)
155+
: settings_(settings),
156+
task_runners_(task_runners),
157+
isolate_snapshot_(std::move(isolate_snapshot)),
158+
vm_(std::move(vm)),
159+
runtime_controller_(std::make_unique<RuntimeController>(
160+
client,
161+
&vm_,
162+
std::move(isolate_snapshot_),
163+
settings.idle_notification_callback, // idle notification callback
164+
flutter::PlatformData(), // platform data
165+
settings.isolate_create_callback, // isolate create callback
166+
settings.isolate_shutdown_callback, // isolate shutdown callback
167+
settings.persistent_isolate_data, // persistent isolate data
168+
UIDartState::Context{task_runners})) {}
169+
170+
Settings settings_;
171+
TaskRunners task_runners_;
172+
fml::RefPtr<const DartSnapshot> isolate_snapshot_;
173+
DartVMRef vm_;
174+
std::unique_ptr<RuntimeController> runtime_controller_;
175+
};
176+
} // namespace
177+
20178
namespace testing {
21179

180+
using ::testing::_;
181+
using ::testing::Return;
182+
22183
class PlatformConfigurationTest : public ShellTest {};
23184

24185
TEST_F(PlatformConfigurationTest, Initialization) {
@@ -332,5 +493,84 @@ TEST_F(PlatformConfigurationTest, SetDartPerformanceMode) {
332493
DestroyShell(std::move(shell), task_runners);
333494
}
334495

496+
TEST_F(PlatformConfigurationTest, OutOfScopeRenderCallsAreIgnored) {
497+
Settings settings = CreateSettingsForFixture();
498+
TaskRunners task_runners = GetTaskRunnersForFixture();
499+
500+
MockRuntimeDelegate client;
501+
auto platform_message_handler =
502+
std::make_shared<MockPlatformMessageHandler>();
503+
EXPECT_CALL(client, GetPlatformMessageHandler)
504+
.WillOnce(Return(platform_message_handler));
505+
// Render should not be called.
506+
EXPECT_CALL(client, Render).Times(0);
507+
508+
auto message_latch = std::make_shared<fml::AutoResetWaitableEvent>();
509+
auto finish = [message_latch](Dart_NativeArguments args) {
510+
message_latch->Signal();
511+
};
512+
AddNativeCallback("Finish", CREATE_NATIVE_ENTRY(finish));
513+
514+
auto runtime_controller_context =
515+
RuntimeControllerContext::Create(settings, task_runners, client);
516+
517+
auto configuration = RunConfiguration::InferFromSettings(settings);
518+
configuration.SetEntrypoint("incorrectImmediateRender");
519+
runtime_controller_context->LaunchRootIsolate(
520+
configuration, [](RuntimeController& runtime_controller) {
521+
runtime_controller.AddView(
522+
kImplicitViewId,
523+
ViewportMetrics(
524+
/*pixel_ratio=*/1.0, /*width=*/20, /*height=*/20,
525+
/*touch_slop=*/2, /*display_id=*/0));
526+
});
527+
528+
// Wait for the Dart main function to end.
529+
message_latch->Wait();
530+
}
531+
532+
TEST_F(PlatformConfigurationTest, DuplicateRenderCallsAreIgnored) {
533+
Settings settings = CreateSettingsForFixture();
534+
TaskRunners task_runners = GetTaskRunnersForFixture();
535+
536+
MockRuntimeDelegate client;
537+
auto platform_message_handler =
538+
std::make_shared<MockPlatformMessageHandler>();
539+
EXPECT_CALL(client, GetPlatformMessageHandler)
540+
.WillOnce(Return(platform_message_handler));
541+
// Render should only be called once, because the second call is ignored.
542+
EXPECT_CALL(client, Render).Times(1);
543+
544+
auto message_latch = std::make_shared<fml::AutoResetWaitableEvent>();
545+
auto finish = [message_latch](Dart_NativeArguments args) {
546+
message_latch->Signal();
547+
};
548+
AddNativeCallback("Finish", CREATE_NATIVE_ENTRY(finish));
549+
550+
auto runtime_controller_context =
551+
RuntimeControllerContext::Create(settings, task_runners, client);
552+
553+
auto configuration = RunConfiguration::InferFromSettings(settings);
554+
configuration.SetEntrypoint("incorrectDoubleRender");
555+
runtime_controller_context->LaunchRootIsolate(
556+
configuration, [](RuntimeController& runtime_controller) {
557+
runtime_controller.AddView(
558+
kImplicitViewId,
559+
ViewportMetrics(
560+
/*pixel_ratio=*/1.0, /*width=*/20, /*height=*/20,
561+
/*touch_slop=*/2, /*display_id=*/0));
562+
});
563+
564+
// Wait for the Dart main function to end.
565+
message_latch->Wait();
566+
567+
runtime_controller_context->ControllerTaskSync(
568+
[](RuntimeController& runtime_controller) {
569+
// This BeginFrame calls PlatformDispatcher's handleBeginFrame and
570+
// handleDrawFrame synchronously. Therefore don't wait after it.
571+
runtime_controller.BeginFrame(fml::TimePoint::Now(), 0);
572+
});
573+
}
574+
335575
} // namespace testing
336576
} // namespace flutter

0 commit comments

Comments
 (0)