Skip to content

Commit d28f3af

Browse files
authored
Clean up EmbedderExternalViewEmbedder creation (flutter#169962)
When creating an EmbedderExternalViewController, we were returning a std::pair of the newly created instance and a bool, where the bool indicated whether creation was successful or not. When true, creation was unsuccessful and we should halt the engine launch. When false, creation was successful and we should continue the engine launch. Instead, we now return fml::StatusOr. When not StatusOr::ok(), we halt engine launch, otherwise we continue. No tests changed since this is a minor cleanup with no semantic change. ## 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 wondered why the hell that first baillout was returning {nullptr, false} too. - [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
1 parent 95cb7ca commit d28f3af

File tree

1 file changed

+18
-14
lines changed

1 file changed

+18
-14
lines changed

engine/src/flutter/shell/platform/embedder/embedder.cc

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
#include "flutter/fml/closure.h"
1717
#include "flutter/fml/make_copyable.h"
1818
#include "flutter/fml/native_library.h"
19+
#include "flutter/fml/status_or.h"
1920
#include "flutter/fml/thread.h"
2021
#include "third_party/dart/runtime/bin/elf_loader.h"
2122
#include "third_party/dart/runtime/include/dart_native_api.h"
@@ -1529,12 +1530,14 @@ CreateEmbedderRenderTarget(
15291530
return render_target;
15301531
}
15311532

1532-
static std::pair<std::unique_ptr<flutter::EmbedderExternalViewEmbedder>,
1533-
bool /* halt engine launch if true */>
1533+
/// Creates an EmbedderExternalViewEmbedder.
1534+
///
1535+
/// When a non-OK status is returned, engine startup should be halted.
1536+
static fml::StatusOr<std::unique_ptr<flutter::EmbedderExternalViewEmbedder>>
15341537
InferExternalViewEmbedderFromArgs(const FlutterCompositor* compositor,
15351538
bool enable_impeller) {
15361539
if (compositor == nullptr) {
1537-
return {nullptr, false};
1540+
return std::unique_ptr<flutter::EmbedderExternalViewEmbedder>{nullptr};
15381541
}
15391542

15401543
auto c_create_callback =
@@ -1550,15 +1553,15 @@ InferExternalViewEmbedderFromArgs(const FlutterCompositor* compositor,
15501553

15511554
// Make sure the required callbacks are present
15521555
if (!c_create_callback || !c_collect_callback) {
1553-
FML_LOG(ERROR) << "Required compositor callbacks absent.";
1554-
return {nullptr, true};
1556+
return fml::Status(fml::StatusCode::kInvalidArgument,
1557+
"Required compositor callbacks absent.");
15551558
}
15561559
// Either the present view or the present layers callback must be provided.
15571560
if ((!c_present_view_callback && !c_present_callback) ||
15581561
(c_present_view_callback && c_present_callback)) {
1559-
FML_LOG(ERROR) << "Either present_layers_callback or present_view_callback "
1560-
"must be provided but not both.";
1561-
return {nullptr, true};
1562+
return fml::Status(fml::StatusCode::kInvalidArgument,
1563+
"Either present_layers_callback or "
1564+
"present_view_callback must be provided but not both.");
15621565
}
15631566

15641567
FlutterCompositor captured_compositor = *compositor;
@@ -1601,10 +1604,9 @@ InferExternalViewEmbedderFromArgs(const FlutterCompositor* compositor,
16011604
};
16021605
}
16031606

1604-
return {std::make_unique<flutter::EmbedderExternalViewEmbedder>(
1605-
avoid_backing_store_cache, create_render_target_callback,
1606-
present_callback),
1607-
false};
1607+
return std::make_unique<flutter::EmbedderExternalViewEmbedder>(
1608+
avoid_backing_store_cache, create_render_target_callback,
1609+
present_callback);
16081610
}
16091611

16101612
// Translates embedder metrics to engine metrics, or returns a string on error.
@@ -2258,7 +2260,8 @@ FlutterEngineResult FlutterEngineInitialize(size_t version,
22582260

22592261
auto external_view_embedder_result = InferExternalViewEmbedderFromArgs(
22602262
SAFE_ACCESS(args, compositor, nullptr), settings.enable_impeller);
2261-
if (external_view_embedder_result.second) {
2263+
if (!external_view_embedder_result.ok()) {
2264+
FML_LOG(ERROR) << external_view_embedder_result.status().message();
22622265
return LOG_EMBEDDER_ERROR(kInvalidArguments,
22632266
"Compositor arguments were invalid.");
22642267
}
@@ -2276,7 +2279,8 @@ FlutterEngineResult FlutterEngineInitialize(size_t version,
22762279

22772280
auto on_create_platform_view = InferPlatformViewCreationCallback(
22782281
config, user_data, platform_dispatch_table,
2279-
std::move(external_view_embedder_result.first), settings.enable_impeller);
2282+
std::move(external_view_embedder_result.value()),
2283+
settings.enable_impeller);
22802284

22812285
if (!on_create_platform_view) {
22822286
return LOG_EMBEDDER_ERROR(

0 commit comments

Comments
 (0)