Use soup's content sniffing

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Gustavo Noronha (kov)]

Now that soup has a content sniffing feature, we should use it instead of our own limited implementation.

[Gustavo Noronha (kov)]

Created attachment 32279 [details]
Use soup's content sniffing

 WebCore/ChangeLog                                  |   21 ++++++++
 WebCore/platform/network/ResourceHandleInternal.h  |    1 -
 .../platform/network/soup/ResourceHandleSoup.cpp   |   54 ++++++++++---------
 WebKit/gtk/ChangeLog                               |   12 ++++
 WebKit/gtk/webkit/webkitprivate.cpp                |    5 ++
 5 files changed, 66 insertions(+), 27 deletions(-)

[Gustavo Noronha (kov)]

Comment on attachment 32279 [details]
Use soup's content sniffing

TODO: depend on 2.27.3 libsoup, when it is released =P.

[Jan Alonzo]

Comment on attachment 32279 [details]
Use soup's content sniffing

> -            , m_reportedHeaders(false)

Please don't forget to remove m_reportedHeaders decl too.

> +static void contentSniffedCallback(SoupMessage* msg, const char* sniffedType, GHashTable *params, gpointer data)
> +{
> +    if (sniffedType) {
> +        const char* officialType = soup_message_headers_get_one(msg->response_headers, "Content-Type");
> +
> +        if (!officialType || strcmp(officialType, sniffedType))
> +            soup_message_headers_set_content_type(msg->response_headers, sniffedType, params);
> +    }

Ok. r=me.

[Jan Alonzo]

(In reply to comment #3)
> (From update of attachment 32279 [details])
> > -            , m_reportedHeaders(false)
> 
> Please don't forget to remove m_reportedHeaders decl too.
> 
> > +static void contentSniffedCallback(SoupMessage* msg, const char* sniffedType, GHashTable *params, gpointer data)
> > +{
> > +    if (sniffedType) {
> > +        const char* officialType = soup_message_headers_get_one(msg->response_headers, "Content-Type");
> > +
> > +        if (!officialType || strcmp(officialType, sniffedType))
> > +            soup_message_headers_set_content_type(msg->response_headers, sniffedType, params);
> > +    }
> 
> Ok. r=me.

FYI, this regressed three XHR tests (Debian amd64, unstable, ToT + this patch)

Running tests from /home/jmalonzo/OpenSource/WebKit/LayoutTests
Testing 77 test cases.
http/tests/xmlhttprequest ........
http/tests/xmlhttprequest/access-control-basic-allow-preflight-cache.html -> failed
............
http/tests/xmlhttprequest/cache-override.html -> failed
...........................................
http/tests/xmlhttprequest/small-chunks-response-text.html -> failed
..
http/tests/xmlhttprequest/web-apps ...........
http/tests/xmlhttprequest/workers .
29.21s total testing time

74 test cases (96%) succeeded
3 test cases (3%) had incorrect layout
1 test case (1%) had stderr output
zsh: exit 1     WEBKITOUTPUTDIR=~/OpenSource/WebKit/build  --gtk --http --no-new-test-results

[Gustavo Noronha (kov)]

Landed in r45558.

(In reply to comment #4)
> FYI, this regressed three XHR tests (Debian amd64, unstable, ToT + this patch)

I'll take a look at this issue, I'm not sure what's going on, but it should be fixed before we release a version. We also need to require soup 2.27.3, like I said. I'll leave this bug open to track those.

[Gustavo Noronha (kov)]

The regressions seem to be caused by a bug in soup. I reported my investigation there, and intend to fix the problem there: http://bugzilla.gnome.org/show_bug.cgi?id=587907

[Eric Seidel (no email)]

Comment on attachment 32279 [details]
Use soup's content sniffing

This was landed, clearing flag.

[Jan Alonzo]

(In reply to comment #6)
> The regressions seem to be caused by a bug in soup. I reported my investigation
> there, and intend to fix the problem there:
> http://bugzilla.gnome.org/show_bug.cgi?id=587907

Raised https://bugs.webkit.org/show_bug.cgi?id=27143 for the regression. Bug fixed in http://trac.webkit.org/changeset/45558.

[Hussam Al-Tayeb]

This change makes it fail to build with libsoup 2.26.x
If 2.27.3 is the minimal dependency, should the configure script complain if I have 2.26 installed?

[Gustavo Noronha (kov)]

(In reply to comment #9)
> This change makes it fail to build with libsoup 2.26.x
> If 2.27.3 is the minimal dependency, should the configure script complain if I
> have 2.26 installed?

The minimal dependency is 2.27.4 (danw decided to skip 2.27.3 to sync up with GNOME release numbering). The configure script was not complaining because libsoup had not yet updated its release numbering, so we couldn't check for it.