Bug 19656 – [SOUP] The gio code should call didFail() instead of didFinishLoading() in case of error

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

[Marco Barisione]

At the moment ther gio code in the soup back-end call didFinishLoading() in case of error, this should be changed to didFail().

[Marco Barisione]

Created attachment 21819 [details]
Call didFail in case of error

[Eric Seidel (no email)]

Comment on attachment 21819 [details]
Call didFail in case of error

We really should have a GOwnPtr which knows how to call g_free on the associated pointer.  In that case:
      gchar* uri = g_file_get_uri (d->m_gfile);
 519         ResourceError resourceError("webkit-network-error", ERROR_TRANSPORT, uri, String::fromUTF8(error->message));
 520         g_free(uri);

would not need the g_free line.  Stupid c-style memory management.  Bleh.  (No offense to your code, I just have a general hatred towards languages and APIs which make it easy for you to stab yourself in the foot, or leak memory.)

WebKit style does not have a space between the function name and the (
+        gchar* uri = g_file_get_uri (d->m_gfile);

So we make 2 copies of the buffer just to send this URI off to ResourceError.  Is it possible to avoid this?

You should consider adding an inline function which created the resource error from the gfile handle and returned it to you.   That would eliminate 6 lines of copy-paste code.

Then these calls just become:

client->didFail(handle, networkErrorForFile(d->m_gfile, error));

static inline ResourceError networkErrorForFile(gfile* file, gerror* error)
{
    gchar* uri = g_file_get_uri(d->m_gfile);
    ResourceError resourceError("webkit-network-error", ERROR_TRANSPORT, uri, String::fromUTF8(error->message));
    g_free(uri);
   return resourceError;
}

r- for the style errors.  And I would encourage you to use a static inline to eliminate this copy-paste code.

[Marco Barisione]

Created attachment 22629 [details]
Call didFail in case of error

Adding something like GOwnPtr and GObjectPtr could be a good idea but it's something for a separate patch.

"So we make 2 copies of the buffer just to send this URI off to ResourceError. 
Is it possible to avoid this?"

What do you mean? The copy from the GFile to a char and then to a String? In this case I don't see any solution.

[Mark Rowe (bdash)]

Comment on attachment 22629 [details]
Call didFail in case of error

r=me

[Jan Alonzo]

landed in r35872. Thanks for the patch and for the review!