RESOLVED FIXED 120541
Make sure remove CachedResourceClient when destructing IconLoader
https://bugs.webkit.org/show_bug.cgi?id=120541
Summary Make sure remove CachedResourceClient when destructing IconLoader
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
Leo Yang
Comment 1 2013-08-30 11:43:13 PDT
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.