Bug 26982

Summary: Use soup's content sniffing
Product: WebKit Reporter: Gustavo Noronha (kov) <gns>
Component: WebKitGTKAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: ht990332, jmalonzo
Priority: P2 Keywords: Gtk, Soup
Version: 528+ (Nightly build)   
Hardware: PC   
OS: All   
Attachments:
Description Flags
Use soup's content sniffing none

Description Gustavo Noronha (kov) 2009-07-05 12:25:17 PDT
Now that soup has a content sniffing feature, we should use it instead of our own limited implementation.
Comment 1 Gustavo Noronha (kov) 2009-07-05 12:32:10 PDT
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(-)
Comment 2 Gustavo Noronha (kov) 2009-07-06 04:02:49 PDT
Comment on attachment 32279 [details]
Use soup's content sniffing

TODO: depend on 2.27.3 libsoup, when it is released =P.
Comment 3 Jan Alonzo 2009-07-06 04:47:33 PDT
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.
Comment 4 Jan Alonzo 2009-07-06 05:04:51 PDT
(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
Comment 5 Gustavo Noronha (kov) 2009-07-06 09:34:51 PDT
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.
Comment 6 Gustavo Noronha (kov) 2009-07-06 10:40:32 PDT
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
Comment 7 Eric Seidel (no email) 2009-07-06 13:22:08 PDT
Comment on attachment 32279 [details]
Use soup's content sniffing

This was landed, clearing flag.
Comment 8 Jan Alonzo 2009-07-09 23:50:07 PDT
(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.
Comment 9 Hussam Al-Tayeb 2009-07-12 10:12:15 PDT
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?
Comment 10 Gustavo Noronha (kov) 2009-07-13 12:12:48 PDT
(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.