[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 #37 from Danilo Cesar Lemes de Paula <danilo.cesar at collabora.co.uk>  2012-10-16 13:55:30 PST ---
(From update of attachment 168593)
View in context: https://bugs.webkit.org/attachment.cgi?id=168593&action=review

>> Source/WebKit/gtk/ChangeLog:6
>> +        When the user make a getUserMedia call, webview will emit a sigial (user-media-requested)
> 
> make -> makes sigial -> signal

fixed

>> Source/WebKit/gtk/ChangeLog:7
>> +        to the application containing then request itself (webkitwebusermediarequest) and two
> 
> then -> the

fixed

>> Source/WebKit/gtk/ChangeLog:8
>> +        list with the available audio and video devices (webkitwebusermedialist).
> 
> list -> lists

fixed.

>> Source/WebKit/gtk/ChangeLog:17
>> +        On the case the user reject the request, webkit_web_user_media_request_fail must
> 
> On the case -> In case?

fixed.

>> Source/WebCore/platform/gtk/GtkMediaChooserDialog.cpp:25
>> +
> 
> No blank line

fixed.

>> Source/WebCore/platform/gtk/GtkMediaChooserDialog.cpp:39
>> +    GtkWidget* videoMessage = 0;
> 
> How about making this a GtkBuilder file that can more easily be changed if necessary?

discussed on irc about it. Decided to keep simple while the dialog is simpler.
Will change that in case it gets much more complex.

>> Source/WebCore/platform/gtk/GtkMediaChooserDialog.cpp:43
>> +    gint videoListLength = videoSource.size();
> 
> We tend not to use glib types for types that are simple C types.

fixed.

>> Source/WebCore/platform/gtk/GtkMediaChooserDialog.cpp:72
>> +        audioMessage = gtk_label_new(hasAudio ? _("Select from available user audio") : _("No user audio available"));
> 
> These messages are quite cryptic for someone that doesn't know the spec I think. User audios are audio inputs? 'audio' should use the plural in this case I imagine?

fixed all the messages, including title.

>> Source/WebCore/platform/gtk/GtkMediaChooserDialog.cpp:88
>> +            gtk_combo_box_text_append_text(GTK_COMBO_BOX_TEXT(m_audioCombo), g_strdup(audioSource[i]->name().utf8().data()));
> 
> You're leaking the string that you create with g_strdup. Why g_strdup, if the function takes a const char*?

fixed.

>> Source/WebKit/gtk/webkit/webkitwebusermediarequest.cpp:131
>> + * webkit_web_user_media_request_succeed:
> 
> How about sticking to the allow/deny terminology we use for our other policy decisions?

in my first prototype the API was request_accept and request_reject.
I changed it back to succeed/fail to reflect the webkit internals (UserMediaRequest->succeed and fail).

What is best? Reflect the internal API or stick to what we have? To be honest I don't like succeed() and fail(), but I thought that using it would be safer.
Any second thought? Mrobinson?

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