Bug 125241 – Add RetainPtr::autorelease to combine leakPtr() + autorelease

• Status: RESOLVED
• Resolution: FIXED
• Component: Platform

[Brian Burg]

Client code should not have to worry about calling leakPtr() manually.

[Pratik Solanki]

Created attachment 225606 [details]
Patch

[Pratik Solanki]

/Volumes/Data/EWS/WebKit/WebKitBuild/Release/usr/local/include/wtf/RetainPtr.h:195:39: error: use of undeclared identifier 'CFAutorelease'
        PtrType ptr = fromStorageType(CFAutorelease(m_ptr));
                                      ^
/Volumes/Data/EWS/WebKit/Source/WebCore/bindings/objc/DOM.mm:558:50: note: in instantiation of member function 'WTF::RetainPtr<NSImage>::autorelease' requested here
    return createDragImageForNode(*frame, *node).autorelease();
                                                 ^
1 error generated.

No CFAutorelease on Mountain Lion. :(

[Darin Adler]

Comment on attachment 225606 [details]
Patch

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

Looks like CFAutorelease is new to Mavericks. We’ll have to do something different to support pre-Mavericks versions of Mac. And maybe something different for Windows too.

r=me, but we need to get this compiling

> Source/WTF/wtf/RetainPtr.h:198
> +    template<typename T> inline typename RetainPtr<T>::PtrType RetainPtr<T>::autorelease()
> +    {
> +        if (!m_ptr)
> +            return 0;
> +        PtrType ptr = fromStorageType(CFAutorelease(m_ptr));
> +        m_ptr = 0;
> +        return ptr;
> +    }

The 0 here should be nullptr.

I am not sure that fromStorageType will always do the right thing with the return value of CFAutorelease. I suggest putting the CFAutorelease on a separate line and ignoring the return value.

[Darin Adler]

(In reply to comment #2)
> No CFAutorelease on Mountain Lion. :(

For MountainLion, we can put a function in some .mm file in WTF that casts the pointer to (id) and then calls -[NSObject autorelease] on it.

[Pratik Solanki]

Created attachment 225671 [details]
For the mountain lion bots

This adds autorelease implementation for Mountain Lion to AutodrainedMac.mm. Lets see how this one fares.

[Darin Adler]

Comment on attachment 225671 [details]
For the mountain lion bots

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

> Source/WTF/wtf/AutodrainedPoolMac.mm:57
> +#if !PLATFORM(IOS) && __MAC_OS_X_VERSION_MIN_REQUIRED < 1090
> +template<typename T> inline typename RetainPtr<T>::PtrType RetainPtr<T>::autorelease()
> +{
> +    if (!m_ptr)
> +        return nullptr;
> +    [(id)m_ptr autorelease];
> +    PtrType ptr = fromStorageType(m_ptr);
> +    m_ptr = nullptr;
> +    return ptr;
> +}
> +#endif

A function template has to be in a header, not an implementation file. I was talking about putting a non-inline function that calls autorelease in the .mm file, which the function template would call.

[Anders Carlsson]

We already did this some time ago.