Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Make fl_engine_send_key_event into a standard async function. #57112

Merged
robert-ancell merged 1 commit into from
Dec 12, 2024
Merged

Make fl_engine_send_key_event into a standard async function. #57112

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 37 additions & 3 deletions shell/platform/linux/fl_engine.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -965,17 +965,51 @@ void fl_engine_send_pointer_pan_zoom_event(FlEngine* self,
self->embedder_api.SendPointerEvent(self->engine, &fl_event, 1);
}

static void send_key_event_cb(bool handled, void* user_data) {
g_autoptr(GTask) task = G_TASK(user_data);
gboolean* return_value = g_new0(gboolean, 1);
*return_value = handled;
g_task_return_pointer(task, return_value, g_free);
}

void fl_engine_send_key_event(FlEngine* self,
const FlutterKeyEvent* event,
FlutterKeyEventCallback callback,
void* user_data) {
GCancellable* cancellable,
GAsyncReadyCallback callback,
gpointer user_data) {
g_return_if_fail(FL_IS_ENGINE(self));

g_autoptr(GTask) task = g_task_new(self, cancellable, callback, user_data);

if (self->engine == nullptr) {
g_task_return_new_error(task, fl_engine_error_quark(),
FL_ENGINE_ERROR_FAILED, "No engine");
return;
}

self->embedder_api.SendKeyEvent(self->engine, event, callback, user_data);
if (self->embedder_api.SendKeyEvent(self->engine, event, send_key_event_cb,
g_object_ref(task)) != kSuccess) {
g_task_return_new_error(task, fl_engine_error_quark(),
FL_ENGINE_ERROR_FAILED, "Failed to send key event");
g_object_unref(task);
}
}

gboolean fl_engine_send_key_event_finish(FlEngine* self,
GAsyncResult* result,
gboolean* handled,
GError** error) {
g_return_val_if_fail(FL_IS_ENGINE(self), FALSE);
g_return_val_if_fail(g_task_is_valid(result, self), FALSE);

g_autofree gboolean* return_value =
static_cast<gboolean*>(g_task_propagate_pointer(G_TASK(result), error));
if (return_value == nullptr) {
return FALSE;
}

*handled = *return_value;
return TRUE;
}

void fl_engine_dispatch_semantics_action(FlEngine* self,
Expand Down
30 changes: 28 additions & 2 deletions shell/platform/linux/fl_engine_private.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -280,11 +280,37 @@ void fl_engine_send_pointer_pan_zoom_event(FlEngine* engine,

/**
* fl_engine_send_key_event:
* @engine: an #FlEngine.
* @event: key event to send.
* @cancellable: (allow-none): a #GCancellable or %NULL.
* @callback: (scope async): a #GAsyncReadyCallback to call when the request is
* satisfied.
* @user_data: (closure): user data to pass to @callback.
*
* Send a key event to the engine.
*/
void fl_engine_send_key_event(FlEngine* engine,
const FlutterKeyEvent* event,
FlutterKeyEventCallback callback,
void* user_data);
GCancellable* cancellable,
GAsyncReadyCallback callback,
gpointer user_data);

/**
* fl_engine_send_key_event_finish:
* @engine: an #FlEngine.
* @result: a #GAsyncResult.
* @handled: location to write if this event was handled by the engine.
* @error: (allow-none): #GError location to store the error occurring, or %NULL
* to ignore.
*
* Completes request started with fl_engine_send_key_event().
*
* Returns: %TRUE on success.
*/
gboolean fl_engine_send_key_event_finish(FlEngine* engine,
GAsyncResult* result,
gboolean* handled,
GError** error);

/**
* fl_engine_dispatch_semantics_action:
Expand Down
128 changes: 128 additions & 0 deletions shell/platform/linux/fl_engine_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -748,4 +748,132 @@ TEST(FlEngineTest, RemoveViewEngineError) {
g_main_loop_run(loop);
}

TEST(FlEngineTest, SendKeyEvent) {
g_autoptr(GMainLoop) loop = g_main_loop_new(nullptr, 0);

g_autoptr(FlEngine) engine = make_mock_engine();
FlutterEngineProcTable* embedder_api = fl_engine_get_embedder_api(engine);

bool called;
embedder_api->SendKeyEvent = MOCK_ENGINE_PROC(
SendKeyEvent,
([&called](auto engine, const FlutterKeyEvent* event,
FlutterKeyEventCallback callback, void* user_data) {
called = true;
EXPECT_EQ(event->timestamp, 1234);
EXPECT_EQ(event->type, kFlutterKeyEventTypeUp);
EXPECT_EQ(event->physical, static_cast<uint64_t>(42));
EXPECT_EQ(event->logical, static_cast<uint64_t>(123));
EXPECT_TRUE(event->synthesized);
EXPECT_EQ(event->device_type, kFlutterKeyEventDeviceTypeKeyboard);
callback(TRUE, user_data);
return kSuccess;
}));

FlutterKeyEvent event = {.struct_size = sizeof(FlutterKeyEvent),
.timestamp = 1234,
.type = kFlutterKeyEventTypeUp,
.physical = 42,
.logical = 123,
.character = nullptr,
.synthesized = true,
.device_type = kFlutterKeyEventDeviceTypeKeyboard};
fl_engine_send_key_event(
engine, &event, nullptr,
[](GObject* object, GAsyncResult* result, gpointer user_data) {
gboolean handled;
g_autoptr(GError) error = nullptr;
EXPECT_TRUE(fl_engine_send_key_event_finish(FL_ENGINE(object), result,
&handled, &error));
EXPECT_EQ(error, nullptr);
EXPECT_TRUE(handled);
g_main_loop_quit(static_cast<GMainLoop*>(user_data));
},
loop);

g_main_loop_run(loop);
EXPECT_TRUE(called);
}

TEST(FlEngineTest, SendKeyEventNotHandled) {
g_autoptr(GMainLoop) loop = g_main_loop_new(nullptr, 0);

g_autoptr(FlEngine) engine = make_mock_engine();
FlutterEngineProcTable* embedder_api = fl_engine_get_embedder_api(engine);

bool called;
embedder_api->SendKeyEvent = MOCK_ENGINE_PROC(
SendKeyEvent,
([&called](auto engine, const FlutterKeyEvent* event,
FlutterKeyEventCallback callback, void* user_data) {
called = true;
callback(FALSE, user_data);
return kSuccess;
}));

FlutterKeyEvent event = {.struct_size = sizeof(FlutterKeyEvent),
.timestamp = 1234,
.type = kFlutterKeyEventTypeUp,
.physical = 42,
.logical = 123,
.character = nullptr,
.synthesized = true,
.device_type = kFlutterKeyEventDeviceTypeKeyboard};
fl_engine_send_key_event(
engine, &event, nullptr,
[](GObject* object, GAsyncResult* result, gpointer user_data) {
gboolean handled;
g_autoptr(GError) error = nullptr;
EXPECT_TRUE(fl_engine_send_key_event_finish(FL_ENGINE(object), result,
&handled, &error));
EXPECT_EQ(error, nullptr);
EXPECT_FALSE(handled);
g_main_loop_quit(static_cast<GMainLoop*>(user_data));
},
loop);

g_main_loop_run(loop);
EXPECT_TRUE(called);
}

TEST(FlEngineTest, SendKeyEventError) {
g_autoptr(GMainLoop) loop = g_main_loop_new(nullptr, 0);

g_autoptr(FlEngine) engine = make_mock_engine();
FlutterEngineProcTable* embedder_api = fl_engine_get_embedder_api(engine);

bool called;
embedder_api->SendKeyEvent = MOCK_ENGINE_PROC(
SendKeyEvent,
([&called](auto engine, const FlutterKeyEvent* event,
FlutterKeyEventCallback callback, void* user_data) {
called = true;
return kInvalidArguments;
}));

FlutterKeyEvent event = {.struct_size = sizeof(FlutterKeyEvent),
.timestamp = 1234,
.type = kFlutterKeyEventTypeUp,
.physical = 42,
.logical = 123,
.character = nullptr,
.synthesized = true,
.device_type = kFlutterKeyEventDeviceTypeKeyboard};
fl_engine_send_key_event(
engine, &event, nullptr,
[](GObject* object, GAsyncResult* result, gpointer user_data) {
gboolean handled;
g_autoptr(GError) error = nullptr;
EXPECT_FALSE(fl_engine_send_key_event_finish(FL_ENGINE(object), result,
&handled, &error));
EXPECT_NE(error, nullptr);
EXPECT_STREQ(error->message, "Failed to send key event");
g_main_loop_quit(static_cast<GMainLoop*>(user_data));
},
loop);

g_main_loop_run(loop);
EXPECT_TRUE(called);
}

// NOLINTEND(clang-analyzer-core.StackAddressEscape)
32 changes: 30 additions & 2 deletions shell/platform/linux/fl_keyboard_manager.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include <memory>
#include <string>

#include "flutter/shell/platform/linux/fl_engine_private.h"
#include "flutter/shell/platform/linux/fl_key_channel_responder.h"
#include "flutter/shell/platform/linux/fl_key_embedder_responder.h"
#include "flutter/shell/platform/linux/fl_keyboard_layout.h"
Expand Down Expand Up @@ -479,8 +480,35 @@ FlKeyboardManager* fl_keyboard_manager_new(
} else {
g_autoptr(FlEngine) engine = FL_ENGINE(g_weak_ref_get(&self->engine));
if (engine != nullptr) {
fl_engine_send_key_event(engine, event, callback,
callback_user_data);
typedef struct {
FlutterKeyEventCallback callback;
void* callback_user_data;
} SendKeyEventData;
SendKeyEventData* data = g_new0(SendKeyEventData, 1);
data->callback = callback;
data->callback_user_data = callback_user_data;
fl_engine_send_key_event(
engine, event, self->cancellable,
[](GObject* object, GAsyncResult* result, gpointer user_data) {
g_autofree SendKeyEventData* data =
static_cast<SendKeyEventData*>(user_data);
gboolean handled = FALSE;
g_autoptr(GError) error = nullptr;
if (!fl_engine_send_key_event_finish(
Copy link

@mattkae mattkae Dec 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make more sense for fl_engine_send_key_event to call fl_engine_send_key_event_finish automatically for us before calling the user provided callback? If it always has to be called after fl_engine_send_key_event gets called, it would make sense to encapsulate it at that level instead of putting the onus on the caller.

Copy link
Contributor Author

@robert-ancell robert-ancell Dec 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the way that GObject does asynchronous calls, the purpose of this change is to make all the code more consistent and reliable (in particular by using GCancellable to make sure all async calls are correctly aborted if the caller is disposed before a call completes). I'm not sure the history of why this pattern was chosen. Some ideas of why it might not be desirable to do the finish internally:

  • This would require making custom callbacks for each function, which might be considered harder to write and have difficulty with language bindings.
  • The finish completes the async call, which might do some specific behaviour. The callback only indicates the function is ready to finish, you might store the GAsyncResult and do it later or chain it into another async call.

FL_ENGINE(object), result, &handled, &error)) {
if (g_error_matches(error, G_IO_ERROR,
G_IO_ERROR_CANCELLED)) {
return;
}

g_warning("Failed to send key event: %s", error->message);
}

if (data->callback != nullptr) {
data->callback(handled, data->callback_user_data);
}
},
data);
}
}
},
Expand Down
Loading