[webkit-reviews] review denied: [Bug 203273] [GTK][WPE] Support setting response headers in custom URI scheme handlers : [Attachment 381614] WIP Patch v2

[bugzilla-daemon at webkit.org]

Carlos Garcia Campos <cgarcia at igalia.com> has denied Adrian Perez
<aperez at igalia.com>'s request for review:
Bug 203273: [GTK][WPE] Support setting response headers in custom URI scheme
handlers
https://bugs.webkit.org/show_bug.cgi?id=203273

Attachment 381614: WIP Patch v2

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




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

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

How does this solves the problem of allowing CORS? This should include a unit
test for that case. My main concern here is that this introduces HTTP headers
for a non-HTTP load, it's weird.

> Source/WebKit/UIProcess/API/glib/WebKitURISchemeRequest.cpp:69
> +    GRefPtr<SoupMessageHeaders> headers {
adoptGRef(soup_message_headers_new(SOUP_MESSAGE_HEADERS_RESPONSE)) };

SoupMessageHeaders is not a GObject, you need to include
<WebCore/GUniquePtrSoup.h> and use GUniquePtr instead. I prefer to use this
initialization here for simple fixed values, so I would move this to
webkitURISchemeRequestCreate().

> Source/WebKit/UIProcess/API/glib/WebKitURISchemeRequest.cpp:165
> + * webkit_uri_scheme_request_get_headers:

request_get_headers sounds like it's returning the request headers. I would use
webkit_uri_scheme_request_get_response_headers().

> Source/WebKit/UIProcess/API/glib/WebKitURISchemeRequest.cpp:227
> + * Note that passing values for @stream_length and @content_type other than
-1
> + * and %NULL will override the corresponding headers which may have been
> + * previously set in the #SoupMessageHeaders object returned by
> + * webkit_uri_scheme_request_get_headers().

This is what makes me doubt about this API. Maybe it would be better to add
webkit_uri_scheme_request_finish_with_headers() or something similar.