[webkit-gtk] WK2: Adapting the Policy Decision Framework for Geolocation

[Carlos Garcia Campos]

El vie, 30-03-2012 a las 13:13 +0200, Mario Sanchez Prada escribió:
> Hi all,
> 

> 
> However, I'm afraid the current implementation of PolicyDecision has
> some problems for reusing that stuff _cleanly_ for geolocation. My
> biggest concerns are:
> 
>  - I understand this 'use', 'ignore' and 'downlad' actions just mimic
>    the C API which, at the end, is mapping the cases in WebCore's
>    PolicyAction. And it's fine as long as we only use this from the
>    Policy Client. But it's weird to have a 'download' operation
>    available for the object emitted with the 'decide-policy' signal when
>    making a decision about geolocation.

Note that download() doesn't make sense for most of navigation actions
either:

typedef enum {
    WEBKIT_NAVIGATION_TYPE_LINK_CLICKED,
    WEBKIT_NAVIGATION_TYPE_FORM_SUBMITTED,
    WEBKIT_NAVIGATION_TYPE_BACK_FORWARD,
    WEBKIT_NAVIGATION_TYPE_RELOAD,
    WEBKIT_NAVIGATION_TYPE_FORM_RESUBMITTED,
    WEBKIT_NAVIGATION_TYPE_OTHER,
} WebKitNavigationType; 

Download it's typically used for WebKitResponsePolicyDecision.

>  - Current implementation of WebKitPolicyDecision (see .cpp file) 
>    assumes there's a listener, of type WKFramePolicyListenerRef, where 
>    the 'use', 'ignore' and 'download' operations would be delegated 
>    when making a decision from the app. For the case of geolocation,
>    though, this "listener" would not be an object of that type but of
>    type WKGeolocationPermissionRequestRef, which provides two methods to
>    allow making a decision: allow() and deny().
> 
> So I'm thinking of slightly changing the Policy Decision design to fit
> this geolocation stuff, and I've thought of two possibilities so far.
> I've attached some quick diagrams I made with Dia to better explain
> what's in my head at this moment.
> 
> * Option 1 (geolocationWK2-option1.png):
> ----------------------------------------
> 
>  - Rename WebKitPolicyDecision to WebKitFramePolicyDecision

We are trying to avoid Frame in public API.

>  - Rename the use() and ignore() methods to accept() and reject(), 
>    which are more general, even if internal implementation is the same,
>    in terms of WKFramePolicyListenerRef's use() and ignore() methods.

Sounds good. Ignore is actually reject, since it cancels the action
indeed.

>  - Create a new abstract class (WebKitPolicyDecision) with two abstract 
>    methods, accept() and reject(). WebKitFramePolicyDecision would be a 
>    subclass of this one, obviously.

While this makes sense internally, even converting WebKitPolicyDecision
into a interface, I think it makes the classes design a bit more complex
for little benefit. The only problem is that download() doesn't make
sense for some decisions, but that's not solved by this approach either,
because download() can't be used for any navigation action. I think
users already know when to call download or not.

>  - Create a new class WebKitGeolocationPolicyDecision, implementing 
>    accept() and reject() in terms of WKGeolocationPermissionRequestRef's
>    allow() and deny() methods.

I simpler approach would be:

 - Rename use()/ignore() to accept()/reject() or allow()/deny().
 - Add WKGeolocationPermissionRequestRef attribute to
WebKitPolicyDecision and
webkitPolicyDecisionSetGeolocationPermissionRequest().
 - Add WebKitGeolocationPolicyDecision that calls
webkitPolicyDecisionSetGeolocationPermissionRequest() when created.
 - That way WebKitPolicyDecision will always have either a listener or a
geolocation request, and it will use whatever it has, doing nothing in
download() if listener is NULL.
 - Document that download() doesn't make sense for geolocation
decisions, if it's not obvious enough.

What do you think?

-- 
Carlos Garcia Campos
http://pgp.rediris.es:11371/pks/lookup?op=get&search=0xF3D322D0EC4582C3
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part
URL: <http://lists.webkit.org/pipermail/webkit-gtk/attachments/20120402/a1321b7f/attachment.bin>