This repository was archived by the owner on Feb 25, 2025. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 6k
Remove get engine #9747
Merged
Merged
Remove get engine #9747
Changes from all commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
593c9b3
Make platform_view_android_jni delegate to platform_view_android
dnfield d3b1538
just SetViewportMetrics
dnfield d6e773d
Don't give shell holders access to the engine
dnfield 650a309
make sure android and ios compile, move logging ot shell, run on plat…
dnfield 907adc3
merge
dnfield d208da1
Merge remote-tracking branch 'upstream/master' into remove_get_engine
dnfield 15d8eb6
Fix tester, fix test, add back ui isolate methods and expose through …
dnfield df66441
add it back and deprecate
dnfield f543223
undeprecate, guard for fuchsia
dnfield 97194c1
Merge remote-tracking branch 'upstream/master' into remove_get_engine
dnfield e510c32
review
dnfield 9036750
fix deadlock in tester_main
dnfield d8d508c
One miss
dnfield File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,7 +3,6 @@ | |
| // found in the LICENSE file. | ||
|
|
||
| #define RAPIDJSON_HAS_STDSTRING 1 | ||
|
|
||
| #include "flutter/shell/common/shell.h" | ||
|
|
||
| #include <memory> | ||
|
|
@@ -372,6 +371,79 @@ void Shell::NotifyLowMemoryWarning() const { | |
| // to purge them. | ||
| } | ||
|
|
||
| void Shell::RunEngine(RunConfiguration run_configuration) { | ||
| RunEngine(std::move(run_configuration), nullptr); | ||
| } | ||
|
|
||
| void Shell::RunEngine(RunConfiguration run_configuration, | ||
| std::function<void(Engine::RunStatus)> result_callback) { | ||
| auto result = [platform_runner = task_runners_.GetPlatformTaskRunner(), | ||
| result_callback](Engine::RunStatus run_result) { | ||
| if (!result_callback) { | ||
| return; | ||
| } | ||
| platform_runner->PostTask( | ||
| [result_callback, run_result]() { result_callback(run_result); }); | ||
| }; | ||
| FML_DCHECK(is_setup_); | ||
| FML_DCHECK(task_runners_.GetPlatformTaskRunner()->RunsTasksOnCurrentThread()); | ||
|
|
||
| if (!weak_engine_) { | ||
| result(Engine::RunStatus::Failure); | ||
| } | ||
| fml::TaskRunner::RunNowOrPostTask( | ||
| task_runners_.GetUITaskRunner(), | ||
| fml::MakeCopyable( | ||
| [run_configuration = std::move(run_configuration), | ||
| weak_engine = weak_engine_, result]() mutable { | ||
| if (!weak_engine) { | ||
| FML_LOG(ERROR) | ||
| << "Could not launch engine with configuration - no engine."; | ||
| result(Engine::RunStatus::Failure); | ||
| return; | ||
| } | ||
| auto run_result = weak_engine->Run(std::move(run_configuration)); | ||
| if (run_result == flutter::Engine::RunStatus::Failure) { | ||
| FML_LOG(ERROR) << "Could not launch engine with configuration."; | ||
| } | ||
| result(run_result); | ||
| })); | ||
| } | ||
|
|
||
| std::optional<DartErrorCode> Shell::GetUIIsolateLastError() const { | ||
| FML_DCHECK(is_setup_); | ||
| FML_DCHECK(task_runners_.GetPlatformTaskRunner()->RunsTasksOnCurrentThread()); | ||
|
|
||
| // We're using the unique_ptr here because we're sure we're on the Platform | ||
| // Thread and callers expect this to be synchronous. | ||
| if (!engine_) { | ||
| return std::nullopt; | ||
| } | ||
| switch (engine_->GetUIIsolateLastError()) { | ||
| case tonic::kCompilationErrorType: | ||
| return DartErrorCode::CompilationError; | ||
| case tonic::kApiErrorType: | ||
| return DartErrorCode::ApiError; | ||
| case tonic::kUnknownErrorType: | ||
| return DartErrorCode::UnknownError; | ||
| case tonic::kNoError: | ||
| return DartErrorCode::NoError; | ||
| } | ||
| return DartErrorCode::UnknownError; | ||
| } | ||
|
|
||
| bool Shell::EngineHasLivePorts() const { | ||
|
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 on the comment in |
||
| FML_DCHECK(is_setup_); | ||
| FML_DCHECK(task_runners_.GetPlatformTaskRunner()->RunsTasksOnCurrentThread()); | ||
|
|
||
| // We're using the unique_ptr here because we're sure we're on the Platform | ||
| // Thread and callers expect this to be synchronous. | ||
| if (!engine_) { | ||
| return false; | ||
| } | ||
| return engine_->UIIsolateHasLivePorts(); | ||
| } | ||
|
|
||
| bool Shell::IsSetup() const { | ||
| return is_setup_; | ||
| } | ||
|
|
@@ -425,10 +497,14 @@ fml::WeakPtr<Rasterizer> Shell::GetRasterizer() { | |
| return weak_rasterizer_; | ||
| } | ||
|
|
||
| // TODO(dnfield): Remove this when either Topaz is up to date or flutter_runner | ||
| // is built out of this repo. | ||
| #ifdef OS_FUCHSIA | ||
| fml::WeakPtr<Engine> Shell::GetEngine() { | ||
| FML_DCHECK(is_setup_); | ||
| return weak_engine_; | ||
| } | ||
| #endif // OS_FUCHSIA | ||
|
|
||
| fml::WeakPtr<PlatformView> Shell::GetPlatformView() { | ||
| FML_DCHECK(is_setup_); | ||
|
|
@@ -907,7 +983,7 @@ void Shell::ReportTimings() { | |
|
|
||
| auto timings = std::move(unreported_timings_); | ||
| unreported_timings_ = {}; | ||
| task_runners_.GetUITaskRunner()->PostTask([timings, engine = GetEngine()] { | ||
| task_runners_.GetUITaskRunner()->PostTask([timings, engine = weak_engine_] { | ||
| if (engine) { | ||
| engine->ReportTimings(std::move(timings)); | ||
| } | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 think it would be better to have the result callback be executed on the calling task runner (the platform task runner in this case). Since doing the re-threading at the each possible call-site is tedious, you can wrap the same in and closure that does the re-threading and then pass that around as normal. See the image decoder for a use of this pattern.
On a related note, since forgetting to invoke the closure in the error case is possible, maybe we should have an RAII wrapper for the same. I have a prototype for this in a different patch that is not in yet.
That way, you can wrap the callback in a closure that does the re-threading and put it in that RAII wrapper. You could then invoke the same only in the success case and ensure that all erroneous cases invoke the callback and on the right thread.
Something like the following should work for now:
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