RESOLVED INVALID 59185
NULL checking for set request's encoding 
https://bugs.webkit.org/show_bug.cgi?id=59185
Summary NULL checking for set request's encoding
Grzegorz Czajkowski
Reported 2011-04-22 03:28:00 PDT
During loading some resources, especially when loading was failed segfault occurred because activeDocumentLoader() has been already destroyed. See callstack in an attachment. It happens when encoding for the request is set and activeDocumentLoader() checks frame's encoding. You can check this on www.latimes.com when WebKit gets resource: https://latimes.signon.trb.com/registration/xd/xd_sender.htm
Attachments
Callstack (5.01 KB, text/plain)
2011-04-22 03:32 PDT, Grzegorz Czajkowski 		no flags
Details
patch (2.01 KB, patch)
2011-04-22 03:55 PDT, Grzegorz Czajkowski 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Grzegorz Czajkowski
Comment 1 2011-04-22 03:32:35 PDT
Created attachment 90685 [details] Callstack
Grzegorz Czajkowski
Comment 2 2011-04-22 03:55:29 PDT
Created attachment 90687 [details] patch
Alexey Proskuryakov
Comment 3 2011-04-22 10:26:08 PDT
This would need a regression test to be landed. However, I doubt that this is the right fix. This problem is not known to occur in other ports, so you should look into what destroys activeDocumentLoader too early, and fix that.
Grzegorz Czajkowski
Comment 4 2011-04-26 05:28:04 PDT
(In reply to comment #3) > This would need a regression test to be landed. > > However, I doubt that this is the right fix. This problem is not known to occur in other ports, so you should look into what destroys activeDocumentLoader too early, and fix that. Ok, the reason of this issue was found. Anyway methods: - FrameLoader::isLoading() - FrameLoader::checkLoadCompleteForThisFrame() checks returned value of activeDocumentLoader() but some of them in FrameLoader not. Do you think that it's safe to have an access to the object if it may return NULL?
Alexey Proskuryakov
Comment 5 2011-04-26 09:20:30 PDT
Adding checks when we know that the returned value can't be null would be very confusing to readers. By having a null check, we're saying that the situation is expected, and we need to handle it adequately. The document encoding is an important part of fallback, so skipping it is not adequate. We would need to find another way to retrieve the document encoding if activeDocumentLoader could be null here.
Grzegorz Czajkowski
Comment 6 2011-04-29 07:41:35 PDT
Ok, thanks for your explanation. I think we can close this bug as invalid.
Note You need to log in before you can comment on or make changes to this bug.