Skip to content

Commit 641765e

Browse files
knopploic-sharma
andauthored
[Windows] Use enum to configure UI thread policy (#163727)
flutter/flutter#162935 added a boolean setting to allow merged platform and UI thread. Using boolean doesn't allow for opting out of the behavior once we enable it by default, which is something we will likely want to allow, at least for a period of time. This PR replaces the boolean with following enum: ```cpp // Configures the thread policy for running the UI isolate. typedef enum { // Default value. Currently will run the UI isolate on separate thread, // later will be changed to running the UI isolate on platform thread. Default, // Run the UI isolate on platform thread. RunOnPlatformThread, // Run the UI isolate on a separate thread. RunOnSeparateThread, } FlutterDesktopUIThreadPolicy; ``` ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md --------- Co-authored-by: Loïc Sharma <737941+loic-sharma@users.noreply.github.com>
1 parent ebd80b7 commit 641765e

File tree

10 files changed

+100
-21
lines changed

10 files changed

+100
-21
lines changed

engine/src/flutter/shell/platform/windows/client_wrapper/flutter_engine.cc

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
#include <iostream>
99

1010
#include "binary_messenger_impl.h"
11+
#include "flutter_windows.h"
1112

1213
namespace flutter {
1314

@@ -19,8 +20,8 @@ FlutterEngine::FlutterEngine(const DartProject& project) {
1920
c_engine_properties.dart_entrypoint = project.dart_entrypoint().c_str();
2021
c_engine_properties.gpu_preference =
2122
static_cast<FlutterDesktopGpuPreference>(project.gpu_preference());
22-
c_engine_properties.merged_platform_ui_thread =
23-
project.merged_platform_ui_thread();
23+
c_engine_properties.ui_thread_policy =
24+
static_cast<FlutterDesktopUIThreadPolicy>(project.ui_thread_policy());
2425

2526
const std::vector<std::string>& entrypoint_args =
2627
project.dart_entrypoint_arguments();

engine/src/flutter/shell/platform/windows/client_wrapper/include/flutter/dart_project.h

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,17 @@ enum class GpuPreference {
2020
LowPowerPreference,
2121
};
2222

23+
// Configures the thread policy for running the UI isolate.
24+
enum class UIThreadPolicy {
25+
// Default value. Currently will run the UI isolate on separate thread,
26+
// later will be changed to running the UI isolate on platform thread.
27+
Default,
28+
// Run the UI isolate on platform thread.
29+
RunOnPlatformThread,
30+
// Run the UI isolate on a separate thread.
31+
RunOnSeparateThread,
32+
};
33+
2334
// A set of Flutter and Dart assets used to initialize a Flutter engine.
2435
class DartProject {
2536
public:
@@ -90,17 +101,14 @@ class DartProject {
90101
// Defaults to NoPreference.
91102
GpuPreference gpu_preference() const { return gpu_preference_; }
92103

93-
// Sets whether the UI isolate should run on the platform thread.
94-
// In a future release, this setting will become a no-op when
95-
// Flutter Windows requires merged platform and UI threads.
96-
void set_merged_platform_ui_thread(bool merged_platform_ui_thread) {
97-
merged_platform_ui_thread_ = merged_platform_ui_thread;
104+
// Sets the thread policy for UI isolate.
105+
void set_ui_thread_policy(UIThreadPolicy policy) {
106+
ui_thread_policy_ = policy;
98107
}
99108

100-
// Returns whether the UI isolate should run on the platform thread.
101-
// Defaults to false. In a future release, this setting will default
102-
// to true.
103-
bool merged_platform_ui_thread() const { return merged_platform_ui_thread_; }
109+
// Returns the policy for UI isolate.
110+
// Defaults to UIThreadPolicy::Default.
111+
UIThreadPolicy ui_thread_policy() const { return ui_thread_policy_; }
104112

105113
private:
106114
// Accessors for internals are private, so that they can be changed if more
@@ -128,8 +136,8 @@ class DartProject {
128136
std::vector<std::string> dart_entrypoint_arguments_;
129137
// The preference for GPU to be used by flutter engine.
130138
GpuPreference gpu_preference_ = GpuPreference::NoPreference;
131-
// Whether the UI isolate should run on the platform thread.
132-
bool merged_platform_ui_thread_ = false;
139+
// Thread policy for UI isolate.
140+
UIThreadPolicy ui_thread_policy_ = UIThreadPolicy::Default;
133141
};
134142

135143
} // namespace flutter

engine/src/flutter/shell/platform/windows/fixtures/main.dart

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -391,3 +391,8 @@ void onMetricsChangedSignalViewIds() {
391391

392392
signal();
393393
}
394+
395+
@pragma('vm:entry-point')
396+
void mergedUIThread() {
397+
signal();
398+
}

engine/src/flutter/shell/platform/windows/flutter_project_bundle.cc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,8 @@ FlutterProjectBundle::FlutterProjectBundle(
3232
gpu_preference_ =
3333
static_cast<FlutterGpuPreference>(properties.gpu_preference);
3434

35-
merged_platform_ui_thread_ = properties.merged_platform_ui_thread;
35+
ui_thread_policy_ =
36+
static_cast<FlutterUIThreadPolicy>(properties.ui_thread_policy);
3637

3738
// Resolve any relative paths.
3839
if (assets_path_.is_relative() || icu_path_.is_relative() ||

engine/src/flutter/shell/platform/windows/flutter_project_bundle.h

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,12 @@ enum class FlutterGpuPreference {
2222
LowPowerPreference,
2323
};
2424

25+
enum class FlutterUIThreadPolicy {
26+
Default,
27+
RunOnPlatformThread,
28+
RunOnSeparateThread,
29+
};
30+
2531
// The data associated with a Flutter project needed to run it in an engine.
2632
class FlutterProjectBundle {
2733
public:
@@ -67,8 +73,8 @@ class FlutterProjectBundle {
6773
// Returns the app's GPU preference.
6874
FlutterGpuPreference gpu_preference() const { return gpu_preference_; }
6975

70-
// Whether the UI isolate should be running on the platform thread.
71-
bool merged_platform_ui_thread() const { return merged_platform_ui_thread_; }
76+
// Returns thread policy for running the UI isolate.
77+
FlutterUIThreadPolicy ui_thread_policy() { return ui_thread_policy_; }
7278

7379
private:
7480
std::filesystem::path assets_path_;
@@ -89,8 +95,8 @@ class FlutterProjectBundle {
8995
// App's GPU preference.
9096
FlutterGpuPreference gpu_preference_;
9197

92-
// Whether the UI isolate should be running on the platform thread.
93-
bool merged_platform_ui_thread_;
98+
// Thread policy for running the UI isolate.
99+
FlutterUIThreadPolicy ui_thread_policy_;
94100
};
95101

96102
} // namespace flutter

engine/src/flutter/shell/platform/windows/flutter_windows_engine.cc

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
#include "flutter/shell/platform/windows/system_utils.h"
2727
#include "flutter/shell/platform/windows/task_runner.h"
2828
#include "flutter/third_party/accessibility/ax/ax_node.h"
29+
#include "shell/platform/windows/flutter_project_bundle.h"
2930

3031
// winbase.h defines GetCurrentTime as a macro.
3132
#undef GetCurrentTime
@@ -294,7 +295,8 @@ bool FlutterWindowsEngine::Run(std::string_view entrypoint) {
294295
custom_task_runners.thread_priority_setter =
295296
&WindowsPlatformThreadPrioritySetter;
296297

297-
if (project_->merged_platform_ui_thread()) {
298+
if (project_->ui_thread_policy() ==
299+
FlutterUIThreadPolicy::RunOnPlatformThread) {
298300
FML_LOG(WARNING)
299301
<< "Running with merged platform and UI thread. Experimental.";
300302
custom_task_runners.ui_task_runner = &platform_task_runner;

engine/src/flutter/shell/platform/windows/flutter_windows_engine_unittests.cc

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// Use of this source code is governed by a BSD-style license that can be
33
// found in the LICENSE file.
44

5+
#include <thread>
56
#include "flutter/shell/platform/windows/flutter_windows_engine.h"
67

78
#include "flutter/fml/logging.h"
@@ -27,6 +28,18 @@
2728
// winbase.h defines GetCurrentTime as a macro.
2829
#undef GetCurrentTime
2930

31+
namespace {
32+
// Process the next win32 message if there is one. This can be used to
33+
// pump the Windows platform thread task runner.
34+
void PumpMessage() {
35+
::MSG msg;
36+
if (::GetMessage(&msg, nullptr, 0, 0)) {
37+
::TranslateMessage(&msg);
38+
::DispatchMessage(&msg);
39+
}
40+
}
41+
} // namespace
42+
3043
namespace flutter {
3144
namespace testing {
3245

@@ -1313,5 +1326,25 @@ TEST_F(FlutterWindowsEngineTest, RemoveViewFailureDoesNotHang) {
13131326
"FlutterEngineRemoveView returned an unexpected result");
13141327
}
13151328

1329+
TEST_F(FlutterWindowsEngineTest, MergedUIThread) {
1330+
auto& context = GetContext();
1331+
WindowsConfigBuilder builder{context};
1332+
builder.SetDartEntrypoint("mergedUIThread");
1333+
builder.SetUIThreadPolicy(FlutterDesktopUIThreadPolicy::RunOnPlatformThread);
1334+
1335+
std::optional<std::thread::id> ui_thread_id;
1336+
1337+
auto native_entry = CREATE_NATIVE_ENTRY([&](Dart_NativeArguments args) {
1338+
ui_thread_id = std::this_thread::get_id();
1339+
});
1340+
context.AddNativeFunction("Signal", native_entry);
1341+
1342+
EnginePtr engine{builder.RunHeadless()};
1343+
while (!ui_thread_id) {
1344+
PumpMessage();
1345+
}
1346+
ASSERT_EQ(*ui_thread_id, std::this_thread::get_id());
1347+
}
1348+
13161349
} // namespace testing
13171350
} // namespace flutter

engine/src/flutter/shell/platform/windows/public/flutter_windows.h

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,17 @@ typedef enum {
4545
LowPowerPreference,
4646
} FlutterDesktopGpuPreference;
4747

48+
// Configures the thread policy for running the UI isolate.
49+
typedef enum {
50+
// Default value. Currently will run the UI isolate on separate thread,
51+
// later will be changed to running the UI isolate on platform thread.
52+
Default,
53+
// Run the UI isolate on platform thread.
54+
RunOnPlatformThread,
55+
// Run the UI isolate on a separate thread.
56+
RunOnSeparateThread,
57+
} FlutterDesktopUIThreadPolicy;
58+
4859
// Properties for configuring a Flutter engine instance.
4960
typedef struct {
5061
// The path to the flutter_assets folder for the application to be run.
@@ -81,8 +92,8 @@ typedef struct {
8192
// GPU choice preference
8293
FlutterDesktopGpuPreference gpu_preference;
8394

84-
// Whether the UI isolate should run on the platform thread.
85-
bool merged_platform_ui_thread;
95+
// Policy for the thread that runs UI isolate.
96+
FlutterDesktopUIThreadPolicy ui_thread_policy;
8697
} FlutterDesktopEngineProperties;
8798

8899
// ========== View Controller ==========

engine/src/flutter/shell/platform/windows/testing/windows_test_config_builder.cc

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,11 @@ void WindowsConfigBuilder::SetDartEntrypoint(std::string_view entrypoint) {
3030
dart_entrypoint_ = entrypoint;
3131
}
3232

33+
void WindowsConfigBuilder::SetUIThreadPolicy(
34+
FlutterDesktopUIThreadPolicy policy) {
35+
ui_thread_policy_ = policy;
36+
}
37+
3338
void WindowsConfigBuilder::AddDartEntrypointArgument(std::string_view arg) {
3439
if (arg.empty()) {
3540
return;
@@ -69,6 +74,7 @@ FlutterDesktopEngineProperties WindowsConfigBuilder::GetEngineProperties()
6974
}
7075

7176
engine_properties.gpu_preference = gpu_preference_;
77+
engine_properties.ui_thread_policy = ui_thread_policy_;
7278

7379
return engine_properties;
7480
}

engine/src/flutter/shell/platform/windows/testing/windows_test_config_builder.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,10 @@ class WindowsConfigBuilder {
5858
// must be decorated with `@pragma('vm:entry-point')`.
5959
void SetDartEntrypoint(std::string_view entrypoint);
6060

61+
// Set the UI Thread policy for the engine.
62+
// If not set defaults to FlutterDesktopUIThreadPolicy::Default;
63+
void SetUIThreadPolicy(FlutterDesktopUIThreadPolicy policy);
64+
6165
// Adds an argument to the Dart entrypoint arguments List<String>.
6266
void AddDartEntrypointArgument(std::string_view arg);
6367

@@ -86,6 +90,8 @@ class WindowsConfigBuilder {
8690
WindowsTestContext& context_;
8791
std::string dart_entrypoint_;
8892
std::vector<std::string> dart_entrypoint_arguments_;
93+
FlutterDesktopUIThreadPolicy ui_thread_policy_ =
94+
FlutterDesktopUIThreadPolicy::Default;
8995

9096
FlutterDesktopGpuPreference gpu_preference_ =
9197
FlutterDesktopGpuPreference::NoPreference;

0 commit comments

Comments
 (0)