[Webkit-unassigned] [Bug 117794] [EFL][WK2] Implement unit test callback: onWordGuesses

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=117794





--- Comment #2 from Grzegorz Czajkowski <g.czajkowski at samsung.com>  2013-06-20 00:17:03 PST ---
(From update of attachment 204997)
View in context: https://bugs.webkit.org/attachment.cgi?id=204997&action=review

Added some comments before review.

> Source/WebKit2/ChangeLog:9
> +        It is finally possible to implement missing unit tests as WebKit2-EFL
> +        has delivered sub menu (r150254).

nit: suggestions do not need sub menu, they appear in the primary context menu.

> Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_text_checker.cpp:188
> +        char* suggestion = (char*)malloc(strlen(clientSuggestionsForWord[i]) + 1);
> +        strcpy(suggestion, clientSuggestionsForWord[i]);

At first sight, it looks wired that we are using C here. But WebKit2-EFL calls free() for them as this part of API.

Anyway I'd duplicate it using:
char* suggestion = strdup(clientSuggestionsForWord[i]);

> Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_text_checker.cpp:289
> +static Eina_Bool checkClientSuggestionsForWord(Ewk_View_Smart_Data*, Evas_Coord, Evas_Coord, Ewk_Context_Menu* contextMenuItems)

Ewk_Context_Menu* contextMenuItems ? 
You've probably thought of contextMenu.

> Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_text_checker.cpp:291
> +    const Eina_List* list = ewk_context_menu_items_get(contextMenuItems);

nit: list -> contextMenuItems

> Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_text_checker.cpp:293
> +    int numberOfSuggestions = WTF_ARRAY_LENGTH(clientSuggestionsForWord);

int -> unsigned or size_t

> Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_text_checker.cpp:629
> +    wasContextMenuShown = false;

That it's confusing. We shouldn't assume that the callback (that overwrite this value) will be called asynchronously. I'd move it somewhere around resetCallbacksExecutionStats().

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.