We want it fail if an invalid vector of tracks is returned from getTrackVectorForType. ASSERT_NOT_REACHED in getTrackVectorForType is taking care of that.
Created attachment 215772 [details] Patch
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.
(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
Created attachment 215777 [details] Patch
Comment on attachment 215777 [details] Patch Clearing flags on attachment: 215777 Committed r158470: <http://trac.webkit.org/changeset/158470>
All reviewed patches have been landed. Closing bug.