Bug 123644 – Removing unnecessary early returns in addTrack, removeTrack and removeRemoteSource methods

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

[Thiago de Barros Lacerda]

We want it fail if an invalid vector of tracks is returned from getTrackVectorForType.
ASSERT_NOT_REACHED in getTrackVectorForType is taking care of that.

[Thiago de Barros Lacerda]

Created attachment 215772 [details]
Patch

[Darin Adler]

Comment on attachment 215772 [details]
Patch

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

r=me because I think this change is OK, but I think we can do better

> Source/WebCore/Modules/mediastream/MediaStream.cpp:166
>      MediaStreamTrackVector* tracks = getTrackVectorForType(track->source()->type());

If this function is guaranteed to never return null, then it should return a reference rather than a pointer. Also, in our coding style we do not use the word “get” for a function with a return value. I’d fix those two things and then it will be more obvious that this change is correct.

[Thiago de Barros Lacerda]

(In reply to comment #2)
> (From update of attachment 215772 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=215772&action=review
> 
> r=me because I think this change is OK, but I think we can do better
> 
> > Source/WebCore/Modules/mediastream/MediaStream.cpp:166
> >      MediaStreamTrackVector* tracks = getTrackVectorForType(track->source()->type());
> 
> If this function is guaranteed to never return null, then it should return a reference rather than a pointer. Also, in our coding style we do not use the word “get” for a function with a return value. I’d fix those two things and then it will be more obvious that this change is correct.

It is supposed to never return null. If for some reason it returns null the failure must be made clear, that is why there is an ASSERT_NOT_REACHED in the last switch case. That is why I'm not return a reference.

Regarding the method name, I will change it

[Thiago de Barros Lacerda]

Created attachment 215777 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 215777 [details]
Patch

Clearing flags on attachment: 215777

Committed r158470: <http://trac.webkit.org/changeset/158470>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.