Bug 118001 – [Qt] REGRESSION(r132916): Mac build broken by createCFString changes

• Status: RESOLVED
• Resolution: INVALID
• Component: WebCore Misc.

[Tobias Netzel]

r132916 changed createCFString() to return a RetainPtr to a CFString instead of a CFString itself.
In that patch there are missing a few cases where adoptCF() is still called on the return value of createCFString(), leading to crashes here where that String is then collected by the ObjC garbage collector, although it still needs to be accessed.

[Tobias Netzel]

Created attachment 205420 [details]
don't adopt CFStrings twice

[Alexey Proskuryakov]

I don't understand how this code can compile. Does any platform actually build it?

HyphenationMac.mm is 10.6 only, and I don't think that we support building WebKit with 10.6. This file should be just deleted.

PluginPackageMac.cpp is referenced from WebCore.pri, but then again, I'm not sure how it compiles.

[Tobias Netzel]

Bug 102057 is referring PluginPackageMac.cpp.

Originally I had found the bug in recently removed Carbon specific code in PluginViewMac.mm and the other occurences I found by running grep on the entire source tree.

[Alexey Proskuryakov]

OK, so it's not "adopted twice", but a compilation failure?

[Tobias Netzel]

First adoption in Source/WebCore/platform/text/cf/StringImplCF.cpp:
RetainPtr<CFStringRef> StringImpl::createCFString()
{
    ...
    return adoptCF(string);
}


Second ones here:
    WTF::RetainPtr<CFStringRef> homeDir = adoptCF(homeDirectoryPath().createCFString());
and here:
    WTF::RetainPtr<CFStringRef> path = adoptCF(m_path.createCFString());

[Anders Carlsson]

(In reply to comment #5)
> First adoption in Source/WebCore/platform/text/cf/StringImplCF.cpp:
> RetainPtr<CFStringRef> StringImpl::createCFString()
> {
>     ...
>     return adoptCF(string);
> }
> 
> 
> Second ones here:
>     WTF::RetainPtr<CFStringRef> homeDir = adoptCF(homeDirectoryPath().createCFString());
> and here:
>     WTF::RetainPtr<CFStringRef> path = adoptCF(m_path.createCFString());

adoptCF does not take a RetainPtr, so the code above would not compile.

[Tobias Netzel]

Indeed I previously had changed the concerning lines to adopt the result of createCFString().get() instead of just createCFString() in order to get the code compiled (but that led to the described crashes).

I guess that means I've found rotten code that has to be eliminated.

[Anders Carlsson]

This patch looks good, but if the code is unused we should just delete it. Is it still used?

[Anders Carlsson]

HyphenationMac.mm has been removed and we haven't seen any build errors from PluginPackageMac so it's clear that code isn't built (we should consider removing it).

I'm going to close this bug.