Bug 164670 – MediaStreamPrivate::create should take vectors of Ref

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

[youenn fablet]

It currently takes vector of RefPtr

[youenn fablet]

Created attachment 294580 [details]
Patch

[youenn fablet]

Created attachment 294615 [details]
Patch

[Sam Weinig]

Comment on attachment 294615 [details]
Patch

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

> Source/WebCore/Modules/webaudio/MediaStreamAudioDestinationNode.cpp:54
> +    m_stream = MediaStream::create(*context.scriptExecutionContext(), MediaStreamPrivate::create(audioSources, Vector<Ref<RealtimeMediaSource>>()));

Depending on the overloading story here, you can probably use { }, for the empty Vector.

> Source/WebCore/platform/mediastream/MediaStreamPrivate.cpp:51
>      tracks.reserveCapacity(audioSources.size() + videoSources.size());

This should use reserveInitialCapacity

> Source/WebCore/platform/mediastream/MediaStreamPrivate.cpp:54
> +        tracks.append(MediaStreamTrackPrivate::create(source.copyRef()));

This can be an uncheckedAppend().

> Source/WebCore/platform/mediastream/MediaStreamPrivate.cpp:57
> +        tracks.append(MediaStreamTrackPrivate::create(source.copyRef()));

This can be an uncheckedAppend().

[youenn fablet]

Created attachment 294704 [details]
Patch for landing

[youenn fablet]

Thanks for the review.

(In reply to comment #3)
> Comment on attachment 294615 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=294615&action=review
> 
> > Source/WebCore/Modules/webaudio/MediaStreamAudioDestinationNode.cpp:54
> > +    m_stream = MediaStream::create(*context.scriptExecutionContext(), MediaStreamPrivate::create(audioSources, Vector<Ref<RealtimeMediaSource>>()));
> 
> Depending on the overloading story here, you can probably use { }, for the
> empty Vector.

Done

> > Source/WebCore/platform/mediastream/MediaStreamPrivate.cpp:51
> >      tracks.reserveCapacity(audioSources.size() + videoSources.size());
> 
> This should use reserveInitialCapacity

Done

> > Source/WebCore/platform/mediastream/MediaStreamPrivate.cpp:54
> > +        tracks.append(MediaStreamTrackPrivate::create(source.copyRef()));
> 
> This can be an uncheckedAppend().

Done

> > Source/WebCore/platform/mediastream/MediaStreamPrivate.cpp:57
> > +        tracks.append(MediaStreamTrackPrivate::create(source.copyRef()));
> 
> This can be an uncheckedAppend().

Done

[WebKit Commit Bot]

Comment on attachment 294704 [details]
Patch for landing

Clearing flags on attachment: 294704

Committed r208687: <http://trac.webkit.org/changeset/208687>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.