Bug 123350 – [MSE] Make MediaSourcePrivate, SourceBufferPrivate classes RefCounted.

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Jer Noble]

[MSE] Make MediaSourcePrivate, SourceBufferPrivate classes RefCounted.

[Jer Noble]

Created attachment 215194 [details]
Patch

[Darin Adler]

Comment on attachment 215194 [details]
Patch

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.

[Jer Noble]

(In reply to comment #2)
> (From update of attachment 215194 [details])
> 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.

Ahh, PassRef is brand new! I'll change all of the instances of PassRefPtr to PassRef.

> > 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.

Good point.  I'll remove this.

[Jer Noble]

Created attachment 215227 [details]
Patch for landing.

[Jer Noble]

Committed r158270: <http://trac.webkit.org/changeset/158270>