Summary: | [WK2][EFL] creating dummy functions for Download class on Efl port. | ||
---|---|---|---|
Product: | WebKit | Reporter: | Keunsoon Lee <keunsoon.lee> |
Component: | WebKit2 | Assignee: | Nobody <webkit-unassigned> |
Status: | RESOLVED FIXED | ||
Severity: | Enhancement | CC: | abarth, gyuyoung.kim, gyuyoung.kim, ibchang, japhet, rakuco, webkit.review.bot |
Priority: | P2 | ||
Version: | 528+ (Nightly build) | ||
Hardware: | Other | ||
OS: | Linux | ||
Attachments: |
Description
Keunsoon Lee
2011-12-27 04:39:55 PST
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.
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.
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.
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)) { 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.
Comment on attachment 120920 [details]
dummy functions for Download class for Efl port
LGTM. :-)
(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. Created attachment 120924 [details]
modified ChangeLog according to changed bug title
Modified this bug's title and depending ChangeLog to emphasize dummy functions.
LGTM. Comment on attachment 120924 [details]
modified ChangeLog according to changed bug title
rs=me
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> All reviewed patches have been landed. Closing bug. |