Bug 114347 – [GTK] Notify WebProcess in WebKitURISchemeRequest when we fail to read the user InputStream

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Manuel Rego Casasnovas]

Until now we were finishing the request without notifying the WebProcess when we fail to read the user InputStream while managing a WebKitURISchemeRequest.

In WebKitURISchemeRequest::webkitURISchemeRequestReadCallback() it's marked as FIXME:
 static void webkitURISchemeRequestReadCallback(GInputStream* inputStream, GAsyncResult* result, WebKitURISchemeRequest* schemeRequest)
 {
     GRefPtr<WebKitURISchemeRequest> request = adoptGRef(schemeRequest);
    gssize bytesRead = g_input_stream_read_finish(inputStream, result, 0);
    // FIXME: notify the WebProcess that we failed to read from the user stream.
     if (bytesRead == -1) {
        webkitWebContextDidFinishURIRequest(request->priv->webContext, request->priv->requestID);
         return;
     }
 [...]
 }

[Manuel Rego Casasnovas]

Created attachment 197242 [details]
Patch

[Martin Robinson]

Comment on attachment 197242 [details]
Patch

You might add a few words about what kind of situation motivates this change.

[Carlos Garcia Campos]

Comment on attachment 197242 [details]
Patch

We also need unit tests.

[WebKit Commit Bot]

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

[Manuel Rego Casasnovas]

Created attachment 197719 [details]
Patch

[Carlos Garcia Campos]

Comment on attachment 197719 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=197719&action=review

> Source/WebKit2/UIProcess/API/gtk/tests/TestWebKitWebContext.cpp:235
> +    g_assert_cmpstr(test->m_error->message, ==, "Stream is already closed");

Don't check the message in this case, this error comes from glib and the message might change making this test fail. Checking the domain and error code is enough.

[Manuel Rego Casasnovas]

Created attachment 197720 [details]
Patch

[Manuel Rego Casasnovas]

This is ready for owner review as bug #94316 has already landed.

[Manuel Rego Casasnovas]

Created attachment 202972 [details]
Patch

Patch still applies and works in trunk. Re-uploading it to check EWSs.

[Carlos Garcia Campos]

Comment on attachment 202972 [details]
Patch

Thanks!

[WebKit Commit Bot]

Comment on attachment 202972 [details]
Patch

Clearing flags on attachment: 202972

Committed r150753: <http://trac.webkit.org/changeset/150753>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.