Bug 49543 – [GTK] Improve FrameLoader signals. Resource loading

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Sergio Villar Senin]

This bug is about implementing resource loading signals proposed by Martin here:

https://lists.webkit.org/pipermail/webkit-gtk/2010-May/000240.html

[Sergio Villar Senin]

Created attachment 73896 [details]
dispatchWillSendRequest

[Sergio Villar Senin]

Created attachment 73897 [details]
didReceiveResponse

[Sergio Villar Senin]

Created attachment 73898 [details]
didFinishLoading

[Sergio Villar Senin]

Created attachment 73899 [details]
didReceiveContentLength

[Sergio Villar Senin]

Created attachment 73900 [details]
didFailLoading

[Sergio Villar Senin]

I'm sure Martin would like to know about his :)

[Sergio Villar Senin]

Created attachment 74739 [details]
dispatchWillSendRequest

[Sergio Villar Senin]

Created attachment 74741 [details]
didReceiveResponse

[Sergio Villar Senin]

Created attachment 74742 [details]
didFinishLoading

Additionally marking 73897 as obsolete. Forgot to do that before

[Sergio Villar Senin]

Created attachment 74743 [details]
didReceiveContentLength

[Sergio Villar Senin]

Created attachment 74744 [details]
didFailLoading

same here with 73899

[Martin Robinson]

I think it makes sense to make this one big patch and to unskip / change results layout tests at the same time. That way we can land the code and have it tested at the same time.

[Philippe Normand]

I'm going to work on this, if Sergio isn't already!

[Philippe Normand]

I updated the patches and started looking at unskipping tests. But there are some issues to match with mac's resource load delegate dumping. Hopefully next week or the week after I'll have more time to work on this.

[Philippe Normand]

Created attachment 92776 [details]
proposed patch

[WebKit Review Bot]

Attachment 92776 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/plat..." exit_code: 1

Tools/DumpRenderTree/gtk/DumpRenderTree.cpp:1122:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebKit/gtk/webkit/webkitnetworkrequest.cpp:175:  Missing spaces around |  [whitespace/operators] [3]
Total errors found: 2 in 13 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Philippe Normand]

Created attachment 92777 [details]
proposed patch

[WebKit Review Bot]

Attachment 92777 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/plat..." exit_code: 1

Source/WebKit/gtk/webkit/webkitnetworkrequest.cpp:175:  Missing spaces around |  [whitespace/operators] [3]
Total errors found: 1 in 13 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Martin Robinson]

Comment on attachment 92777 [details]
proposed patch

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

Awesome! This looks good to me. Please see my comments below, which comprise mostly stylistic nits. We need to get tha approval of another GTK+ reviewer for this.

> Source/WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp:-389
> -    if (!redirectResponse.isNull()) {
> -        // This is a redirect, so we need to update the WebResource's knowledge
> -        // of the URI.
> -        g_free(webResource->priv->uri);
> -        webResource->priv->uri = g_strdup(request.url().string().utf8().data());
> -    }
> -    

In this ChangeLog you should describe why you removed this bit in the FrameLoaderClient::dispatchWillSendRequest method line.

> Source/WebKit/gtk/webkit/webkitnetworkrequest.cpp:174
> +                                                        NULL,

NULL -> 0 here.

> Source/WebKit/gtk/webkit/webkitnetworkrequest.cpp:306
> +    WebKitNetworkRequestPrivate* priv = request->priv;
> +
> +    return priv->mainURI;

Please just say request->priv->mainURI here. :)

> Source/WebKit/gtk/webkit/webkitwebframe.cpp:332
> +     * the lifetime of the resource, but the contents may change from
> +     * inbetween signal emissions.

"from inbetween" should just be "between" Awesome doc!

> Source/WebKit/gtk/webkit/webkitwebframe.cpp:340
> +            NULL, NULL,

NULL -> 0 here.

> Source/WebKit/gtk/webkit/webkitwebframe.cpp:353
> +     * Emitted when the first byte of data arrives

Period needed here.

> Source/WebKit/gtk/webkit/webkitwebframe.cpp:361
> +            NULL, NULL,

NULL -> 0.

> Source/WebKit/gtk/webkit/webkitwebframe.cpp:372
> +     * Emitted when all the data for the resource was loaded

Period necessary.

> Source/WebKit/gtk/webkit/webkitwebframe.cpp:380
> +            NULL, NULL,

NULL -> 0.

> Source/WebKit/gtk/webkit/webkitwebframe.cpp:391
> +     * Emitted when all the data for the resource was loaded

Period.

> Source/WebKit/gtk/webkit/webkitwebframe.cpp:399
> +            NULL, NULL,

NULL -> 0.

> Source/WebKit/gtk/webkit/webkitwebframe.cpp:411
> +     * Invoked when a resource failed to load

Period.

> Source/WebKit/gtk/webkit/webkitwebframe.cpp:419
> +            NULL, NULL,

NULL -> 0.

> Tools/DumpRenderTree/gtk/DumpRenderTree.cpp:1058
> +static char* urlPath(SoupURI* uri)

Please call this convertSoupURIToURLPath or something equally descriptive, to meet the style guidelines.

> Tools/DumpRenderTree/gtk/DumpRenderTree.cpp:1068
> +    gchar* basename = g_path_get_basename(uri->path);
> +    gchar* dirname = g_path_get_basename(g_path_get_dirname(uri->path));
> +    return g_strdup_printf("%s/%s", dirname, basename);

Please just make this one line. Might want to investigate whether it is possible to use CString here without leaking memory. That would avoid needing to use GOwnPtr. If not, no worries.

> Tools/DumpRenderTree/gtk/DumpRenderTree.cpp:1071
> +static char* urlPath(SoupMessage* soupMessage)

Please make the method name more descriptive, per the style guidelines.

> Tools/DumpRenderTree/gtk/DumpRenderTree.cpp:1080
> +    if (soupMessage) {
> +        SoupURI* requestURI = soup_message_get_uri(soupMessage);
> +        if (requestURI)
> +            path = urlPath(requestURI);
> +    }
> +    return path;

Please use early returns here with something like this:

if (!soupMessage)
    return 0;
if (SoupURI* requestURI = soup_message_get_uri(soupMessage))
    return urlPath(requestURI);
return 0;

> Tools/DumpRenderTree/gtk/DumpRenderTree.cpp:1083
> +static char* urlPath(WebKitNetworkRequest* request)

Please make the helper name more descriptive.

> Tools/DumpRenderTree/gtk/DumpRenderTree.cpp:1086
> +    SoupMessage* soupMessage = webkit_network_request_get_message(request);
> +    return urlPath(soupMessage);

Please make this one line.

> Tools/DumpRenderTree/gtk/DumpRenderTree.cpp:1092
> +    SoupURI* uri = soup_uri_new(webkit_web_resource_get_uri(webResource));
> +    return urlPath(uri);

Ditto.

> Tools/DumpRenderTree/gtk/DumpRenderTree.cpp:1171
> +    if (!g_ascii_strncasecmp(errorDomain, "webkit-network-error-quark", 26))

Wouldn't it be better to use g_str_equal here or is there a chance that error->domain may not be null escaped or have weird case?

> Tools/DumpRenderTree/gtk/DumpRenderTree.cpp:1210
> +    char* description;

You can eliminate this variable entirely.

> Tools/DumpRenderTree/gtk/DumpRenderTree.cpp:1218
> +    description = g_strdup_printf("<NSURLResponse %s, http status code %d>", path.get(), statusCode);
> +    return description;

See above.

[Sergio Villar Senin]

(In reply to comment #19)
> (From update of attachment 92777 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=92777&action=review
> 
> Awesome! This looks good to me. Please see my comments below, which comprise mostly stylistic nits. We need to get tha approval of another GTK+ reviewer for this.
> 
> > Source/WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp:-389
> > -    if (!redirectResponse.isNull()) {
> > -        // This is a redirect, so we need to update the WebResource's knowledge
> > -        // of the URI.
> > -        g_free(webResource->priv->uri);
> > -        webResource->priv->uri = g_strdup(request.url().string().utf8().data());
> > -    }
> > -    

I don't remember the exact details here but when I checked with test results it looked like all of them were assuming that web resources should have the original URL and not the redirected.

[Philippe Normand]

Created attachment 92992 [details]
updated patch

[Martin Robinson]

Comment on attachment 92992 [details]
updated patch

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

Thank you very much for updating the patch! I still have a few concerns though. See below.

> LayoutTests/platform/gtk/Skipped:907
>  fast/loader/recursive-before-unload-crash.html 

What's missing for this one still?

> Source/WebKit/gtk/ChangeLog:23
> +        Based on patch by Sergio Villar Senin <[e-mail]>

Please just put this in the top line:

2011-05-09  Philippe Normand  <[e-mail]> and Sergio Villar Senin  <[e-mail]>

> Source/WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp:389
> -        // This is a redirect, so we need to update the WebResource's knowledge
> -        // of the URI.
> -        g_free(webResource->priv->uri);
> -        webResource->priv->uri = g_strdup(request.url().string().utf8().data());
> -    }
> -    
> +         // This is a redirect, so we need to update the WebResource's knowledge
> +         // of the URI.
> +         g_free(webResource->priv->uri);
> +         webResource->priv->uri = g_strdup(request.url().string().utf8().data());
> +     }
> +

This change was probably accidental?

> Source/WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp:1178
> +    // Notify listeners

You can just omit this comment, in my opinion. :)

> Source/WebKit/gtk/WebCoreSupport/FrameLoaderClientGtk.cpp:1195
> +    // Notify listeners

Ditto.

> Source/WebKit/gtk/webkit/webkitnetworkrequest.cpp:169
> +    /**
> +     * WebKitNetworkRequest:main-uri:
> +     *
> +     * The #ResourceRequest that backs the request.
> +     *
> +     * Since: 1.5.1
> +     */

I'm not sure I understand the difference between the URI and MainURI.  The comment above doesn't seem to explain it either. Why is this property necessary? I don't see a similar one on the Mac port's API surface.

> Source/WebKit/gtk/webkit/webkitnetworkrequest.cpp:300
> +const gchar*
> +webkit_network_request_get_main_uri(WebKitNetworkRequest* request)

Should just be one line here.

> Source/WebKit/gtk/webkit/webkitwebresource.cpp:138
> +            NULL, NULL,

Should turn these NULLs into 0.

> Source/WebKit/gtk/webkit/webkitwebresource.cpp:156
> +            NULL, NULL,

Ditto.

> Source/WebKit/gtk/webkit/webkitwebresource.cpp:173
> +            NULL, NULL,

Ditto.

> Source/WebKit/gtk/webkit/webkitwebresource.cpp:190
> +            NULL, NULL,

Ditto.

> Source/WebKit/gtk/webkit/webkitwebview.cpp:2766
> +            NULL, NULL,

Ditto.

> Source/WebKit/gtk/webkit/webkitwebview.cpp:2787
> +            NULL, NULL,

Ditto.

> Source/WebKit/gtk/webkit/webkitwebview.cpp:2808
> +            NULL, NULL,

Ditto.

> Source/WebKit/gtk/webkit/webkitwebview.cpp:2830
> +            NULL, NULL,

Ditto.

> Tools/ChangeLog:10
> +        Based on patch by Sergio Villar Senin <[e-mail]>

Please move this one up as well.

> Tools/DumpRenderTree/gtk/DumpRenderTree.cpp:70
> +extern WebKitWebResource* webkit_web_view_get_main_resource(WebKitWebView*);

It's unfortuante that we are still using private APIs here. :( Hopefully a later patch can push whatever is needed into the public API. See below where I've suggested an alternative approach though.

> Tools/DumpRenderTree/gtk/DumpRenderTree.cpp:1062
> +    GOwnPtr<gchar> urlPath(g_strdup_printf("%s/%s", g_path_get_basename(g_path_get_dirname(uri->path)), g_path_get_basename(uri->path)));

This is a memory leak because g_path_get_basename returns a newly allocated string.

> Tools/DumpRenderTree/gtk/DumpRenderTree.cpp:1082
> +    return convertSoupURIToURLPath(soup_uri_new(webkit_web_resource_get_uri(webResource)));

Doesn't this leak the SoupURI?

> Tools/DumpRenderTree/gtk/DumpRenderTree.cpp:1090
> +    return CString(g_path_get_basename(uriString));

This is a memory leak, because the g_path_get_basename returns a newly allocated string.

> Tools/DumpRenderTree/gtk/DumpRenderTree.cpp:1107
> +    if (webResource == webkit_web_view_get_main_resource(webView)

Couldn't you use the public API here by doing: webkit_web_frame_get_data_source  and then webkit_web_data_source_get_main_resource?

[Philippe Normand]

Created attachment 93083 [details]
updated patch

[Philippe Normand]

I removed the main-uri property, doesn't seem needed, at least for now :)

[Martin Robinson]

Comment on attachment 93083 [details]
updated patch

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

This looks pretty good to me, in general. I think Gustavo or Xan should sign off on the API changes now.

> Source/WebKit/gtk/webkit/webkitwebview.cpp:185
>      DATABASE_QUOTA_EXCEEDED,
> -    RESOURCE_REQUEST_STARTING,
>      DOCUMENT_LOAD_FINISHED,

I think reordering the signals is an ABI break unfortunatley.

> Source/WebKit/gtk/webkit/webkitwebview.cpp:2803
> +    /*
> +     * WebKitWebView::resource-content-length-received
> +     * @webView: the object which received the signal
> +     * @webFrame: the #WebKitWebFrame the response was received for
> +     * @webResource: the #WebKitWebResource that was loaded
> +     * @lengthReceived: the resource data length in bytes
> +     *
> +     * Emitted when all the data for the resource was loaded
> +     *
> +     * Since: 1.5.1
> +     */

This documentation appears to be incorrect.

> Tools/DumpRenderTree/gtk/DumpRenderTree.cpp:1115
> +    if (webResource == webkit_web_data_source_get_main_resource(dataSource)
> +        && (!webkit_web_view_get_progress(webView) || g_str_equal(uri->scheme, "file")))
> +        description = CString("<unknown>");

This is slightly different than before where you were getting the main resource from the main frame, right? I'm not sure which is correct, because I'm not sure exactly what this is doing. Why do we only return "<unknown>" if this is the main data source of the frame? This probably needs a comment.

> Tools/DumpRenderTree/gtk/DumpRenderTree.cpp:1184
> +        printf("%s - willSendRequest %s redirectResponse %s\n", path.data(),
> +               requestDescription.data(), responseDescription.data());

Can just be:
 printf("%s - willSendRequest %s redirectResponse %s\n",
        convertNetworkRequestToURLPath(request).data(),
        requestDescription(descriptionSuitableForTestResult(request)).data(),
        responseDescription(descriptionSuitableForTestResult(response)).data());

> Tools/DumpRenderTree/gtk/DumpRenderTree.cpp:1223
> +    // TODO: add "has MIME type" whenever dumpResourceResponseMIMETypes()

I think this one is missing a word. :)

[Eric Seidel (no email)]

8 months since last comment.  Can we close this or is this moving forward silently? :)

[Martin Robinson]

Philippe, do you mind deciding whether you want to invest the time in this or wait for these features in WebKit2? I think your patch is very close to being done!

[Philippe Normand]

(In reply to comment #27)
> Philippe, do you mind deciding whether you want to invest the time in this or wait for these features in WebKit2? I think your patch is very close to being done!

Indeed I totally forgot about this patch. I'll update it today.

[Gustavo Noronha (kov)]

Comment on attachment 93083 [details]
updated patch

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

> Source/WebKit/gtk/webkit/webkitwebframe.cpp:106
> +    RESOURCE_LOADING_FINISHED,

It all looks OK to me, the only thing that bothered me was using 'loading' only for this signal, I think we should be consistent and use load everywhere?

> Source/WebKit/gtk/webkit/webkitwebresource.cpp:51
> +    LOADING_FINISHED,

Ditto.

>> Source/WebKit/gtk/webkit/webkitwebview.cpp:185
>>      DOCUMENT_LOAD_FINISHED,
> 
> I think reordering the signals is an ABI break unfortunatley.

I don't think so, the enum is only used internally isn't it?

> Source/WebKit/gtk/webkit/webkitwebview.cpp:205
> +    RESOURCE_LOADING_FINISHED,

And here...

> Tools/DumpRenderTree/gtk/DumpRenderTree.cpp:1051
> +static CString convertSoupURIToURLPath(SoupURI* uri)

How about pathFromSoupURI?

[Philippe Normand]

*** Bug 62585 has been marked as a duplicate of this bug. ***

[Philippe Normand]

Created attachment 124721 [details]
Patch

[WebKit Review Bot]

Attachment 124721 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/update-webkit']" exit_code: 9

Updating OpenSource
From git://git.webkit.org/WebKit
   7fbc464..f5e58c7  master     -> origin/master
Partial-rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc ...
Currently at 106357 = 7fbc464108a336b4b5cc9d5561e013ca51529334
r106358 = f5e58c7fca5f55729d43f04848daca0e72ec5075
Done rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc
First, rewinding head to replay your work on top of it...
Applying: Fix compilation errors on build-webkit --debug --no-workers on mac.
Using index info to reconstruct a base tree...
Falling back to patching base and 3-way merge...
Auto-merging LayoutTests/ChangeLog
CONFLICT (content): Merge conflict in LayoutTests/ChangeLog
Auto-merging LayoutTests/platform/qt/Skipped
CONFLICT (content): Merge conflict in LayoutTests/platform/qt/Skipped
Auto-merging Source/WebCore/ChangeLog
CONFLICT (content): Merge conflict in Source/WebCore/ChangeLog
Failed to merge in the changes.
Patch failed at 0001 Fix compilation errors on build-webkit --debug --no-workers on mac.

When you have resolved this problem run "git rebase --continue".
If you would prefer to skip this patch, instead run "git rebase --skip".
To restore the original branch and stop rebasing run "git rebase --abort".

rebase refs/remotes/origin/master: command returned error: 1

Died at Tools/Scripts/update-webkit line 164.


If any of these errors are false positives, please file a bug against check-webkit-style.

[Martin Robinson]

Comment on attachment 124721 [details]
Patch

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

Okay. Thanks!

> Source/WebKit/gtk/webkit/webkitwebframe.cpp:344
> +            (GSignalFlags)(G_SIGNAL_RUN_LAST | G_SIGNAL_ACTION),

Please use static_cast here.

> Source/WebKit/gtk/webkit/webkitwebframe.cpp:365
> +            (GSignalFlags)(G_SIGNAL_RUN_LAST | G_SIGNAL_ACTION),

Ditto.

> Source/WebKit/gtk/webkit/webkitwebframe.cpp:384
> +            (GSignalFlags)(G_SIGNAL_RUN_LAST | G_SIGNAL_ACTION),

Ditto and for the rest of the patch.

[Philippe Normand]

Committed r106445: <http://trac.webkit.org/changeset/106445>

[Philippe Normand]

(In reply to comment #33)
> (From update of attachment 124721 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=124721&action=review
> 
> Okay. Thanks!
> 
> > Source/WebKit/gtk/webkit/webkitwebframe.cpp:344
> > +            (GSignalFlags)(G_SIGNAL_RUN_LAST | G_SIGNAL_ACTION),
> 
> Please use static_cast here.

G_SIGNAL_ACTION wasn't needed, so I just removed it and the casts as well.

[Carlos Garcia Campos]

Comment on attachment 124721 [details]
Patch

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

I know this already landed, but I've found some issues with the patch.

> Source/WebKit/gtk/webkit/webkitwebframe.cpp:353
> +    /*

Double * missing here

> Source/WebKit/gtk/webkit/webkitwebframe.cpp:354
> +     * WebKitWebFrame::resource-response-received

Trailing : missing

> Source/WebKit/gtk/webkit/webkitwebframe.cpp:355
> +     * @webFrame: the #WebKitWebFrame the response was received for

web_frame

> Source/WebKit/gtk/webkit/webkitwebframe.cpp:356
> +     * @webResource: the #WebKitWebResource being loaded

web_resource

> Source/WebKit/gtk/webkit/webkitwebframe.cpp:359
> +     * Emitted when the first byte of data arrives

Are you sure, this is emitted when the response is received from the server, when the first byte of data arrives, received-content-length is emitted.

> Source/WebKit/gtk/webkit/webkitwebframe.cpp:373
> +    /*

Double * missing here too

> Source/WebKit/gtk/webkit/webkitwebframe.cpp:374
> +     * WebKitWebFrame::resource-load-finished

And trailing :

> Source/WebKit/gtk/webkit/webkitwebframe.cpp:375
> +     * @webFrame: the #WebKitWebFrame the response was received for

web_frame

> Source/WebKit/gtk/webkit/webkitwebframe.cpp:376
> +     * @webResource: the #WebKitWebResource being loaded

web_resource

> Source/WebKit/gtk/webkit/webkitwebframe.cpp:391
> +    /*

Double *

> Source/WebKit/gtk/webkit/webkitwebframe.cpp:393
> +     * @webFrame: the #WebKitWebFrame the response was received for

web_frame

> Source/WebKit/gtk/webkit/webkitwebframe.cpp:394
> +     * @webResource: the #WebKitWebResource that was loaded

web_resource

> Source/WebKit/gtk/webkit/webkitwebframe.cpp:395
> +     * @lengthReceived: the resource data length in bytes

length_received. It's not the length of the resource, it the bytes received in this moment

> Source/WebKit/gtk/webkit/webkitwebframe.cpp:397
> +     * Emitted when all the data for the resource was loaded.

Received every time data is received. This is called every time data has been received and length_received is the amount of bytes received. This is useful to provide progress information about the resource load operation.

> Source/WebKit/gtk/webkit/webkitwebframe.cpp:411
> +    /*

Double *

> Source/WebKit/gtk/webkit/webkitwebframe.cpp:412
> +     * WebKitWebFrame::resource-load-failed

Trailing :

> Source/WebKit/gtk/webkit/webkitwebframe.cpp:413
> +     * @webFrame: the #WebKitWebFrame the response was received for

web_frame

> Source/WebKit/gtk/webkit/webkitwebframe.cpp:414
> +     * @webResource: the #WebKitWebResource that was loaded

web_resource

> Source/WebKit/gtk/webkit/webkitwebframe.cpp:415
> +     * @webError: the #GError that was triggered

error

> Source/WebKit/gtk/webkit/webkitwebresource.cpp:125
> +    /*

Double *

> Source/WebKit/gtk/webkit/webkitwebresource.cpp:126
> +     * WebKitWebResource::response-received

Trailing :

> Source/WebKit/gtk/webkit/webkitwebresource.cpp:127
> +     * @webResource: the #WebKitWebResource being loaded

web_resource

> Source/WebKit/gtk/webkit/webkitwebresource.cpp:130
> +     * Emitted when the first byte of data arrives

Same comment here about the first byte of data

> Source/WebKit/gtk/webkit/webkitwebresource.cpp:143
> +    /*

Double *

> Source/WebKit/gtk/webkit/webkitwebresource.cpp:144
> +     * WebKitWebResource::load-failed

Trailing :

> Source/WebKit/gtk/webkit/webkitwebresource.cpp:145
> +     * @webResource: the #WebKitWebResource that was loaded

web_resource

> Source/WebKit/gtk/webkit/webkitwebresource.cpp:146
> +     * @webError: the #GError that was triggered

error

> Source/WebKit/gtk/webkit/webkitwebresource.cpp:148
> +     * Invoked when a resource failed to load

This is emitted on the resource object, it should probably say 'Invoked when the resource failed to load' or even 'Invoked when @resource failed to load'

> Source/WebKit/gtk/webkit/webkitwebresource.cpp:161
> +    /*

Double *

> Source/WebKit/gtk/webkit/webkitwebresource.cpp:162
> +     * WebKitWebResource::load-finished

Trailing :

> Source/WebKit/gtk/webkit/webkitwebresource.cpp:163
> +     * @webResource: the #WebKitWebResource being loaded

web_resource

> Source/WebKit/gtk/webkit/webkitwebresource.cpp:177
> +    /*

Double *

> Source/WebKit/gtk/webkit/webkitwebresource.cpp:178
> +     * WebKitWebResource::content-length-received

Trailing :

> Source/WebKit/gtk/webkit/webkitwebresource.cpp:179
> +     * @webResource: the #WebKitWebResource that was loaded

web_resource

> Source/WebKit/gtk/webkit/webkitwebresource.cpp:180
> +     * @lengthReceived: the resource data length in bytes

length_received

> Source/WebKit/gtk/webkit/webkitwebresource.cpp:182
> +     * Emitted when all the data for the resource was loaded

Same comment here about what this signal is for.

> Source/WebKit/gtk/webkit/webkitwebview.cpp:214
> +    RESOURCE_RESPONSE_RECEIVED,
> +    RESOURCE_LOAD_FINISHED,
> +    RESOURCE_CONTENT_LENGTH_RECEIVED,
> +    RESOURCE_LOAD_FAILED,

Same comments for this file too.

[Philippe Normand]

Can this be fixed in a new patch/bugzilla entry?

[Carlos Garcia Campos]

(In reply to comment #37)
> Can this be fixed in a new patch/bugzilla entry?

Please, add me to the CC if you open a new bug for this.