Bug 132939 – [WebKit2-EFL] Fix safety check failed errors in API tests

• Component: WebKit EFL
• Status: RESOLVED REMIND

[Shivakumar J M]

Fix erros in below API tests, ewk_context_additional_plugin_path_set(), ewk_view_contents_size_get(), ewk_context_menu_item_select(), ewk_view_script_execute().

[ RUN      ] EWK2ContextTest.ewk_context_additional_plugin_path_set
ERR<9873>: /home/shiva.jm/webkit-git/svngitmay14/WebKit/Source/WebKit2/UIProcess/API/efl/ewk_context.cpp:490 ewk_context_additional_plugin_path_set() safety check failed: path == NULL
[       OK ] EWK2ContextTest.ewk_context_additional_plugin_path_set (131 ms)
[----------] 12 tests from EWK2ContextTest (11581 ms total)


[ RUN      ] EWK2ViewTest.ewk_view_contents_size_get
ERR<11319>: /home/shiva.jm/webkit-git/svngitmay14/WebKit/Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:54 toEwkViewChecked() safety check failed: evasObject == NULL
CRI<11319>: /home/shiva.jm/webkit-git/svngitmay14/WebKit/Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:763 ewk_view_contents_size_get() no private data for object (nil)
[       OK ] EWK2ViewTest.ewk_view_contents_size_get (790 ms)
[----------] 36 tests from EWK2ViewTest (29847 ms total)

[----------] 1 test from EWK2UnitTestBase
[ RUN      ] EWK2UnitTestBase.ewk_view_script_execute
ERR<11319>: /home/shiva.jm/webkit-git/svngitmay14/WebKit/Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:692 ewk_view_script_execute() safety check failed: script == NULL
[       OK ] EWK2UnitTestBase.ewk_view_script_execute (675 ms)

Expected: true
ERR<11033>: /home/shiva.jm/webkit-git/svngitmay14/WebKit/Source/WebKit2/UIProcess/API/efl/ewk_context_menu.cpp:158 ewk_context_menu_item_select() safety check failed: item == NULL
/home/shiva.jm/webkit-git/svngitmay14/WebKit/Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_text_checker.cpp:167: Failure
Value of: text
  Actual: "aa"

[Shivakumar J M]

Created attachment 231498 [details]
Patch

ewk_view_contents_size_get unit test has tested unsupported evas object type. But ewk APIs gives critical error, when given evas object
is not ewk type. So, we can't test wrong evas object type by using current test. so Removing the test line.

[Gyuyoung Kim]

Comment on attachment 231498 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=231498&action=review

> Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_view.cpp:-1146
> -    ewk_view_contents_size_get(0, &contentsWidth, &contentsHeight);

I think this is to check if width and height can be initialized with 0 when Evas_Object is null. To be honest, I don't know why we have to initialize width/height when evas object is null.

Eina_Bool ewk_view_contents_size_get(const Evas_Object* ewkView, Evas_Coord* width, Evas_Coord* height)
{
    EwkView* impl = toEwkViewChecked(ewkView);
    if (EINA_UNLIKELY(!impl)) {
        EINA_LOG_CRIT("no private data for object %p", ewkView);
        if (width)
            *width = 0;
        if (height)
            *height = 0;

        return false;
    }

If we remove this test logic, I think we have to remove the initialization code together.

[Ryuan Choi]

Comment on attachment 231498 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=231498&action=review

> Source/WebKit2/ChangeLog:9
> +        ewk_view_contents_size_get unit test has tested unsupported evas object type. But ewk APIs gives critical error, when given evas object
> +        is not ewk type. So, we can't test wrong evas object type by using current test. so Removing the test line.

I am not sure what is the problem. It is intentionally tested.

>> Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_view.cpp:-1146
>> -    ewk_view_contents_size_get(0, &contentsWidth, &contentsHeight);
> 
> I think this is to check if width and height can be initialized with 0 when Evas_Object is null. To be honest, I don't know why we have to initialize width/height when evas object is null.
> 
> Eina_Bool ewk_view_contents_size_get(const Evas_Object* ewkView, Evas_Coord* width, Evas_Coord* height)
> {
>     EwkView* impl = toEwkViewChecked(ewkView);
>     if (EINA_UNLIKELY(!impl)) {
>         EINA_LOG_CRIT("no private data for object %p", ewkView);
>         if (width)
>             *width = 0;
>         if (height)
>             *height = 0;
> 
>         return false;
>     }
> 
> If we remove this test logic, I think we have to remove the initialization code together.

Maybe, I don't have objection about removing the logic that clears the value.
But I still want test case although we remove this logic.

This test case explains the behavior of ewebkit well when wrong ewkView is given.

[Ryuan Choi]

If you want clear log, it looks better to turn off the EINA_LOG for our test cases.

Test cases should cover not only positive cases but also negative cases.

[Gyuyoung Kim]

(In reply to comment #4)
> If you want clear log, it looks better to turn off the EINA_LOG for our test cases.
> 
> Test cases should cover not only positive cases but also negative cases.

How about turning on EINA_LOG only when debug mode ? Personally I hope to see the message on debug mode. ;)

[Shivakumar J M]

I would like to postpone to fix this problem later, since to remove error logs for negative test, we may need to seperate negative tests or enable error logs in debug mode only