Summary: | [WebKit2-EFL] Fix safety check failed errors in API tests | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Shivakumar J M <shiva.jm> | ||||
Component: | WebKit EFL | Assignee: | Nobody <webkit-unassigned> | ||||
Status: | RESOLVED REMIND | ||||||
Severity: | Normal | CC: | bunhere, cdumez, commit-queue, gyuyoung.kim, gyuyoung.kim, lucas.de.marchi, ryuan.choi, sergio | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Bug Depends on: | |||||||
Bug Blocks: | 132813 | ||||||
Attachments: |
|
Description
Shivakumar J M
2014-05-14 20:44:23 PDT
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.
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. 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. 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. (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. ;) 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 |