Bug 126130

Summary: [GTK] Test /webkit2/WebKitWebContext/languages crashes with network process enabled
Product: WebKit Reporter: Carlos Garcia Campos <cgarcia>
Component: WebKit2Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: andersca, danw
Priority: P2 Keywords: Gtk
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 126208, 126966    
Bug Blocks: 108832    
Attachments:
Description Flags
Patch
none
Updated patch
andersca: review-
Updated patch just removing the confusing comment
none
Rebased patch andersca: review+

Carlos Garcia Campos
Reported 2013-12-22 03:08:50 PST
$ Programs/WebKit2APITests/TestWebKitWebContext -p /webkit2/WebKitWebContext/languages /webkit2/WebKitWebContext/languages: OK $ WEBKIT_USE_NETWORK_PROCESS=1 Programs/WebKit2APITests/TestWebKitWebContext -p /webkit2/WebKitWebContext/languages /webkit2/WebKitWebContext/languages: Segmentation fault
Attachments
Patch (9.47 KB, patch)
2013-12-24 06:37 PST, Carlos Garcia Campos 		no flags
DetailsFormatted DiffDiff
Updated patch (10.13 KB, patch)
2013-12-28 02:26 PST, Carlos Garcia Campos 		andersca: review-
DetailsFormatted DiffDiff
Updated patch just removing the confusing comment (10.02 KB, patch)
2014-01-10 01:57 PST, Carlos Garcia Campos 		no flags
DetailsFormatted DiffDiff
Rebased patch (7.76 KB, patch)
2014-01-13 10:33 PST, Carlos Garcia Campos 		andersca: review+
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Carlos Garcia Campos
Comment 1 2013-12-24 06:37:01 PST
Created attachment 219969 [details] Patch This patch applies on top of patch attached to bug #126208. It's specific to soup because I don't know if mac needs to also notify the network process about user preferred languages change. Anders, could you confirm it? $ WEBKIT_USE_NETWORK_PROCESS=1 Programs/WebKit2APITests/TestWebKitWebContext -p /webkit2/WebKitWebContext/languages /webkit2/WebKitWebContext/languages: OK
Carlos Garcia Campos
Comment 2 2013-12-28 02:26:24 PST
Created attachment 220072 [details] Updated patch Just rebased to apply on current git master
Anders Carlsson
Comment 3 2014-01-09 09:28:48 PST
Comment on attachment 220072 [details] Updated patch View in context: https://bugs.webkit.org/attachment.cgi?id=220072&action=review > Source/WebKit2/NetworkProcess/soup/NetworkProcessSoup.cpp:80 > +// This function is based on Epiphany code in ephy-embed-prefs.c. > +static CString buildAcceptLanguages(Vector<String> languages) Looking around, it seems that ephy-embed-prefs.c is GPL which is a big no-no. Maybe you can get Xan to relicense it since he seems to be the only copyright holder.
Carlos Garcia Campos
Comment 4 2014-01-09 09:44:23 PST
(In reply to comment #3) > (From update of attachment 220072 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=220072&action=review > > > Source/WebKit2/NetworkProcess/soup/NetworkProcessSoup.cpp:80 > > +// This function is based on Epiphany code in ephy-embed-prefs.c. > > +static CString buildAcceptLanguages(Vector<String> languages) > > Looking around, it seems that ephy-embed-prefs.c is GPL which is a big no-no. Maybe you can get Xan to relicense it since he seems to be the only copyright holder. It's true that the code was copied from ephy and then adapted to wk coding style, but the original code in ephy (which is not part of ephy anymore since it's wk2 only) was copied from libsoup, that is LGPL. Dan, could you confirm that piece of code is LGPL? I'll update the comment (or just remove it) to avoid confusions.
Carlos Garcia Campos
Comment 5 2014-01-10 01:57:04 PST
Created attachment 220824 [details] Updated patch just removing the confusing comment Removed the comment. The code is actually based on accept_languages_from_system() from libsoup, see: https://git.gnome.org/browse/libsoup/tree/libsoup/soup-session.c?id=2.45.3#n390
Dan Winship
Comment 6 2014-01-13 10:28:12 PST
yeah, that code was copied into epiphany from libsoup, so it should be fine
Carlos Garcia Campos
Comment 7 2014-01-13 10:31:06 PST
(In reply to comment #6) > yeah, that code was copied into epiphany from libsoup, so it should be fine Thanks for confirming it. I've moved that code to a common file in WebCore in another patch so that it's shared by web and network processes instead of having it duplicated. I'll submit a rebased patch.
Carlos Garcia Campos
Comment 8 2014-01-13 10:33:46 PST
Created attachment 221061 [details] Rebased patch This is simpler patch now that doesn't need the build accept languages code.
Anders Carlsson
Comment 9 2014-01-13 11:03:57 PST
Comment on attachment 221061 [details] Rebased patch View in context: https://bugs.webkit.org/attachment.cgi?id=221061&action=review > Source/WebKit2/NetworkProcess/NetworkProcess.h:118 > + void userPreferredLanguagesChanged(Vector<String>); Should be const Vector<String>&.
Carlos Garcia Campos
Comment 10 2014-01-14 09:26:21 PST
Committed r161976: <http://trac.webkit.org/changeset/161976>
Note You need to log in before you can comment on or make changes to this bug.