Bug 26982 - Use soup's content sniffing
Summary: Use soup's content sniffing
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC All
: P2 Normal
Assignee: Nobody
URL:
Keywords: Gtk, Soup
Depends on:
Blocks:
 
Reported: 2009-07-05 12:25 PDT by Gustavo Noronha (kov)
Modified: 2009-07-13 12:12 PDT (History)
2 users (show)

See Also:


Attachments
Use soup's content sniffing (7.17 KB, patch)
2009-07-05 12:32 PDT, Gustavo Noronha (kov)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.