-
Notifications
You must be signed in to change notification settings - Fork 6k
Add flushUIThreadTasks service RPC #3898
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -128,6 +128,9 @@ void PlatformViewServiceProtocol::RegisterHook(bool running_precompiled_code) { | |
| } | ||
| Dart_RegisterRootServiceRequestCallback(kRunInViewExtensionName, &RunInView, | ||
| nullptr); | ||
| // [benchmark helper] Wait for the UI Thread to idle. | ||
| Dart_RegisterRootServiceRequestCallback(kWaitUIThreadIdleExtensionName, | ||
| &WaitUIThreadIdle, nullptr); | ||
| } | ||
|
|
||
| const char* PlatformViewServiceProtocol::kRunInViewExtensionName = | ||
|
|
@@ -202,11 +205,10 @@ bool PlatformViewServiceProtocol::ListViews(const char* method, | |
| intptr_t num_params, | ||
| void* user_data, | ||
| const char** json_object) { | ||
| // Ask the Shell for the list of platform views. This will run a task on | ||
| // the UI thread before returning. | ||
| // Ask the Shell for the list of platform views. | ||
| Shell& shell = Shell::Shared(); | ||
| std::vector<Shell::PlatformViewInfo> platform_views; | ||
| shell.WaitForPlatformViewIds(&platform_views); | ||
| shell.GetPlatformViewIds(&platform_views); | ||
|
|
||
| std::stringstream response; | ||
|
|
||
|
|
@@ -314,4 +316,29 @@ void PlatformViewServiceProtocol::ScreenshotGpuTask(SkBitmap* bitmap) { | |
| canvas->flush(); | ||
| } | ||
|
|
||
| const char* PlatformViewServiceProtocol::kWaitUIThreadIdleExtensionName = | ||
| "_flutter.waitUIThreadIdle"; | ||
|
|
||
| // This API should not be invoked by production code. | ||
| // It can potentially starve the service isolate if the main isolate pauses | ||
| // at a breakpoint or is in an infinite loop. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Might be worth mentioning that this method must be invoked from the VM service thread and blocks it until UI thread tasks are processed.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we add this description even to the other services? some of them are blocking too.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That would be great. We have comments like that in
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
| bool PlatformViewServiceProtocol::WaitUIThreadIdle(const char* method, | ||
| const char** param_keys, | ||
| const char** param_values, | ||
| intptr_t num_params, | ||
| void* user_data, | ||
| const char** json_object) { | ||
| ftl::AutoResetWaitableEvent latch; | ||
| blink::Threads::UI()->PostTask([&latch]() { | ||
| // This task is empty because we just need to synchronize this RPC with the | ||
| // UI Thread | ||
| latch.Signal(); | ||
| }); | ||
|
|
||
| latch.Wait(); | ||
|
|
||
| *json_object = strdup("{\"type\":\"Success\"}"); | ||
| return true; | ||
| } | ||
|
|
||
| } // namespace shell | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -43,6 +43,17 @@ class PlatformViewServiceProtocol { | |
| void* user_data, | ||
| const char** json_object); | ||
| static void ScreenshotGpuTask(SkBitmap* bitmap); | ||
|
|
||
| // This API should not be invoked by production code. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ditto re calling from VM isolate thread
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
| // It can potentially starve the service isolate if the main isolate pauses | ||
| // at a breakpoint or is in an infinite loop. | ||
| static const char* kWaitUIThreadIdleExtensionName; | ||
| static bool WaitUIThreadIdle(const char* method, | ||
| const char** param_keys, | ||
| const char** param_values, | ||
| intptr_t num_params, | ||
| void* user_data, | ||
| const char** json_object); | ||
| }; | ||
|
|
||
| } // namespace shell | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Strictly speaking this is not waiting for the UI thread to idle it is more wait for UI thread to finish processing the first event, I don't have a better name for it though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about
FlushUIThreadTasks?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is not actually flushing all the tasks, but just the first one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you sure? The implementation calls
blink::Threads::UI()->PostTask, which will queue up a task. Given that the message queue is FIFO, all tasks that were scheduled prior to this task will execute first. No?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I misunderstood the meaning of Tasks, you are right. It will wait for all the previous tasks to be flushed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done