[Webkit-unassigned] [Bug 231880] [GTK][WPE] Support setting status code and getting HTTP method in custom URI scheme handlers

[bugzilla-daemon at webkit.org]

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

Carlos Garcia Campos <cgarcia at igalia.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #442335|review?                     |review-
              Flags|                            |

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

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

> Source/WebKit/ChangeLog:9
> +        * SourcesGTK.txt: Added WebKitURISchemeRequest.cpp
> +        * SourcesWPE.txt: Added WebKitURISchemeRequest.cpp

You mean WebKitURISchemeResponse.cpp

> Source/WebKit/UIProcess/API/glib/WebKitURISchemeRequest.cpp:199
> +    WebKitURISchemeResponsePrivate* respPriv = priv->response->priv;

You shouldn't need this.

> Source/WebKit/UIProcess/API/glib/WebKitURISchemeRequest.cpp:201
> +        CString contentType = WebKitURISchemeResponseGetContentType(respPriv);

Private getter should receive a WebKitURISchemeResponse as parameter.

> Source/WebKit/UIProcess/API/glib/WebKitURISchemeRequest.cpp:208
> +        if (statusMessage.isNull())
> +            webkit_uri_scheme_response_set_status(priv->response.get(), 200, "OK");
> +        response.setHTTPStatusCode(WebKitURISchemeResponseGetStatusCode(respPriv));
> +        response.setHTTPStatusText(statusMessage.data());

I think you should set the status code and if message is not null set status text text too, but we don't need to call webkit_uri_scheme_response_set_status() in any case.

> Source/WebKit/UIProcess/API/glib/WebKitURISchemeRequestPrivate.h:23
> +#include "WebKitURISchemeResponsePrivate.h"

Include this from WebKitURISchemeRequest.cpp not from here.

> Source/WebKit/UIProcess/API/glib/WebKitURISchemeResponse.cpp:27
> +#include "APIData.h"
> +#include "WebKitPrivate.h"
> +#include "WebURLSchemeTask.h"
> +#include <WebCore/HTTPParsers.h>
> +#include <WebCore/MIMETypeRegistry.h>

I don't think these are needed.

> Source/WebKit/UIProcess/API/glib/WebKitURISchemeResponse.cpp:29
> +#include <wtf/glib/RunLoopSourcePriority.h>

Nor this one

> Source/WebKit/UIProcess/API/glib/WebKitURISchemeResponse.cpp:80
> +CString WebKitURISchemeResponseGetStatusMessage(const WebKitURISchemeResponsePrivate* response)

const CString

> Source/WebKit/UIProcess/API/glib/WebKitURISchemeResponse.cpp:85
> +CString WebKitURISchemeResponseGetContentType(const WebKitURISchemeResponsePrivate* response)

Ditto

> Source/WebKit/UIProcess/API/glib/WebKitURISchemeResponse.cpp:98
> +void WebKitURISchemeResponseDisposeStream(WebKitURISchemeResponsePrivate* response)
> +{
> +    response->stream = nullptr;
> +}

This is unused.

> Source/WebKit/UIProcess/API/glib/WebKitURISchemeResponse.cpp:110
> +/**
> + * webkit_uri_scheme_response_set_status_body:
> + * @response: a #WebKitURISchemeResponse
> + * @stream: a #GInputStream to read the contents of the request
> + * @stream_length: the length of the stream or -1 if not known
> + * @content_type: (allow-none): the content type of the stream or %NULL if not known
> + *
> + * Sets the status message for the @response
> + *
> + * Since: 2.36
> + */

This documentation isn't correct, this is webkit_uri_scheme_response_new().

> Source/WebKit/UIProcess/API/glib/WebKitURISchemeResponse.cpp:120
> +    response->priv->statusCode = -1;
> +    response->priv->stream = inputStream;
> +    // We use -1 in the API for consistency with soup when the content length is not known, but 0 internally.
> +    response->priv->streamLength = streamLength == -1 ? 0 : streamLength;

I've just realized that this is a problem for bindings because this is a public constructor. The statusCode initialization could be done in the struct declaration and stream and length should be construct only properties.

> Source/WebKit/UIProcess/API/glib/WebKitURISchemeResponse.cpp:127
> + * @content_type: (allow-none): the content type of the stream or %NULL if not known

I'm not sure we should allow none now that we have a specific setter for this, if unknown don't call this function.

> Source/WebKit/UIProcess/API/glib/WebKitURISchemeResponsePrivate.h:26
> +#include "WebKitURISchemeResponse.h"
> +#include "WebKitWebContext.h"
> +#include "WebPageProxy.h"
> +#include "WebURLSchemeTask.h"
> +#include <WebCore/ResourceRequest.h>

You don't need all this headers here.

-- 
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/20211027/e61a6ea9/attachment.htm>