Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "Enforce the rule of calling FlutterView.Render" #45525

Merged
merged 1 commit into from
Sep 7, 2023
Merged

Revert "Enforce the rule of calling FlutterView.Render" #45525

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 0 additions & 42 deletions lib/ui/fixtures/ui_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1076,45 +1076,3 @@ external void _callHook(
Object? arg20,
Object? arg21,
]);

Scene _createRedBoxScene(Size size) {
final SceneBuilder builder = SceneBuilder();
builder.pushOffset(0.0, 0.0);
final Paint paint = Paint()
..color = Color.fromARGB(255, 255, 0, 0)
..style = PaintingStyle.fill;
final PictureRecorder baseRecorder = PictureRecorder();
final Canvas canvas = Canvas(baseRecorder);
canvas.drawRect(Rect.fromLTRB(0.0, 0.0, size.width, size.height), paint);
final Picture picture = baseRecorder.endRecording();
builder.addPicture(Offset(0.0, 0.0), picture);
builder.pop();
return builder.build();
}

@pragma('vm:entry-point')
void incorrectImmediateRender() {
PlatformDispatcher.instance.views.first.render(_createRedBoxScene(Size(2, 2)));
_finish();
// Don't schedule a frame here. This test only checks if the
// [FlutterView.render] call is propagated to PlatformConfiguration.render
// and thus doesn't need anything from `Animator` or `Engine`, which,
// besides, are not even created in the native side at all.
}

@pragma('vm:entry-point')
void incorrectDoubleRender() {
PlatformDispatcher.instance.onBeginFrame = (Duration value) {
PlatformDispatcher.instance.views.first.render(_createRedBoxScene(Size(2, 2)));
PlatformDispatcher.instance.views.first.render(_createRedBoxScene(Size(3, 3)));
};
PlatformDispatcher.instance.onDrawFrame = () {
PlatformDispatcher.instance.views.first.render(_createRedBoxScene(Size(4, 4)));
PlatformDispatcher.instance.views.first.render(_createRedBoxScene(Size(5, 5)));
};
_finish();
// Don't schedule a frame here. This test only checks if the
// [FlutterView.render] call is propagated to PlatformConfiguration.render
// and thus doesn't need anything from `Animator` or `Engine`, which,
// besides, are not even created in the native side at all.
}
33 changes: 0 additions & 33 deletions lib/ui/platform_dispatcher.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -308,21 +308,6 @@ class PlatformDispatcher {
_invoke(onMetricsChanged, _onMetricsChangedZone);
}

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

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

// Called from the engine, via hooks.dart
void _beginFrame(int microseconds) {
assert(_renderedViews == null);
assert(_renderedViewsBetweenCallbacks == null);

_renderedViews = <FlutterView>{};
_invoke1<Duration>(
onBeginFrame,
_onBeginFrameZone,
Duration(microseconds: microseconds),
);
_renderedViewsBetweenCallbacks = _renderedViews;
_renderedViews = null;

assert(_renderedViews == null);
assert(_renderedViewsBetweenCallbacks != null);
}

/// A callback that is invoked for each frame after [onBeginFrame] has
Expand All @@ -375,16 +351,7 @@ 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.
Expand Down
9 changes: 1 addition & 8 deletions lib/ui/window.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -353,14 +353,7 @@ class FlutterView {
/// 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);
Expand Down
240 changes: 0 additions & 240 deletions lib/ui/window/platform_configuration_unittests.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -493,84 +332,5 @@ TEST_F(PlatformConfigurationTest, SetDartPerformanceMode) {
DestroyShell(std::move(shell), task_runners);
}

TEST_F(PlatformConfigurationTest, OutOfScopeRenderCallsAreIgnored) {
Settings settings = CreateSettingsForFixture();
TaskRunners task_runners = GetTaskRunnersForFixture();

MockRuntimeDelegate client;
auto platform_message_handler =
std::make_shared<MockPlatformMessageHandler>();
EXPECT_CALL(client, GetPlatformMessageHandler)
.WillOnce(Return(platform_message_handler));
// Render should not be called.
EXPECT_CALL(client, Render).Times(0);

auto message_latch = std::make_shared<fml::AutoResetWaitableEvent>();
auto finish = [message_latch](Dart_NativeArguments args) {
message_latch->Signal();
};
AddNativeCallback("Finish", CREATE_NATIVE_ENTRY(finish));

auto runtime_controller_context =
RuntimeControllerContext::Create(settings, task_runners, client);

auto configuration = RunConfiguration::InferFromSettings(settings);
configuration.SetEntrypoint("incorrectImmediateRender");
runtime_controller_context->LaunchRootIsolate(
configuration, [](RuntimeController& runtime_controller) {
runtime_controller.AddView(
kImplicitViewId,
ViewportMetrics(
/*pixel_ratio=*/1.0, /*width=*/20, /*height=*/20,
/*touch_slop=*/2, /*display_id=*/0));
});

// Wait for the Dart main function to end.
message_latch->Wait();
}

TEST_F(PlatformConfigurationTest, DuplicateRenderCallsAreIgnored) {
Settings settings = CreateSettingsForFixture();
TaskRunners task_runners = GetTaskRunnersForFixture();

MockRuntimeDelegate client;
auto platform_message_handler =
std::make_shared<MockPlatformMessageHandler>();
EXPECT_CALL(client, GetPlatformMessageHandler)
.WillOnce(Return(platform_message_handler));
// Render should only be called once, because the second call is ignored.
EXPECT_CALL(client, Render).Times(1);

auto message_latch = std::make_shared<fml::AutoResetWaitableEvent>();
auto finish = [message_latch](Dart_NativeArguments args) {
message_latch->Signal();
};
AddNativeCallback("Finish", CREATE_NATIVE_ENTRY(finish));

auto runtime_controller_context =
RuntimeControllerContext::Create(settings, task_runners, client);

auto configuration = RunConfiguration::InferFromSettings(settings);
configuration.SetEntrypoint("incorrectDoubleRender");
runtime_controller_context->LaunchRootIsolate(
configuration, [](RuntimeController& runtime_controller) {
runtime_controller.AddView(
kImplicitViewId,
ViewportMetrics(
/*pixel_ratio=*/1.0, /*width=*/20, /*height=*/20,
/*touch_slop=*/2, /*display_id=*/0));
});

// Wait for the Dart main function to end.
message_latch->Wait();

runtime_controller_context->ControllerTaskSync(
[](RuntimeController& runtime_controller) {
// This BeginFrame calls PlatformDispatcher's handleBeginFrame and
// handleDrawFrame synchronously. Therefore don't wait after it.
runtime_controller.BeginFrame(fml::TimePoint::Now(), 0);
});
}

} // namespace testing
} // namespace flutter
Loading