2008-04-07 Luca Bruno <lethalman88@gmail.com>
authoralp@webkit.org <alp@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 7 Apr 2008 13:34:45 +0000 (13:34 +0000)
committeralp@webkit.org <alp@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 7 Apr 2008 13:34:45 +0000 (13:34 +0000)
        Reviewed by Alp Toker.

        http://bugs.webkit.org/show_bug.cgi?id=18297
        Bug #18297 - Acid2/Acid3 -tests don't load load with soup.

        Fixes in the libsoup backend: data url parsing, acid tests, redirects,
        response headers handling and re-entrancy issues on job cancellation.
        Thanks to Dan Winship for libsoup hints.

        * platform/network/ResourceHandleInternal.h:
        (WebCore::ResourceHandleInternal::ResourceHandleInternal): add m_cancelled and remove unuseful m_session
        * platform/network/soup/ResourceHandleSoup.cpp:
        (WebCore::restartedCallback): added to route redirects to webkit
        (WebCore::dataCallback): add response headers, some checks and fix re-entrancy
        (WebCore::parseDataUrl): be an idle callback for the main loop
        (WebCore::ResourceHandle::start):
        (WebCore::ResourceHandle::cancel): fixed re-entrancy

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@31682 268f45cc-cd09-0410-ab3c-d52691b4dbfc

WebCore/ChangeLog
WebCore/platform/network/ResourceHandleInternal.h
WebCore/platform/network/soup/ResourceHandleSoup.cpp

index 29596a4..10741bf 100644 (file)
@@ -1,3 +1,23 @@
+2008-04-07  Luca Bruno  <lethalman88@gmail.com>
+
+        Reviewed by Alp Toker.
+
+        http://bugs.webkit.org/show_bug.cgi?id=18297
+        Bug #18297 - Acid2/Acid3 -tests don't load load with soup.
+
+        Fixes in the libsoup backend: data url parsing, acid tests, redirects,
+        response headers handling and re-entrancy issues on job cancellation.
+        Thanks to Dan Winship for libsoup hints.
+
+        * platform/network/ResourceHandleInternal.h:
+        (WebCore::ResourceHandleInternal::ResourceHandleInternal): add m_cancelled and remove unuseful m_session
+        * platform/network/soup/ResourceHandleSoup.cpp:
+        (WebCore::restartedCallback): added to route redirects to webkit
+        (WebCore::dataCallback): add response headers, some checks and fix re-entrancy
+        (WebCore::parseDataUrl): be an idle callback for the main loop
+        (WebCore::ResourceHandle::start):
+        (WebCore::ResourceHandle::cancel): fixed re-entrancy
+
 2008-04-07  Jan Michael Alonzo  <jmalonzo@unpluggable.com>
 
         Build fix, rubber-stamped and landed by ap.
index 7835ddb..db98ae2 100644 (file)
@@ -109,6 +109,7 @@ namespace WebCore {
 #endif
 #if USE(SOUP)
             , m_msg(0)
+            , m_cancelled(false)
 #endif
 #if PLATFORM(QT)
             , m_job(0)
@@ -173,8 +174,8 @@ namespace WebCore {
 #endif
 #if USE(SOUP)
         SoupMessage* m_msg;
-        SoupSession* session;
         ResourceResponse m_response;
+        bool m_cancelled;
 #endif
 #if PLATFORM(QT)
 #if QT_VERSION < 0x040400
index 2776adb..966428e 100644 (file)
@@ -40,6 +40,12 @@ namespace WebCore {
 
 static SoupSession* session = 0;
 
+typedef enum
+{
+    ERROR_TRANSPORT,
+    ERROR_UNKNOWN_PROTOCOL
+};
+
 ResourceHandleInternal::~ResourceHandleInternal()
 {
     if (m_msg) {
@@ -52,33 +58,95 @@ ResourceHandle::~ResourceHandle()
 {
 }
 
+static void fillResponseFromMessage(SoupMessage* msg, ResourceResponse* response)
+{
+    SoupMessageHeadersIter iter;
+    const char* name = NULL;
+    const char* value = NULL;
+    soup_message_headers_iter_init(&iter, msg->response_headers);
+    while (soup_message_headers_iter_next(&iter, &name, &value))
+        response->setHTTPHeaderField(name, value);
+
+    String contentType = soup_message_headers_get(msg->response_headers, "Content-Type");
+    char* uri = soup_uri_to_string(soup_message_get_uri(msg), FALSE);
+    response->setUrl(KURL(uri));
+    g_free(uri);
+    response->setMimeType(extractMIMETypeFromMediaType(contentType));
+    response->setTextEncodingName(extractCharsetFromMediaType(contentType));
+    response->setExpectedContentLength(msg->response_body->length);
+    response->setHTTPStatusCode(msg->status_code);
+    response->setSuggestedFilename(filenameFromHTTPContentDisposition(response->httpHeaderField("Content-Disposition")));
+}
+
+// Called each time the message is going to be sent again except the first time.
+// It's used mostly to let webkit know about redirects.
+static void restartedCallback(SoupMessage* msg, gpointer data)
+{
+    ResourceHandle* handle = static_cast<ResourceHandle*>(data);
+    if (!handle)
+        return;
+    ResourceHandleInternal* d = handle->getInternal();
+    if (d->m_cancelled)
+        return;
+
+    char* uri = soup_uri_to_string(soup_message_get_uri(msg), FALSE);
+    String location = String(uri);
+    g_free(uri);
+    KURL newURL = KURL(handle->request().url(), location);
+            
+    ResourceRequest request = handle->request();
+    ResourceResponse response;
+    request.setURL(newURL);
+    fillResponseFromMessage(msg, &response);
+    if (d->client())
+        d->client()->willSendRequest(handle, request, response);
+    
+    d->m_request.setURL(newURL);
+}
+
+// Called at the end of the message, with all the necessary about the last informations.
+// Doesn't get called for redirects.
 static void dataCallback(SoupSession *session, SoupMessage* msg, gpointer data)
 {
     ResourceHandle* handle = static_cast<ResourceHandle*>(data);
     // TODO: maybe we should run this code even if there's no client?
     if (!handle)
         return;
+
+    ResourceHandleInternal* d = handle->getInternal();
+    // The message has been handled.
+    d->m_msg = NULL;
+
     ResourceHandleClient* client = handle->client();
     if (!client)
         return;
 
-    ResourceResponse response;
+    if (d->m_cancelled)
+        return;
 
-    String contentType = String(soup_message_headers_get(msg->response_headers, "Content-Type"));
-    response.setMimeType(extractMIMETypeFromMediaType(contentType));
-    response.setTextEncodingName(extractCharsetFromMediaType(contentType));
+    if (SOUP_STATUS_IS_TRANSPORT_ERROR(msg->status_code)) {
+        char* uri = soup_uri_to_string(soup_message_get_uri(msg), FALSE);
+        ResourceError error("webkit-network-error", ERROR_TRANSPORT, uri, String::fromUTF8(msg->reason_phrase));
+        g_free(uri);
+        client->didFail(handle, error);
+        return;
+    }
+   
+    fillResponseFromMessage(msg, &d->m_response);
+    client->didReceiveResponse(handle, d->m_response);
 
-    response.setExpectedContentLength(msg->response_body->length);
-    response.setHTTPStatusCode(msg->status_code);
+    // WebCore might have cancelled the job in the while
+    if (d->m_cancelled)
+        return;
 
-    client->didReceiveResponse(handle, response);
     if (msg->response_body->data)
         client->didReceiveData(handle, msg->response_body->data, msg->response_body->length, 0);
     client->didFinishLoading(handle);
 }
 
-static void parseDataUrl(ResourceHandle* handle)
+static gboolean parseDataUrl(gpointer callback_data)
 {
+    ResourceHandle* handle = static_cast<ResourceHandle*>(callback_data);
     String data = handle->request().url().string();
 
     ASSERT(data.startsWith("data:", false));
@@ -133,6 +201,8 @@ static void parseDataUrl(ResourceHandle* handle)
     g_free(outData);
 
     client->didFinishLoading(handle);
+
+    return FALSE;
 }
 
 bool ResourceHandle::start(Frame* frame)
@@ -148,17 +218,23 @@ bool ResourceHandle::start(Frame* frame)
     String protocol = url.protocol();
 
     if (equalIgnoringCase(protocol, "data")) {
-        parseDataUrl(this);
-        return false;
+        // If parseDataUrl is called syncronously the job is not yet effectively started
+        // and webkit won't never know that the data has been parsed even didFinishLoading is called.
+        g_idle_add(parseDataUrl, this);
+        return true;
     }
 
+    String urlString = url.string();
+
     if (!equalIgnoringCase(protocol, "http") && !equalIgnoringCase(protocol, "https")) {
-        // TODO: didFail()?
+        // If we don't call didFail the job is not complete for webkit even false is returned.
+        if (d->client()) {
+            ResourceError error("webkit-network-error", ERROR_UNKNOWN_PROTOCOL, urlString, protocol);
+            d->client()->didFail(this, error);
+        }
         return false;
     }
 
-    String urlString = url.string();
-
     if (url.isLocalFile()) {
         String query = url.query();
         // Remove any query part sent to a local file.
@@ -172,20 +248,8 @@ bool ResourceHandle::start(Frame* frame)
         session = soup_session_async_new();
 
     SoupMessage* msg;
-    const char* method = 0;
-
-    if (request().httpMethod() == "GET")
-        method = SOUP_METHOD_GET;
-    else if (request().httpMethod() == "POST")
-        method = SOUP_METHOD_POST;
-    else if (request().httpMethod() == "HEAD")
-        method = SOUP_METHOD_HEAD;
-    else if (request().httpMethod() == "PUT")
-        method = SOUP_METHOD_PUT;
-    else
-        g_debug ("Unknown method!");
-
-    msg = soup_message_new(method? method : SOUP_METHOD_GET, urlString.utf8().data());
+    msg = soup_message_new(request().httpMethod().utf8().data(), urlString.utf8().data());
+    g_signal_connect(msg, "restarted", G_CALLBACK(restartedCallback), this);
 
     HTTPHeaderMap customHeaders = d->m_request.httpHeaderFields();
     if (!customHeaders.isEmpty()) {
@@ -209,8 +273,7 @@ bool ResourceHandle::start(Frame* frame)
                                  SOUP_MEMORY_COPY, body.data(), body.size());
     }
 
-    d->m_msg = (SoupMessage*)g_object_ref(msg);
-    d->session = session;
+    d->m_msg = static_cast<SoupMessage*>(g_object_ref(msg));
     soup_session_queue_message(session, d->m_msg, dataCallback, this);
 
     return true;
@@ -218,10 +281,12 @@ bool ResourceHandle::start(Frame* frame)
 
 void ResourceHandle::cancel()
 {
-    if (d->m_msg)
-        soup_session_cancel_message (d->session, d->m_msg, SOUP_STATUS_CANCELLED);
-
-    client()->didFinishLoading(this);
+    d->m_cancelled = true;
+    if (d->m_msg) {
+        soup_session_cancel_message(session, d->m_msg, SOUP_STATUS_CANCELLED);
+        // For re-entrancy troubles we call didFinishLoading when the message hasn't been handled yet.
+        d->client()->didFinishLoading(this);
+    }
 }
 
 PassRefPtr<SharedBuffer> ResourceHandle::bufferedData()