[webkit-reviews] review granted: [Bug 24051] Soup backend needs content sniffing capabilities : [Attachment 28356] content sniffing for the soup backend

[bugzilla-daemon at webkit.org]

Holger Freyther <zecke at selfish.org> has granted Gustavo Noronha (kov)
<gns at gnome.org>'s request for review:
Bug 24051: Soup backend needs content sniffing capabilities
https://bugs.webkit.org/show_bug.cgi?id=24051

Attachment 28356: content sniffing for the soup backend
https://bugs.webkit.org/attachment.cgi?id=28356&action=review

------- Additional Comments from Holger Freyther <zecke at selfish.org>
> From 2c0d6818a8549c54ff66e9f0ee6a14e2c4c0be86 Mon Sep 17 00:00:00 2001
> From: Gustavo Noronha Silva <kov at kov.eti.br>
> Date: Fri, 6 Mar 2009 10:30:52 -0300
> Subject: [PATCH] 2009-03-06  Gustavo Noronha Silva  <gns at gnome.org>
> 
>	  Reviewed by NOBODY (OOPS!).
> 
>	  https://bugs.webkit.org/show_bug.cgi?id=24051
>	  Soup backend needs content sniffing capabilities
> 
>	  Perform content sniffing when using soup, so that we have a chance
>	  of figuring out the Content-Type of the file if it's not sent by
>	  the server.
> 
>	  * platform/network/ResourceHandleInternal.h:
>	  (WebCore::ResourceHandleInternal::ResourceHandleInternal):
>	  * platform/network/soup/ResourceHandleSoup.cpp:
>	  (WebCore::gotHeadersCallback):
>	  (WebCore::gotChunkCallback):
> ---
>  WebCore/ChangeLog				      |   17 ++++++++++++++++
>  WebCore/platform/network/ResourceHandleInternal.h  |    2 +
>  .../platform/network/soup/ResourceHandleSoup.cpp   |   21
+++++++++++++++++++-
>  3 files changed, 39 insertions(+), 1 deletions(-)
> 
> diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
> index 614e0db..2a18f28 100644
> --- a/WebCore/ChangeLog
> +++ b/WebCore/ChangeLog
> @@ -1,3 +1,20 @@
> +2009-03-06  Gustavo Noronha Silva  <gns at gnome.org>
> +
> +	   Reviewed by NOBODY (OOPS!).
> +
> +	   https://bugs.webkit.org/show_bug.cgi?id=24051
> +	   Soup backend needs content sniffing capabilities
> +
> +	   Perform content sniffing when using soup, so that we have a chance
> +	   of figuring out the Content-Type of the file if it's not sent by
> +	   the server.
> +
> +	   * platform/network/ResourceHandleInternal.h:
> +	   (WebCore::ResourceHandleInternal::ResourceHandleInternal):
> +	   * platform/network/soup/ResourceHandleSoup.cpp:
> +	   (WebCore::gotHeadersCallback):
> +	   (WebCore::gotChunkCallback):
> +
>  2009-03-06  Hironori Bono  <hbono at chromium.org>
>  
>	   Reviewed by Alexey Proskuryakov.
> diff --git a/WebCore/platform/network/ResourceHandleInternal.h
b/WebCore/platform/network/ResourceHandleInternal.h
> index a55f945..875854b 100644
> --- a/WebCore/platform/network/ResourceHandleInternal.h
> +++ b/WebCore/platform/network/ResourceHandleInternal.h
> @@ -111,6 +111,7 @@ namespace WebCore {
>  #if USE(SOUP)
>	       , m_msg(0)
>	       , m_cancelled(false)
> +	       , m_reportedHeaders(false)
>	       , m_gfile(0)
>	       , m_inputStream(0)
>	       , m_cancellable(0)
> @@ -183,6 +184,7 @@ namespace WebCore {
>	   SoupMessage* m_msg;
>	   ResourceResponse m_response;
>	   bool m_cancelled;
> +	   bool m_reportedHeaders;
>	   GFile* m_gfile;
>	   GInputStream* m_inputStream;
>	   GCancellable* m_cancellable;
> diff --git a/WebCore/platform/network/soup/ResourceHandleSoup.cpp
b/WebCore/platform/network/soup/ResourceHandleSoup.cpp
> index 82ed142..2169450 100644
> --- a/WebCore/platform/network/soup/ResourceHandleSoup.cpp
> +++ b/WebCore/platform/network/soup/ResourceHandleSoup.cpp
> @@ -214,6 +214,14 @@ static void gotHeadersCallback(SoupMessage* msg,
gpointer data)
>      if (!SOUP_STATUS_IS_SUCCESSFUL(msg->status_code))
>	   return;
>  
> +    soup_message_set_flags(msg, SOUP_MESSAGE_OVERWRITE_CHUNKS);

okay, we can kill that too?


> +
> +    // We still don't know anything about Content-Type, so we will try
> +    // sniffing the contents of the file, and then report that we got
> +    // headers
> +    if (!soup_message_headers_get_content_type(msg->response_headers, NULL))

> +	   return;
> +

according to Danw the header callback is only called once, so this guard is
good enough.


do you happen to have a test case for that as well?