Bug 123648 - Removing MediaStreamTrackVector and MediaStreamSourceVector typedefs
Summary: Removing MediaStreamTrackVector and MediaStreamSourceVector typedefs
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Thiago de Barros Lacerda
URL:
Keywords:
Depends on: 123644
Blocks:
  Show dependency treegraph
 
Reported: 2013-11-01 16:58 PDT by Thiago de Barros Lacerda
Modified: 2013-11-01 19:11 PDT (History)
2 users (show)

See Also:


Attachments
Patch (19.92 KB, patch)
2013-11-01 17:05 PDT, Thiago de Barros Lacerda
no flags Details | Formatted Diff | Diff
Patch (19.87 KB, patch)
2013-11-01 18:27 PDT, Thiago de Barros Lacerda
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Thiago de Barros Lacerda 2013-11-01 16:58:45 PDT
Talking with Eric Carlson we agreed that those reduce code readability.
Comment 1 Thiago de Barros Lacerda 2013-11-01 17:05:30 PDT
Created attachment 215781 [details]
Patch
Comment 2 Eric Carlson 2013-11-01 18:02:45 PDT
Comment on attachment 215781 [details]
Patch

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

This looks good, thanks for doing it! 

r=me, but it looks like it needs to be rebased so I listed a couple of drive-by changes you might make.

> Source/WebCore/Modules/mediastream/MediaStream.cpp:137
> +void MediaStream::cloneMediaStreamTrackVector(Vector<RefPtr<MediaStreamTrack>>& destination, const Vector<RefPtr<MediaStreamTrack>>& origin)

Nit: while you are here, can you change "origin" to "source"?

> Source/WebCore/Modules/mediastream/MediaStream.cpp:219
> +        for (Vector<RefPtr<MediaStreamTrack>>::iterator iter = m_audioTracks.begin(); iter != m_audioTracks.end(); ++iter) {

Nit: while you are here, can you fix this to use "auto"?

> Source/WebCore/Modules/mediastream/MediaStream.cpp:226
> +    for (Vector<RefPtr<MediaStreamTrack>>::iterator iter = m_videoTracks.begin(); iter != m_videoTracks.end(); ++iter) {

Ditto.

> Source/WebCore/Modules/mediastream/MediaStream.cpp:236
> +    for (Vector<RefPtr<MediaStreamTrack>>::iterator iter = m_audioTracks.begin(); iter != m_audioTracks.end(); ++iter) {

Ditto.

> Source/WebCore/Modules/mediastream/MediaStream.cpp:241
> +    for (Vector<RefPtr<MediaStreamTrack>>::iterator iter = m_videoTracks.begin(); iter != m_videoTracks.end(); ++iter) {

Ditto.

> Source/WebCore/Modules/mediastream/UserMediaRequest.cpp:169
> +    for (Vector<RefPtr<MediaStreamTrack>>::iterator iter = tracks.begin(); iter != tracks.end(); ++iter)

Ditto.

> Source/WebCore/Modules/mediastream/UserMediaRequest.cpp:173
> +    for (Vector<RefPtr<MediaStreamTrack>>::iterator iter = tracks.begin(); iter != tracks.end(); ++iter)

Ditto.
Comment 3 Thiago de Barros Lacerda 2013-11-01 18:03:52 PDT
(In reply to comment #2)
> (From update of attachment 215781 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=215781&action=review
> 
> This looks good, thanks for doing it! 
> 
> r=me, but it looks like it needs to be rebased so I listed a couple of drive-by changes you might make.
> 
> > Source/WebCore/Modules/mediastream/MediaStream.cpp:137
> > +void MediaStream::cloneMediaStreamTrackVector(Vector<RefPtr<MediaStreamTrack>>& destination, const Vector<RefPtr<MediaStreamTrack>>& origin)
> 
> Nit: while you are here, can you change "origin" to "source"?
> 
> > Source/WebCore/Modules/mediastream/MediaStream.cpp:219
> > +        for (Vector<RefPtr<MediaStreamTrack>>::iterator iter = m_audioTracks.begin(); iter != m_audioTracks.end(); ++iter) {
> 
> Nit: while you are here, can you fix this to use "auto"?
> 
> > Source/WebCore/Modules/mediastream/MediaStream.cpp:226
> > +    for (Vector<RefPtr<MediaStreamTrack>>::iterator iter = m_videoTracks.begin(); iter != m_videoTracks.end(); ++iter) {
> 
> Ditto.
> 
> > Source/WebCore/Modules/mediastream/MediaStream.cpp:236
> > +    for (Vector<RefPtr<MediaStreamTrack>>::iterator iter = m_audioTracks.begin(); iter != m_audioTracks.end(); ++iter) {
> 
> Ditto.
> 
> > Source/WebCore/Modules/mediastream/MediaStream.cpp:241
> > +    for (Vector<RefPtr<MediaStreamTrack>>::iterator iter = m_videoTracks.begin(); iter != m_videoTracks.end(); ++iter) {
> 
> Ditto.
> 
> > Source/WebCore/Modules/mediastream/UserMediaRequest.cpp:169
> > +    for (Vector<RefPtr<MediaStreamTrack>>::iterator iter = tracks.begin(); iter != tracks.end(); ++iter)
> 
> Ditto.
> 
> > Source/WebCore/Modules/mediastream/UserMediaRequest.cpp:173
> > +    for (Vector<RefPtr<MediaStreamTrack>>::iterator iter = tracks.begin(); iter != tracks.end(); ++iter)
> 
> Ditto.

OK for all
Comment 4 Thiago de Barros Lacerda 2013-11-01 18:27:48 PDT
Created attachment 215791 [details]
Patch
Comment 5 WebKit Commit Bot 2013-11-01 19:11:29 PDT
Comment on attachment 215791 [details]
Patch

Clearing flags on attachment: 215791

Committed r158480: <http://trac.webkit.org/changeset/158480>
Comment 6 WebKit Commit Bot 2013-11-01 19:11:31 PDT
All reviewed patches have been landed.  Closing bug.