[Webkit-unassigned] [Bug 136449] [GTK] UserMedia Permission Request API

[bugzilla-daemon at webkit.org]

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

--- Comment #17 from Carlos Garcia Campos <cgarcia at igalia.com> ---
Comment on attachment 241791
  --> https://bugs.webkit.org/attachment.cgi?id=241791
Patch

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

> Source/WebKit2/UIProcess/API/gtk/WebKitUserMediaPermissionRequest.cpp:106
> + * Returns: TRUE if access to an audio device was requested.

%TRUE

> Source/WebKit2/UIProcess/API/gtk/WebKitUserMediaPermissionRequest.cpp:110
> +gboolean webkit_user_media_permission_is_audio(WebKitUserMediaPermissionRequest* request)

I have the same doubts about the names, this sounds to me like if the permission itself was audio, maybe is_audio_device? or is_for_audio_device?

> Source/WebKit2/UIProcess/API/gtk/WebKitUserMediaPermissionRequest.cpp:120
> + * Returns: TRUE if access to a video device was requested.

%TRUE

> Source/WebKit2/UIProcess/API/gtk/WebKitUserMediaPermissionRequest.cpp:155
> +     * Wether the media device requested has a microphone or not.

We are not requesting a device, but permission to access a device, no?

> Source/WebKit2/UIProcess/API/gtk/WebKitUserMediaPermissionRequest.cpp:177
> +

Extra line here.

> Source/WebKit2/UIProcess/API/gtk/WebKitUserMediaPermissionRequest.h:65
> +WEBKIT_API gboolean
> +webkit_user_media_permission_is_audio(WebKitUserMediaPermissionRequest *request);
> +
> +WEBKIT_API gboolean
> +webkit_user_media_permission_is_video(WebKitUserMediaPermissionRequest *request);

Nit: the parameters should be lined up with the previous method webkit_user_media_permission_request_get_type

> Tools/TestWebKitAPI/Tests/WebKit2Gtk/TestUIClient.cpp:203
> +            g_assert_cmpint(webkit_user_media_permission_is_audio(userMediaRequest), ==, test->m_expectedAudioMedia);
> +            g_assert_cmpint(webkit_user_media_permission_is_video(userMediaRequest), ==, test->m_expectedVideoMedia);

Those are not actually int, use g_assert(a == b);

> Tools/TestWebKitAPI/Tests/WebKit2Gtk/TestUIClient.cpp:302
> +    gboolean m_verifyMediaTypes;
> +    gboolean m_expectedAudioMedia;
> +    gboolean m_expectedVideoMedia;

Use bool instead of gboolean.

> Tools/TestWebKitAPI/Tests/WebKit2Gtk/TestUIClient.cpp:702
> +    test->loadHtml(userMediaRequestHTML, 0);

nullptr instead of 0

> Tools/TestWebKitAPI/Tests/WebKit2Gtk/TestUIClient.cpp:707
> +    test->loadHtml(userMediaRequestHTML, 0);

Ditto.

> Tools/TestWebKitAPI/Tests/WebKit2Gtk/TestUIClient.cpp:739
> +    test->loadHtml(userMediaRequestHTML, 0);

Ditto.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20141119/20cefe71/attachment-0002.html>