Summary: | [EFL] Set proxy address for Soup | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Gyuyoung Kim <gyuyoung.kim> | ||||||||||
Component: | WebKit EFL | Assignee: | Nobody <webkit-unassigned> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | barbieri, commit-queue, hyuki.kim, kenneth, leandro, lucas.de.marchi | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | PC | ||||||||||||
OS: | Linux | ||||||||||||
Attachments: |
|
Description
Gyuyoung Kim
2010-07-20 19:38:40 PDT
Created attachment 62145 [details]
Patch
Patch
(In reply to comment #0) > If PC network needs to use proxy address, EWebLauncher cannot use network without proxy address setting. Nice, but this breaks when libsoub is not enabled (bug 42286, which did not get in yet because of a problem with changelogs). I'd rather let the part of getting the environment variable to browser and add a method to set proxy. This way browser might decide for whatever reason to ask for a proxy to be used (be the environment var or because user explicitly configured browser for this). Then we would have 2 implementations surrounded by ifdefs, 1 for libsoup and another for libcurl (which can be let blank for now). Thank you lucas, I understand your point as below, void ewk_XXX_proxy_uri_set(...) { #if USE(SOUP) .... #elif USE(CURL) .... #endif } And, EWebLauncher invokes the API. In my opinion, this API is located in ewk_settings.cpp. How do you think about it ? (In reply to comment #4) > Thank you lucas, > > I understand your point as below, > > void ewk_XXX_proxy_uri_set(...) > { > #if USE(SOUP) > .... > #elif USE(CURL) > .... > #endif > } > > And, EWebLauncher invokes the API. > > In my opinion, this API is located in ewk_settings.cpp. How do you think about it ? I think ewk_settings.cpp is indeed the best place. Created attachment 62267 [details]
Patch
I modify this patch according to Lucas's opinion. However, I don't support libcurl yet.
WebKit's Libcurl port already has proxy method. In order to use the function, proxy address needs to be modified. I will support it later.
Created attachment 62268 [details]
Patch
Oops, sorry. Other patch was uploaded. This is new patch.
Comment on attachment 62268 [details] Patch > Index: WebKit/efl/ChangeLog > =================================================================== > --- WebKit/efl/ChangeLog (revision 63877) > +++ WebKit/efl/ChangeLog (working copy) > @@ -1,3 +1,18 @@ > +2010-07-22 Gyuyoung Kim <gyuyoung.kim@samsung.com> > + > + Reviewed by NOBODY (OOPS!). > + > + [EFL] Set proxy address for Soup > + https://bugs.webkit.org/show_bug.cgi?id=42721 > + > + Set proxy address by http_proxy enviroment variable. Now the description doesn't make much sense. > Index: WebKit/efl/ewk/ewk_settings.cpp > =================================================================== > --- WebKit/efl/ewk/ewk_settings.cpp (revision 63876) > +++ WebKit/efl/ewk/ewk_settings.cpp (working copy) > @@ -29,6 +29,7 @@ > #include "Image.h" > #include "IntSize.h" > #include "KURL.h" > +#include "NotImplemented.h" There's no other place inside WebKit/efl/ewk/ where we use notImplemented(). I'd rather let this out and use a warning in eina_log or an EINA_SAFETY_ON_TRUE_RETURN(1). > +/** > + * Sets proxy address. > + * > + * @param proxy address. If proxy address is null, proxy address isn't set. > + */ > +void ewk_settings_proxy_uri_set(const char* proxy) > +{ > +#ifdef WTF_USE_SOUP > + SoupSession* session = WebCore::ResourceHandle::defaultSession(); > + g_object_set(session, SOUP_SESSION_PROXY_URI, soup_uri_new(proxy), NULL); You are leaking the SoupURI struct here, since g_object_set always copies. Change to something like: SoupURI* uri = soup_uri_new(proxy); EINA_SAFETY_ON_NULL_RETURN(uri); g_object_set(session, SOUP_SESSION_PROXY_URI, uri, NULL); soup_uri_free(soupUri); Comment on attachment 62268 [details] Patch Not so important, but: > +/** > + * Sets proxy address. > + * > + * @param proxy address. If proxy address is null, proxy address isn't set. > + */ > +void ewk_settings_proxy_uri_set(const char* proxy) > +{ > +#ifdef WTF_USE_SOUP #if USE(SOUP) > + SoupSession* session = WebCore::ResourceHandle::defaultSession(); > + g_object_set(session, SOUP_SESSION_PROXY_URI, soup_uri_new(proxy), NULL); > +#elif defined WTF_USE_CURL #elif USE(CURL) Created attachment 62290 [details]
Patch
Hello lucas,
Thank you for your advice.
I modify this patch. If there are still nits, please let me know.
Hello Comment on attachment 62290 [details]
Patch
Looks fine to me. If leandro is ok with this, please cq+
Comment on attachment 62290 [details]
Patch
LGTM
Comment on attachment 62290 [details] Patch Clearing flags on attachment: 62290 Committed r63972: <http://trac.webkit.org/changeset/63972> All reviewed patches have been landed. Closing bug. |