[Webkit-unassigned] [Bug 84133] [GTK] Add API to register custom URI schemes to WebKit2 GTK+ API

Thu May 31 09:57:22 PDT 2012

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

--- Comment #15 from Carlos Garcia Campos <cgarcia at igalia.com>  2012-05-31 09:57:22 PST ---
(In reply to comment #12)
> (From update of attachment 141144 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=141144&action=review
> 

> > Source/WebKit2/UIProcess/API/gtk/WebKitURISchemeRequest.cpp:137
> > +        WKSoupRequestManagerDidHandleURIRequest(priv->wkRequestManager.get(), wkData.get(), priv->streamLength, wkMimeType.get(), priv->requestID);
> 
> Here you are passing zero to the WebProcess for streams that do not have predetermined length. How do you differentiate between this situation and empty responses?

No, the input stream returns a 0 byte read when it has finished, if there's nothing to read yet, the callback is not emitted. The case of zero-length request is hanlded here indeed.

> > Source/WebKit2/UIProcess/API/gtk/WebKitURISchemeRequest.cpp:139
> > +    } else if (bytesRead || (!bytesRead && !priv->streamLength))
> > +        WKSoupRequestManagerDidReceiveURIRequestData(priv->wkRequestManager.get(), wkData.get(), priv->requestID);
> 
> Is it important to call WKSoupRequestManagerDidReceiveURIRequestData when priv->streamLength == 0 (undetermined stream length) case because otherwise the WebProcess won't know that the request is complete? I think this merits a small comment explaining the situation.

WKSoupRequestManagerDidHandleURIRequest handles the case of a zero-length request.

> > Source/WebKit2/UIProcess/API/gtk/WebKitURISchemeRequest.cpp:162
> > +    g_return_if_fail(streamLength == -1 || streamLength > 0);
> 
> So it's not possible to send an empty reply?

hmm, we could change that to be streamLength == -1 || streamLength >= 0.

> > Source/WebKit2/UIProcess/API/gtk/WebKitWebContext.cpp:52
> > +        {
> > +        }
> > +
> 
> Nit: I think the curly braces are one indentation level too far to the right.

emacs . . . :-)

> > Source/WebKit2/UIProcess/API/gtk/tests/TestWebKitWebContext.cpp:36
> > +static const char* kBarHtml = "<html><body>Bar</body></html>";
> > +static const char* kEchoHtmlFormat = "<html><body>%s</body></html>";
> 
> Nit: Html -> HTML
> 
> > Source/WebKit2/UIProcess/API/gtk/tests/TestWebKitWebContext.cpp:53
> > +            g_memory_input_stream_add_data(G_MEMORY_INPUT_STREAM(inputStream.get()), reinterpret_cast<gconstpointer>(kBarHtml), inputStreamLength, 0);
> 
> Do you actually need to castkBarHTML and echoHTML here? I tried compiling this locally with g++ and had no errors:

I don't know, I'll try again.

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