[webkit-reviews] review granted: [Bug 231467] Switch WTF::bridge_cast to use type traits : [Attachment 440712] Patch v2

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Oct 11 17:48:45 PDT 2021


Darin Adler <darin at apple.com> has granted David Kilzer (:ddkilzer)
<ddkilzer at webkit.org>'s request for review:
Bug 231467: Switch WTF::bridge_cast to use type traits
https://bugs.webkit.org/show_bug.cgi?id=231467

Attachment 440712: Patch v2

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




--- Comment #12 from Darin Adler <darin at apple.com> ---
Comment on attachment 440712
  --> https://bugs.webkit.org/attachment.cgi?id=440712
Patch v2

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

>>> Source/WTF/wtf/RetainPtr.h:224
>>> +	 static_assert(!std::is_convertible_v<PtrType, id> &&
IsTypeComplete<CFTollFreeBridgingTraits<PtrType>>, "bridgingAutorelease() may
only be used for toll-free bridged CF types.");
>> 
>> The idea behind my saying "base the return type on the type that is bridged"
was not to generate compile time errors when we wanted to bridge to just "id".
So I’m not sure it’s worth making this change yet.
> 
> Okay, I'll revert this.  We don't have to solve fixing the return type of
bridgingAutorelease() before landing the patch, correct?

That’s right. A separate thought that we can pursue in its own time.

>>> Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:4134
>>> +		 return bridge_id_cast([protectedSelf
nextTextMarkerForCharacterOffset:characterOffset]);
>> 
>> Is this really right? We need autorelease here. Does bridge_id_cast
implicitly do an autorelease?
> 
> I'm pretty sure this lambda returns a RetainPtr<id>, so this is way more
efficient than autoreleasing it and then re-retaining it, correct?
> 
>	  return
Accessibility::retrieveAutoreleasedValueFromMainThread<id>([&textMarker,
protectedSelf = retainPtr(self)] () -> RetainPtr<id> {
> 
> Maybe I amI missing something?  Is there some unexpected requirement that
this be autoreleased before returning it?

I did not see retrieveAutoreleasedValueFromMainThread, clearly that’s what does
the autorelease. Thanks, my mistake.


More information about the webkit-reviews mailing list