[Webkit-unassigned] [Bug 83879] [GTK][WK2] Implement API for Geolocation permission requests in the GTK port

[bugzilla-daemon at webkit.org]

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





--- Comment #9 from Carlos Garcia Campos <cgarcia at igalia.com>  2012-04-16 01:23:00 PST ---
(From update of attachment 137092)
View in context: https://bugs.webkit.org/attachment.cgi?id=137092&action=review

> Source/WebKit2/UIProcess/API/gtk/WebKitPermissionRequest.cpp:62
> +    WebKitPermissionRequestIface* iface = WEBKIT_PERMISSION_REQUEST_GET_IFACE(request);
> +    if (iface->allow)
> +        iface->allow(request);

I guess this should be a required method, so we should enforce to be implemented, no?

> Source/WebKit2/UIProcess/API/gtk/WebKitPermissionRequest.cpp:77
> +    WebKitPermissionRequestIface* iface = WEBKIT_PERMISSION_REQUEST_GET_IFACE(request);
> +    if (iface->deny)
> +        iface->deny(request);

Ditto.

> Source/WebKit2/UIProcess/API/gtk/WebKitPermissionRequest.h:36
> +#define WEBKIT_TYPE_PERMISSION_REQUEST           (webkit_permission_request_get_type())
> +#define WEBKIT_PERMISSION_REQUEST(obj)           (G_TYPE_CHECK_INSTANCE_CAST((obj), WEBKIT_TYPE_PERMISSION_REQUEST, WebKitPermissionRequest))
> +#define WEBKIT_PERMISSION_REQUEST_CLASS(obj)     (G_TYPE_CHECK_CLASS_CAST ((obj), WEBKIT_TYPE_PERMISSION_REQUEST, WebKitPermissionRequestIface))
> +#define WEBKIT_IS_PERMISSION_REQUEST(obj)        (G_TYPE_CHECK_INSTANCE_TYPE((obj), WEBKIT_TYPE_PERMISSION_REQUEST))
> +#define WEBKIT_PERMISSION_REQUEST_GET_IFACE(obj) (G_TYPE_INSTANCE_GET_INTERFACE((obj), WEBKIT_TYPE_PERMISSION_REQUEST, WebKitPermissionRequestIface))

Don't add CLASS macros for interfaces, just WEBKIT_TYPE_PERMISSION_REQUEST, WEBKIT_PERMISSION_REQUEST, WEBKIT_IS_PERMISSION_REQUEST and WEBKIT_PERMISSION_REQUEST_GET_IFACE

> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:323
> +    webViewClass->permission_request = webkitWebViewDecidePermissionRequest;

We use the same name as the signal, so remove 'Decide'.

> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:652
> +     * @request_type: a #WebKitPermissionRequestType denoting the type of @request

WebKitPermissionRequest is an interface, so request is an instance of an object that implements the interface. We usually use FOOT_IS_BAR macros to know the concrete type, so we don't need an enum with all types. I'm not opposed to use an enum though, if you think it's clearer and consistent with policy client API.

> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:660
> +     * It is possible to handle permission requests asynchronously, by
> +     * simply calling g_object_ref() on the @request argument and
> +     * returning %TRUE to block the default signal handler.

I think this is something that depends on the concrete request object, unless we make clear that implementations of WebKitPermissionRequest should allow it.

> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:665
> +     * If the last reference is removed on a #WebKitPermissionRequest
> +     * and no decision has been made, webkit_permission_request_deny()
> +     * will be the default decision. The default signal handler will
> +     * simply call webkit_permission_request_deny().

Ditto, this will be implemented in the finalize of every request implementation.

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