WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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
Details
Formatted Diff
Diff
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.
Top of Page
Format For Printing
XML
Clone This Bug