Bug 58852 - [GTK] Add proxy support to GtkLauncher
Summary: [GTK] Add proxy support to GtkLauncher
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-04-18 16:33 PDT by Ryuan Choi
Modified: 2011-05-26 05:43 PDT (History)
5 users (show)

See Also:


Attachments
Patch (1.23 KB, patch)
2011-04-18 16:36 PDT, Ryuan Choi
no flags Details | Formatted Diff | Diff
Patch (1.33 KB, patch)
2011-04-22 00:19 PDT, Ryuan Choi
no flags Details | Formatted Diff | Diff
Patch (1.45 KB, patch)
2011-04-24 21:40 PDT, Ryuan Choi
no flags Details | Formatted Diff | Diff
Patch (1.88 KB, patch)
2011-04-26 21:41 PDT, Ryuan Choi
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ryuan Choi 2011-04-18 16:33:47 PDT
I recreate from Bug 55929.

Proxy support to Gtklauncher will help some developers behind proxy.
Comment 1 Ryuan Choi 2011-04-18 16:36:53 PDT
Created attachment 90117 [details]
Patch
Comment 2 Ryuan Choi 2011-04-18 16:39:37 PDT
(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.
Comment 3 Martin Robinson 2011-04-18 19:07:34 PDT
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?
Comment 4 Ryuan Choi 2011-04-22 00:19:59 PDT
Created attachment 90675 [details]
Patch
Comment 5 Ryuan Choi 2011-04-22 00:25:34 PDT
(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.
Comment 6 Martin Robinson 2011-04-22 09:06:59 PDT
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.
Comment 7 Ryuan Choi 2011-04-24 21:40:58 PDT
Created attachment 90899 [details]
Patch
Comment 8 Ryuan Choi 2011-04-24 21:44:07 PDT
(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.
Comment 9 WebKit Commit Bot 2011-04-26 13:56:29 PDT
The commit-queue encountered the following flaky tests while processing attachment 90899 [details]:

fast/loader/file-protocol-fragment.html bug 59488 (author: koivisto@iki.fi)
The commit-queue is continuing to process your patch.
Comment 10 WebKit Commit Bot 2011-04-26 13:58:13 PDT
Comment on attachment 90899 [details]
Patch

Clearing flags on attachment: 90899

Committed r84949: <http://trac.webkit.org/changeset/84949>
Comment 11 WebKit Commit Bot 2011-04-26 13:58:18 PDT
All reviewed patches have been landed.  Closing bug.
Comment 12 WebKit Commit Bot 2011-04-26 14:21:53 PDT
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.
Comment 13 Alejandro G. Castro 2011-04-26 15:47:38 PDT
Reverted r84949 for reason:

Broke GTK+ compilation

Committed r84965: <http://trac.webkit.org/changeset/84965>
Comment 14 Ryuan Choi 2011-04-26 21:41:17 PDT
Created attachment 91227 [details]
Patch
Comment 15 Ryuan Choi 2011-04-26 21:48:22 PDT
(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
Comment 16 WebKit Commit Bot 2011-04-27 02:44:25 PDT
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
Comment 17 Ryuan Choi 2011-05-10 21:36:12 PDT
I think that this is already landed.
Comment 18 ChangSeok Oh 2011-05-25 19:32:03 PDT
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. :)
Comment 19 Martin Robinson 2011-05-26 05:43:12 PDT
(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.