[webkit-reviews] review granted: [Bug 227902] [GLib] Expose API to access/modify capture devices states : [Attachment 433871] Patch

[bugzilla-daemon at webkit.org]

Carlos Garcia Campos <cgarcia at igalia.com> has granted Philippe Normand
<pnormand at igalia.com>'s request for review:
Bug 227902: [GLib] Expose API to access/modify capture devices states
https://bugs.webkit.org/show_bug.cgi?id=227902

Attachment 433871: Patch

https://bugs.webkit.org/attachment.cgi?id=433871&action=review




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

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

> Source/WebKit/UIProcess/API/glib/WebKitUserMediaPermissionRequest.cpp:149
> +    g_return_val_if_fail(request->priv->request, FALSE);

I don't understand this. I know we are doing the same in the other methods, but
I didn't know it. The request is set after object is constructed and we take a
ref, so it can't be nullptr, but even in that case, it wouldn't be a programmer
error, so g_return macros are not a good way to check that. I think this should
just check the request passed to the function with
g_return_val_if_fail(WEBKIT_IS_USER_MEDIA_PERMISSION_REQUEST(request), FALSE);
And the same for the other functions.

> Source/WebKit/UIProcess/API/glib/WebKitWebView.cpp:4957
> + * If WebKitSettings:enable-mediastream is %FALSE, this method will have no
visible effect. Once the

#WebKitSettings:enable-mediastream

> Source/WebKit/UIProcess/API/glib/WebKitWebView.cpp:4999
> + * If WebKitSettings:enable-mediastream is %FALSE, this method will have no
visible effect. Once the

#WebKitSettings:enable-mediastream

> Source/WebKit/UIProcess/API/glib/WebKitWebView.cpp:5041
> + * If WebKitSettings:enable-mediastream is %FALSE, this method will have no
visible effect. Once the

#WebKitSettings:enable-mediastream

> Tools/TestWebKitAPI/glib/WebKitGLib/WebViewTest.h:67
> +    void waitUntilCameraCaptureStateChangedTo(WebKitMediaCaptureState);
> +    void waitUntilDisplayCaptureStateChangedTo(WebKitMediaCaptureState);
> +    void waitUntilMicrophoneCaptureStateChangedTo(WebKitMediaCaptureState);

Since this is only used by UIClient test, I think we could move it there
instead.