[Webkit-unassigned] [Bug 75246] [WK2][EFL] implemented Download module for Efl port

[bugzilla-daemon at webkit.org]

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





--- Comment #4 from Gyuyoung Kim <gyuyoung.kim at samsung.com>  2011-12-28 05:20:02 PST ---
(From update of attachment 120650)
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)) {

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