[webkit-reviews] review granted: [Bug 24758] [Gtk] Implement a WebDataSource for the gtk port : [Attachment 38721] updated patch

[bugzilla-daemon at webkit.org]

Xan Lopez <xan.lopez at gmail.com> has granted Jan Alonzo <jmalonzo at gmail.com>'s
request for review:
Bug 24758: [Gtk] Implement a WebDataSource for the gtk port
https://bugs.webkit.org/show_bug.cgi?id=24758

Attachment 38721: updated patch
https://bugs.webkit.org/attachment.cgi?id=38721&action=review

------- Additional Comments from Xan Lopez <xan.lopez at gmail.com>
> +    if (priv->data) {
> +	   g_string_free(priv->data, TRUE);
> +	   priv->data = NULL;
> +    }

Being pedantic, priv->data should be freed in finalize, since it's not a
reference counted object :)

> +
> +    G_OBJECT_CLASS(webkit_web_data_source_parent_class)->dispose(object);
> +}
> +


> +/**
> + * webkit_web_data_source_new_with_request:
> + * @request: the #WebKitNetworkRequest to use to create this data source
> + *
> + * Creates a new #WebKitWebDataSource from a #WebKitNetworkRequest.
Normally,
> + * #WebKitWebFrame objects create their data sources so you will almost
never
> + * want to invoke this method directly.

Do you have any use case for users where it would make sense to use this? It
would be good to mention it I think.

> +/**
> + * webkit_web_data_source_get_initial_request:
> + * @data_source: a #WebKitWebDataSource
> + *
> + * Returns a reference to the original request that was used to load the web

> + * content

I think I would use this to expand the introductory section at the begining of
the file.

> + *
> + * Return value: the original #WebKitNetworkRequest
> + *
> + * Since: 1.1.14
> + */

> +
> +/**
> + * webkit_web_data_source_get_network_request:
> + * @data_source: a #WebKitWebDataSource
> + *
> + * Returns a #WebKitNetworkRequest that was used to create this
#WebKitWebDataSource
> + *
> + * Return value: the #WebKitNetworkRequest that created the @data_source or
> + * %NULL if the @data_source is not attached to the frame or the frame
hasn't been loaded.

It's still not clear to me from the doc what's the difference between this and
the initial request. Only that if this function returns !NULL it means the
resource has been loaded and attached to a frame? Ie, something similar to the
provisional vs non-provisional data source? Could you clarify it in the
documentation? And add tests for it :)

> + *
> + * Since 1.1.14
> + */

I'm going to r+, but let's clarify the documentation a bit and work on adding
tests ASAP.
+	 Reviewed by NOBODY (OOPS!).
> +
> +	   [Gtk] Implement a WebDataSource for the gtk port
> +	   https://bugs.webkit.org/show_bug.cgi?id=24758
> +
>	   Implement WebKitWebResource for the resource-related API for
>	   WebKitWebDataSource.
>