Bug 162547 - [GTK][EFL] imported/w3c/web-platform-tests/fetch/api/basic/accept-header.html is failing
Summary: [GTK][EFL] imported/w3c/web-platform-tests/fetch/api/basic/accept-header.html...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: youenn fablet
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-09-26 01:02 PDT by youenn fablet
Modified: 2016-09-27 06:13 PDT (History)
11 users (show)

See Also:


Attachments
Patch (1.98 KB, patch)
2016-09-26 03:00 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Patch for landing (2.11 KB, patch)
2016-09-26 12:25 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Patch (1.73 KB, patch)
2016-09-27 03:42 PDT, youenn fablet
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description youenn fablet 2016-09-26 01:02:27 PDT
It seems GTK WTR is not setting Accept-Language header.
Comment 1 youenn fablet 2016-09-26 01:05:53 PDT
Using minibrowser, the test is passing.
This may be an issue with GTK WTR setup?
Comment 2 youenn fablet 2016-09-26 03:00:46 PDT
Created attachment 289814 [details]
Patch
Comment 3 youenn fablet 2016-09-26 03:02:52 PDT
(In reply to comment #2)
> Created attachment 289814 [details]
> Patch

The soup session accept-language is well initialized, making minibrowser work as expected.
But we are creating a new soup session for testing in WRT.
This patch takes the simple approach to set Accept-Language for testing session with a fixed default value.
Comment 4 Carlos Garcia Campos 2016-09-26 05:04:26 PDT
So, maybe the bug is that we are using SoupNetworkSession::defaultSession() directly in NetworkProcess::userPreferredLanguagesChanged() and we should use NetworkStorageSession::defaultStorageSession().soupNetworkSession()?
Comment 5 youenn fablet 2016-09-26 05:39:17 PDT
AIUI, the default soup session gets updated after the network process is launched and properly set.

When we launch tests using WTR, test controller asks to use the testing network session. At that time, we lose all soup session specific options previously set and we are back to the default values.
Comment 6 Carlos Garcia Campos 2016-09-26 05:42:13 PDT
(In reply to comment #5)
> AIUI, the default soup session gets updated after the network process is
> launched and properly set.
> 
> When we launch tests using WTR, test controller asks to use the testing
> network session. At that time, we lose all soup session specific options
> previously set and we are back to the default values.

Ok, there's probably more places where we are using the default soup session directly, we need to check that carefully, for now it make sense to use a sane default value for the accept-language property.
Comment 7 Carlos Garcia Campos 2016-09-26 05:42:46 PDT
Comment on attachment 289814 [details]
Patch

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

> Source/WebCore/platform/network/soup/SoupNetworkSession.cpp:69
> +    auto cookieJar = adoptGRef(createPrivateBrowsingCookieJar());
> +    auto newSoupSession = std::unique_ptr<SoupNetworkSession>(new SoupNetworkSession(cookieJar.get()));
> +    g_object_set(newSoupSession.get(), "accept-language", "en-us", nullptr);
> +    return newSoupSession;

Maybe we could simply add a fixme here.
Comment 8 Michael Catanzaro 2016-09-26 11:56:22 PDT
Please do add a FIXME
Comment 9 youenn fablet 2016-09-26 12:25:23 PDT
Created attachment 289842 [details]
Patch for landing
Comment 10 WebKit Commit Bot 2016-09-26 12:57:27 PDT
Comment on attachment 289842 [details]
Patch for landing

Clearing flags on attachment: 289842

Committed r206387: <http://trac.webkit.org/changeset/206387>
Comment 11 WebKit Commit Bot 2016-09-26 12:57:31 PDT
All reviewed patches have been landed.  Closing bug.
Comment 12 youenn fablet 2016-09-27 03:41:25 PDT
Patch is not working according bots.
Comment 13 youenn fablet 2016-09-27 03:42:42 PDT
Created attachment 289931 [details]
Patch
Comment 14 youenn fablet 2016-09-27 03:43:01 PDT
(In reply to comment #13)
> Created attachment 289931 [details]
> Patch

This time, it should work...
Comment 15 Csaba Osztrogonác 2016-09-27 04:06:16 PDT
(In reply to comment #10)
> Comment on attachment 289842 [details]
> Patch for landing
> 
> Clearing flags on attachment: 289842
> 
> Committed r206387: <http://trac.webkit.org/changeset/206387>

It made all perf tests fail on EFL and GTK performance bots:
- https://build.webkit.org/builders/GTK%20Linux%2064-bit%20Release%20%28Perf%29/builds/6718
- https://build.webkit.org/builders/EFL%20Linux%2064-bit%20Release%20WK2%20%28Perf%29/builds/10018


(process:23353): GLib-GObject-CRITICAL **: g_object_set: assertion 'G_IS_OBJECT (object)' failed

...
Comment 16 WebKit Commit Bot 2016-09-27 06:13:33 PDT
Comment on attachment 289931 [details]
Patch

Clearing flags on attachment: 289931

Committed r206431: <http://trac.webkit.org/changeset/206431>
Comment 17 WebKit Commit Bot 2016-09-27 06:13:38 PDT
All reviewed patches have been landed.  Closing bug.