Bug 75246 – [WK2][EFL] creating dummy functions for Download class on Efl port.

• Component: WebKit2
• Status: RESOLVED FIXED

[Keunsoon Lee]

Added download module for Efl port on WebKit2/WebProcess/Downloads/efl.

Codes will be uploaded soon.

[Keunsoon Lee]

Created attachment 120648 [details]
initial implementation for download module specific to Efl port

This is initial version.
I'll try to make a test code if necessary.

EflDownloader is not dependent on Efl or soup port indeed, but uses only common classes on WebCore and WebKit2.
Efl port is using soup port for Network/HTTP stack, though.

PS, there is a known issue; EflDownloader::start() cannot obtain UserAgent when it is called from WKContextDownloadURLRequest() because it does not pass WebPage.
I'll create another bug thread for it.

Thank you.

[WebKit Review Bot]

Attachment 120648 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit..." exit_code: 1

Source/WebKit2/WebProcess/Downloads/efl/EflDownloader.h:34:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Source/WebKit2/WebProcess/Downloads/efl/EflDownloader.h:85:  The parameter name "download" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 2 in 6 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Keunsoon Lee]

Created attachment 120650 [details]
fix style error

I leaved the style errors because it is a kind of style from another files on Source/WebKit2/WebProcess/Downloads.
But, I fixed them according to check-webkit-style script.

[Gyuyoung Kim]

Comment on attachment 120650 [details]
fix style error

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

I think this patch is too huge to review. I think you need to make a patch which has dummy functions. Then, you submit concrete patches step by step. In addition, this patch doesn't include any test cases. Informal r- on my side.

> Source/WebKit2/ChangeLog:6
> +        newly implemented download module for Efl port

This description is not sufficient. I think you need to add more detail description ?

> Source/WebKit2/WebProcess/Downloads/efl/EflDownloader.cpp:44
> +    , m_bufferCapacity(64*1024) // FIXME: find proper value or give a chance to adjust it for client

Do place spaces around binary and ternary operators.

> Source/WebKit2/WebProcess/Downloads/efl/EflDownloader.cpp:66
> +            userAgent = "Mozilla/5.0 (Unknown; Linux armv7l) AppleWebKit/534.16+ (KHTML, like Gecko) Version/5.0 Safari/534.16+";

Why do you add armv7 as default ? In WebKit1, we make user agent string according to platform information and os version information. See ewk_settings.cpp files.

> Source/WebKit2/WebProcess/Downloads/efl/EflDownloader.cpp:71
> +    NetworkingContext* context = m_initiatingPage?m_initiatingPage->mainWebFrame()->coreFrame()->loader()->networkingContext():0;

ditto. Add space between ? : and so on.

> Source/WebKit2/WebProcess/Downloads/efl/EflDownloader.cpp:109
> +            || !destination.contains(PlatformFilePathSeparator)) {

Coding style violation.

 if (destination.isEmpty()
     || !destination.contains(PlatformFilePathSeparator)) {

[Keunsoon Lee]

Created attachment 120920 [details]
dummy functions for Download class for Efl port

This patch has only dummy functions for Download class because previous patch was too big to review.
I'll upload working patches in another bug threads.

Thank you in advance.

[Gyuyoung Kim]

Comment on attachment 120920 [details]
dummy functions for Download class for Efl port

LGTM. :-)

[Gyuyoung Kim]

(In reply to comment #6)
> (From update of attachment 120920 [details])
> LGTM. :-)

Would you change this Bug title according to your new patch ? If you change bug title, you have to change it in ChangeLog as well.

[Keunsoon Lee]

Created attachment 120924 [details]
modified ChangeLog according to changed bug title

Modified this bug's title and depending ChangeLog to emphasize dummy functions.

[Gyuyoung Kim]

LGTM.

[Hajime Morrita]

Comment on attachment 120924 [details]
modified ChangeLog according to changed bug title

rs=me

[WebKit Review Bot]

Comment on attachment 120924 [details]
modified ChangeLog according to changed bug title

Clearing flags on attachment: 120924

Committed r104122: <http://trac.webkit.org/changeset/104122>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.