[webkit-reviews] review denied: [Bug 24758] [Gtk] Implement a WebDataSource for the gtk port : [Attachment 33016] Updated frame data source API patch

[bugzilla-daemon at webkit.org]

Xan Lopez <xan.lopez at gmail.com> has denied 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 33016: Updated frame data source API patch
https://bugs.webkit.org/attachment.cgi?id=33016&action=review

------- Additional Comments from Xan Lopez <xan.lopez at gmail.com>
> +static void web_data_source_fixture_teardown(WebDataSourceFixture* fixture,
> +						gconstpointer data)
> +{
> +    g_assert(fixture->mainFrame != NULL);
> +    g_assert(fixture->webView != NULL);

Just g_assert (fixture->mainFrame);, etc?


>  /**
> + * webkit_web_frame_get_data_source:
> + * @frame: a #WebKitWebFrame
> + *
> + * Returns the committed data source.
> + *
> + * Return value: the committed #WebKitWebDataSource or %NULL if the
> + * provisional data source is not done loading.
> + *
> + * Since: 1.1.5

1.1.5? :)

> + */
> +WebKitWebDataSource* webkit_web_frame_get_data_source(WebKitWebFrame* frame)

> +{
> +    g_return_val_if_fail(WEBKIT_IS_WEB_FRAME(frame), NULL);
> +
> +    Frame* coreFrame = core(frame);
> +    WebKitWebDataSource* dataSource;

You need to initialize this to NULL here, otherwise you could crash below.

> +
> +    if (coreFrame && coreFrame->loader()->frameHasLoaded())
> +	   dataSource =
webkit_web_frame_get_data_source_from_core_loader(coreFrame->loader()->document
Loader());
> +
> +    if (!dataSource)
> +	   return NULL;
> +
> +    return dataSource;
> +}
> +
> +/**
> + * webkit_web_frame_get_provisional_data_source:
> + * @frame: a #WebKitWebFrame
> + *
> + * You use the webkit_web_frame_load_request method to initiate a request
that
> + * creates a provisional data source. The provisional data source will
> + * transition to a committed data source once any data has been received.
Use
> + * webkit_web_frame_get_data_source to get the committed data source.
> + *
> + * Return value: the provisional #WebKitWebDataSource or %NULL if a load
> + * request is not in progress or has been completed.
> + *
> + * Since: 1.1.5

Version again.

> + */
> +WebKitWebDataSource*
webkit_web_frame_get_provisional_data_source(WebKitWebFrame* frame)
> +{
> +    g_return_val_if_fail(WEBKIT_IS_WEB_FRAME(frame), NULL);
> +
> +    Frame* coreFrame = core(frame);
> +    WebKitWebDataSource* dataSource =
webkit_web_frame_get_data_source_from_core_loader(coreFrame->loader()->provisio
nalDocumentLoader());
> +
> +    if (!dataSource)
> +	   return NULL;
> +
> +    return dataSource;

You can jsut return the dataSource, this does not really add anything.

Marking as r- for now.