[Webkit-unassigned] [Bug 94373] [gtk] adding methods and signals for usermedia communication between webkit and browser

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=94373





--- Comment #41 from Gustavo Noronha (kov) <gns at gnome.org>  2012-10-19 10:07:30 PST ---
(From update of attachment 169431)
View in context: https://bugs.webkit.org/attachment.cgi?id=169431&action=review

The API looks good to me, I'm just wondering about passing the list of chocies to allow; the spec says currently in its implementation suggestions: "A single call to getUserMedia() will always return a stream with either zero or one audio tracks, and either zero or one video tracks.", what do you think?

> Source/WebKit/gtk/WebCoreSupport/UserMediaClientGtk.cpp:49
> +    RefPtr<UserMediaRequest> request = prpRequest;
> +    g_signal_emit_by_name(m_webView, "choose-media-device", kitNew(request.get()), kitNew(audioSource), kitNew(videoSource));

The spec says already in use devices should be considered busy and not shown, do you know whether we're doing that kind of filtering atm?

> Source/WebKit/gtk/webkit/webkitwebusermedialist.cpp:32
> + * @Title: WebKitWebUserMediaList

>From the looks of it the names here are case insensitive, but might as well be consistent and keep everything lower-case.

> Source/WebKit/gtk/webkit/webkitwebusermedialist.cpp:70
> +    // placement new syntax

This comment's not very useful.

> Source/WebKit/gtk/webkit/webkitwebusermediarequest.cpp:141
> +void webkit_web_user_media_request_allow(WebKitWebUserMediaRequest* webRequest, WebKitWebUserMediaList* audioMediaList, WebKitWebUserMediaList* videoMediaList)

The implementation suggestion seems to say that there should be either 0 or 1 selected items in a reply to getUserMedia, currently, I'm wondering if using a list with possibly multiple selections is the way to go (http://dev.w3.org/2011/webrtc/editor/getusermedia.html#implementation-suggestions)

> Source/WebKit/gtk/webkit/webkitwebview.cpp:126
> +

Left over new line?

> Source/WebKit/gtk/webkit/webkitwebview.cpp:1217
> +    UserMediaSelectorData* data = g_new(UserMediaSelectorData, 1);

This looks like a good place to use g_slice_new =)

> Source/WebKit/gtk/webkit/webkitwebview.cpp:1221
> +    data->request = (WebKitWebUserMediaRequest*) g_object_ref(request);
> +    data->audioMediaList = (WebKitWebUserMediaList*) g_object_ref(audioMediaList);
> +    data->videoMediaList = (WebKitWebUserMediaList*) g_object_ref(videoMediaList);

We usually do C++-style casts, static_cast<WebKit...*>()

> Source/WebKit/gtk/webkit/webkitwebview.h:162
> +    gboolean                   (* choose_media_device)    (WebKitWebView             *web_view,

Adding this to the middle of the class struct breaks ABI. That's not a problem if we decide to break ABI for 2.0, but something to keep in mind.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.