[webkit-reviews] review requested: [Bug 121123] Web Inspector: Do not try to parse incomplete HTTP requests : [Attachment 211778] Patch addressing Carlos review (new round)
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Mon Sep 16 07:07:43 PDT 2013
Andre Moreira Magalhaes <andrunko at gmail.com> has asked for review:
Bug 121123: Web Inspector: Do not try to parse incomplete HTTP requests
https://bugs.webkit.org/show_bug.cgi?id=121123
Attachment 211778: Patch addressing Carlos review (new round)
https://bugs.webkit.org/attachment.cgi?id=211778&action=review
------- Additional Comments from Andre Moreira Magalhaes <andrunko at gmail.com>
(In reply to comment #8)
> (From update of attachment 211555 [details])
> View in context:
https://bugs.webkit.org/attachment.cgi?id=211555&action=review
>
> > Source/WebKit2/UIProcess/API/gtk/tests/TestInspectorServer.cpp:249
> > + GRefPtr<GSocketClient> client = g_socket_client_new();
> > + GRefPtr<GSocketConnection> connection =
g_socket_client_connect_to_host(client.get(), "127.0.0.1", 2999, 0,
&error.outPtr());
>
> This is still wrong, you have to adopt the reference with adoptGRef().
>
Done.
> > Source/WebKit2/UIProcess/API/gtk/tests/TestInspectorServer.cpp:264
> > + g_input_stream_read_async(istream, response, sizeof(response) - 1,
G_PRIORITY_DEFAULT, 0, 0, 0);
>
> Since the operation might never finish, I think it would be better to pass a
cancellable, and cancel the operation before the test finish so that the thread
used to implement the async operation exits.
>
Done
> > Source/WebKit2/UIProcess/API/gtk/tests/TestInspectorServer.cpp:268
> > g_assert(String(response).isEmpty());
>
> You don't need to convert the buffer to a String just to check if it's empty,
you can simply check response[0] == '\0'
Of course, don't ask me why I did that :D. Done
More information about the webkit-reviews
mailing list