Bug 120541

Summary: Make sure remove CachedResourceClient when destructing IconLoader
Product: WebKit Reporter: Leo Yang <leoyang>
Component: Page LoadingAssignee: Leo Yang <leoyang>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, japhet, mark.toller, staikos
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch none

Leo Yang
Reported 2013-08-30 11:37:11 PDT
It's a good practice to call CachedResource::removeClient(client) when the client destructing. We need to do this for InconLoader to prevent m_resource from keeping dangling client in case m_resource is referenced by someone else in the future.
Attachments
Patch (2.22 KB, patch)
2013-08-30 11:43 PDT, Leo Yang 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Leo Yang
Comment 1 2013-08-30 11:43:13 PDT
Created attachment 210141 [details] Patch
Darin Adler
Comment 2 2013-08-30 11:50:18 PDT
Comment on attachment 210141 [details] Patch I am really concerned that we are doing these fixes without regression tests. Over the years we have found the extra effort to demonstrate the problem with a test is well worth it.
Leo Yang
Comment 3 2013-08-30 11:58:11 PDT
(In reply to comment #2) > (From update of attachment 210141 [details]) > I am really concerned that we are doing these fixes without regression tests. Over the years we have found the extra effort to demonstrate the problem with a test is well worth it. I understand the importance of regression tests. But for this specific change it's just defensive and following the common good practice of removing client when destructing. At the moment this is not a bug without the patch because m_resource is going to be destroyed soon anyway.
WebKit Commit Bot
Comment 4 2013-08-30 12:20:31 PDT
Comment on attachment 210141 [details] Patch Clearing flags on attachment: 210141 Committed r154905: <http://trac.webkit.org/changeset/154905>
WebKit Commit Bot
Comment 5 2013-08-30 12:20:32 PDT
All reviewed patches have been landed. Closing bug.
Alexey Proskuryakov
Comment 6 2013-10-03 11:51:03 PDT
*** Bug 106663 has been marked as a duplicate of this bug. ***
Note You need to log in before you can comment on or make changes to this bug.