[Webkit-unassigned] [Bug 210422] [GTK][WPE] Add API to expose UIClient::requestStorageAccessConfirm

[bugzilla-daemon at webkit.org]

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

Michael Catanzaro <mcatanzaro at gnome.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #401725|review?                     |review+
              Flags|                            |

--- Comment #6 from Michael Catanzaro <mcatanzaro at gnome.org> ---
Comment on attachment 401725
  --> https://bugs.webkit.org/attachment.cgi?id=401725
Patch

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

I think it would be pretty hard to create an API test for this. Manual testing should suffice. (Bonus points for Epiphany implementation.) Do we know of any existing websites that make use of the storage access API with either Safari or Firefox?

> Source/WebKit/UIProcess/API/glib/WebKitWebsiteDataAccessPermissionRequest.cpp:33
> + * WebKitUserMediaPermissionRequest represents a request for

WebKitWebsiteDataPermissionRequest

> Source/WebKit/UIProcess/API/glib/WebKitWebsiteDataAccessPermissionRequest.cpp:34
> + * permission to allow a third-party domain access cookies and website data.

access to cookies and website data

Although, I'm not actually certain it *does* allow a third-party domain to access website data other than cookies. Are you sure about this? Firefox allows this according to https://developer.mozilla.org/en-US/docs/Web/API/Storage_Access_API, but its documentation implies that WebKit does not. John would know exactly how this works.

> Source/WebKit/UIProcess/API/glib/WebKitWebsiteDataAccessPermissionRequest.cpp:81
> +    // Default behaviour when no decision has been made is denying the request.
> +    webkitWebsiteDataAccessPermissionRequestDeny(WEBKIT_PERMISSION_REQUEST(object));

So I see this is copied from our various other permission request classes, but all those classes have a bool madeDecision member variable to track whether a permission request has previously been completed or not and only call the CompletionHandler if so. You don't have that here, so it's going to call the CompletionHandler again unconditionally. If you try this patch in a debug build, I think it's going to assert, because CompletionHandler is designed to assert when called twice.

> Source/WebKit/UIProcess/API/glib/WebKitWebsiteDataAccessPermissionRequest.cpp:125
> +/**
> + * webkit_website_data_access_permission_request_get_current_domain:
> + * @request: a #WebKitWebsiteDataAccessPermissionRequest
> + *
> + * Get the domain from which the requesting domain wants to access cookies and website data while browsing.
> + *
> + * Returns: the current domain name
> + *
> + * Since: 2.30

Hm, I assumed that the domain requesting storage access only gets access to its *own* cookies (and maybe also other website data), not access to the first-party domain's data. Does it really get access to the data of the first-party domain? John?

Please investigate before landing; we need to be sure about this before we land this API.

> Source/WebKit/UIProcess/API/gtk/WebKitWebsiteDataAccessPermissionRequest.h:56
> +struct _WebKitWebsiteDataAccessPermissionRequestClass {
> +    GObjectClass parent_class;
> +
> +    void (*_webkit_reserved0) (void);
> +    void (*_webkit_reserved1) (void);
> +    void (*_webkit_reserved2) (void);
> +    void (*_webkit_reserved3) (void);
> +};

Idle brainstorming: now that GTK 4 is upon us, it might be a good idea to figure out if it's possible to hide these class structs from the GTK 4 API. There's no reason for this type to be derivable, and no value in exposing class structs for non-derivable types. Problem is we can't use #if USE(GTK4) in the header files, so we'd need a third copy of all the public header files... possibly not worth it.

-- 
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/20200612/efc8ecbc/attachment.htm>