Bug 114347 - [GTK] Notify WebProcess in WebKitURISchemeRequest when we fail to read the user InputStream
Summary: [GTK] Notify WebProcess in WebKitURISchemeRequest when we fail to read the us...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 94316
Blocks:
  Show dependency treegraph
 
Reported: 2013-04-10 03:49 PDT by Manuel Rego Casasnovas
Modified: 2013-05-27 06:31 PDT (History)
6 users (show)

See Also:


Attachments
Patch (2.13 KB, patch)
2013-04-10 04:03 PDT, Manuel Rego Casasnovas
no flags Details | Formatted Diff | Diff
Patch (4.38 KB, patch)
2013-04-11 23:08 PDT, Manuel Rego Casasnovas
no flags Details | Formatted Diff | Diff
Patch (4.31 KB, patch)
2013-04-11 23:31 PDT, Manuel Rego Casasnovas
no flags Details | Formatted Diff | Diff
Patch (4.31 KB, patch)
2013-05-27 05:11 PDT, Manuel Rego Casasnovas
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Manuel Rego Casasnovas 2013-04-10 03:49:53 PDT
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;
     }
 [...]
 }
Comment 1 Manuel Rego Casasnovas 2013-04-10 04:03:14 PDT
Created attachment 197242 [details]
Patch
Comment 2 Martin Robinson 2013-04-10 09:34:44 PDT
Comment on attachment 197242 [details]
Patch

You might add a few words about what kind of situation motivates this change.
Comment 3 Carlos Garcia Campos 2013-04-10 10:30:49 PDT
Comment on attachment 197242 [details]
Patch

We also need unit tests.
Comment 4 WebKit Commit Bot 2013-04-10 11:08:35 PDT
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
Comment 5 Manuel Rego Casasnovas 2013-04-11 23:08:06 PDT
Created attachment 197719 [details]
Patch
Comment 6 Carlos Garcia Campos 2013-04-11 23:20:06 PDT
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.
Comment 7 Manuel Rego Casasnovas 2013-04-11 23:31:39 PDT
Created attachment 197720 [details]
Patch
Comment 8 Manuel Rego Casasnovas 2013-05-07 02:23:36 PDT
This is ready for owner review as bug #94316 has already landed.
Comment 9 Manuel Rego Casasnovas 2013-05-27 05:11:37 PDT
Created attachment 202972 [details]
Patch

Patch still applies and works in trunk. Re-uploading it to check EWSs.
Comment 10 Carlos Garcia Campos 2013-05-27 06:10:18 PDT
Comment on attachment 202972 [details]
Patch

Thanks!
Comment 11 WebKit Commit Bot 2013-05-27 06:31:51 PDT
Comment on attachment 202972 [details]
Patch

Clearing flags on attachment: 202972

Committed r150753: <http://trac.webkit.org/changeset/150753>
Comment 12 WebKit Commit Bot 2013-05-27 06:31:54 PDT
All reviewed patches have been landed.  Closing bug.