Bug 116680

Summary:

We need to clear main resource when detaching DocumentLoader from the frame.

Product:

WebKit

Reporter:

Yongjun Zhang <yongjun_zhang>

Component:

Page Loading

Assignee:

Yongjun Zhang <yongjun_zhang>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

ap, beidson, commit-queue, ddkilzer, japhet, koivisto, yongjun_zhang

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
clear main resource in detachFromFrame.	none
don't clear the main resource, just remove docLoader from its client set to avoid getting callback after it is detached.	none
fix style issue.	none

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	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
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Yongjun Zhang

Comment 1 2013-05-23 09:34:42 PDT

<rdar://problem/13924120>

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.