Bug 24600

Summary: [GTK] responses with status code >= 400 should not be given special treatment
Product: WebKit Reporter: Gustavo Noronha (kov) <gns>
Component: WebKitGTKAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal Keywords: Gtk, Soup
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: All   
Attachments:
Description Flags
proposed fix
none
proposed fix zecke: review+

Description Gustavo Noronha (kov) 2009-03-14 15:04:52 PDT
Current code avoids calling gotHeaders and gotChunk for messages that are not "successful". This makes them skip things such as content sniffing, and would hog the memory for responses with big bodies, were accumulate not disabled =). This last fact is the reason why error pages with content, such as plone.org's 404 pages are not displayed.
Comment 1 Gustavo Noronha (kov) 2009-03-14 15:07:26 PDT
Created attachment 28630 [details]
proposed fix
Comment 2 Holger Freyther 2009-03-17 10:26:17 PDT
Okay, any idea about the impact on the http tests?
Comment 3 Gustavo Noronha (kov) 2009-03-17 12:31:26 PDT
(In reply to comment #2)
> Okay, any idea about the impact on the http tests?
> 

I was getting some tests fixed, and one regression. I have reworked the patch to fix the xmlhttprequest regression, and we are now down to 2 failing tests, and no crash, in my tests. I'm going to upload the patch now.
Comment 4 Gustavo Noronha (kov) 2009-03-17 12:33:42 PDT
Created attachment 28695 [details]
proposed fix

This patch differs from the other in that it also does not give special treatment to 304 regarding gotHeaders and gotChunk, but does avoid trying a content sniff on it. On my local setup, I get the following impact on the http tests:

Without the patch:

Fails:

http/tests/xmlhttprequest/access-control-basic-whitelist-request-headers.html
http/tests/xmlhttprequest/xmlhttprequest-missing-file-exception.html
http/tests/xmlhttprequest/xmlhttprequest-no-content-length-onProgress.html
http/tests/xmlhttprequest/web-apps/013.html

Crashes:

http/tests/xmlhttprequest/web-apps/002-simple.html

With the patch:

Fails:

http/tests/xmlhttprequest/access-control-basic-whitelist-request-headers.html
http/tests/xmlhttprequest/xmlhttprequest-missing-file-exception.html
Comment 5 Gustavo Noronha (kov) 2009-03-17 14:17:46 PDT
Comment on attachment 28630 [details]
proposed fix

Forgot to obsolete when uploading the new one.
Comment 6 Holger Freyther 2009-03-25 00:17:24 PDT
Comment on attachment 28695 [details]
proposed fix


> +    if ((msg->status_code != SOUP_STATUS_NOT_MODIFIED)
> +        &&!soup_message_headers_get_content_type(msg->response_headers, NULL))

style. put a space in there...
Comment 7 Gustavo Noronha (kov) 2009-03-25 07:50:12 PDT
Landed as r41974 with the style fix.