[webkit-reviews] review denied: [Bug 26854] [GTK] Needs API to allow more control over outgoing requests : [Attachment 32084] Map the willSendRequest delegate to the 'outgoing-request' signal

[bugzilla-daemon at webkit.org]

Jan Alonzo <jmalonzo at gmail.com> has denied 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 32084: Map the willSendRequest delegate to the 'outgoing-request'
signal
https://bugs.webkit.org/attachment.cgi?id=32084&action=review

------- Additional Comments from Jan Alonzo <jmalonzo at gmail.com>
> -void FrameLoaderClient::dispatchWillSendRequest(DocumentLoader*, unsigned
long, ResourceRequest&, const ResourceResponse&)
> +void FrameLoaderClient::dispatchWillSendRequest(DocumentLoader*, unsigned
long identifier, ResourceRequest& request, const ResourceResponse&)
>  {
> -    notImplemented();
> +    WebKitWebView* webView = getViewFromFrame(m_frame);
> +    WebKitNetworkRequest* networkRequest =
webkit_network_request_new_with_core_request(request);
> +
> +    g_signal_emit_by_name(webView, "outgoing-request", m_frame, identifier,
networkRequest);

We probably need to send the data source object as well and possibly make
identifier an attribute of that object.

As for naming, I'm not sure what outgoing-request means. I think it needs to be
more meaningful.

> +	* 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

Will changing the URI have any effect on security?

> +    webkit_web_view_signals[OUTGOING_REQUEST] =
g_signal_new("outgoing-request",
> +	       G_TYPE_FROM_CLASS(webViewClass),
> +	       (GSignalFlags)(G_SIGNAL_RUN_LAST | G_SIGNAL_ACTION),
> +	       0,
> +	       NULL, NULL,
> +	       webkit_marshal_VOID__OBJECT_OBJECT_ULONG,
> +	       G_TYPE_NONE, 3,
> +	       WEBKIT_TYPE_WEB_FRAME,
> +	       G_TYPE_ULONG,
> +	       WEBKIT_TYPE_NETWORK_REQUEST);

The marshaller seems to be off here.

Also, the DRT support for this is missing + the layout tests that will be
enabled by this feature.

r- for now because of this.