[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 #442655|review?                     |review+, commit-queue-
              Flags|                            |

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

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

Looks good to me, but there are still a few minor issues to fix before landing.

> Source/WebKit/UIProcess/API/glib/WebKitURISchemeRequest.cpp:205
> +        CString statusMessage = WebKitURISchemeResponseGetStatusMessage(resp);

const CString& or const auto&

> Source/WebKit/UIProcess/API/glib/WebKitURISchemeResponse.cpp:25
> +#include "APIData.h"
> +#include "WebKitPrivate.h"
> +#include "WebURLSchemeTask.h"

Are these needed?

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

This is already included in WebKitURISchemeResponsePrivate.h

> Source/WebKit/UIProcess/API/glib/WebKitURISchemeResponse.cpp:65
> +    int statusCode = -1;

int statusCode { -1 };

> Source/WebKit/UIProcess/API/glib/WebKitURISchemeResponse.cpp:86
> +static void webkitURISchemeResponseGetProperty(GObject* object, guint propId, GValue* value, GParamSpec* paramSpec)
> +{
> +    WebKitURISchemeResponse* response = WEBKIT_URI_SCHEME_RESPONSE(object);
> +
> +    switch (propId) {
> +    case PROP_STREAM:
> +        g_value_set_object(value, response->priv->stream.get());
> +        break;
> +    case PROP_STREAM_LENGTH:
> +        g_value_set_int64(value, response->priv->streamLength);
> +        break;
> +    default:
> +        G_OBJECT_WARN_INVALID_PROPERTY_ID(object, propId, paramSpec);
> +    }
> +}

We don't have public getters, so maybe it's better to make the properties as writable + construct_only and don't implement get_property.

> Source/WebKit/UIProcess/API/glib/WebKitURISchemeResponse.cpp:91
> +    gint64 streamLength;

Move this to the case PROP_STREAM_LENGTH

> Source/WebKit/UIProcess/API/glib/WebKitURISchemeResponse.cpp:98
> +        streamLength = g_value_get_int64(value);

gint64 streamLength = g_value_get_int64(value);

> Source/WebKit/UIProcess/API/glib/WebKitURISchemeResponse.cpp:117
> +     */

Since: 2.36

> Source/WebKit/UIProcess/API/glib/WebKitURISchemeResponse.cpp:130
> +     */

Since: 2.36

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

const CString&

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

Ditto.

> Source/WebKit/UIProcess/API/glib/WebKitURISchemeResponse.cpp:185
> +    WebKitURISchemeResponse* response = WEBKIT_URI_SCHEME_RESPONSE(g_object_new(WEBKIT_TYPE_URI_SCHEME_RESPONSE, "stream", inputStream, "stream-length", streamLength, nullptr));
> +    return response;

just return WEBKIT_URI_SCHEME_RESPONSE(g_object_new(WEBKIT_TYPE_URI_SCHEME_RESPONSE, "stream", inputStream, "stream-length", streamLength, nullptr));

-- 
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/20211029/2d5c9bc2/attachment.htm>