Bug 144683

Summary: [EFL] ewk_view_page_contents_get() API test is flaky
Product: WebKit Reporter: Ryuan Choi <ryuan.choi>
Component: WebKit EFLAssignee: Ryuan Choi <ryuan.choi>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, lucas.de.marchi
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Description Ryuan Choi 2015-05-06 05:59:55 PDT
ewk_view_page_contents_get() API test is sometime failing like below.

https://build.webkit.org/builders/EFL%20Linux%2064-bit%20Release%20WK2/builds/21679/steps/API%20tests/logs/stdio

../../Source/WebKit2/UIProcess/API/efl/tests/test_ewk2_view.cpp:226: Failure
Value of: String(data).length() == fileSize || String(data).length() == fileSize + 1
  Actual: false
Expected: true
ERR<10438>: ../../Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:633 ewk_view_page_contents_get() safety check failed: callback == NULL
ERR<10438>: ../../Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:54 toEwkViewChecked() safety check failed: evasObject == NULL
CRI<10438>: ../../Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:634 ewk_view_page_contents_get() no private data for object (nil)
[  FAILED  ] EWK2ViewTest.ewk_view_page_contents_get (204 ms)


It's because callback of WKPageGetContentsAsMHTMLData returns WKData, which is not null-terminated string, but it is considered as common string.
We should pass null terminated string instead of WKData.
Comment 1 Ryuan Choi 2015-05-06 16:50:57 PDT
Created attachment 252538 [details]
Patch
Comment 2 Darin Adler 2015-05-06 16:58:16 PDT
Comment on attachment 252538 [details]
Patch

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

> Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:616
> +    contentsContext->callback(contentsContext->type, String(WKDataGetBytes(wkData), WKDataGetSize(wkData)).utf8().data(), contentsContext->userData);

It doesn’t make sense to take this data and convert it to UTF-8 as if it was Latin-1. If you want to put it in a CString so it gets null terminated, then I suggest creating the CString directly, not through a String.
Comment 3 Ryuan Choi 2015-05-06 18:27:19 PDT
Created attachment 252549 [details]
Patch
Comment 4 Ryuan Choi 2015-05-06 18:30:16 PDT
(In reply to comment #2)
> Comment on attachment 252538 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=252538&action=review
> 
> > Source/WebKit2/UIProcess/API/efl/ewk_view.cpp:616
> > +    contentsContext->callback(contentsContext->type, String(WKDataGetBytes(wkData), WKDataGetSize(wkData)).utf8().data(), contentsContext->userData);
> 
> It doesn’t make sense to take this data and convert it to UTF-8 as if it was
> Latin-1. If you want to put it in a CString so it gets null terminated, then
> I suggest creating the CString directly, not through a String.

Thanks, I updated to use CString.
Comment 5 Gyuyoung Kim 2015-05-06 18:49:01 PDT
Comment on attachment 252549 [details]
Patch

r+ed based on Darin's comment.
Comment 6 WebKit Commit Bot 2015-05-06 20:21:04 PDT
Comment on attachment 252549 [details]
Patch

Clearing flags on attachment: 252549

Committed r183908: <http://trac.webkit.org/changeset/183908>
Comment 7 WebKit Commit Bot 2015-05-06 20:21:09 PDT
All reviewed patches have been landed.  Closing bug.