Bug 91602 – [EFL][WK2] EFL should use DownloadSoup instead of defining DownloadEfl

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Chris Dumez]

The EFL port is using libsoup and therefore we should reuse DownloadSoup.cpp in WK2-EFL instead of defining our own (empty) DownloadEfl.cpp.

The only issue with that is that currently DownloadSoup.cpp includes "ErrorsGtk.h". We need to fix that.

[Keunsoon Lee]

I don't think we need to use DownloadSoup.cpp instead of DownloadEfl.cpp on WebKit2/WebProcess/Downloads/efl.
Because Download itself should not depend on Loader or Soup port.

By the way, I'm going to upstream the core of DownloadEfl.cpp soon.
(https://bugs.webkit.org/show_bug.cgi?id=88077)

What do you think so?

[Chris Dumez]

Actually it depend(In reply to comment #1)
> I don't think we need to use DownloadSoup.cpp instead of DownloadEfl.cpp on WebKit2/WebProcess/Downloads/efl.
> Because Download itself should not depend on Loader or Soup port.
> 
> By the way, I'm going to upstream the core of DownloadEfl.cpp soon.
> (https://bugs.webkit.org/show_bug.cgi?id=88077)
> 
> What do you think so?

Actually, it depends on soup for the response (it is a SoupMessage). Otherwise, why would GTK port put the implementation in a soup folder instead of GTK.
I don't want to duplicate the code in EFL for no reason. We are using soup and there is a DownloadSoup, therefore we should use it.

My patch to use DownloadSoup in WebKit2-EFL is almost ready and it is small. I think it is the way to go.

[Chris Dumez]

Created attachment 152987 [details]
Patch

[Gustavo Noronha (kov)]

Comment on attachment 152987 [details]
Patch

Attachment 152987 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/13284469

[Chris Dumez]

Created attachment 152996 [details]
Patch

Attempt to make gtk-ews happy.

[Gyuyoung Kim]

(In reply to comment #1)
> I don't think we need to use DownloadSoup.cpp instead of DownloadEfl.cpp on WebKit2/WebProcess/Downloads/efl.
> Because Download itself should not depend on Loader or Soup port.
> 
> By the way, I'm going to upstream the core of DownloadEfl.cpp soon.
> (https://bugs.webkit.org/show_bug.cgi?id=88077)
> 
> What do you think so?

Keunsoon, do you have any features DownloadSoup can't support ? Basically, this is a way to go I think. So, I'd like to know if there is any reasons.

[Chris Dumez]

Created attachment 152998 [details]
Patch

Fix a small typo.

[Kenneth Rohde Christiansen]

Comment on attachment 152998 [details]
Patch

good point, rs=me

[Chris Dumez]

Comment on attachment 152998 [details]
Patch

Clearing cq flag for now as I need the dependency to land first.

[Chris Dumez]

Created attachment 153073 [details]
Patch for landing

Rebase on master now that the dependency landed. Could someone please cq+ ?

[WebKit Review Bot]

Comment on attachment 153073 [details]
Patch for landing

Clearing flags on attachment: 153073

Committed r123019: <http://trac.webkit.org/changeset/123019>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.