Bug 126130 – [GTK] Test /webkit2/WebKitWebContext/languages crashes with network process enabled

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

[Carlos Garcia Campos]

$ 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

[Carlos Garcia Campos]

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]

Created attachment 220072 [details]
Updated patch

Just rebased to apply on current git master

[Anders Carlsson]

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]

(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]

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]

yeah, that code was copied into epiphany from libsoup, so it should be fine

[Carlos Garcia Campos]

(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]

Created attachment 221061 [details]
Rebased patch

This is simpler patch now that doesn't need the build accept languages code.

[Anders Carlsson]

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]

Committed r161976: <http://trac.webkit.org/changeset/161976>