Bug 58852 – [GTK] Add proxy support to GtkLauncher

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Ryuan Choi]

I recreate from Bug 55929.

Proxy support to Gtklauncher will help some developers behind proxy.

[Ryuan Choi]

Created attachment 90117 [details]
Patch

[Ryuan Choi]

(In reply to comment #1)
> Created an attachment (id=90117) [details]
> Patch

I tried to use SoupProxyResolverDefault but it looks not available on libsoup 2.33.6.

IMO, g_object_set now is simple way for test browser.

[Martin Robinson]

Comment on attachment 90117 [details]
Patch

Can't you just guard the code with an #ifdef so that when compiler with older versions of Soup, it's not seen. We generally develop against the latest stable version of the Gnome stack. Is there some particular version of libsoup that you require?

[Ryuan Choi]

Created attachment 90675 [details]
Patch

[Ryuan Choi]

(In reply to comment #3)
> (From update of attachment 90117 [details])
> Can't you just guard the code with an #ifdef so that when compiler with older versions of Soup, it's not seen. We generally develop against the latest stable version of the Gnome stack. Is there some particular version of libsoup that you require?

Sorry about late answer.
I tried to find checking libsoup version, but it looks not simple.

Instead, I checked SOUP_TYPE_PROXY_RESOLVER_DEFAULT.

And I changed my libsoup version, but previous patch works well for lower version of libsoup.
I update my patch for both SoupProxyResolverDefault and previous libsoup.

[Martin Robinson]

Comment on attachment 90675 [details]
Patch

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

Okay. This seems alright in the end.

> Tools/ChangeLog:9
> +        * GtkLauncher/main.c:
> +        (main):

Please fill this out before landing.

> Tools/GtkLauncher/main.c:245
> +    const char *httpProxy = getenv("http_proxy");

I think it would be better to call g_getenv here instead.

[Ryuan Choi]

Created attachment 90899 [details]
Patch

[Ryuan Choi]

(In reply to comment #6)
> (From update of attachment 90675 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=90675&action=review
> 
> Okay. This seems alright in the end.
> 
> > Tools/ChangeLog:9
> > +        * GtkLauncher/main.c:
> > +        (main):
> 
> Please fill this out before landing.

added some comment.

> 
> > Tools/GtkLauncher/main.c:245
> > +    const char *httpProxy = getenv("http_proxy");
> 
> I think it would be better to call g_getenv here instead.

Done.

Thank you for review.

[WebKit Commit Bot]

The commit-queue encountered the following flaky tests while processing attachment 90899 [details]:

fast/loader/file-protocol-fragment.html bug 59488 (author: [e-mail])
The commit-queue is continuing to process your patch.

[WebKit Commit Bot]

Comment on attachment 90899 [details]
Patch

Clearing flags on attachment: 90899

Committed r84949: <http://trac.webkit.org/changeset/84949>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[WebKit Commit Bot]

The commit-queue encountered the following flaky tests while processing attachment 90899 [details]:

storage/domstorage/localstorage/storagetracker/storage-tracker-1-prepare.html bug 59494
storage/domstorage/localstorage/storagetracker/storage-tracker-2-create.html bug 59495
storage/domstorage/localstorage/storagetracker/storage-tracker-3-delete-all.html bug 59496
The commit-queue is continuing to process your patch.

[Alejandro G. Castro]

Reverted r84949 for reason:

Broke GTK+ compilation

Committed r84965: <http://trac.webkit.org/changeset/84965>

[Ryuan Choi]

Created attachment 91227 [details]
Patch

[Ryuan Choi]

(In reply to comment #13)
> Reverted r84949 for reason:
> 
> Broke GTK+ compilation
> 
> Committed r84965: <http://trac.webkit.org/changeset/84965>

I am sorry. I didn't see build break.
I update patch to solve it, although I couldn't reproduce it on my desktop.

Below is a message from http://build.webkit.org/builders/GTK%20Linux%2032-bit%20Release/builds/13144/steps/compile-webkit/logs/stdio

/usr/bin/ld: Tools/GtkLauncher/Programs_GtkLauncher-main.o: undefined reference to symbol 'soup_session_add_feature_by_type'
/usr/bin/ld: note: 'soup_session_add_feature_by_type' is defined in DSO /usr/lib/libsoup-2.4.so.1 so try adding it to the linker command line
/usr/lib/libsoup-2.4.so.1: could not read symbols: Invalid operation

[WebKit Commit Bot]

Comment on attachment 91227 [details]
Patch

Rejecting attachment 91227 [details] from commit-queue.

Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=cr-jail-3', 'land-a..." exit_code: 1

Last 500 characters of output:
autoinstalled/mechanize/_urllib2_fork.py", line 332, in _call_chain
    result = func(*args)
  File "/mnt/git/webkit-commit-queue/Tools/Scripts/webkitpy/thirdparty/autoinstalled/mechanize/_urllib2_fork.py", line 1170, in https_open
    return self.do_open(conn_factory, req)
  File "/mnt/git/webkit-commit-queue/Tools/Scripts/webkitpy/thirdparty/autoinstalled/mechanize/_urllib2_fork.py", line 1118, in do_open
    raise URLError(err)
urllib2.URLError: <urlopen error [Errno 60] Operation timed out>

Full output: http://queues.webkit.org/results/8508915

[Ryuan Choi]

I think that this is already landed.

[ChangSeok Oh]

I think there is an issue to use SOUP_TYPE_PROXY_RESOLVER_DEFAULT.
A crash is occured while running GtkLauncher, if I set a proxy using by xxx.pac file.
Please refer http://code.google.com/p/libproxy/issues/detail?id=153#makechanges
This issue is still there on Ubuntu 11.04.
If I disable SOUP_TYPE_PROXY_RESOLVER_DEFAULT, No problem.
And If I don't use .pac file to set proxy, No problem, too.

Actually, this issue might not be webkit' one. But we need to track this issue and be careful to use .pac file to set proxy. :)

[Martin Robinson]

(In reply to comment #18)
> I think there is an issue to use SOUP_TYPE_PROXY_RESOLVER_DEFAULT.
> A crash is occured while running GtkLauncher, if I set a proxy using by xxx.pac file.
> Please refer http://code.google.com/p/libproxy/issues/detail?id=153#makechanges
> This issue is still there on Ubuntu 11.04.
> If I disable SOUP_TYPE_PROXY_RESOLVER_DEFAULT, No problem.
> And If I don't use .pac file to set proxy, No problem, too.
> 
> Actually, this issue might not be webkit' one. But we need to track this issue and be careful to use .pac file to set proxy. :)

Thanks for linking to the libproxy issue. That will be very helpful if people report the segfault.