[Webkit-unassigned] [Bug 40222] Use the new SoupURILoader API
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Thu Jul 1 05:05:09 PDT 2010
https://bugs.webkit.org/show_bug.cgi?id=40222
--- Comment #9 from Xan Lopez <xan.lopez at gmail.com> 2010-07-01 05:05:09 PST ---
(From update of attachment 59635)
>- GFile* m_gfile;
>+ SoupRequest* m_req;
Maybe we can start using GRefPtr for this?
>- String url = handle->request().url().string();
>- ASSERT(url.startsWith("data:", false));
>+ String urlStr = handle->request().url().string();
>+ ASSERT(urlStr.startsWith("data:", false));
>
>- int index = url.find(',');
>+ int index = urlStr.find(',');
> if (index == -1) {
> client->cannotShowURL(handle);
> return false;
> }
>
>- String mediaType = url.substring(5, index - 5);
>- String data = url.substring(index + 1);
>+ String mediaType = urlStr.substring(5, index - 5);
This seems to be a bit random? Should go in a different patch at the very least.
>+ SoupSession* session = handle->defaultSession();
>+ GError* error = 0;
Should use GOwnPtr for GError here.
>+ d->m_req = soup_session_request(session, handle->request().url().string().utf8().data(), &error);
>+ if (error) {
>+ g_error_free(error);
>+ return false;
>+ }
>+
>+ GInputStream* in = soup_request_send(d->m_req, 0, &error);
>+ if (error) {
>+ g_error_free(error);
>+ return false;
>+ }
>+
>+ d->m_inputStream = in;
>+ d->m_bufferSize = 8192;
>+ d->m_buffer = static_cast<char*>(g_malloc(d->m_bufferSize));
>+ d->m_total = 0;
>+
>+ g_object_set_data(G_OBJECT(d->m_inputStream), "webkit-resource", handle);
>+ handle->ref();
>+
>+ d->m_cancellable = g_cancellable_new();
>+ g_input_stream_read_async(d->m_inputStream, d->m_buffer, d->m_bufferSize,
>+ G_PRIORITY_DEFAULT, d->m_cancellable,
>+ readCallback, (isBase64) ? 0 : GINT_TO_POINTER(1));
I suppose it's good to unify stuff and always go through libsoup for everything, but isn't parsing a simple string asynchronously a bit overkill? /me wonders
>
> return false;
> }
>@@ -428,7 +404,7 @@ static bool startData(ResourceHandle* handle, String urlString)
>
> // If parseDataUrl is called synchronously the job is not yet effectively started
> // and webkit won't never know that the data has been parsed even didFinishLoading is called.
>- d->m_idleHandler = g_timeout_add(0, parseDataUrl, handle);
>+ d->m_idleHandler = g_idle_add(parseDataUrl, handle);
This is not the same thing. Do you need to change it or you just felt it was better? Needs an explanation.
> return true;
> }
>
>@@ -469,6 +445,151 @@ static void ensureSessionIsInitialized(SoupSession* session)
> g_object_set_data(G_OBJECT(session), "webkit-init", reinterpret_cast<void*>(0xdeadbeef));
> }
>
>+static void sendRequestCallback(GObject* source, GAsyncResult* res, gpointer userData)
>+{
>+ RefPtr<ResourceHandle> handle = static_cast<ResourceHandle*>(g_object_get_data(source, "webkit-resource"));
>+ if (!handle)
>+ return;
>+
>+ ResourceHandleInternal* d = handle->getInternal();
>+ ResourceHandleClient* client = handle->client();
>+
>+ if (userData) {
>+ // No need to call gotChunkHandler anymore. Received data will
>+ // be reported by readCallback
>+ gulong gotChunkHandler = GPOINTER_TO_UINT(userData);
>+ if (g_signal_handler_is_connected(d->m_msg, gotChunkHandler))
>+ g_signal_handler_disconnect(d->m_msg, gotChunkHandler);
>+ }
>+
>+ if (d->m_cancelled || !client) {
>+ cleanupSoupRequestOperation(handle.get());
>+ return;
>+ }
>+
>+ GError* error = 0;
GOwnPtr. This is repeated elsewhere, the same applies.
>+ GInputStream* in = soup_request_send_finish(d->m_req, res, &error);
>+
>+ if (error) {
>+
Extra blank line!
>+ if (d->m_msg && SOUP_STATUS_IS_TRANSPORT_ERROR(d->m_msg->status_code)) {
>+ SoupURI* uri = soup_request_get_uri(d->m_req);
>+ char* uristr = soup_uri_to_string(uri, false);
uriStr? With GOwnPtr. This applies in a bunch of places too.
>+ ResourceError resourceError(g_quark_to_string(SOUP_HTTP_ERROR),
>+ d->m_msg->status_code,
>+ uristr,
>+ String::fromUTF8(d->m_msg->reason_phrase));
>+ g_free(uristr);
>+ g_error_free(error);
>+ cleanupSoupRequestOperation(handle.get());
>+ client->didFail(handle.get(), resourceError);
>+ return;
>+ }
>+
>+ if (error->domain == G_IO_ERROR) {
>+ SoupURI* uri = soup_request_get_uri(d->m_req);
>+ char* uristr = soup_uri_to_string(uri, false);
>+ ResourceError resourceError(g_quark_to_string(G_IO_ERROR),
>+ error->code,
>+ uristr,
>+ error ? String::fromUTF8(error->message) : String());
>+ g_free(uristr);
>+ g_error_free(error);
>+ cleanupSoupRequestOperation(handle.get());
>+ client->didFail(handle.get(), resourceError);
>+ return;
>+ }
>+
>+ g_error_free(error);
>+
>+ if (d->m_msg && d->m_msg->status_code == SOUP_STATUS_UNAUTHORIZED) {
>+ fillResponseFromMessage(d->m_msg, &d->m_response);
>+ client->didReceiveResponse(handle.get(), d->m_response);
>+
>+ // WebCore might have cancelled the job in the while
>+ if (d->m_cancelled)
>+ return;
>+
>+ if (d->m_msg->response_body->data)
>+ client->didReceiveData(handle.get(), d->m_msg->response_body->data, d->m_msg->response_body->length, true);
>+ }
>+
>+ client->didFinishLoading(handle.get());
>+ return;
>+ }
>+
>+ if (d->m_cancelled) {
>+ cleanupSoupRequestOperation(handle.get());
>+ return;
>+ }
>+
>+ d->m_inputStream = in;
>+ d->m_bufferSize = 8192;
This number appears around more than once, should be a #define
>+ d->m_buffer = static_cast<char*>(g_malloc(d->m_bufferSize));
>+ d->m_total = 0;
>+
>+ // readCallback needs it
>+ g_object_set_data(G_OBJECT(d->m_inputStream), "webkit-resource", handle.get());
>+
>+ // We need to check if it's a file: URL and if it is a regular
>+ // file as it could be a directory. In that case Soup properly
>+ // returns a stream whose outcome is a HTML with a list of files
>+ // in the directory
>+ if (equalIgnoringCase(handle->request().url().protocol(), "file")
>+ && G_IS_FILE_INPUT_STREAM(in)) {
>+
>+ GFile* file = soup_request_file_get_file(SOUP_REQUEST_FILE(d->m_req));
GRefPtr.
>+
>+ ResourceResponse response;
>+
>+ response.setURL(handle->request().url());
>+ response.setMimeType(soup_request_get_content_type(d->m_req));
>+ response.setExpectedContentLength(soup_request_get_content_length(d->m_req));
>+ client->didReceiveResponse(handle.get(), response);
>+
>+ if (d->m_cancelled) {
>+ cleanupSoupRequestOperation(handle.get());
>+ return;
>+ }
>+
>+ g_object_unref(file);
>+ }
>+
>+ g_input_stream_read_async(d->m_inputStream, d->m_buffer, d->m_bufferSize,
>+ G_PRIORITY_DEFAULT, d->m_cancellable,
>+ readCallback, 0);
>+}
>+
>
>+
Not extra line.
> bool ResourceHandle::start(Frame* frame)
> {
> ASSERT(!d->m_msg);
>@@ -591,21 +720,21 @@ bool ResourceHandle::start(Frame* frame)
> return false;
>
> KURL url = request().url();
>- String urlString = url.string();
>- String protocol = url.protocol();
>
> // Used to set the authentication dialog toplevel; may be NULL
> d->m_frame = frame;
>
>- if (equalIgnoringCase(protocol, "data"))
>- return startData(this, urlString);
>+ if (url.protocolIs("data"))
>+ return startData(this, url);
>
>- if (equalIgnoringCase(protocol, "http") || equalIgnoringCase(protocol, "https")) {
>+ if (url.protocolInHTTPFamily()) {
> if (startHttp(this))
> return true;
> }
>
>- if (equalIgnoringCase(protocol, "file") || equalIgnoringCase(protocol, "ftp") || equalIgnoringCase(protocol, "ftps")) {
>+ if (url.protocolIs("file")
>+ || url.protocolIs("ftp")
>+ || url.protocolIs("ftps")) {
> // FIXME: should we be doing any other protocols here?
> if (startGio(this, url))
> return true;
This is all nice but I don't believe it's strictly needed in the patch, you could push it separately and earlier.
>@@ -665,38 +794,6 @@ void ResourceHandle::loadResourceSynchronously(const ResourceRequest& request, S
> syncLoader.run();
> }
>
> static void closeCallback(GObject* source, GAsyncResult* res, gpointer)
> {
> RefPtr<ResourceHandle> handle = static_cast<ResourceHandle*>(g_object_get_data(source, "webkit-resource"));
>@@ -707,7 +804,6 @@ static void closeCallback(GObject* source, GAsyncResult* res, gpointer)
> ResourceHandleClient* client = handle->client();
>
> g_input_stream_close_finish(d->m_inputStream, res, 0);
>- cleanupGioOperation(handle.get());
>
> // The load may have been cancelled, the client may have been
> // destroyed already. In such cases calling didFinishLoading is a
>@@ -716,9 +812,11 @@ static void closeCallback(GObject* source, GAsyncResult* res, gpointer)
> return;
>
> client->didFinishLoading(handle.get());
>+
>+ cleanupSoupRequestOperation(handle.get());
Any reason to move this?
> }
>
>@@ -755,141 +854,36 @@ static void readCallback(GObject* source, GAsyncResult* res, gpointer)
> }
>
> d->m_total += bytesRead;
>- client->didReceiveData(handle.get(), d->m_buffer, bytesRead, d->m_total);
>+ if (G_LIKELY(!data))
>+ client->didReceiveData(handle.get(), d->m_buffer, bytesRead, d->m_total);
So the likely case is that there's no data read? Uh?
>+ else {
>+ // We have to convert it to UTF-16 due to limitations in KURL
>+ GOwnPtr<gunichar2> unicodeStr(g_utf8_to_utf16(d->m_buffer, bytesRead, 0, 0, 0));
>+ client->didReceiveData(handle.get(), reinterpret_cast<const char*>(unicodeStr.get()), g_utf8_strlen(d->m_buffer, bytesRead) * sizeof(UChar), 0);
Mmm, we don't seem to do this in the other didReceiveData calls, we pass UTF8 data and lenght in bytes, not characters. If the tests pass I suppose this is ok, but it seems puzzling.
>+ }
>
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
More information about the webkit-unassigned
mailing list