RESOLVED FIXED Bug 116680
We need to clear main resource when detaching DocumentLoader from the frame.
https://bugs.webkit.org/show_bug.cgi?id=116680
Summary We need to clear main resource when detaching DocumentLoader from the frame.
Yongjun Zhang
Reported 2013-05-23 09:34:28 PDT
Normally, when we detach the documentLoader in DocumentLoader::detachFromFrame, main resource is also cleared in stopLoading(). There is possibility that main resource not being cleared, and this could cause crash later since docLoader could still receive resource callbacks.
Attachments
clear main resource in detachFromFrame. (1.59 KB, patch)
2013-05-23 09:59 PDT, Yongjun Zhang
no flags
don't clear the main resource, just remove docLoader from its client set to avoid getting callback after it is detached. (3.43 KB, patch)
2013-05-23 12:45 PDT, Yongjun Zhang
no flags
fix style issue. (3.39 KB, patch)
2013-05-23 12:52 PDT, Yongjun Zhang
no flags
Yongjun Zhang
Comment 1 2013-05-23 09:34:42 PDT
Yongjun Zhang
Comment 2 2013-05-23 09:59:16 PDT
Created attachment 202723 [details] clear main resource in detachFromFrame.
Alexey Proskuryakov
Comment 3 2013-05-23 10:56:27 PDT
What if the document gets attached to a frame again (when restoring from page cache)? I suspect that it still needs its main resource then.
Yongjun Zhang
Comment 4 2013-05-23 11:11:13 PDT
(In reply to comment #3) > What if the document gets attached to a frame again (when restoring from page cache)? I suspect that it still needs its main resource then. Good question! that would be a problem. I think instead of clear the main resource, we should just remove DocumentLoader from its client set, that way m_mainResource would still be alive and we won't receive callbacks when it is detached. Will post another patch.
Yongjun Zhang
Comment 5 2013-05-23 12:45:40 PDT
Created attachment 202734 [details] don't clear the main resource, just remove docLoader from its client set to avoid getting callback after it is detached.
WebKit Commit Bot
Comment 6 2013-05-23 12:46:19 PDT
Attachment 202734 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/loader/DocumentLoader.cpp', u'Source/WebCore/loader/cache/CachedResource.h']" exit_code: 1 Source/WebCore/loader/DocumentLoader.cpp:1416: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Total errors found: 1 in 3 files If any of these errors are false positives, please file a bug against check-webkit-style.
Yongjun Zhang
Comment 7 2013-05-23 12:52:02 PDT
Created attachment 202736 [details] fix style issue.
Alexey Proskuryakov
Comment 8 2013-05-23 13:03:00 PDT
Makes sense to me, but I think that Brady would be the best reviewer here.
Yongjun Zhang
Comment 9 2013-05-23 13:55:41 PDT
Comment on attachment 202736 [details] fix style issue. thanks Brady.
WebKit Commit Bot
Comment 10 2013-05-23 14:17:25 PDT
Comment on attachment 202736 [details] fix style issue. Clearing flags on attachment: 202736 Committed r150613: <http://trac.webkit.org/changeset/150613>
WebKit Commit Bot
Comment 11 2013-05-23 14:17:27 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.