[webkit-reviews] review granted: [Bug 123350] [MSE] Make MediaSourcePrivate, SourceBufferPrivate classes RefCounted. : [Attachment 215194] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Jer Noble <jer.noble at apple.com>'s
request for review:
Bug 123350: [MSE] Make MediaSourcePrivate, SourceBufferPrivate classes
RefCounted.
https://bugs.webkit.org/show_bug.cgi?id=123350

Attachment 215194: Patch
https://bugs.webkit.org/attachment.cgi?id=215194&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=215194&action=review


> Source/WebCore/Modules/mediasource/SourceBuffer.h:54
> +    static PassRefPtr<SourceBuffer> create(PassRefPtr<SourceBufferPrivate>,
MediaSource*);

Since this argument can never be null, it should be PassRef instead of
PassRefPtr.

> Source/WebCore/Modules/mediasource/SourceBuffer.h:87
> +    SourceBuffer(PassRefPtr<SourceBufferPrivate>, MediaSource*);

Since this argument can never be null, it should be PassRef instead of
PassRefPtr.

> Source/WebCore/Modules/mediasource/WebKitSourceBuffer.h:50
> +    static RefPtr<WebKitSourceBuffer>
create(PassRefPtr<SourceBufferPrivate>, PassRefPtr<WebKitMediaSource>);

Since this argument can never be null, it should be PassRef instead of
PassRefPtr.

Since the result of this function can never be null, it could be PassRef
instead of RefPtr.

> Source/WebCore/html/HTMLMediaSource.h:56
> +    virtual void setPrivateAndOpen(PassRefPtr<MediaSourcePrivate>) = 0;

Since this argument can never be null, it should be PassRef instead of
PassRefPtr.

> Source/WebCore/platform/graphics/SourceBufferPrivate.h:51
> +protected:
> +    SourceBufferPrivate() { }

This isn’t needed and can just be omitted. Since there are already pure virtual
functions in this class, nobody can create an instance by calling the
constructor except for a derived class, so the constructor is already
effectively protected. And an empty constructor need not be written explicitly
except to make it protected. So I suggest you just leave this out.