Bug 164670 - MediaStreamPrivate::create should take vectors of Ref
Summary: MediaStreamPrivate::create should take vectors of Ref
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: youenn fablet
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-11-11 18:55 PST by youenn fablet
Modified: 2016-11-14 09:49 PST (History)
1 user (show)

See Also:

Attachments
Patch (10.76 KB, patch)
2016-11-11 18:58 PST, youenn fablet 		no flags Details | Formatted Diff | Diff
Patch (10.81 KB, patch)
2016-11-12 09:30 PST, youenn fablet 		no flags Details | Formatted Diff | Diff
Patch for landing (10.83 KB, patch)
2016-11-14 09:13 PST, youenn fablet 		no flags Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description youenn fablet 2016-11-11 18:55:22 PST 
It currently takes vector of RefPtr
Comment 1 youenn fablet 2016-11-11 18:58:32 PST 
Created attachment 294580 [details]
Patch
Comment 2 youenn fablet 2016-11-12 09:30:25 PST 
Created attachment 294615 [details]
Patch
Comment 3 Sam Weinig 2016-11-12 20:30:21 PST 
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().
Comment 4 youenn fablet 2016-11-14 09:13:17 PST 
Created attachment 294704 [details]
Patch for landing
Comment 5 youenn fablet 2016-11-14 09:19:39 PST 
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
Comment 6 WebKit Commit Bot 2016-11-14 09:49:19 PST 
Comment on attachment 294704 [details]
Patch for landing

Clearing flags on attachment: 294704

Committed r208687: <http://trac.webkit.org/changeset/208687>
Comment 7 WebKit Commit Bot 2016-11-14 09:49:22 PST 
All reviewed patches have been landed.  Closing bug.