Skip to content

Commit

Permalink
Log non-kSuccess returns from embedder API calls. (flutter#8096)
Browse files Browse the repository at this point in the history 
Embedders don’t realize that some calls to the API return an error and don’t handle the same. Log such erroneous returns.
  • Loading branch information
@chinmaygarde
chinmaygarde authored Mar 10, 2019
1 parent 14d1584 commit a28b530
Showing 1 changed file with 45 additions and 35 deletions.
80 changes: 45 additions & 35 deletions shell/platform/embedder/embedder.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,16 @@ extern const intptr_t kPlatformStrongDillSize;
return static_cast<decltype(pointer->member)>((default_value)); \
})()

#define LOG_EMBEDDER_ERROR(code) \
({ \
do { \
FML_LOG(ERROR) << "Returning error '" << #code << "' (" << code \
<< ") from Flutter Embedder API call to '" \
<< __FUNCTION__ << "'."; \
} while (0); \
(code); \
})

static bool IsOpenGLRendererConfigValid(const FlutterRendererConfig* config) {
if (config->type != kOpenGL) {
return false;
Expand Down Expand Up @@ -308,19 +318,19 @@ FlutterEngineResult FlutterEngineRun(size_t version,
FlutterEngine* engine_out) {
// Step 0: Figure out arguments for shell creation.
if (version != FLUTTER_ENGINE_VERSION) {
return kInvalidLibraryVersion;
return LOG_EMBEDDER_ERROR(kInvalidLibraryVersion);
}

if (engine_out == nullptr) {
return kInvalidArguments;
return LOG_EMBEDDER_ERROR(kInvalidArguments);
}

if (args == nullptr) {
return kInvalidArguments;
return LOG_EMBEDDER_ERROR(kInvalidArguments);
}

if (SAFE_ACCESS(args, assets_path, nullptr) == nullptr) {
return kInvalidArguments;
return LOG_EMBEDDER_ERROR(kInvalidArguments);
}

if (SAFE_ACCESS(args, main_path__unused__, nullptr) != nullptr) {
Expand All @@ -334,7 +344,7 @@ FlutterEngineResult FlutterEngineRun(size_t version,
}

if (!IsRendererValid(config)) {
return kInvalidArguments;
return LOG_EMBEDDER_ERROR(kInvalidArguments);
}

std::string icu_data_path;
Expand Down Expand Up @@ -375,7 +385,7 @@ FlutterEngineResult FlutterEngineRun(size_t version,
if (!fml::IsFile(application_kernel_path)) {
FML_LOG(ERROR) << "Not running in AOT mode but could not resolve the "
"kernel binary.";
return kInvalidArguments;
return LOG_EMBEDDER_ERROR(kInvalidArguments);
}
settings.application_kernel_asset = kApplicationKernelSnapshotFileName;
}
Expand Down Expand Up @@ -529,7 +539,7 @@ FlutterEngineResult FlutterEngineRun(size_t version,
config, user_data, platform_dispatch_table);

if (!on_create_platform_view) {
return kInvalidArguments;
return LOG_EMBEDDER_ERROR(kInvalidArguments);
}

shell::Shell::CreateCallback<shell::Rasterizer> on_create_rasterizer =
Expand Down Expand Up @@ -599,12 +609,12 @@ FlutterEngineResult FlutterEngineRun(size_t version,
);

if (!embedder_engine->IsValid()) {
return kInvalidArguments;
return LOG_EMBEDDER_ERROR(kInvalidArguments);
}

// Step 2: Setup the rendering surface.
if (!embedder_engine->NotifyCreated()) {
return kInvalidArguments;
return LOG_EMBEDDER_ERROR(kInvalidArguments);
}

// Step 3: Run the engine.
Expand All @@ -618,11 +628,11 @@ FlutterEngineResult FlutterEngineRun(size_t version,
std::make_unique<blink::DirectoryAssetBundle>(fml::OpenDirectory(
settings.assets_path.c_str(), false, fml::FilePermission::kRead)));
if (!run_configuration.IsValid()) {
return kInvalidArguments;
return LOG_EMBEDDER_ERROR(kInvalidArguments);
}

if (!embedder_engine->Run(std::move(run_configuration))) {
return kInvalidArguments;
return LOG_EMBEDDER_ERROR(kInvalidArguments);
}

// Finally! Release the ownership of the embedder engine to the caller.
Expand All @@ -632,7 +642,7 @@ FlutterEngineResult FlutterEngineRun(size_t version,

FlutterEngineResult FlutterEngineShutdown(FlutterEngine engine) {
if (engine == nullptr) {
return kInvalidArguments;
return LOG_EMBEDDER_ERROR(kInvalidArguments);
}
auto embedder_engine = reinterpret_cast<shell::EmbedderEngine*>(engine);
embedder_engine->NotifyDestroyed();
Expand All @@ -644,7 +654,7 @@ FlutterEngineResult FlutterEngineSendWindowMetricsEvent(
FlutterEngine engine,
const FlutterWindowMetricsEvent* flutter_metrics) {
if (engine == nullptr || flutter_metrics == nullptr) {
return kInvalidArguments;
return LOG_EMBEDDER_ERROR(kInvalidArguments);
}

blink::ViewportMetrics metrics;
Expand All @@ -656,7 +666,7 @@ FlutterEngineResult FlutterEngineSendWindowMetricsEvent(
return reinterpret_cast<shell::EmbedderEngine*>(engine)->SetViewportMetrics(
std::move(metrics))
? kSuccess
: kInvalidArguments;
: LOG_EMBEDDER_ERROR(kInvalidArguments);
}

// Returns the blink::PointerData::Change for the given FlutterPointerPhase.
Expand Down Expand Up @@ -699,7 +709,7 @@ FlutterEngineResult FlutterEngineSendPointerEvent(
const FlutterPointerEvent* pointers,
size_t events_count) {
if (engine == nullptr || pointers == nullptr || events_count == 0) {
return kInvalidArguments;
return LOG_EMBEDDER_ERROR(kInvalidArguments);
}

auto packet = std::make_unique<blink::PointerDataPacket>(events_count);
Expand Down Expand Up @@ -728,19 +738,19 @@ FlutterEngineResult FlutterEngineSendPointerEvent(
return reinterpret_cast<shell::EmbedderEngine*>(engine)
->DispatchPointerDataPacket(std::move(packet))
? kSuccess
: kInvalidArguments;
: LOG_EMBEDDER_ERROR(kInvalidArguments);
}

FlutterEngineResult FlutterEngineSendPlatformMessage(
FlutterEngine engine,
const FlutterPlatformMessage* flutter_message) {
if (engine == nullptr || flutter_message == nullptr) {
return kInvalidArguments;
return LOG_EMBEDDER_ERROR(kInvalidArguments);
}

if (SAFE_ACCESS(flutter_message, channel, nullptr) == nullptr ||
SAFE_ACCESS(flutter_message, message, nullptr) == nullptr) {
return kInvalidArguments;
return LOG_EMBEDDER_ERROR(kInvalidArguments);
}

auto message = fml::MakeRefCounted<blink::PlatformMessage>(
Expand All @@ -753,7 +763,7 @@ FlutterEngineResult FlutterEngineSendPlatformMessage(
return reinterpret_cast<shell::EmbedderEngine*>(engine)->SendPlatformMessage(
std::move(message))
? kSuccess
: kInvalidArguments;
: LOG_EMBEDDER_ERROR(kInvalidArguments);
}

FlutterEngineResult FlutterEngineSendPlatformMessageResponse(
Expand All @@ -762,7 +772,7 @@ FlutterEngineResult FlutterEngineSendPlatformMessageResponse(
const uint8_t* data,
size_t data_length) {
if (data_length != 0 && data == nullptr) {
return kInvalidArguments;
return LOG_EMBEDDER_ERROR(kInvalidArguments);
}

auto response = handle->message->response();
Expand All @@ -788,11 +798,11 @@ FlutterEngineResult FlutterEngineRegisterExternalTexture(
FlutterEngine engine,
int64_t texture_identifier) {
if (engine == nullptr || texture_identifier == 0) {
return kInvalidArguments;
return LOG_EMBEDDER_ERROR(kInvalidArguments);
}
if (!reinterpret_cast<shell::EmbedderEngine*>(engine)->RegisterTexture(
texture_identifier)) {
return kInternalInconsistency;
return LOG_EMBEDDER_ERROR(kInternalInconsistency);
}
return kSuccess;
}
Expand All @@ -806,7 +816,7 @@ FlutterEngineResult FlutterEngineUnregisterExternalTexture(

if (!reinterpret_cast<shell::EmbedderEngine*>(engine)->UnregisterTexture(
texture_identifier)) {
return kInternalInconsistency;
return LOG_EMBEDDER_ERROR(kInternalInconsistency);
}

return kSuccess;
Expand All @@ -816,23 +826,23 @@ FlutterEngineResult FlutterEngineMarkExternalTextureFrameAvailable(
FlutterEngine engine,
int64_t texture_identifier) {
if (engine == nullptr || texture_identifier == 0) {
return kInvalidArguments;
return LOG_EMBEDDER_ERROR(kInvalidArguments);
}
if (!reinterpret_cast<shell::EmbedderEngine*>(engine)
->MarkTextureFrameAvailable(texture_identifier)) {
return kInternalInconsistency;
return LOG_EMBEDDER_ERROR(kInternalInconsistency);
}
return kSuccess;
}

FlutterEngineResult FlutterEngineUpdateSemanticsEnabled(FlutterEngine engine,
bool enabled) {
if (engine == nullptr) {
return kInvalidArguments;
return LOG_EMBEDDER_ERROR(kInvalidArguments);
}
if (!reinterpret_cast<shell::EmbedderEngine*>(engine)->SetSemanticsEnabled(
enabled)) {
return kInternalInconsistency;
return LOG_EMBEDDER_ERROR(kInternalInconsistency);
}
return kSuccess;
}
Expand All @@ -841,11 +851,11 @@ FlutterEngineResult FlutterEngineUpdateAccessibilityFeatures(
FlutterEngine engine,
FlutterAccessibilityFeature flags) {
if (engine == nullptr) {
return kInvalidArguments;
return LOG_EMBEDDER_ERROR(kInvalidArguments);
}
if (!reinterpret_cast<shell::EmbedderEngine*>(engine)
->SetAccessibilityFeatures(flags)) {
return kInternalInconsistency;
return LOG_EMBEDDER_ERROR(kInternalInconsistency);
}
return kSuccess;
}
Expand All @@ -857,14 +867,14 @@ FlutterEngineResult FlutterEngineDispatchSemanticsAction(
const uint8_t* data,
size_t data_length) {
if (engine == nullptr) {
return kInvalidArguments;
return LOG_EMBEDDER_ERROR(kInvalidArguments);
}
auto engine_action = static_cast<blink::SemanticsAction>(action);
if (!reinterpret_cast<shell::EmbedderEngine*>(engine)
->DispatchSemanticsAction(
id, engine_action,
std::vector<uint8_t>({data, data + data_length}))) {
return kInternalInconsistency;
return LOG_EMBEDDER_ERROR(kInternalInconsistency);
}
return kSuccess;
}
Expand All @@ -874,7 +884,7 @@ FlutterEngineResult FlutterEngineOnVsync(FlutterEngine engine,
uint64_t frame_start_time_nanos,
uint64_t frame_target_time_nanos) {
if (engine == nullptr) {
return kInvalidArguments;
return LOG_EMBEDDER_ERROR(kInvalidArguments);
}

auto start_time = fml::TimePoint::FromEpochDelta(
Expand All @@ -885,7 +895,7 @@ FlutterEngineResult FlutterEngineOnVsync(FlutterEngine engine,

if (!reinterpret_cast<shell::EmbedderEngine*>(engine)->OnVsyncEvent(
baton, start_time, target_time)) {
return kInternalInconsistency;
return LOG_EMBEDDER_ERROR(kInternalInconsistency);
}

return kSuccess;
Expand All @@ -907,13 +917,13 @@ FlutterEngineResult FlutterEnginePostRenderThreadTask(FlutterEngine engine,
VoidCallback callback,
void* baton) {
if (engine == nullptr || callback == nullptr) {
return kInvalidArguments;
return LOG_EMBEDDER_ERROR(kInvalidArguments);
}

auto task = [callback, baton]() { callback(baton); };

return reinterpret_cast<shell::EmbedderEngine*>(engine)->PostRenderThreadTask(
task)
? kSuccess
: kInternalInconsistency;
: LOG_EMBEDDER_ERROR(kInternalInconsistency);
}

0 comments on commit a28b530

Please sign in to comment.