Bug 104865

Summary: Handle HTTP 204 status codes in MainResourceLoader
Product: WebKit Reporter: Martin Robinson <mrobinson>
Component: Page LoadingAssignee: Martin Robinson <mrobinson>
Status: RESOLVED DUPLICATE    
Severity: Normal CC: ap, beidson, gyuyoung.kim, japhet, laszlo.gombos, luiz, rakuco, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Martin Robinson
Reported 2012-12-12 17:17:09 PST
This follows the fix for Qt in https://bugs.webkit.org/show_bug.cgi?id=60206.
Attachments
Patch (6.92 KB, patch)
2012-12-13 01:47 PST, Martin Robinson 		no flags
DetailsFormatted DiffDiff
Patch (9.27 KB, patch)
2012-12-23 09:13 PST, Martin Robinson 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Martin Robinson
Comment 1 2012-12-13 01:47:01 PST
Created attachment 179230 [details] Patch
Laszlo Gombos
Comment 2 2012-12-17 16:49:52 PST
looks good to me.
Alexey Proskuryakov
Comment 3 2012-12-17 16:57:54 PST
Does this work on Mac now? If if does, why do we need to change cross-platform code?
Martin Robinson
Comment 4 2012-12-17 21:39:33 PST
The test is still skipped on Mac. On reflection, my patch should probably unskip the test on all platforms. Assuming that one platform handles this properly, is it still better to repeat the logic in all n-1 WebKit layers or to wrap the code in an #ifdef?
Alexey Proskuryakov
Comment 5 2012-12-17 22:01:29 PST
Well, if Mac already handles this properly, my question would be how it does that, and whether it's better design. I haven't thought about this patch deeply.
Laszlo Gombos
Comment 6 2012-12-18 18:42:30 PST
I think comment https://bugs.webkit.org/show_bug.cgi?id=60206#c6 is relevant for discussing this bug.
Alexey Proskuryakov
Comment 7 2012-12-18 19:33:28 PST
Indeed!
Alexey Proskuryakov
Comment 8 2012-12-18 20:27:34 PST
Looks reasonable to me. Someone who recently worked on this code should probably do final review. Can you unskip the test on other platforms, to let Mac EWS run the test? Please also consider duping to the older bug.
Martin Robinson
Comment 9 2012-12-23 09:13:22 PST
Created attachment 180623 [details] Patch
Martin Robinson
Comment 10 2012-12-23 09:14:45 PST
*** Bug 60206 has been marked as a duplicate of this bug. ***
Martin Robinson
Comment 11 2012-12-23 09:16:35 PST
Thanks for the comments. I uploaded a new patch unskipping this test everywhere, except WebKit2. For WebKit2 the TestExpectations comments suggested there is still more work to do on testRunner.queueLoad. I've also marked 60206 a duplicate of this bug.
Alexey Proskuryakov
Comment 12 2012-12-25 19:52:51 PST
> I've also marked 60206 a duplicate of this bug. Why not the other way around? It's normally advisable to keep an older bug.
Martin Robinson
Comment 13 2012-12-25 21:10:54 PST
(In reply to comment #12) > > I've also marked 60206 a duplicate of this bug. > > Why not the other way around? It's normally advisable to keep an older bug. I usually dupe the ones that don't have a patch, but I'm happy to upload the patch at the other bug and mark this one as the duplicate.
Martin Robinson
Comment 14 2012-12-27 10:27:41 PST
*** This bug has been marked as a duplicate of bug 60206 ***
Eric Seidel (no email)
Comment 15 2013-01-04 00:56:37 PST
Comment on attachment 180623 [details] Patch Cleared review? from attachment 180623 [details] so that this bug does not appear in http://webkit.org/pending-review. If you would like this patch reviewed, please attach it to a new bug (or re-open this bug before marking it for review again).
Note You need to log in before you can comment on or make changes to this bug.