[webkit-reviews] review granted: [Bug 197079] Crash in FrameLoader::stopAllLoaders via [WebView dealloc] inside ~ObjCEventListener : [Attachment 367770] Fixes the bug

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Ryosuke Niwa <rniwa at webkit.org>'s
request for review:
Bug 197079: Crash in FrameLoader::stopAllLoaders via [WebView dealloc] inside
~ObjCEventListener
https://bugs.webkit.org/show_bug.cgi?id=197079

Attachment 367770: Fixes the bug

https://bugs.webkit.org/attachment.cgi?id=367770&action=review




--- Comment #3 from Darin Adler <darin at apple.com> ---
Comment on attachment 367770
  --> https://bugs.webkit.org/attachment.cgi?id=367770
Fixes the bug

View in context: https://bugs.webkit.org/attachment.cgi?id=367770&action=review

> Source/WebKitLegacy/mac/DOM/ObjCEventListener.mm:75
> +    // Avoid executing arbitrary code during GC; e.g. inside Node::~Node.
> +    [m_listener.get() retain];
> +    [m_listener.get() autorelease];

Some day we will move to ARC. To be prepared for when we do, it will be much
more future-proof to use CFRetain and CFAutorelease, which can be used for this
purpose even under ARC. I suggest doing that.

    CFRetain((__bridge CFTypeRef)m_listener.get());
    CFAutorelease((__bridge CFTypeRef)m_listener.get());

To make it cleaner, if this idiom comes up more often, we could add a RetainPtr
member function designed for this purpose. Unfortunately, the current
RetainPtr::autorelease is not what we want because it compiles to a bridging
release under ARC, not an autorelease.