It will be needed by Download and Policy clients.
Created attachment 116213 [details] Patch It only returns uri, content_length and status_code for now. It doesn't include unit tests, because it will be covered by other unit tests like downloads and policy client.
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API
Comment on attachment 116213 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=116213&action=review > Source/WebKit2/UIProcess/API/gtk/WebKitNetworkResponse.cpp:130 > + * the server. It will normally be a #SoupKnownStatusCode, eg, > + * %SOUP_STATUS_OK, though of course it might actually be an unknown > + * status code. I would rephrase this slightly to: It will normally be a #SoupKnownStatusCode, for example %SOUP_STATUS_OK, though the server can respond with any unsigned integer. We need to look into how we can properly link to soup documentation so this doesn't produce errors during the gktdoc-fixxref run. > Source/WebKit2/UIProcess/API/gtk/WebKitNetworkResponse.cpp:153 > +guint64 webkit_network_response_get_content_length(WebKitNetworkResponse* response) Any reason this isn't a property? It is appropriate to use gsize here instead of guint64? Soup seems to use goffset, which I assume is the same data type as gsize. > Source/WebKit2/UIProcess/API/gtk/WebKitNetworkResponse.cpp:170 > + WebKitNetworkResponse* response = WEBKIT_NETWORK_RESPONSE(g_object_new(WEBKIT_TYPE_NETWORK_RESPONSE, "uri", uri.get(), NULL)); Ah, is this why you've chosen to use a property for uri, but not the others? I guess I would prefer that all private data be properties or none of them. > Source/WebKit2/UIProcess/API/gtk/WebKitNetworkResponse.cpp:176 > +SoupMessage* > +webkitNetworkResponseGetSoupMessage(WebKitNetworkResponse* response) Extra newline here. :)
(In reply to comment #3) > (From update of attachment 116213 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=116213&action=review > > > Source/WebKit2/UIProcess/API/gtk/WebKitNetworkResponse.cpp:130 > > + * the server. It will normally be a #SoupKnownStatusCode, eg, > > + * %SOUP_STATUS_OK, though of course it might actually be an unknown > > + * status code. > > I would rephrase this slightly to: It will normally be a #SoupKnownStatusCode, for example %SOUP_STATUS_OK, though the server can respond with any unsigned integer. I copied it literally from the libsoup api docs :-P > We need to look into how we can properly link to soup documentation so this doesn't produce errors during the gktdoc-fixxref run. Right. > > Source/WebKit2/UIProcess/API/gtk/WebKitNetworkResponse.cpp:153 > > +guint64 webkit_network_response_get_content_length(WebKitNetworkResponse* response) > > Any reason this isn't a property? It is appropriate to use gsize here instead of guint64? Soup seems to use goffset, which I assume is the same data type as gsize. Because data length used by WebKit2 internal API is uint64_t > > Source/WebKit2/UIProcess/API/gtk/WebKitNetworkResponse.cpp:170 > > + WebKitNetworkResponse* response = WEBKIT_NETWORK_RESPONSE(g_object_new(WEBKIT_TYPE_NETWORK_RESPONSE, "uri", uri.get(), NULL)); > > Ah, is this why you've chosen to use a property for uri, but not the others? I guess I would prefer that all private data be properties or none of them. Yes, because it's construct only. > > Source/WebKit2/UIProcess/API/gtk/WebKitNetworkResponse.cpp:176 > > +SoupMessage* > > +webkitNetworkResponseGetSoupMessage(WebKitNetworkResponse* response) > > Extra newline here. :) Ok.
(In reply to comment #4) > > We need to look into how we can properly link to soup documentation so this doesn't produce errors during the gktdoc-fixxref run. I think if you have the libsoup gtk-docs installed in the prefix you are installing webkit into, it will find them automatically. > > > +guint64 webkit_network_response_get_content_length(WebKitNetworkResponse* response) > > > > Any reason this isn't a property? It is appropriate to use gsize here instead of guint64? Soup seems to use goffset, which I assume is the same data type as gsize. (Not always. gsize is size_t and goffset is offset_t. On x86_64 they're both 64-bit but on x86 size_t is 32-bit.) > > Ah, is this why you've chosen to use a property for uri, but not the others? I guess I would prefer that all private data be properties or none of them. > > Yes, because it's construct only. Having properties for everything is more language-bindings-friendly.
(In reply to comment #5) > Having properties for everything is more language-bindings-friendly. In this case, I think it makes sense to have all members be properties or none of them, but not only some of them.
(In reply to comment #6) > (In reply to comment #5) > > > Having properties for everything is more language-bindings-friendly. > > In this case, I think it makes sense to have all members be properties or none of them, but not only some of them. Good point, I agree, I'll update the wiki accordingly http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API btw, I'll rename this and NetworkRequest as WebKitURIRequest/Response if nobody objects.
(In reply to comment #7) > btw, I'll rename this and NetworkRequest as WebKitURIRequest/Response if nobody objects. I like WebKitURIRequest. It's more accurate and less typing. :)
Created attachment 116749 [details] Updated patch Updated according to review comments and renamed to WebKitURIResponse
Comment on attachment 116749 [details] Updated patch View in context: https://bugs.webkit.org/attachment.cgi?id=116749&action=review Nice. > Source/WebKit2/UIProcess/API/gtk/WebKitURIResponse.cpp:98 > + * The URI to which the response will be made. Maybe change this to "The URI for which the response was made." > Source/WebKit2/UIProcess/API/gtk/WebKitURIResponse.cpp:104 > + _("The URI to which the response will be made."), Ditto. > Source/WebKit2/UIProcess/API/gtk/WebKitURIResponse.cpp:182 > + * be 0 if the server provided incorrect or missing Content-Length. You have "an" after provided.
Committed r101350: <http://trac.webkit.org/changeset/101350>