[webkit-reviews] review granted: [Bug 188799] WeakPtr breaks vtables when upcasting to base classes : [Attachment 370809] Patch

[bugzilla-daemon at webkit.org]

youenn fablet <youennf at gmail.com> has granted Geoffrey Garen
<ggaren at apple.com>'s request for review:
Bug 188799: WeakPtr breaks vtables when upcasting to base classes
https://bugs.webkit.org/show_bug.cgi?id=188799

Attachment 370809: Patch

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




--- Comment #34 from youenn fablet <youennf at gmail.com> ---
Comment on attachment 370809
  --> https://bugs.webkit.org/attachment.cgi?id=370809
Patch

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

> Source/WTF/wtf/WeakPtr.h:97
> +    WeakPtr(Ref<WeakReference>&& ref) :
m_ref(std::forward<Ref<WeakReference>>(ref)) { }

Do we need std::forward here?
We could mark the constructor as explicit.

>
Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourc
eAVFObjC.mm:411
> +    auto weakThis = makeWeakPtr(*this);

Can we remove this line?

>
Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourc
eAVFObjC.mm:862
> +    auto weakThis = makeWeakPtr(*this);

Can we do weakThis = makeWeakPtr(*this) in the lambda below.

>
Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.m
m:923
> +	       WeakPtr<SourceBufferPrivateAVFObjC> weakThis =
makeWeakPtr(*this);

auto, but probably can be created at lambda creation time below.

>
Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.m
m:1310
>      m_decompressionSession->requestMediaDataWhenReady([weakThis] {

weakThis = makeWeakPtr(*this)