[webkit-reviews] review granted: [Bug 26854] [GTK] Needs API to allow more control over outgoing requests : [Attachment 39146] one more draft, with Xan's comments addressed

[bugzilla-daemon at webkit.org]

Xan Lopez <xan.lopez at gmail.com> has granted Gustavo Noronha (kov)
<gns at gnome.org>'s request for review:
Bug 26854: [GTK] Needs API to allow more control over outgoing requests
https://bugs.webkit.org/show_bug.cgi?id=26854

Attachment 39146: one more draft, with Xan's comments addressed
https://bugs.webkit.org/attachment.cgi?id=39146&action=review

------- Additional Comments from Xan Lopez <xan.lopez at gmail.com>
> +    GOwnPtr<WebKitWebResource>
webResource(WEBKIT_WEB_RESOURCE(g_object_new(WEBKIT_TYPE_WEB_RESOURCE, 0)));
> +    webResource->priv->uri = g_strdup(request.url().string().utf8().data());


As discussed on IRC, set the property as CONSTRUCT_ONLY and pass as parameter
to g_object_new

> +
> +    GOwnPtr<WebKitNetworkRequest>
networkRequest(webkit_network_request_new_with_core_request(request));
> +
> +    g_signal_emit_by_name(webView, "resource-request-starting", m_frame,
webResource.get(), networkRequest.get(), networkResponse.get());
> +
> +    // Feed any changes back into the ResourceRequest object.
> +    request = core(networkRequest.get());
>  }
>  
> -void FrameLoaderClient::assignIdentifierToInitialRequest(unsigned long
identifier, WebCore::DocumentLoader*, const ResourceRequest&)
> +void FrameLoaderClient::assignIdentifierToInitialRequest(unsigned long,
WebCore::DocumentLoader*, const ResourceRequest&)
>  {
>      notImplemented();
>  }
> @@ -792,7 +812,7 @@ void FrameLoaderClient::setTitle(const String& title,
const KURL& url)
>      frameData->title = g_strdup(title.utf8().data());
>  }
>  
> -void
FrameLoaderClient::dispatchDidReceiveContentLength(WebCore::DocumentLoader*,
unsigned long identifier, int lengthReceived)
> +void
FrameLoaderClient::dispatchDidReceiveContentLength(WebCore::DocumentLoader*
loader, unsigned long identifier, int lengthReceived)
>  {
>      notImplemented();
>  }

No need to add the name here...

> @@ -800,13 +820,17 @@ void
FrameLoaderClient::dispatchDidReceiveContentLength(WebCore::DocumentLoader*
>  void FrameLoaderClient::dispatchDidFinishLoading(WebCore::DocumentLoader*
loader, unsigned long identifier)
>  {
>     
static_cast<WebKit::DocumentLoader*>(loader)->decreaseLoadCount(identifier);
> +
> +    notImplemented();
>  }
>  

As discussed on IRC, add a comment on this one too.

>  void FrameLoaderClient::dispatchDidFailLoading(WebCore::DocumentLoader*
loader, unsigned long identifier, const ResourceError& error)
>  {
> -    // FIXME: when does this occur and what should happen?
> -
>     
static_cast<WebKit::DocumentLoader*>(loader)->decreaseLoadCount(identifier);
> +
> +    // This function should notify the application that a resource failed
> +    // loading (maybe a 'load-error' signal in the WebKitWebResource object
> +    notImplemented();
>  }
>  



> @@ -1975,6 +1978,46 @@ static void
webkit_web_view_class_init(WebKitWebViewClass* webViewClass)
>	       G_TYPE_NONE, 2,
>	       G_TYPE_OBJECT, G_TYPE_OBJECT);
>  
> +    /**
> +	* WebKitWebView::resource-request-starting:
> +	* @web_view: the object which received the signal
> +	* @web_frame: the #WebKitWebFrame whose load dispatched this request
> +	* @web_resource: an empty #WebKitWebResource object
> +	* @request: the #WebKitNetworkRequest that will be dispatched
> +	* @response: the #WebKitNetworkResponse representing the redirect
> +	* response, if any
> +	*
> +	* Emitted when a request is about to be sent. You can modify the
> +	* request, by adding/removing/replacing headers, or changing the
> +	* URI, using the #SoupMessage object it carries, if it is
> +	* present. See webkit_network_request_get_message(). Setting the
> +	* request URI to "about:blank" will effectively cause the request
> +	* to load nothing, and can be used to disable the loading of
> +	* specific resources.

I read this like you have to modify the request through the SoupMessage if it's
there, but you don't need to get the message to, for example, set the URI.
Maybe reword it a bit?

r=me with those changes wooo!