Add tests for FlMouseCursorHandler#188554
Conversation
Add unit tests covering cursor kind mapping, the fallback to the default cursor, the cursor-changed signal, and method call error handling for the mouse cursor handler.
An activateSystemCursor method call whose argument map omits the "kind" key (or has a non-string value) passes a null kind to the handler, which was passed straight to g_hash_table_lookup and crashed in g_str_hash. Guard against a null kind and fall back to the default cursor. Also guard the channel's fl_value_get_type lookup against a missing "kind" key, which otherwise triggered a CRITICAL assertion warning. Add a regression test covering a call with a missing kind.
github-actions
Bot
added
engine
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds unit tests for FlMouseCursorHandler and introduces null checks in fl_mouse_cursor_channel.cc and fl_mouse_cursor_handler.cc to prevent potential crashes. The review feedback highlights a critical double-free or use-after-free issue in the new test file, where fl_value_set_string takes ownership of kind while it is also managed by g_autoptr. The reviewer suggests refactoring activate_system_cursor to accept a const gchar* and simplifying the test cases to resolve this issue.
There was a problem hiding this comment.
The fl_value_set_string function takes ownership of the value passed to it (transfer full). Passing kind directly to fl_value_set_string transfers its ownership to args. Since kind is also managed by a g_autoptr in the calling test cases, this leads to a double-free / use-after-free bug when args is destroyed at the end of this function.
To fix this and simplify the test cases, we can change activate_system_cursor to accept a const gchar* kind string instead of an FlValue*.
static gboolean activate_system_cursor(FlMockBinaryMessenger* messenger,
const gchar* kind) {
g_autoptr(FlValue) args = fl_value_new_map();
fl_value_set_string(args, "kind", fl_value_new_string(kind));
gboolean called = FALSE;
fl_mock_binary_messenger_invoke_standard_method(
messenger, "flutter/mousecursor", "activateSystemCursor", args,
[](FlMockBinaryMessenger* messenger, FlMethodResponse* response,
gpointer user_data) {
gboolean* called = static_cast<gboolean*>(user_data);
*called = TRUE;
EXPECT_TRUE(FL_IS_METHOD_SUCCESS_RESPONSE(response));
},
&called);
return called;
}| g_autoptr(FlValue) kind = fl_value_new_string("click"); | ||
| EXPECT_TRUE(activate_system_cursor(messenger, kind)); |
There was a problem hiding this comment.
Simplify the test case by passing the string directly to activate_system_cursor, which avoids the double-free of kind.
EXPECT_TRUE(activate_system_cursor(messenger, "click"));| g_autoptr(FlValue) kind = fl_value_new_string("basic"); | ||
| EXPECT_TRUE(activate_system_cursor(messenger, kind)); |
There was a problem hiding this comment.
Simplify the test case by passing the string directly to activate_system_cursor, which avoids the double-free of kind.
EXPECT_TRUE(activate_system_cursor(messenger, "basic"));| g_autoptr(FlValue) kind = fl_value_new_string("madeUpCursorKind"); | ||
| EXPECT_TRUE(activate_system_cursor(messenger, kind)); |
There was a problem hiding this comment.
Simplify the test case by passing the string directly to activate_system_cursor, which avoids the double-free of kind.
EXPECT_TRUE(activate_system_cursor(messenger, "madeUpCursorKind"));There was a problem hiding this comment.
Simplify the test case by passing the string directly to activate_system_cursor, which avoids the double-free of kind.
EXPECT_TRUE(activate_system_cursor(messenger, "grab"));
2 participants
Add unit tests covering cursor kind mapping, the fallback to the default cursor, the cursor-changed signal, and method call error handling for the mouse cursor handler.