Bug 116680 - We need to clear main resource when detaching DocumentLoader from the frame.
Summary: We need to clear main resource when detaching DocumentLoader from the frame.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Page Loading (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Yongjun Zhang
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-05-23 09:34 PDT by Yongjun Zhang
Modified: 2013-05-23 14:17 PDT (History)
7 users (show)

See Also:


Attachments
clear main resource in detachFromFrame. (1.59 KB, patch)
2013-05-23 09:59 PDT, Yongjun Zhang
no flags Details | Formatted Diff | Diff
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 Details | Formatted Diff | Diff
fix style issue. (3.39 KB, patch)
2013-05-23 12:52 PDT, Yongjun Zhang
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yongjun Zhang 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.
Comment 1 Yongjun Zhang 2013-05-23 09:34:42 PDT
<rdar://problem/13924120>
Comment 2 Yongjun Zhang 2013-05-23 09:59:16 PDT
Created attachment 202723 [details]
clear main resource in detachFromFrame.
Comment 3 Alexey Proskuryakov 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.
Comment 4 Yongjun Zhang 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.
Comment 5 Yongjun Zhang 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.
Comment 6 WebKit Commit Bot 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.
Comment 7 Yongjun Zhang 2013-05-23 12:52:02 PDT
Created attachment 202736 [details]
fix style issue.
Comment 8 Alexey Proskuryakov 2013-05-23 13:03:00 PDT
Makes sense to me, but I think that Brady would be the best reviewer here.
Comment 9 Yongjun Zhang 2013-05-23 13:55:41 PDT
Comment on attachment 202736 [details]
fix style issue.

thanks Brady.
Comment 10 WebKit Commit Bot 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>
Comment 11 WebKit Commit Bot 2013-05-23 14:17:27 PDT
All reviewed patches have been landed.  Closing bug.