[Webkit-unassigned] [Bug 141508] [SOUP] Synchronous XMLHttpRequests can time out when we reach the max connections limit

[bugzilla-daemon at webkit.org]

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

--- Comment #11 from Carlos Garcia Campos <cgarcia at igalia.com> ---
(In reply to comment #9)
> Comment on attachment 247751 [details]
> Updated patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=247751&action=review
> 
> Nice!

Thanks for the review.

> > Source/WebCore/ChangeLog:10
> > +        connections allowed if the soup version is recent enough.
> 
> Perhaps we should explain here why the increase/decrease was not a good idea
> sometimes.

Added more explanation there.

> > Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp:935
> > +    // Ignore the connection limits in synchronous loads to avoid freezing the networking process.
> 
> This comment doesn't add much as the code bellow is really explicit. I'd
> better include a link to the bug or a more detailed description of the
> issue. I'd prefer the former.

Added the bug URL.

> > Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp:942
> >      g_signal_connect(d->m_soupMessage.get(), "network-event", G_CALLBACK(networkEventCallback), handle);
> 
> As you're fixing some other stuff like override,final, etc... I think we
> could use this bug to also store the SoupMessage in a local variable and use
> it all along this method which is full of the ugly d->m_soupMessage.get().

Fixed the sync loader because I was changing that, but for more general cleanups I prefer to use a separate bug.

> > Tools/TestWebKitAPI/Tests/WebKit2Gtk/TestResources.cpp:704
> > +        GUniquePtr<char> xhr(g_strdup_printf("xhr = new XMLHttpRequest; xhr.open('GET', '/sync-request-on-max-conns-xhr%u', false); xhr.send();", i));
> 
> If it were me I'd have stored this string in a static variable putting each
> instruction in a different line for better readability. But since this is a
> unit test I could live with it.

:-)

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20150303/afb1f65e/attachment-0002.html>