[Webkit-unassigned] [Bug 85880] [SOUP] Allow sending URI request data in chunks

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=85880





--- Comment #4 from Carlos Garcia Campos <cgarcia at igalia.com>  2012-05-08 08:24:46 PST ---
(In reply to comment #3)
> (From update of attachment 140703 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=140703&action=review

Thanks for looking at it!

> > Source/WebKit2/WebProcess/soup/WebKitSoupRequestInputStream.cpp:43
> > +    GRefPtr<GSource> baseSource = adoptGRef(g_timeout_source_new(0));
> 
> That means the source will always trigger immediately, and so if there's not actually any data available, the reader will spin in a tight loop. When the stream is "empty" but not finished, you have to return a source that won't trigger until after the next time webkitSoupRequestInputStreamAddData() has been called.

Yeah, I thought about it, but assumed the ui client would send more data soon. I agree we should use a custom GSource to implement it properly. 

> One way to do that would be to have a "GCancellable *waiting_for_data;" in priv, and create a source with g_cancellable_create_source(). Then do g_cancellable_cancel(priv->waiting_for_data) in webkitSoupRequestInputStreamAddData() and g_cancellable_reset(priv->waiting_for_data) in webkitSoupRequestInputStreamPollableReadNonblocking(). (Using GCancellable for this is semantically backwards, but... it works.)

It's a bit tricky, but easier than implementing a new GSource.

> > Source/WebKit2/WebProcess/soup/WebKitSoupRequestInputStream.cpp:44
> > +    return g_pollable_source_new_full(stream, baseSource.get(), cancellable);
> 
> as you noted, g_pollable_source_new_full() is very new. but you can just reimplement it by hand; it's just a small wrapper around g_pollable_source_new(), which is older.

Yes, I was looking at the latest changes in gio.

> > Source/WebKit2/WebProcess/soup/WebKitSoupRequestInputStream.cpp:82
> > +static void webkit_soup_request_input_stream_class_init(WebKitSoupRequestInputStreamClass* requestStreamClass)
> 
> Since you're not overriding GInputStreamClass->read_fn, it will use GMemoryInputStream's implementation, which won't deal with streaming data correctly. It *can't* deal with it correctly, but it seems like it might be better to have it either return G_IO_ERROR_NOT_SUPPORTED, or else do g_return_val_if_reached(-1);

Yes, I assumed it was not going to be used, since this input stream is not a generic object in the end, and will only be used by ResourceHandleSoup.

> > Source/WebKit2/WebProcess/soup/WebKitSoupRequestInputStream.cpp:108
> > +    return stream->priv->bytesAdded == stream->priv->contentLength;
> 
> Hm... what about for chunked data, when the final length won't be known in advance? What does WebKit2 do in that case?

The API always has to provide the content_length in advance, doesn't SoupRequest also require to set content_length before finish()?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.