// FIXME: How could this ever happen when RenderWidget retains the Widget!? Indeed.
Created attachment 215965 [details] Patch
Comment on attachment 215965 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=215965&action=review > Source/WebCore/ChangeLog:8 > + There's no way we can be in ~FrameView() while also being owned by > + a RenderWidget. Remove some overly paranoid code that was making sure > + the renderer didn't have a reference on us. Could you have put in an assertion or is this so obviously true that an assertion would be a waste?
(In reply to comment #2) > (From update of attachment 215965 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=215965&action=review > > > Source/WebCore/ChangeLog:8 > > + There's no way we can be in ~FrameView() while also being owned by > > + a RenderWidget. Remove some overly paranoid code that was making sure > > + the renderer didn't have a reference on us. > > Could you have put in an assertion or is this so obviously true that an assertion would be a waste? I thought about it, and it seemed to me that asserting in ~Foo() that someone's RefPtr<Foo> isn't pointing to |this| is overly paranoid. WDYT?
(In reply to comment #3) > I thought about it, and it seemed to me that asserting in ~Foo() that someone's RefPtr<Foo> isn't pointing to |this| is overly paranoid. WDYT? Yes, and no. Paranoid, yes. But catching overrelease in the destructor might be great instead of having to debug use of the deallocated memory later. I wish every class could have a check like this one. If this would help us debug a case where we someone does an extra deref, then it would be great. Not really sure how to judge.
Comment on attachment 215965 [details] Patch Clearing flags on attachment: 215965 Committed r158625: <http://trac.webkit.org/changeset/158625>
All reviewed patches have been landed. Closing bug.
(In reply to comment #4) > (In reply to comment #3) > > I thought about it, and it seemed to me that asserting in ~Foo() that someone's RefPtr<Foo> isn't pointing to |this| is overly paranoid. WDYT? > > Yes, and no. Paranoid, yes. But catching overrelease in the destructor might be great instead of having to debug use of the deallocated memory later. I wish every class could have a check like this one. If this would help us debug a case where we someone does an extra deref, then it would be great. Doesn't RefCounted already do something like this with the deletion-has-begun assertions?
(In reply to comment #7) > Doesn't RefCounted already do something like this with the deletion-has-begun assertions? Maybe it does!