Created attachment 123091 [details] Patch

Please wait for approval from fishd@chromium.org before submitting because this patch contains changes to the Chromium public API.

Comment on attachment 123091 [details] Patch Attachment 123091 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/11299055 New failing tests: platform/chromium/media/video-capture-preview.html

Created attachment 123108 [details] Patch

Comment on attachment 123108 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=123108&action=review > Source/WebCore/mediastream/MediaStream.cpp:41 > +void processTrackList(PassRefPtr<MediaStreamTrackList> prpTracks, String kind, MediaStreamSourceVector& sources, ExceptionCode& ec) nit: this function should be marked static or placed in an anonymous namespace. nit: String should be passed as |const String&| > Source/WebCore/mediastream/MediaStream.cpp:44 > + if (tracks.get()) { nit: consider an early return if !tracks.get() what is the reason for storing prpTracks as tracks? > Source/WebKit/chromium/public/WebUserMediaClient.h:46 > + virtual void requestUserMedia(const WebUserMediaRequest&, const WebVector<WebMediaStreamSource>&, const WebVector<WebMediaStreamSource>&) { } nit: add parameter names for the new function? it isn't always obvious which is the set of the audio tracks and which is the set of video tracks. shouldn't the comment just be about declaring the first function as DEPRECATED? the second function is the new one, right? > Source/WebKit/chromium/public/WebUserMediaRequest.h:74 > + WEBKIT_EXPORT void requestSucceeded(const WebVector<WebMediaStreamSource>&, const WebVector<WebMediaStreamSource>&); same nit: please name the parameters. it is not clear otherwise which one is for audio and which one is for video. > Source/WebKit/chromium/public/platform/WebMediaStreamDescriptor.h:59 > + WEBKIT_EXPORT void initialize(const WebString& label, const WebVector<WebMediaStreamSource>&, const WebVector<WebMediaStreamSource>&); same nit.

Comment on attachment 123108 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=123108&action=review >> Source/WebCore/mediastream/MediaStream.cpp:41 >> +void processTrackList(PassRefPtr<MediaStreamTrackList> prpTracks, String kind, MediaStreamSourceVector& sources, ExceptionCode& ec) > > nit: this function should be marked static or placed in an anonymous namespace. > nit: String should be passed as |const String&| Fixed * 2 >> Source/WebCore/mediastream/MediaStream.cpp:44 >> + if (tracks.get()) { > > nit: consider an early return if !tracks.get() > > what is the reason for storing prpTracks as tracks? Fixed nit. Don't think I understand your question. I need to store the PassRefPtr into a RefPtr so I can use it more than once. >> Source/WebKit/chromium/public/WebUserMediaClient.h:46 >> + virtual void requestUserMedia(const WebUserMediaRequest&, const WebVector<WebMediaStreamSource>&, const WebVector<WebMediaStreamSource>&) { } > > nit: add parameter names for the new function? it isn't always obvious which is the set of the audio tracks and which is the set of video tracks. > > shouldn't the comment just be about declaring the first function as DEPRECATED? the second function is the new one, right? Fixed >> Source/WebKit/chromium/public/WebUserMediaRequest.h:74 >> + WEBKIT_EXPORT void requestSucceeded(const WebVector<WebMediaStreamSource>&, const WebVector<WebMediaStreamSource>&); > > same nit: please name the parameters. it is not clear otherwise which one is for audio and which one is for video. Fixed. >> Source/WebKit/chromium/public/platform/WebMediaStreamDescriptor.h:59 >> + WEBKIT_EXPORT void initialize(const WebString& label, const WebVector<WebMediaStreamSource>&, const WebVector<WebMediaStreamSource>&); > > same nit. Fixed.

Created attachment 123539 [details] Patch

Comment on attachment 123539 [details] Patch Clearing flags on attachment: 123539 Committed r105774: <http://trac.webkit.org/changeset/105774>

All reviewed patches have been landed. Closing bug.